Implement consistent tombstone and follow-up visibility behavior for cloud agent conversations.#10939
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
zachbai
force-pushed
the
zb/tombstone-input-ui
branch
from
d546606 to
88b464a
Compare
…cloud agent conversations.
…dering of pending user query and tombstone upon failure cases.
zachbai
force-pushed
the
graphite-base/10939
branch
from
7cc792b to
f78edb7
Compare
zachbai
force-pushed
the
zb/tombstone-input-ui
branch
from
88b464a to
92cefc9
Compare
|
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}
There was a problem hiding this comment.
Overview
This PR centralizes cloud conversation ended-state behavior around conversation metadata and data-driven tombstone CTAs. The direction is sound, but two state-resolution regressions would show the wrong UI in important edge cases.
Concerns
- Missing conversation metadata still falls back to ambient task creator ownership, which can show mutation affordances before the conversation permission source of truth is loaded.
- The legacy tombstone path for
HandoffCloudClouddisabled is gated on!CloudModeSetupV2, which removes the existing ended tombstone for CloudModeSetupV2 sessions even though this PR is scoped behind the handoff flag.
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
| } | ||
|
|
||
| let access = task_creator_access(&task, app); | ||
| if access == ConversationAccess::Edit { |
There was a problem hiding this comment.
ServerAIConversationMetadata.permissions is available, otherwise a user can briefly get followup/continue affordances after permissions were revoked or before the conversation access model loads.
| let should_insert_legacy_tombstone = { | ||
| let model = self.model.lock(); | ||
| FeatureFlag::CloudModeSetupV2.is_enabled() | ||
| !FeatureFlag::CloudModeSetupV2.is_enabled() |
There was a problem hiding this comment.
HandoffCloudCloud disabled and CloudModeSetupV2 enabled, ended shared ambient sessions no longer insert the tombstone. The PR is scoped behind HandoffCloudCloud, so preserve the existing CloudModeSetupV2 tombstone path when the handoff resolver returns None.
|
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 |
There was a problem hiding this comment.
Overview
This PR centralizes cloud conversation tombstone/follow-up decisions and makes tombstone CTAs data-driven. The main structure is sound, and the supplemental security pass did not find a separate security-specific issue.
Concerns
- Missing conversation metadata can still enable mutation UI from task creator identity, which contradicts the stated safe default for missing metadata.
- The session-ended legacy tombstone path is disabled for
CloudModeSetupV2whenHandoffCloudCloudis off, changing behavior outside the feature flag.
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
| } | ||
|
|
||
| let access = task_creator_access(&task, app); | ||
| if access == ConversationAccess::Edit { |
There was a problem hiding this comment.
FollowupInput/Continue instead of the stated safe default tombstone with no action. Return a missing-metadata/unknown state here unless metadata-derived permissions confirm edit access.
| let should_insert_legacy_tombstone = { | ||
| let model = self.model.lock(); | ||
| FeatureFlag::CloudModeSetupV2.is_enabled() | ||
| !FeatureFlag::CloudModeSetupV2.is_enabled() |
There was a problem hiding this comment.
CloudModeSetupV2 when HandoffCloudCloud is disabled: the new resolver returns None in that flag state, and this legacy branch now skips setup-v2 sessions. Keep the legacy setup-v2 path for the disabled-handoff case so the feature flag does not change existing behavior.
…cloud agent conversations. (warpdotdev#10939) ## Description [Demo](https://www.loom.com/share/f591784f65444cc894cedb194a2416a7) [Demo pt II with 3rd party harnesses](https://www.loom.com/share/df53c75e0deb4c3eba7110eb85861691) This cleans up the ended-session-state UI for cloud agent conversations behind `FeatureFlag::HandoffCloudCloud`. The important bit is that we now make the tombstone / followup input / CTA decision from the underlying conversation metadata. - Oz/Warp agent + edit access: hide the tombstone and show the input directly. - Oz/warp agent + view-only access: show the tombstone with `Continue locally`. - Third-party harness + edit access: show the tombstone with `Continue`. - Third-party harness + view-only access, or missing metadata: show the tombstone, no continue action. I also pulled that branching into a shared resolver, made tombstone CTAs data-driven, and added specs/tests around the behavior so we do not keep re-implementing the same checks in terminal view code. ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode Co-Authored-By: Oz <oz-agent@warp.dev>
2 participants
Description
Demo
Demo pt II with 3rd party harnesses
This cleans up the ended-session-state UI for cloud agent conversations behind
FeatureFlag::HandoffCloudCloud.The important bit is that we now make the tombstone / followup input / CTA decision from the underlying conversation metadata.
Continue locally.Continue.I also pulled that branching into a shared resolver, made tombstone CTAs data-driven, and added specs/tests around the behavior so we do not keep re-implementing the same checks in terminal view code.
Agent Mode
Co-Authored-By: Oz oz-agent@warp.dev