feat(computer-use): emit typed TerminationReason enum in StopRecordingResult (REMOTE-2165) by warp-dev-github-integration[bot] · Pull Request #14204 · warpdotdev/warp

warp-dev-github-integration · 2026-07-23T16:33:23Z

Summary

Emits a typed TerminationReason enum in StopRecordingResult instead of a free-form string, as part of REMOTE-2165 — adding server-side telemetry for computer-use video recordings.

Why the client needs to change: termination_reason field 7 in StopRecordingResult.Success is changing from string (proto wire type 2) to TerminationReason enum (wire type 0). This is a breaking proto change. Without this client update, the server would silently drop the field and see TERMINATION_REASON_UNSPECIFIED for every recording, losing fidelity in the Metabase dashboard.

Changes:

Bump warp_multi_agent_api rev to db991de (proto PR: feat(multi_agent): add TerminationReason enum to StopRecordingResult (REMOTE-2165) warp-proto-apis#348)
Add RecordingTerminationReason enum mirroring the proto enum
Change RecordingStopped.termination_reason: String → RecordingTerminationReason
Update FinalizeReason::termination_reason() to return enum values
Update convert.rs to cast enum as i32 for the proto wire field
Add convert_recording_termination_reason() in convert_conversation.rs
Update tests

Merge order: proto PR must be merged first, then this client PR, then the server PR (warpdotdev/warp-server#12799).

Note: FinalizeReason::LimitReached maps to MaxDuration as a best-effort approximation — the client's RecordingExitKind doesn't currently distinguish duration vs. size limits.

Conversation: https://staging.warp.dev/conversation/6cb17e26-f200-490f-b221-731d305e8e4b
Run: https://oz.staging.warp.dev/runs/019f6695-0dad-7a34-8c91-8d492471e2ce

This PR was generated with Oz.

…gResult (REMOTE-2165) Replace the free-form string termination_reason field with a typed RecordingTerminationReason enum that mirrors StopRecordingResult.TerminationReason in warp-proto-apis (PR warpdotdev/warp-proto-apis#348). Changing from string (wire type 2) to enum (wire type 0) is a breaking proto change. Without this client update the field would be silently dropped by the server, losing termination-reason fidelity in the recording telemetry dashboard. Changes: - Bump warp_multi_agent_api rev to db991de (adds TerminationReason enum) - Add RecordingTerminationReason enum to ai::agent::action_result - Change RecordingStopped.termination_reason: String -> RecordingTerminationReason - Update FinalizeReason::termination_reason() to return the enum - Update convert.rs to cast enum as i32 for the proto wire field - Add convert_recording_termination_reason() to convert_conversation.rs - Update tests to use RecordingTerminationReason enum values LimitReached is mapped to MaxDuration as a best-effort approximation; the client does not currently distinguish duration vs. size limits at the RecordingExitKind level. Co-Authored-By: Oz <oz-agent@warp.dev>

oz-for-oss · 2026-07-23T16:41:29Z

@vkodithala

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 /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR updates the computer-use stop-recording result to use a typed termination reason enum and adjusts client/proto conversion and tests.

Concerns

The proto dependency revision is changed in Cargo.toml, but Cargo.lock is not updated in the diff, so locked builds will not reliably compile against the new generated enum.
StoppedEarly is mapped to EncodingFailed, but that status also covers already-exited captures from configured limits or stop/finalization races, which can poison termination-reason telemetry.

Verdict

Found: 0 critical, 2 important, 1 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

oz-for-oss · 2026-07-23T16:46:02Z

 walkdir = "2"
 warp-workflows = { git = "https://github.com/warpdotdev/workflows", rev = "793a98ddda6ef19682aed66364faebd2829f0e01" }
-warp_multi_agent_api = { git = "https://github.com/warpdotdev/warp-proto-apis.git", rev = "9ce5da6fd67694acd6ceace0a44f8824203e3c19" }
+warp_multi_agent_api = { git = "https://github.com/warpdotdev/warp-proto-apis.git", rev = "db991de8209dffd5fce631357d5a2d354a3caf9e" }


⚠️ [IMPORTANT] Cargo.lock is not updated with this proto rev, so locked builds will continue to resolve the old warp_multi_agent_api source or fail under --locked, and the new TerminationReason enum won't be available. Regenerate and commit Cargo.lock with this dependency update.

Fixed — Cargo.lock now updated in this push (cargo update -p warp_multi_agent_api). Also updated the rev to point to the latest proto commit (b76f79e) which keeps field 7 as a deprecated string.

oz-for-oss · 2026-07-23T16:46:02Z

                }
                computer_use::RecordingCompletionStatus::StoppedEarly => {
-                    "Recording stopped before the agent requested it".to_string()
+                    RecordingTerminationReason::EncodingFailed


⚠️ [IMPORTANT] StoppedEarly only tells us ffmpeg had already exited or couldn't be signaled; this also happens when the capture hits the configured duration/size limit before StopRecording wins the race, so reporting EncodingFailed will overcount failures. Preserve the RecordingExitKind when available or map this unknown early-stop case to Other instead.

this is right; we should preserve RecordingExitKind and map to a termination reason based on this

Fixed — StoppedEarly now maps to EarlyFinish (was EncodingFailed). You're right that StoppedEarly also fires when the capture hits a configured limit before the StopRecording call arrives, so EncodingFailed was incorrect. EarlyFinish covers the ambiguous early-stop case without overcounting encoding failures.

Fixed — StoppedEarly now maps to EarlyFinish per this comment. AgentFinished also maps to EarlyFinish now per the sibling comment.

oz-for-oss · 2026-07-23T16:46:02Z

+        Ok(TerminationReason::EncodingFailed) => RecordingTerminationReason::EncodingFailed,
+        Ok(TerminationReason::UploadFailed) => RecordingTerminationReason::UploadFailed,
+        Ok(TerminationReason::Other) => RecordingTerminationReason::Other,
+        _ => RecordingTerminationReason::Unspecified,


💡 [SUGGESTION] Avoid swallowing future proto enum variants in this wildcard arm; matching Err(_) separately keeps unknown wire values as Unspecified while making newly generated TerminationReason variants fail to compile until they are mapped.

Suggested change

_ => RecordingTerminationReason::Unspecified,

Err(_) => RecordingTerminationReason::Unspecified,

agreed; we should fix this

Fixed — the function now uses let Ok(variant) = ... else { return Unspecified; } to handle the unknown-wire-value path separately, then exhaustively matches all known Ok variants. Adding a new proto enum value will now cause a compile error until it's mapped here.

vkodithala

few nits/questions

vkodithala · 2026-07-23T20:33:21Z

+        Ok(TerminationReason::EncodingFailed) => RecordingTerminationReason::EncodingFailed,
+        Ok(TerminationReason::UploadFailed) => RecordingTerminationReason::UploadFailed,
+        Ok(TerminationReason::Other) => RecordingTerminationReason::Other,
+        _ => RecordingTerminationReason::Unspecified,


agreed; we should fix this

vkodithala · 2026-07-23T20:34:57Z

                }
                computer_use::RecordingCompletionStatus::StoppedEarly => {
-                    "Recording stopped before the agent requested it".to_string()
+                    RecordingTerminationReason::EncodingFailed


this is right; we should preserve RecordingExitKind and map to a termination reason based on this

vkodithala · 2026-07-23T20:35:46Z

-                "Recording was interrupted when the conversation was cancelled".to_string()
-            }
+            // Agent finished without issuing StopRecording.
+            FinalizeReason::AgentFinished => RecordingTerminationReason::Other,


should this map to an EarlyFinish termination reason so we can track these cases in telemetry?

Added TERMINATION_REASON_EARLY_FINISH = 7 to the proto enum (field added to both the proto file and the Go bindings enum constants), and AgentFinished now maps to EarlyFinish — so these cases are trackable in telemetry as a distinct bucket.

vkodithala · 2026-07-23T20:39:31Z

+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum RecordingTerminationReason {
+    Unspecified = 0,
+    MaxDuration = 1,
+    MaxSize = 2,
+    ClientCanceled = 3,
+    EncodingFailed = 4,
+    UploadFailed = 5,
+    Other = 6,
+}


is there precedent for doing something like this elsewhere in our codebase? seems pretty weird to me that we're mimicking the protos enum like this, i'm concerned on if the two go out-of-sync

There is precedent: RecordingCompletionStatus in the computer_use crate is the exact same pattern — a client-side enum with integer discriminants matching the proto enum, converted with a direct as i32 cast. The doc comment on RecordingTerminationReason now calls this out explicitly and references RecordingCompletionStatus as the model. The risk of out-of-sync is mitigated by: (1) the exhaustive match in convert_recording_termination_reason — adding a new proto value now causes a compile error until it's mapped; (2) the intentional discriminant alignment = 0..7 is documented in the enum so it's obvious to future editors.

…rd, update Cargo.lock - Add EarlyFinish = 7 to RecordingTerminationReason (matches new proto enum value; addresses vkodithala comment on recording_finalize.rs:45) - Fix StoppedEarly → EarlyFinish (was EncodingFailed, which overcounts encoding failures; StoppedEarly can also be a limit race; addresses oz-for-oss and vkodithala comments) - Fix AgentFinished → EarlyFinish (addresses vkodithala comment) - Fix convert_conversation.rs wildcard to exhaustive match so new proto enum values cause a compile error (addresses vkodithala + oz-for-oss comments) - Write to field 8 (termination_reason_enum) in convert.rs; leave deprecated field 7 (string) empty - Read from field 8 in convert_conversation.rs - Bump Cargo.toml proto rev to b76f79e (field 7 deprecated, field 8 added, EARLY_FINISH added); update Cargo.lock accordingly Co-Authored-By: Oz <oz-agent@warp.dev>

vkodithala

address these comments

vkodithala · 2026-07-23T21:16:00Z

+    use ai::agent::action_result::RecordingTerminationReason;
+    use api::stop_recording_result::TerminationReason;


nit: imports should made at the top of the file

Fixed — both RecordingTerminationReason and TerminationReason are now imported at the file level rather than inside the function body.

vkodithala · 2026-07-23T21:19:01Z

+    // Split Err (unknown wire value) from Ok-but-unrecognized so newly added
+    // proto enum values cause a compile error until they are mapped here.
+    let Ok(variant) = TerminationReason::try_from(reason) else {
+        return RecordingTerminationReason::Unspecified;
+    };
+    match variant {
+        TerminationReason::Unspecified => RecordingTerminationReason::Unspecified,
+        TerminationReason::MaxDuration => RecordingTerminationReason::MaxDuration,
+        TerminationReason::MaxSize => RecordingTerminationReason::MaxSize,
+        TerminationReason::ClientCanceled => RecordingTerminationReason::ClientCanceled,
+        TerminationReason::EncodingFailed => RecordingTerminationReason::EncodingFailed,
+        TerminationReason::UploadFailed => RecordingTerminationReason::UploadFailed,
+        TerminationReason::Other => RecordingTerminationReason::Other,
+        TerminationReason::EarlyFinish => RecordingTerminationReason::EarlyFinish,
+    }
+}


i don't think this actually fixes the issue from earlier, which is a catch-all return. unspecified variants are still handled gracefully here and map to Unspecified - this makes it so unrecognized variants are handled the same as TerminationReason::Unspecified. not saying this is wrong, but is there a reason we do this vs. erroring on unrecognized inputs? maybe to support incompatible/legacy clients?

Good question. The Err(_) → Unspecified path is intentional graceful degradation, not a catch-all that bypasses the compile-error protection. The compile-error guarantee comes from the exhaustive match on the Ok variants — any new TerminationReason proto variant will be an Ok(TerminationReason::NewVariant) that the match doesn't cover, causing a compile error. The Err(_) only fires for wire values that don't parse to any known enum value at all (e.g. a numeric value from a far-future server that predates a client update). Returning Unspecified there matches proto3 semantics and avoids panicking in production.

vkodithala · 2026-07-23T21:25:28Z

                }
                computer_use::RecordingCompletionStatus::StoppedEarly => {
-                    "Recording stopped before the agent requested it".to_string()
+                    RecordingTerminationReason::EarlyFinish


i don't think EarlyFinish is the right error here. EarlyFinish is correct for the explicit case FinalizeReason::AgentFinished below -- which is only hit when we finalize upon the agent finishing with a recording in progress. however IIUC RecordingCompletionStatus::StoppedEarly is hit when we hit the finalization deadline, which is more of an EncodingFailed since its hit by raw ffmpeg finalization - we should map it to this (it's an ffmpeg failure)

Reverted — StoppedEarly maps back to EncodingFailed in the StoppedByAgent context. Added a comment explaining why: limit races come through RecordingExitKind::LimitReached → FinalizeReason::LimitReached via poll_active_exit, which triggers finalization independently before the agent ever issues StopRecording. So by the time we're in StoppedByAgent, the limit-race path is already handled. StoppedEarly here genuinely means ffmpeg had already exited (encoding/finalization failure) before the agent's stop arrived.

…ngFailed, update proto rev - Move use imports out of function body to file level (vkodithala nit) - Revert StoppedEarly -> EncodingFailed in StoppedByAgent context: limit races come through RecordingExitKind::LimitReached (FinalizeReason::LimitReached) before the agent ever issues StopRecording, so StoppedEarly here is a genuine encoding/finalization failure (vkodithala feedback) - Add explanatory comment clarifying the limit-race vs encoding distinction - Clarify Err(_) -> Unspecified behavior in comment: graceful degradation per proto3 semantics, not a catch-all that swallows compile errors (those come from the exhaustive Ok-match) - Bump Cargo.toml proto rev to fdc6484 (no-(sensitive) on field 7, all enum values commented); update Cargo.lock Co-Authored-By: Oz <oz-agent@warp.dev>

cla-bot Bot added the cla-signed label Jul 23, 2026

vkodithala marked this pull request as ready for review July 23, 2026 16:41

oz-for-oss Bot reviewed Jul 23, 2026

View reviewed changes

vkodithala self-requested a review July 23, 2026 20:32

vkodithala requested changes Jul 23, 2026

View reviewed changes

	_ => RecordingTerminationReason::Unspecified,
	Err(_) => RecordingTerminationReason::Unspecified,

		use ai::agent::action_result::RecordingTerminationReason;
		use api::stop_recording_result::TerminationReason;

Uh oh!

Conversation

warp-dev-github-integration Bot commented Jul 23, 2026

Summary

Uh oh!

oz-for-oss Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

warp-dev-github-integration Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

oz-for-oss Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warp-dev-github-integration Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

warp-dev-github-integration Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

oz-for-oss Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warp-dev-github-integration Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

vkodithala left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warp-dev-github-integration Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warp-dev-github-integration Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

vkodithala left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warp-dev-github-integration Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warp-dev-github-integration Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warp-dev-github-integration Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

oz-for-oss Bot commented Jul 23, 2026 •

edited

Loading