Ensure cloud conversation status updates in response to execution completion#9315
Ensure cloud conversation status updates in response to execution completion#9315zachbai wants to merge 5 commits intozb/add-execution-support-ambient-agent-modelfrom
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
I'm starting a first review of this pull request. I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment 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 adds cloud-to-cloud follow-up UI plumbing for ambient agent tasks: tombstone Continue actions, pending follow-up submission routing, optimistic prompt rendering, execution-ended status refreshes, and related tests/spec updates.
Concerns
- A new test helper constructs
AmbientAgentTaskwithout all required fields, which will break compilation. - Owned ambient sessions can enter cloud-follow-up mode even when
HandoffCloudCloudis disabled, bypassing the existing read-only viewer cleanup. - Follow-up submission leaves the input editable and the pending mode active while waiting for a new session, allowing duplicate follow-up submissions for the same run.
- Optimistic follow-up rendering is triggered before the API accepts the follow-up, so immediate API rejection can leave a prompt permanently rendered for a run that never started.
Verdict
Found: 1 critical, 3 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
| request_usage: None, | ||
| is_sandbox_running: false, | ||
| agent_config_snapshot: None, | ||
| artifacts: vec![], |
There was a problem hiding this comment.
🚨 [CRITICAL] This AmbientAgentTask literal is missing the current last_event_sequence and children fields, so the test target will not compile.
| artifacts: vec![], | |
| artifacts: vec![], | |
| last_event_sequence: None, | |
| children: vec![], |
|
|
||
| // When the session is ended, the input should be uneditable iff this is a viewer. | ||
| if self.model.lock().shared_session_status().is_viewer() { | ||
| if let Some(task_id) = owned_ambient_task_id { |
There was a problem hiding this comment.
HandoffCloudCloud, so with the feature disabled an owned ambient viewer skips the normal read-only cleanup and enters cloud-follow-up mode that later just errors. Gate this branch, or fall through to the viewer cleanup, when the flag is off.
| ambient_agent_view_model.update(ctx, |model, ctx| { | ||
| model.submit_cloud_followup(prompt, ctx); | ||
| }); | ||
| self.input.update(ctx, |input, ctx| { |
There was a problem hiding this comment.
pending_cloud_followup_task_id set and re-enables editable agent input, so a second Enter before FollowupSessionReady sends another follow-up for the same run. Freeze or disable input, or clear the pending mode, until success/failure.
| None, | ||
| ); | ||
|
|
||
| self.pending_followup_prompt = Some(prompt); |
There was a problem hiding this comment.
submit_run_followup is accepted; because FollowupDispatched inserts the optimistic query immediately, an API rejection still leaves a prompt rendered for a follow-up that never started. Defer optimistic rendering until acceptance or remove it on pre-accept failure.
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
a18c6a2 to
1077445
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
04c8f6b to
0cf8a89
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
1077445 to
a495742
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
0cf8a89 to
318f8b9
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
a495742 to
ecbc7f9
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
318f8b9 to
184e5a8
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
ecbc7f9 to
6599846
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
184e5a8 to
0440a49
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
6599846 to
1f1ac58
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
0440a49 to
3d57414
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
1f1ac58 to
608bcc4
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
2 times, most recently
from
5fa260b to
7abe5b4
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
608bcc4 to
2dcf967
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
7abe5b4 to
36a1eac
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
2dcf967 to
973da8c
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
36a1eac to
cb69d57
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
973da8c to
b4a2e4a
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
2 times, most recently
from
a9e2039 to
3471287
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
b4a2e4a to
c5c0843
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
3471287 to
c0bef65
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
c5c0843 to
5394505
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
c0bef65 to
d4e99a0
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
5394505 to
c69a2ac
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
d4e99a0 to
bf8c47e
Compare
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
c69a2ac to
d466711
Compare
There was a problem hiding this comment.
Overview
This PR adds cloud-to-cloud follow-up UI/state handling, execution-ended status refreshes, replay suppression for appended follow-up sessions, optimistic follow-up query rendering, and related telemetry/tests.
Concerns
- Append-followup replay suppression currently skips any response for an existing conversation that already has exchanges, which can drop newly replayed follow-up responses for the same conversation.
- Owned ambient session teardown enters cloud follow-up input even when
HandoffCloudCloudis disabled, so the feature-off path is not unchanged. - Follow-up submission leaves the input editable and the pending task id set, allowing duplicate concurrent submissions before the first follow-up session is ready.
- Optimistic query suppression is keyed only by prompt text, so later identical prompts can have their AI block query/header hidden incorrectly.
- Supplemental security pass found no additional security-specific findings.
Verdict
Found: 0 critical, 4 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
|
|
||
| BlocklistAIHistoryModel::as_ref(ctx) | ||
| .conversation(&conversation_id) | ||
| .is_some_and(|conversation| conversation.exchange_count() > 0) |
There was a problem hiding this comment.
exchange_count() > 0.
|
|
||
| // When the session is ended, the input should be uneditable iff this is a viewer. | ||
| if self.model.lock().shared_session_status().is_viewer() { | ||
| if let Some(task_id) = owned_ambient_task_id { |
There was a problem hiding this comment.
HandoffCloudCloud is disabled, causing the next agent prompt to be intercepted and rejected instead of preserving the feature-off behavior; gate this branch on the flag.
| ambient_agent_view_model.update(ctx, |model, ctx| { | ||
| model.submit_cloud_followup(prompt, ctx); | ||
| }); | ||
| self.input.update(ctx, |input, ctx| { |
There was a problem hiding this comment.
pending_cloud_followup_task_id remains set and this resets the editor to editable, so a second Enter can submit another cloud follow-up before the first session is ready; freeze or clear the pending mode until ready/failure.
| self.optimistically_rendered_user_queries.push(prompt); | ||
| } | ||
|
|
||
| pub fn has_optimistic_user_query(&self, prompt: &str) -> bool { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Overview
This PR adds cloud-to-cloud follow-up UI for ambient agent tasks: execution-ended handling updates task status, tombstones can start cloud follow-ups, the terminal input routes pending follow-up prompts through the ambient agent model, and optimistic follow-up query UI is added.
Concerns
- Follow-up submission currently leaves the pane in pending follow-up mode and editable, allowing multiple concurrent submissions for the same ended task before a new session is ready.
- Pending follow-up mode intercepts all non-empty agent prompts, so slash-command prompts can be misrouted to the cloud follow-up API instead of keeping their existing local behavior.
- Failure handling clears the pending follow-up state after the tombstone has been removed, which can leave users without a retry path and subsequent prompts on the wrong route.
- Security pass: no additional security-specific findings beyond the lifecycle/routing issues above.
Verdict
Found: 0 critical, 3 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 false; | ||
| }; | ||
|
|
||
| if prompt.trim().is_empty() { |
There was a problem hiding this comment.
SendAgentPrompt, including slash-command prompts such as /fork; those should keep the existing local path instead of being sent to submit_cloud_followup.
| ambient_agent_view_model.update(ctx, |model, ctx| { | ||
| model.submit_cloud_followup(prompt, ctx); | ||
| }); | ||
| self.input.update(ctx, |input, ctx| { |
There was a problem hiding this comment.
pending_cloud_followup_task_id set and resets the editor to editable immediately after starting a follow-up, so repeated submits can dispatch multiple concurrent follow-up executions for the same task before FollowupSessionReady or failure. Clear or mark the pending submission as in-flight until the ambient model resolves.
| ctx.notify(); | ||
| } | ||
| AmbientAgentViewModelEvent::Failed { error_message } => { | ||
| self.pending_cloud_followup_task_id = None; |
There was a problem hiding this comment.
start_cloud_followup_from_tombstone has removed the tombstone, leaving no retry entrypoint and causing later prompts to fall back to the ended shared-session path. Restore the tombstone or keep follow-up compose state available on failure.
zachbai
force-pushed
the
zb/continue-cloud-tombstone
branch
from
d466711 to
cc116e8
Compare
zachbai
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
bf8c47e to
560ca94
Compare
There was a problem hiding this comment.
Overview
This PR adds cloud-to-cloud follow-up UI/state plumbing for ambient agent tombstones, execution-ended status updates, optimistic follow-up prompt rendering, and related tests.
Concerns
- The new
CloudModeFollowupUserQueryrender path callsrender_user_querywith the wrong number of arguments, so the branch will not compile. - Follow-up compose state remains active and editable after submission, allowing duplicate concurrent follow-up executions before a session is ready.
- Failure/auth/cancel cleanup can remove the tombstone and clear pending follow-up state, leaving no reliable retry path for tombstone-started follow-ups.
- Optimistic prompt suppression is tracked by prompt text only and is never consumed, so later live exchanges with the same text can have their AI block query/header hidden incorrectly.
- Security pass: no security-specific findings.
Verdict
Found: 1 critical, 3 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
| } | ||
|
|
||
| fn render(&self, app: &AppContext) -> Box<dyn Element> { | ||
| render_user_query(&self.prompt, &self.view_model, app) |
There was a problem hiding this comment.
🚨 [CRITICAL] This call no longer matches render_user_query(prompt, query_prefix_highlight_len, view_model, app), so the PR will not compile.
| render_user_query(&self.prompt, &self.view_model, app) | |
| render_user_query(&self.prompt, None, &self.view_model, app) |
| } | ||
|
|
||
| pub fn record_optimistic_user_query(&mut self, prompt: String) { | ||
| self.optimistically_rendered_user_queries.push(prompt); |
There was a problem hiding this comment.
has_optimistic_user_query suppress any later live AI block with the same user text; consume the matching entry or key it to a specific pending exchange/session instead.
| model.submit_cloud_followup(prompt, ctx); | ||
| }); | ||
| self.input.update(ctx, |input, ctx| { | ||
| input.reset_after_cloud_followup_submission(ctx); |
There was a problem hiding this comment.
pending_cloud_followup_task_id remains set and the input is made editable again, so another prompt before FollowupSessionReady starts a second follow-up against the same ended execution; clear the pending state or keep the input frozen until ready/failure.
| ctx.notify(); | ||
| } | ||
| AmbientAgentViewModelEvent::Failed { error_message } => { | ||
| self.pending_cloud_followup_task_id = None; |
There was a problem hiding this comment.
pending_cloud_followup_task_id on failure leaves no cloud retry entrypoint and subsequent prompts fall through the normal path; reinsert the tombstone or keep follow-up compose state for retry.
There was a problem hiding this comment.
Overview
This PR adds cloud-to-cloud follow-up handling for ambient agent sessions, including a tombstone Continue action, follow-up input routing, optimistic query rendering, replay suppression, and details/status refreshes when executions end.
Concerns
- The owned ambient session cleanup path changes behavior even when
HandoffCloudCloudis disabled, which violates the feature flag isolation expected for this rollout. - Replay suppression is keyed only on whether the conversation already has any exchanges, so append-followup replay can suppress the new follow-up response along with already-present responses.
- Follow-up submission leaves compose state active and input editable while the new session is starting, allowing concurrent duplicate follow-up submissions.
- Failure cleanup removes the retry route after a tombstone-started follow-up because the tombstone was removed and the pending follow-up id is cleared.
- Supplemental security pass: no direct security-specific issues were found in the changed lines.
Verdict
Found: 0 critical, 4 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
|
|
||
| // When the session is ended, the input should be uneditable iff this is a viewer. | ||
| if self.model.lock().shared_session_status().is_viewer() { | ||
| if let Some(task_id) = owned_ambient_task_id { |
There was a problem hiding this comment.
HandoffCloudCloud, so disabling the flag still suppresses the existing tombstone and enables cloud-follow-up input; keep the old tombstone/read-only behavior unless the handoff flag is enabled.
|
|
||
| BlocklistAIHistoryModel::as_ref(ctx) | ||
| .conversation(&conversation_id) | ||
| .is_some_and(|conversation| conversation.exchange_count() > 0) |
There was a problem hiding this comment.
| model.submit_cloud_followup(prompt, ctx); | ||
| }); | ||
| self.input.update(ctx, |input, ctx| { | ||
| input.reset_after_cloud_followup_submission(ctx); |
There was a problem hiding this comment.
submit_cloud_followup, the pending task id remains set and the input is reset to editable, so users can submit multiple follow-ups concurrently while the first session is still starting; clear/freeze the compose state until ready or failure.
| ctx.notify(); | ||
| } | ||
| AmbientAgentViewModelEvent::Failed { error_message } => { | ||
| self.pending_cloud_followup_task_id = None; |
There was a problem hiding this comment.
harryalbert
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
560ca94 to
3db61ca
Compare
harryalbert
force-pushed
the
zb/continue-cloud-tombstone
branch
from
cc116e8 to
70f0742
Compare
There was a problem hiding this comment.
Overview
This PR adds the cloud-to-cloud follow-up entrypoint for ambient Cloud Mode tombstones, routes pending input to the ambient follow-up API, adds optimistic follow-up query rendering, and suppresses duplicate replayed agent conversation content while appending follow-up scrollback.
Concerns
- Append-followup replay suppression is conversation-wide, so a replay for an existing conversation can skip the new follow-up response as soon as the conversation has any prior exchange.
- Follow-up submission leaves the input editable and the pending task id set, allowing repeated Enter presses to dispatch multiple concurrent cloud follow-ups for the same task.
- Starting from the tombstone removes the only retry affordance before a new session is established; failure/auth/cancel paths clear pending state without restoring the tombstone.
- Optimistic follow-up query blocks are inserted on dispatch but are not removed when submission fails before a session is ready, leaving prompts in the transcript that may not have run.
Verdict
Found: 0 critical, 4 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
|
|
||
| BlocklistAIHistoryModel::as_ref(ctx) | ||
| .conversation(&conversation_id) | ||
| .is_some_and(|conversation| conversation.exchange_count() > 0) |
There was a problem hiding this comment.
| model.submit_cloud_followup(prompt, ctx); | ||
| }); | ||
| self.input.update(ctx, |input, ctx| { | ||
| input.reset_after_cloud_followup_submission(ctx); |
There was a problem hiding this comment.
pending_cloud_followup_task_id remains set, so repeated submits can create multiple concurrent follow-up executions for the same task.
| self.show_error_toast("Couldn't continue this cloud task.".to_string(), ctx); | ||
| return; | ||
| } | ||
| self.remove_conversation_ended_tombstone(ctx); |
There was a problem hiding this comment.
FollowupSessionReady leaves no retry path if submit/polling fails, needs auth, or is cancelled because those handlers only clear pending state and never restore the tombstone.
| ctx, | ||
| ) | ||
| }); | ||
| self.insert_rich_content( |
There was a problem hiding this comment.
FollowupDispatched, but failure/auth/cancel paths only clear model state, so a rejected follow-up can leave a prompt in the transcript that never actually ran.
There was a problem hiding this comment.
Overview
This PR adds cloud-to-cloud follow-up UI from ambient-agent tombstones, execution-ended status updates, optimistic follow-up prompt rendering, and related tests/spec documentation. I also performed a security-focused pass and did not find a security-specific issue in the changed lines.
Concerns
CloudModeFollowupUserQuery::rendercallsrender_user_querywith the wrong arity, which will break compilation.- The cloud follow-up submission success path makes the input editable while the pending follow-up task id remains set, allowing repeated submissions before the new session is ready.
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
| } | ||
|
|
||
| fn render(&self, app: &AppContext) -> Box<dyn Element> { | ||
| render_user_query(&self.prompt, &self.view_model, app) |
There was a problem hiding this comment.
🚨 [CRITICAL] This call no longer matches render_user_query's signature; the helper still takes query_prefix_highlight_len, so this will not compile.
| render_user_query(&self.prompt, &self.view_model, app) | |
| render_user_query(&self.prompt, None, &self.view_model, app) |
| model.submit_cloud_followup(prompt, ctx); | ||
| }); | ||
| self.input.update(ctx, |input, ctx| { | ||
| input.reset_after_cloud_followup_submission(ctx); |
There was a problem hiding this comment.
pending_cloud_followup_task_id stays set until session-ready/failure, so a second Enter before the new session is ready dispatches another submit_cloud_followup and overwrites the pending prompt; freeze or clear the pending mode after dispatch.
…or owned cloud mode convos
harryalbert
force-pushed
the
zb/continue-cloud-tombstone
branch
from
70f0742 to
275dd8b
Compare
harryalbert
force-pushed
the
zb/add-execution-support-ambient-agent-model
branch
from
3db61ca to
036037d
Compare
1 participant
Iron out UI for follow-up run execution triggering on ambient agent tasks.