From 524516cb56e738e3ce50ec19f8e91f2b7952b928 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Chaonan=E2=80=9D?= Date: Mon, 17 Aug 2026 20:26:42 +0800 Subject: [PATCH] fix(protocol): align Trace v2/v3 classification and hover steps Require Trace v3 version const 3, reject mixed pages/states payloads, and align TypeScript selection optionality with the generated schema. Trace v2 on main already records hover steps, so the frozen v2 schema must accept them instead of dropping them during reduction. Co-authored-by: Cursor --- .../src/lib/__tests__/trace-reducer.test.ts | 8 +- apps/extension/src/lib/trace-reducer.ts | 8 +- apps/extension/src/transport/types.ts | 5 +- .../schema/tool_record_await_result.json | 49 ++++++++++- .../schema/tool_record_stop_result.json | 49 ++++++++++- crates/bsk-protocol/schema/trace.json | 4 +- crates/bsk-protocol/schema/trace_v2.json | 41 +++++++++ crates/bsk-protocol/src/tools/record.rs | 84 ++++++++++++++++--- crates/bsk-protocol/src/tools/record_v2.rs | 17 +++- 9 files changed, 234 insertions(+), 31 deletions(-) diff --git a/apps/extension/src/lib/__tests__/trace-reducer.test.ts b/apps/extension/src/lib/__tests__/trace-reducer.test.ts index b1153be..f5c6db5 100644 --- a/apps/extension/src/lib/__tests__/trace-reducer.test.ts +++ b/apps/extension/src/lib/__tests__/trace-reducer.test.ts @@ -151,7 +151,7 @@ describe("reduceTraceSteps", () => { }); }); - it("drops hover steps unsupported by historical v2 clients", () => { + it("keeps hover steps before menu clicks", () => { const { steps } = reduceTraceSteps( [ { @@ -167,11 +167,11 @@ describe("reduceTraceSteps", () => { ], "https://example.com/app", ); - expect(steps.map((s) => s.op)).toEqual(["click"]); + expect(steps.map((s) => s.op)).toEqual(["hover", "click"]); expect(steps[0]).toMatchObject({ - op: "click", + op: "hover", page: "p1", - target: { name: "Profile" }, + target: { name: "Account" }, }); }); diff --git a/apps/extension/src/lib/trace-reducer.ts b/apps/extension/src/lib/trace-reducer.ts index 2d5b0df..8f83fff 100644 --- a/apps/extension/src/lib/trace-reducer.ts +++ b/apps/extension/src/lib/trace-reducer.ts @@ -18,7 +18,6 @@ export function shouldRecordPress( } function shouldIncludeDraft(step: DraftTraceStep): boolean { - if (step.op === "hover") return false; if (step.op === "press" && !shouldRecordPress(step.key, step.modifiers)) return false; return true; } @@ -140,7 +139,12 @@ function toV2Step( effectForNavigation(step.navigated_to, urlToId), ); case "hover": - return null; + return { + op: "hover", + id, + page, + target: step.target, + }; case "fill": return { op: "fill", diff --git a/apps/extension/src/transport/types.ts b/apps/extension/src/transport/types.ts index e855f67..7d87667 100644 --- a/apps/extension/src/transport/types.ts +++ b/apps/extension/src/transport/types.ts @@ -793,6 +793,7 @@ export type DraftTraceStep = export type Step = | ({ op: "navigate" } & StepCommon & { to: string }) | ({ op: "click" } & StepCommon & { target: TargetDescriptor }) + | ({ op: "hover" } & StepCommon & { target: TargetDescriptor }) | ({ op: "fill" } & StepCommon & { target: TargetDescriptor; value: string; @@ -834,7 +835,7 @@ export type StepV3 = }) | ({ op: "select" } & StepCommonV3 & { target: TargetDescriptorV3; - selection: SelectedOption[]; + selection?: SelectedOption[]; }) | ({ op: "press" } & StepCommonV3 & { key: string; @@ -844,7 +845,7 @@ export type StepV3 = | ({ op: "scroll" } & StepCommonV3); export interface TraceV3 { - version: number; + version: 3; recorded_at: string; started_at?: string; purpose?: string; diff --git a/crates/bsk-protocol/schema/tool_record_await_result.json b/crates/bsk-protocol/schema/tool_record_await_result.json index 622e270..a16fdf8 100644 --- a/crates/bsk-protocol/schema/tool_record_await_result.json +++ b/crates/bsk-protocol/schema/tool_record_await_result.json @@ -512,6 +512,46 @@ } } }, + { + "description": "Fields shared by every v2 step variant (flattened in JSON).", + "type": "object", + "required": [ + "id", + "op", + "page", + "target" + ], + "properties": { + "effect": { + "anyOf": [ + { + "$ref": "#/definitions/StepEffectV2" + }, + { + "type": "null" + } + ] + }, + "id": { + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, + "op": { + "type": "string", + "enum": [ + "hover" + ] + }, + "page": { + "description": "Reference into `pages[]`.", + "type": "string" + }, + "target": { + "$ref": "#/definitions/TargetDescriptorV2" + } + } + }, { "description": "Fields shared by every v2 step variant (flattened in JSON).", "type": "object", @@ -803,10 +843,10 @@ }, "version": { "type": "integer", - "format": "uint32", - "minimum": 0.0 + "const": 3 } - } + }, + "additionalProperties": false }, "TraceEntry": { "description": "Recording entry point — first URL the flow starts from.", @@ -900,7 +940,8 @@ "$ref": "#/definitions/StepV2" } } - } + }, + "additionalProperties": false } } } diff --git a/crates/bsk-protocol/schema/tool_record_stop_result.json b/crates/bsk-protocol/schema/tool_record_stop_result.json index 49d84c0..6fe6d48 100644 --- a/crates/bsk-protocol/schema/tool_record_stop_result.json +++ b/crates/bsk-protocol/schema/tool_record_stop_result.json @@ -512,6 +512,46 @@ } } }, + { + "description": "Fields shared by every v2 step variant (flattened in JSON).", + "type": "object", + "required": [ + "id", + "op", + "page", + "target" + ], + "properties": { + "effect": { + "anyOf": [ + { + "$ref": "#/definitions/StepEffectV2" + }, + { + "type": "null" + } + ] + }, + "id": { + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, + "op": { + "type": "string", + "enum": [ + "hover" + ] + }, + "page": { + "description": "Reference into `pages[]`.", + "type": "string" + }, + "target": { + "$ref": "#/definitions/TargetDescriptorV2" + } + } + }, { "description": "Fields shared by every v2 step variant (flattened in JSON).", "type": "object", @@ -803,10 +843,10 @@ }, "version": { "type": "integer", - "format": "uint32", - "minimum": 0.0 + "const": 3 } - } + }, + "additionalProperties": false }, "TraceEntry": { "description": "Recording entry point — first URL the flow starts from.", @@ -900,7 +940,8 @@ "$ref": "#/definitions/StepV2" } } - } + }, + "additionalProperties": false } } } diff --git a/crates/bsk-protocol/schema/trace.json b/crates/bsk-protocol/schema/trace.json index f17a028..2abfd55 100644 --- a/crates/bsk-protocol/schema/trace.json +++ b/crates/bsk-protocol/schema/trace.json @@ -51,10 +51,10 @@ }, "version": { "type": "integer", - "format": "uint32", - "minimum": 0.0 + "const": 3 } }, + "additionalProperties": false, "definitions": { "FillCommit": { "type": "string", diff --git a/crates/bsk-protocol/schema/trace_v2.json b/crates/bsk-protocol/schema/trace_v2.json index 22b0b32..c301ee9 100644 --- a/crates/bsk-protocol/schema/trace_v2.json +++ b/crates/bsk-protocol/schema/trace_v2.json @@ -42,6 +42,7 @@ } } }, + "additionalProperties": false, "definitions": { "KeyModifier": { "description": "Keyboard modifier flags. Multiple flags may be combined; the extension folds them into CDP's bitfield (`alt=1, ctrl=2, meta=4, shift=8`).", @@ -189,6 +190,46 @@ } } }, + { + "description": "Fields shared by every v2 step variant (flattened in JSON).", + "type": "object", + "required": [ + "id", + "op", + "page", + "target" + ], + "properties": { + "effect": { + "anyOf": [ + { + "$ref": "#/definitions/StepEffectV2" + }, + { + "type": "null" + } + ] + }, + "id": { + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, + "op": { + "type": "string", + "enum": [ + "hover" + ] + }, + "page": { + "description": "Reference into `pages[]`.", + "type": "string" + }, + "target": { + "$ref": "#/definitions/TargetDescriptorV2" + } + } + }, { "description": "Fields shared by every v2 step variant (flattened in JSON).", "type": "object", diff --git a/crates/bsk-protocol/src/tools/record.rs b/crates/bsk-protocol/src/tools/record.rs index 74cd634..8111965 100644 --- a/crates/bsk-protocol/src/tools/record.rs +++ b/crates/bsk-protocol/src/tools/record.rs @@ -4,7 +4,7 @@ //! observations (VOM) captured before and after the action. use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use super::interaction::KeyModifier; @@ -17,6 +17,28 @@ pub const TRACE_VERSION_V2: u32 = 2; pub const DEFAULT_TRACE_VERSION: u32 = 2; pub const VOM_FORMAT_VERSION: u32 = 1; +fn deserialize_trace_v3_version<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + let version = u32::deserialize(deserializer)?; + if version != TRACE_VERSION { + return Err(serde::de::Error::custom(format!( + "unsupported trace version {version} (expected {TRACE_VERSION})" + ))); + } + Ok(version) +} + +fn trace_v3_version_schema(_: &mut schemars::r#gen::SchemaGenerator) -> schemars::schema::Schema { + schemars::schema::SchemaObject { + instance_type: Some(schemars::schema::InstanceType::Integer.into()), + const_value: Some(serde_json::json!(TRACE_VERSION)), + ..Default::default() + } + .into() +} + // --------------------------------------------------------------------------- // Target // --------------------------------------------------------------------------- @@ -179,7 +201,10 @@ pub enum Step { /// Persisted user-action trace exported by `tool.record_stop` / `await`. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields)] pub struct Trace { + #[serde(deserialize_with = "deserialize_trace_v3_version")] + #[schemars(schema_with = "trace_v3_version_schema")] pub version: u32, #[serde(default, skip_serializing_if = "Option::is_none")] pub purpose: Option, @@ -236,21 +261,22 @@ pub enum RecordedTrace { impl RecordedTrace { pub fn classify_value(v: &serde_json::Value) -> Result { if let Some(ver) = v.get("version").and_then(|x| x.as_u64()) { - if ver == u64::from(TRACE_VERSION) { - return serde_json::from_value(v.clone()) - .map(RecordedTrace::V3) - .map_err(|e| e.to_string()); + if ver != u64::from(TRACE_VERSION) { + return Err(format!("unsupported trace version {ver}")); + } + if v.get("pages").is_some() { + return Err("trace v3 must not include legacy pages[]".into()); } - return Err(format!("unsupported trace version {ver}")); - } - if v.get("pages").is_some() && v.get("states").is_none() { return serde_json::from_value(v.clone()) - .map(RecordedTrace::V2) + .map(RecordedTrace::V3) .map_err(|e| e.to_string()); } if v.get("states").is_some() { + return Err("trace v2 must not include states[]; set version: 3 for Trace v3".into()); + } + if v.get("pages").is_some() { return serde_json::from_value(v.clone()) - .map(RecordedTrace::V3) + .map(RecordedTrace::V2) .map_err(|e| e.to_string()); } Err("ambiguous or unparseable trace".into()) @@ -569,6 +595,42 @@ mod tests { } } + #[test] + fn recorded_trace_rejects_mixed_and_unsupported_versions() { + let v2_with_states = json!({ + "recorded_at": "2026-07-21T08:00:00Z", + "entry": { "start_url": "https://example.com/" }, + "pages": [{ "id": "p1", "url": "https://example.com/" }], + "states": [], + "steps": [] + }); + assert!(RecordedTrace::classify_value(&v2_with_states).is_err()); + + let v3_with_pages = json!({ + "version": 3, + "recorded_at": "2026-07-21T08:00:00Z", + "stopped_by": "user_finish", + "entry": { "start_url": "https://example.com/" }, + "recorder": { "bsk": "0.1.10", "vom": 1 }, + "pages": [{ "id": "p1", "url": "https://example.com/" }], + "states": [], + "steps": [] + }); + assert!(RecordedTrace::classify_value(&v3_with_pages).is_err()); + + let unsupported = json!({ + "version": 2, + "recorded_at": "2026-07-21T08:00:00Z", + "stopped_by": "user_finish", + "entry": { "start_url": "https://example.com/" }, + "recorder": { "bsk": "0.1.10", "vom": 1 }, + "states": [], + "steps": [] + }); + assert!(RecordedTrace::classify_value(&unsupported).is_err()); + assert!(serde_json::from_value::(unsupported).is_err()); + } + #[test] fn recorded_trace_schema_includes_v2_and_v3() { let schema = serde_json::to_value(schemars::schema_for!(RecordStopResult)).unwrap(); @@ -577,6 +639,8 @@ mod tests { .expect("RecordedTrace schema should use oneOf"); assert_eq!(variants.len(), 2); + let trace_schema = schema["definitions"]["Trace"].clone(); + assert_eq!(trace_schema["properties"]["version"]["const"], 3); } #[test] diff --git a/crates/bsk-protocol/src/tools/record_v2.rs b/crates/bsk-protocol/src/tools/record_v2.rs index 77b3a37..27f7f3f 100644 --- a/crates/bsk-protocol/src/tools/record_v2.rs +++ b/crates/bsk-protocol/src/tools/record_v2.rs @@ -75,6 +75,11 @@ pub enum StepV2 { common: StepCommonV2, target: TargetDescriptorV2, }, + Hover { + #[serde(flatten)] + common: StepCommonV2, + target: TargetDescriptorV2, + }, Fill { #[serde(flatten)] common: StepCommonV2, @@ -102,6 +107,7 @@ pub enum StepV2 { /// Persisted user-action trace exported by legacy `tool.record_stop` / `await`. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields)] pub struct TraceV2 { /// RFC 3339 timestamp when recording stopped. pub recorded_at: String, @@ -197,7 +203,7 @@ mod tests { } #[test] - fn historical_v2_rejects_hover_steps() { + fn v2_hover_steps_round_trip() { let value = json!({ "recorded_at": "2026-07-21T08:00:00Z", "entry": { "start_url": "https://example.com/" }, @@ -206,10 +212,15 @@ mod tests { "op": "hover", "id": 1, "page": "p1", - "target": { "tag": "button", "name": "结束" } + "target": { "tag": "span", "role": "button", "name": "Account" } }] }); - assert!(serde_json::from_value::(value).is_err()); + let trace: TraceV2 = serde_json::from_value(value).unwrap(); + assert!(matches!(trace.steps.as_slice(), [StepV2::Hover { .. }])); + assert_eq!( + serde_json::to_value(&trace).unwrap()["steps"][0]["op"], + "hover" + ); } }