Skip to content

Commit 8e3ad97

Browse files
benbrandtbenbrandt
authored
fix(unstable-v2): keep optional fields of unknown variants verbatim (#2561)
1 parent 97de389 commit 8e3ad97

6 files changed

Lines changed: 49 additions & 207 deletions

File tree

‎agent-client-protocol-schema/src/v2/agent.rs‎

Lines changed: 35 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,10 @@ impl AuthMethod {
551551
pub fn description(&self) -> Option<&str> {
552552
match self {
553553
Self::Agent(a) => a.description.as_deref(),
554-
Self::Other(a) => a.description.as_deref(),
554+
Self::Other(a) => a
555+
.fields
556+
.get("description")
557+
.and_then(serde_json::Value::as_str),
555558
Self::Terminal(t) => t.description.as_deref(),
556559
}
557560
}
@@ -565,7 +568,7 @@ impl AuthMethod {
565568
pub fn meta(&self) -> Option<&Meta> {
566569
match self {
567570
Self::Agent(a) => a.meta.as_ref(),
568-
Self::Other(a) => a.meta.as_ref(),
571+
Self::Other(a) => a.fields.get("_meta").and_then(serde_json::Value::as_object),
569572
Self::Terminal(t) => t.meta.as_ref(),
570573
}
571574
}
@@ -592,22 +595,8 @@ pub struct OtherAuthMethod {
592595
pub method_id: AuthMethodId,
593596
/// Human-readable name of the authentication method.
594597
pub name: String,
595-
/// Optional description providing more details about this authentication method.
596-
#[serde_as(deserialize_as = "DefaultOnError")]
597-
#[cfg_attr(feature = "schemars", schemars(extend("x-deserialize-default-on-error" = true)))]
598-
#[serde(default)]
599-
pub description: Option<String>,
600-
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
601-
/// metadata to their interactions. Omitted and `null` are equivalent and mean no metadata.
602-
/// Implementations MUST NOT make assumptions about values at these keys.
603-
///
604-
/// See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/extensibility)
605-
#[serde_as(deserialize_as = "DefaultOnError")]
606-
#[cfg_attr(feature = "schemars", schemars(extend("x-deserialize-default-on-error" = true)))]
607-
#[serde(default)]
608-
#[serde(rename = "_meta")]
609-
pub meta: Option<Meta>,
610-
/// Additional fields from the unknown authentication method payload.
598+
/// The remaining fields of the unknown authentication method payload, preserved verbatim,
599+
/// including `description` and `_meta`.
611600
#[serde(flatten)]
612601
pub fields: BTreeMap<String, serde_json::Value>,
613602
}
@@ -624,35 +613,13 @@ impl OtherAuthMethod {
624613
fields.remove("type");
625614
fields.remove("methodId");
626615
fields.remove("name");
627-
fields.remove("description");
628-
fields.remove("_meta");
629616
Self {
630617
type_: type_.into(),
631618
method_id: method_id.into(),
632619
name: name.into(),
633-
description: None,
634-
meta: None,
635620
fields,
636621
}
637622
}
638-
639-
/// Optional description providing more details about this authentication method.
640-
#[must_use]
641-
pub fn description(mut self, description: impl IntoOption<String>) -> Self {
642-
self.description = description.into_option();
643-
self
644-
}
645-
646-
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
647-
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at
648-
/// these keys.
649-
///
650-
/// See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/extensibility)
651-
#[must_use]
652-
pub fn meta(mut self, meta: impl IntoOption<Meta>) -> Self {
653-
self.meta = meta.into_option();
654-
self
655-
}
656623
}
657624

658625
impl<'de> Deserialize<'de> for OtherAuthMethod {
@@ -667,9 +634,6 @@ impl<'de> Deserialize<'de> for OtherAuthMethod {
667634
type_: String,
668635
method_id: AuthMethodId,
669636
name: String,
670-
description: Option<String>,
671-
#[serde(rename = "_meta")]
672-
meta: Option<Meta>,
673637
#[serde(flatten)]
674638
fields: BTreeMap<String, serde_json::Value>,
675639
}
@@ -686,8 +650,6 @@ impl<'de> Deserialize<'de> for OtherAuthMethod {
686650
type_: raw.type_,
687651
method_id: raw.method_id,
688652
name: raw.name,
689-
description: raw.description,
690-
meta: raw.meta,
691653
fields: raw.fields,
692654
})
693655
}
@@ -1477,17 +1439,8 @@ pub struct OtherReplayFrom {
14771439
/// future ACP variants.
14781440
#[serde(rename = "type")]
14791441
pub type_: String,
1480-
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
1481-
/// metadata to their interactions. Omitted and `null` are equivalent and mean no metadata.
1482-
/// Implementations MUST NOT make assumptions about values at these keys.
1483-
///
1484-
/// See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/extensibility)
1485-
#[serde_as(deserialize_as = "DefaultOnError")]
1486-
#[cfg_attr(feature = "schemars", schemars(extend("x-deserialize-default-on-error" = true)))]
1487-
#[serde(default)]
1488-
#[serde(rename = "_meta")]
1489-
pub meta: Option<Meta>,
1490-
/// Additional fields from the unknown replay cursor payload.
1442+
/// The remaining fields of the unknown replay cursor payload, preserved verbatim,
1443+
/// including `_meta`.
14911444
#[serde(flatten)]
14921445
pub fields: BTreeMap<String, serde_json::Value>,
14931446
}
@@ -1497,24 +1450,11 @@ impl OtherReplayFrom {
14971450
#[must_use]
14981451
pub fn new(type_: impl Into<String>, mut fields: BTreeMap<String, serde_json::Value>) -> Self {
14991452
fields.remove("type");
1500-
fields.remove("_meta");
15011453
Self {
15021454
type_: type_.into(),
1503-
meta: None,
15041455
fields,
15051456
}
15061457
}
1507-
1508-
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
1509-
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at
1510-
/// these keys.
1511-
///
1512-
/// See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/extensibility)
1513-
#[must_use]
1514-
pub fn meta(mut self, meta: impl IntoOption<Meta>) -> Self {
1515-
self.meta = meta.into_option();
1516-
self
1517-
}
15181458
}
15191459

15201460
impl<'de> Deserialize<'de> for OtherReplayFrom {
@@ -1536,15 +1476,7 @@ impl<'de> Deserialize<'de> for OtherReplayFrom {
15361476
)));
15371477
}
15381478

1539-
let meta = fields
1540-
.remove("_meta")
1541-
.and_then(|value| serde_json::from_value(value).ok());
1542-
1543-
Ok(Self {
1544-
type_,
1545-
meta,
1546-
fields,
1547-
})
1479+
Ok(Self { type_, fields })
15481480
}
15491481
}
15501482

@@ -5913,6 +5845,31 @@ mod test_serialization {
59135845
json!({ "source": "test" }).as_object().unwrap().clone()
59145846
}
59155847

5848+
#[test]
5849+
fn other_fallbacks_preserve_optional_fields_verbatim() {
5850+
let valid = json!({ "type": "_custom", "methodId": "m", "name": "M",
5851+
"description": "d", "_meta": { "k": 1 } });
5852+
let method: AuthMethod = serde_json::from_value(valid.clone()).unwrap();
5853+
assert_eq!(method.description(), Some("d"));
5854+
assert_eq!(
5855+
method.meta(),
5856+
Some(&json!({ "k": 1 }).as_object().unwrap().clone())
5857+
);
5858+
assert_eq!(serde_json::to_value(&method).unwrap(), valid);
5859+
5860+
// Malformed optional fields are kept as sent and read as absent.
5861+
let malformed = json!({ "type": "_custom", "methodId": "m", "name": "M",
5862+
"description": 42, "_meta": "bad", "extra": true });
5863+
let method: AuthMethod = serde_json::from_value(malformed.clone()).unwrap();
5864+
assert_eq!(method.description(), None);
5865+
assert_eq!(method.meta(), None);
5866+
assert_eq!(serde_json::to_value(&method).unwrap(), malformed);
5867+
5868+
let replay = json!({ "type": "_custom", "_meta": "bad", "cursor": 3 });
5869+
let decoded: ReplayFrom = serde_json::from_value(replay.clone()).unwrap();
5870+
assert_eq!(serde_json::to_value(&decoded).unwrap(), replay);
5871+
}
5872+
59165873
fn serialized_meta_key_count(value: &impl serde::Serialize) -> usize {
59175874
serde_json::to_string(value)
59185875
.unwrap()

‎agent-client-protocol-schema/src/v2/client.rs‎

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2360,7 +2360,7 @@ impl AvailableCommand {
23602360
pub fn meta(&self) -> Option<&Meta> {
23612361
match self {
23622362
Self::Command(c) => c.meta.as_ref(),
2363-
Self::Other(c) => c.meta.as_ref(),
2363+
Self::Other(c) => c.fields.get("_meta").and_then(serde_json::Value::as_object),
23642364
}
23652365
}
23662366
}
@@ -2386,17 +2386,8 @@ pub struct OtherAvailableCommand {
23862386
pub name: String,
23872387
/// Human-readable description of what the command does.
23882388
pub description: String,
2389-
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
2390-
/// metadata to their interactions. Omitted and `null` are equivalent and mean no metadata.
2391-
/// Implementations MUST NOT make assumptions about values at these keys.
2392-
///
2393-
/// See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/extensibility)
2394-
#[serde_as(deserialize_as = "DefaultOnError")]
2395-
#[cfg_attr(feature = "schemars", schemars(extend("x-deserialize-default-on-error" = true)))]
2396-
#[serde(default)]
2397-
#[serde(rename = "_meta")]
2398-
pub meta: Option<Meta>,
2399-
/// Additional fields from the unknown command payload.
2389+
/// The remaining fields of the unknown command payload, preserved verbatim, including
2390+
/// `_meta`.
24002391
#[serde(flatten)]
24012392
pub fields: BTreeMap<String, serde_json::Value>,
24022393
}
@@ -2413,26 +2404,13 @@ impl OtherAvailableCommand {
24132404
fields.remove("type");
24142405
fields.remove("name");
24152406
fields.remove("description");
2416-
fields.remove("_meta");
24172407
Self {
24182408
type_: type_.into(),
24192409
name: name.into(),
24202410
description: description.into(),
2421-
meta: None,
24222411
fields,
24232412
}
24242413
}
2425-
2426-
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
2427-
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at
2428-
/// these keys.
2429-
///
2430-
/// See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/extensibility)
2431-
#[must_use]
2432-
pub fn meta(mut self, meta: impl IntoOption<Meta>) -> Self {
2433-
self.meta = meta.into_option();
2434-
self
2435-
}
24362414
}
24372415

24382416
impl<'de> Deserialize<'de> for OtherAvailableCommand {
@@ -2446,8 +2424,6 @@ impl<'de> Deserialize<'de> for OtherAvailableCommand {
24462424
type_: String,
24472425
name: String,
24482426
description: String,
2449-
#[serde(default, rename = "_meta")]
2450-
meta: Option<serde_json::Value>,
24512427
#[serde(flatten)]
24522428
fields: BTreeMap<String, serde_json::Value>,
24532429
}
@@ -2464,8 +2440,6 @@ impl<'de> Deserialize<'de> for OtherAvailableCommand {
24642440
type_: raw.type_,
24652441
name: raw.name,
24662442
description: raw.description,
2467-
// Match the `DefaultOnError` handling of `_meta` on known commands.
2468-
meta: raw.meta.and_then(|meta| serde_json::from_value(meta).ok()),
24692443
fields: raw.fields,
24702444
})
24712445
}
@@ -3710,6 +3684,17 @@ impl AgentNotification {
37103684
#[cfg(test)]
37113685
mod tests {
37123686

3687+
#[test]
3688+
fn other_available_command_preserves_meta_verbatim() {
3689+
for meta in [serde_json::json!({ "k": 1 }), serde_json::json!("bad")] {
3690+
let wire = serde_json::json!({ "type": "_custom", "name": "c",
3691+
"description": "d", "_meta": meta, "extra": true });
3692+
let command: super::AvailableCommand = serde_json::from_value(wire.clone()).unwrap();
3693+
assert_eq!(command.meta(), meta.as_object());
3694+
assert_eq!(serde_json::to_value(&command).unwrap(), wire);
3695+
}
3696+
}
3697+
37133698
#[test]
37143699
fn cancelled_permission_outcome_wire_format() {
37153700
let outcome: RequestPermissionOutcome =

‎docs/protocol/v2/draft/schema.mdx‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2127,17 +2127,6 @@ data, and otherwise ignore the method or display it generically.
21272127

21282128
<Expandable title="Properties">
21292129

2130-
<ResponseField name="_meta" type={"object | null"} >
2131-
The _meta property is reserved by ACP to allow clients and agents to attach additional
2132-
metadata to their interactions. Omitted and `null` are equivalent and mean no metadata.
2133-
Implementations MUST NOT make assumptions about values at these keys.
2134-
2135-
See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/draft/extensibility)
2136-
2137-
</ResponseField>
2138-
<ResponseField name="description" type={"string | null"} >
2139-
Optional description providing more details about this authentication method.
2140-
</ResponseField>
21412130
<ResponseField name="methodId" type={<a href="#authmethodid">AuthMethodId</a>} required>
21422131
Unique identifier for this authentication method.
21432132
</ResponseField>
@@ -2281,14 +2270,6 @@ command using its `name` and `description`.
22812270

22822271
<Expandable title="Properties">
22832272

2284-
<ResponseField name="_meta" type={"object | null"} >
2285-
The _meta property is reserved by ACP to allow clients and agents to attach additional
2286-
metadata to their interactions. Omitted and `null` are equivalent and mean no metadata.
2287-
Implementations MUST NOT make assumptions about values at these keys.
2288-
2289-
See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/draft/extensibility)
2290-
2291-
</ResponseField>
22922273
<ResponseField name="description" type={"string"} required>
22932274
Human-readable description of what the command does.
22942275
</ResponseField>
@@ -5970,14 +5951,6 @@ substituting another cursor or resuming without replay.
59705951

59715952
<Expandable title="Properties">
59725953

5973-
<ResponseField name="_meta" type={"object | null"} >
5974-
The _meta property is reserved by ACP to allow clients and agents to attach additional
5975-
metadata to their interactions. Omitted and `null` are equivalent and mean no metadata.
5976-
Implementations MUST NOT make assumptions about values at these keys.
5977-
5978-
See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/draft/extensibility)
5979-
5980-
</ResponseField>
59815954
<ResponseField name="type" type={"string"} required>
59825955
Custom or future replay cursor type.
59835956

‎docs/protocol/v2/schema.mdx‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,17 +1662,6 @@ data, and otherwise ignore the method or display it generically.
16621662

16631663
<Expandable title="Properties">
16641664

1665-
<ResponseField name="_meta" type={"object | null"} >
1666-
The _meta property is reserved by ACP to allow clients and agents to attach additional
1667-
metadata to their interactions. Omitted and `null` are equivalent and mean no metadata.
1668-
Implementations MUST NOT make assumptions about values at these keys.
1669-
1670-
See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/extensibility)
1671-
1672-
</ResponseField>
1673-
<ResponseField name="description" type={"string | null"} >
1674-
Optional description providing more details about this authentication method.
1675-
</ResponseField>
16761665
<ResponseField name="methodId" type={<a href="#authmethodid">AuthMethodId</a>} required>
16771666
Unique identifier for this authentication method.
16781667
</ResponseField>
@@ -1816,14 +1805,6 @@ command using its `name` and `description`.
18161805

18171806
<Expandable title="Properties">
18181807

1819-
<ResponseField name="_meta" type={"object | null"} >
1820-
The _meta property is reserved by ACP to allow clients and agents to attach additional
1821-
metadata to their interactions. Omitted and `null` are equivalent and mean no metadata.
1822-
Implementations MUST NOT make assumptions about values at these keys.
1823-
1824-
See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/extensibility)
1825-
1826-
</ResponseField>
18271808
<ResponseField name="description" type={"string"} required>
18281809
Human-readable description of what the command does.
18291810
</ResponseField>
@@ -5057,14 +5038,6 @@ substituting another cursor or resuming without replay.
50575038

50585039
<Expandable title="Properties">
50595040

5060-
<ResponseField name="_meta" type={"object | null"} >
5061-
The _meta property is reserved by ACP to allow clients and agents to attach additional
5062-
metadata to their interactions. Omitted and `null` are equivalent and mean no metadata.
5063-
Implementations MUST NOT make assumptions about values at these keys.
5064-
5065-
See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/extensibility)
5066-
5067-
</ResponseField>
50685041
<ResponseField name="type" type={"string"} required>
50695042
Custom or future replay cursor type.
50705043

0 commit comments

Comments
 (0)