mirror of
https://github.com/Tencent/BrowserSkill.git
synced 2026-09-10 13:01:23 +08:00
fix(protocol): align trace schemas with v2/v3 classify rules
RecordedTrace validation now follows classify_value — numeric version selects v3, otherwise v2 — so mixed pages/states and v2 envelopes with a version field fail Schema checks before Recall import. Keep standalone trace_step as a v2|v3 union for tools that validate a single step.
This commit is contained in:
@@ -16,6 +16,7 @@ pub use super::record_v3::{
|
||||
VOM_FORMAT_VERSION,
|
||||
};
|
||||
|
||||
/// Logical v2 identifier. Not a wire field — v2 envelopes omit `version`.
|
||||
pub const TRACE_VERSION_V2: u32 = 2;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -45,6 +46,10 @@ pub struct RecordStopParams {
|
||||
}
|
||||
|
||||
/// Wire trace payload — v2 (legacy `pages[]`) or v3 (`version: 3`, `states[]`).
|
||||
///
|
||||
/// Classification matches [`RecordedTrace::classify_value`]: a numeric `version`
|
||||
/// selects v3 (and forbids `pages[]`); otherwise the envelope is v2 and must
|
||||
/// not include `states[]`.
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub enum RecordedTrace {
|
||||
V2(TraceV2),
|
||||
@@ -104,12 +109,134 @@ impl JsonSchema for RecordedTrace {
|
||||
}
|
||||
|
||||
fn json_schema(generator: &mut schemars::SchemaGenerator) -> schemars::schema::Schema {
|
||||
// Mirror classify_value: a numeric `version` selects v3; otherwise v2.
|
||||
// TraceV2 stays open for unknown extension fields but forbids `states[]`
|
||||
// and a numeric `version`. TraceV3 already denies `pages[]`.
|
||||
let mut version_props = schemars::Map::new();
|
||||
version_props.insert(
|
||||
"version".into(),
|
||||
schemars::schema::SchemaObject {
|
||||
instance_type: Some(schemars::schema::InstanceType::Integer.into()),
|
||||
..Default::default()
|
||||
}
|
||||
.into(),
|
||||
);
|
||||
let mut required = schemars::Set::new();
|
||||
required.insert("version".into());
|
||||
|
||||
schemars::schema::SchemaObject {
|
||||
subschemas: Some(Box::new(schemars::schema::SubschemaValidation {
|
||||
one_of: Some(vec![
|
||||
generator.subschema_for::<TraceV2>(),
|
||||
generator.subschema_for::<TraceV3>(),
|
||||
]),
|
||||
if_schema: Some(Box::new(
|
||||
schemars::schema::SchemaObject {
|
||||
object: Some(Box::new(schemars::schema::ObjectValidation {
|
||||
properties: version_props,
|
||||
required,
|
||||
..Default::default()
|
||||
})),
|
||||
..Default::default()
|
||||
}
|
||||
.into(),
|
||||
)),
|
||||
then_schema: Some(Box::new(generator.subschema_for::<TraceV3>())),
|
||||
else_schema: Some(Box::new(generator.subschema_for::<TraceV2>())),
|
||||
..Default::default()
|
||||
})),
|
||||
..Default::default()
|
||||
}
|
||||
.into()
|
||||
}
|
||||
}
|
||||
|
||||
/// One recorded step — v2 (`page`) or v3 (`state` / `result.state`).
|
||||
///
|
||||
/// Classification matches [`RecordedStep::classify_value`]: `state` selects v3
|
||||
/// (and forbids `page`); otherwise the step is v2 and must include `page`.
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub enum RecordedStep {
|
||||
V2(StepV2),
|
||||
V3(StepV3),
|
||||
}
|
||||
|
||||
impl RecordedStep {
|
||||
pub fn classify_value(v: &serde_json::Value) -> Result<Self, String> {
|
||||
if v.get("state").is_some() {
|
||||
if v.get("page").is_some() {
|
||||
return Err("step v3 must not include legacy page".into());
|
||||
}
|
||||
return serde_json::from_value(v.clone())
|
||||
.map(RecordedStep::V3)
|
||||
.map_err(|e| e.to_string());
|
||||
}
|
||||
if v.get("page").is_some() {
|
||||
return serde_json::from_value(v.clone())
|
||||
.map(RecordedStep::V2)
|
||||
.map_err(|e| e.to_string());
|
||||
}
|
||||
Err("ambiguous or unparseable step".into())
|
||||
}
|
||||
}
|
||||
|
||||
impl Serialize for RecordedStep {
|
||||
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
|
||||
where
|
||||
S: serde::Serializer,
|
||||
{
|
||||
match self {
|
||||
RecordedStep::V2(s) => s.serialize(serializer),
|
||||
RecordedStep::V3(s) => s.serialize(serializer),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for RecordedStep {
|
||||
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
let value = serde_json::Value::deserialize(deserializer)?;
|
||||
Self::classify_value(&value).map_err(serde::de::Error::custom)
|
||||
}
|
||||
}
|
||||
|
||||
impl JsonSchema for RecordedStep {
|
||||
fn schema_name() -> String {
|
||||
"RecordedStep".into()
|
||||
}
|
||||
|
||||
fn json_schema(generator: &mut schemars::SchemaGenerator) -> schemars::schema::Schema {
|
||||
// Mirror classify_value: `state` selects v3; otherwise v2.
|
||||
// Mixed page + state is rejected at this layer because StepV2/StepV3
|
||||
// still allow unknown extension fields.
|
||||
let mut required = schemars::Set::new();
|
||||
required.insert("state".into());
|
||||
let mut mixed_keys = schemars::Set::new();
|
||||
mixed_keys.insert("page".into());
|
||||
mixed_keys.insert("state".into());
|
||||
|
||||
schemars::schema::SchemaObject {
|
||||
subschemas: Some(Box::new(schemars::schema::SubschemaValidation {
|
||||
if_schema: Some(Box::new(
|
||||
schemars::schema::SchemaObject {
|
||||
object: Some(Box::new(schemars::schema::ObjectValidation {
|
||||
required,
|
||||
..Default::default()
|
||||
})),
|
||||
..Default::default()
|
||||
}
|
||||
.into(),
|
||||
)),
|
||||
then_schema: Some(Box::new(generator.subschema_for::<StepV3>())),
|
||||
else_schema: Some(Box::new(generator.subschema_for::<StepV2>())),
|
||||
not: Some(Box::new(
|
||||
schemars::schema::SchemaObject {
|
||||
object: Some(Box::new(schemars::schema::ObjectValidation {
|
||||
required: mixed_keys,
|
||||
..Default::default()
|
||||
})),
|
||||
..Default::default()
|
||||
}
|
||||
.into(),
|
||||
)),
|
||||
..Default::default()
|
||||
})),
|
||||
..Default::default()
|
||||
@@ -204,26 +331,43 @@ mod tests {
|
||||
assert!(serde_json::from_value::<TraceV3>(unsupported).is_err());
|
||||
}
|
||||
|
||||
fn assert_classifies_by_integer_version(schema: &serde_json::Value) {
|
||||
assert_eq!(
|
||||
schema["if"]["required"],
|
||||
json!(["version"]),
|
||||
"numeric version is the RecordedTrace classification key"
|
||||
);
|
||||
assert_eq!(schema["if"]["properties"]["version"]["type"], "integer");
|
||||
assert_eq!(schema["then"]["$ref"], "#/definitions/TraceV3");
|
||||
assert_eq!(schema["else"]["$ref"], "#/definitions/TraceV2");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recorded_trace_schema_includes_v2_and_v3() {
|
||||
let schema = serde_json::to_value(schemars::schema_for!(RecordStopResult)).unwrap();
|
||||
let variants = schema["definitions"]["RecordedTrace"]["oneOf"]
|
||||
.as_array()
|
||||
.expect("RecordedTrace schema should use oneOf");
|
||||
assert_classifies_by_integer_version(&schema["definitions"]["RecordedTrace"]);
|
||||
assert_eq!(
|
||||
schema["definitions"]["TraceV3"]["properties"]["version"]["const"],
|
||||
3
|
||||
);
|
||||
}
|
||||
|
||||
assert_eq!(variants.len(), 2);
|
||||
let trace_schema = schema["definitions"]["TraceV3"].clone();
|
||||
assert_eq!(trace_schema["properties"]["version"]["const"], 3);
|
||||
#[test]
|
||||
fn recorded_trace_schema_follows_classify_value() {
|
||||
let standalone = serde_json::to_value(schemars::schema_for!(RecordedTrace)).unwrap();
|
||||
assert_classifies_by_integer_version(&standalone);
|
||||
assert!(standalone["definitions"].get("TraceV2").is_some());
|
||||
assert!(standalone["definitions"].get("TraceV3").is_some());
|
||||
|
||||
let stop_result = serde_json::to_value(schemars::schema_for!(RecordStopResult)).unwrap();
|
||||
assert_classifies_by_integer_version(&stop_result["definitions"]["RecordedTrace"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn standalone_trace_schema_is_recorded_trace_union() {
|
||||
let schema = serde_json::to_value(schemars::schema_for!(RecordedTrace)).unwrap();
|
||||
assert_eq!(schema["title"], "RecordedTrace");
|
||||
let variants = schema["oneOf"]
|
||||
.as_array()
|
||||
.expect("standalone trace schema should be a v2|v3 oneOf");
|
||||
assert_eq!(variants.len(), 2);
|
||||
assert_classifies_by_integer_version(&schema);
|
||||
assert!(schema["definitions"].get("TraceV2").is_some());
|
||||
assert!(schema["definitions"].get("TraceV3").is_some());
|
||||
}
|
||||
@@ -242,4 +386,70 @@ mod tests {
|
||||
other => panic!("expected v2, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
fn v2_click() -> serde_json::Value {
|
||||
json!({
|
||||
"op": "click",
|
||||
"id": 1,
|
||||
"page": "p1",
|
||||
"target": { "tag": "button", "role": "button", "name": "发布" }
|
||||
})
|
||||
}
|
||||
|
||||
fn v3_click() -> serde_json::Value {
|
||||
json!({
|
||||
"op": "click",
|
||||
"id": 1,
|
||||
"state": "s1",
|
||||
"result": { "state": "s2" },
|
||||
"target": { "ref": "e1", "role": "button", "name": "发布" }
|
||||
})
|
||||
}
|
||||
|
||||
fn assert_step_classifies_by_state(schema: &serde_json::Value) {
|
||||
assert_eq!(
|
||||
schema["if"]["required"],
|
||||
json!(["state"]),
|
||||
"state is the RecordedStep classification key"
|
||||
);
|
||||
assert_eq!(schema["then"]["$ref"], "#/definitions/StepV3");
|
||||
assert_eq!(schema["else"]["$ref"], "#/definitions/StepV2");
|
||||
let mut forbidden = schema["not"]["required"]
|
||||
.as_array()
|
||||
.expect("RecordedStep schema must reject mixed page + state steps")
|
||||
.iter()
|
||||
.filter_map(|v| v.as_str())
|
||||
.collect::<Vec<_>>();
|
||||
forbidden.sort_unstable();
|
||||
assert_eq!(forbidden, ["page", "state"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recorded_step_classifies_v2_and_v3() {
|
||||
match RecordedStep::classify_value(&v2_click()).unwrap() {
|
||||
RecordedStep::V2(StepV2::Click { .. }) => {}
|
||||
other => panic!("expected v2 click, got {other:?}"),
|
||||
}
|
||||
match RecordedStep::classify_value(&v3_click()).unwrap() {
|
||||
RecordedStep::V3(StepV3::Click { .. }) => {}
|
||||
other => panic!("expected v3 click, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recorded_step_rejects_mixed_page_and_state() {
|
||||
let mut mixed = v2_click();
|
||||
mixed["state"] = json!("s1");
|
||||
mixed["result"] = json!({ "state": "s2" });
|
||||
assert!(RecordedStep::classify_value(&mixed).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recorded_step_schema_follows_classify_value() {
|
||||
let schema = serde_json::to_value(schemars::schema_for!(RecordedStep)).unwrap();
|
||||
assert_eq!(schema["title"], "RecordedStep");
|
||||
assert_step_classifies_by_state(&schema);
|
||||
assert!(schema["definitions"].get("StepV2").is_some());
|
||||
assert!(schema["definitions"].get("StepV3").is_some());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -107,8 +107,9 @@ pub enum StepV2 {
|
||||
/// Persisted user-action trace exported by legacy `tool.record_stop` / `await`.
|
||||
///
|
||||
/// Unknown extension fields are ignored so older traces remain readable.
|
||||
/// Mixed v2/v3 envelopes are rejected by `RecordedTrace` classification.
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)]
|
||||
/// `states[]` and a numeric `version` are reserved for Trace v3 / `RecordedTrace`
|
||||
/// classification — serde still ignores them, but the JSON Schema rejects them.
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub struct TraceV2 {
|
||||
/// RFC 3339 timestamp when recording stopped.
|
||||
pub recorded_at: String,
|
||||
@@ -121,6 +122,70 @@ pub struct TraceV2 {
|
||||
pub steps: Vec<StepV2>,
|
||||
}
|
||||
|
||||
fn constrain_trace_v2_schema(mut schema: schemars::schema::Schema) -> schemars::schema::Schema {
|
||||
let schemars::schema::Schema::Object(obj) = &mut schema else {
|
||||
return schema;
|
||||
};
|
||||
obj.metadata().title = Some("TraceV2".into());
|
||||
obj.metadata().description = Some(
|
||||
"Persisted user-action trace exported by legacy `tool.record_stop` / `await`.\n\n\
|
||||
Unknown extension fields are ignored so older traces remain readable. \
|
||||
`states[]` and a numeric `version` are reserved for Trace v3 / `RecordedTrace` classification."
|
||||
.into(),
|
||||
);
|
||||
|
||||
let object = obj.object.get_or_insert_with(Default::default);
|
||||
object
|
||||
.properties
|
||||
.insert("states".into(), schemars::schema::Schema::Bool(false));
|
||||
object.properties.insert(
|
||||
"version".into(),
|
||||
schemars::schema::SchemaObject {
|
||||
metadata: Some(Box::new(schemars::schema::Metadata {
|
||||
description: Some(
|
||||
"Numeric version selects Trace v3. Legacy v2 envelopes omit this field.".into(),
|
||||
),
|
||||
..Default::default()
|
||||
})),
|
||||
subschemas: Some(Box::new(schemars::schema::SubschemaValidation {
|
||||
not: Some(Box::new(
|
||||
schemars::schema::SchemaObject {
|
||||
instance_type: Some(schemars::schema::InstanceType::Integer.into()),
|
||||
..Default::default()
|
||||
}
|
||||
.into(),
|
||||
)),
|
||||
..Default::default()
|
||||
})),
|
||||
..Default::default()
|
||||
}
|
||||
.into(),
|
||||
);
|
||||
schema
|
||||
}
|
||||
|
||||
impl JsonSchema for TraceV2 {
|
||||
fn schema_name() -> String {
|
||||
"TraceV2".into()
|
||||
}
|
||||
|
||||
fn json_schema(generator: &mut schemars::SchemaGenerator) -> schemars::schema::Schema {
|
||||
#[derive(JsonSchema)]
|
||||
#[allow(dead_code)]
|
||||
struct TraceV2Shape {
|
||||
/// RFC 3339 timestamp when recording stopped.
|
||||
recorded_at: String,
|
||||
started_at: Option<String>,
|
||||
purpose: Option<String>,
|
||||
entry: TraceEntry,
|
||||
pages: Vec<PageRefV2>,
|
||||
steps: Vec<StepV2>,
|
||||
}
|
||||
|
||||
constrain_trace_v2_schema(TraceV2Shape::json_schema(generator))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -248,4 +313,18 @@ mod tests {
|
||||
"Trace v2 must keep accepting traces with unknown extension fields"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trace_v2_schema_forbids_classification_keys() {
|
||||
let schema = serde_json::to_value(schemars::schema_for!(TraceV2)).unwrap();
|
||||
assert_eq!(
|
||||
schema["properties"]["states"],
|
||||
json!(false),
|
||||
"Trace v2 schema must reject states[] (reserved for v3)"
|
||||
);
|
||||
assert_eq!(
|
||||
schema["properties"]["version"]["not"]["type"], "integer",
|
||||
"Trace v2 schema must reject a numeric version (that selects the v3 path)"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user