Skip to content

Commit 1a873bb

Browse files
committed
Add optional tracing for skipped list entries
Introduce a `tracing` feature that logs `VecSkipError` drops via a shared `SkipListener` inspector, while remaining a zero-cost no-op when the feature is disabled. Also add tests covering inspector callbacks for malformed entries.
1 parent aa60197 commit 1a873bb

9 files changed

Lines changed: 138 additions & 34 deletions

File tree

Cargo.lock

Lines changed: 23 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ unstable_session_usage = []
4545
unstable_message_id = []
4646
unstable_boolean_config = []
4747

48+
# Emit `tracing::warn!` events when `VecSkipError` drops a malformed list
49+
# entry during deserialization. When disabled (the default), the inspector
50+
# hook compiles down to serde_with's built-in no-op and has zero runtime cost.
51+
tracing = ["dep:tracing"]
52+
4853
[[bin]]
4954
name = "generate"
5055
path = "src/bin/generate.rs"
@@ -57,6 +62,7 @@ serde = { version = "1", features = ["derive", "rc"] }
5762
serde_json = { version = "1", features = ["raw_value"] }
5863
serde_with = { version = "3.18.0", features = ["json", "schemars_1"] }
5964
strum = { version = "0.28", features = ["derive"] }
65+
tracing = { version = "0.1", default-features = false, optional = true }
6066

6167
[lints.rust]
6268
future_incompatible = { level = "warn", priority = -1 }

src/agent.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use serde_with::{DefaultOnError, VecSkipError, serde_as, skip_serializing_none};
1717
use crate::RequiredNullable;
1818
use crate::{
1919
ClientCapabilities, ContentBlock, ExtNotification, ExtRequest, ExtResponse, IntoOption, Meta,
20-
ProtocolVersion, SessionId,
20+
ProtocolVersion, SessionId, SkipListener,
2121
};
2222

2323
#[cfg(feature = "unstable_nes")]
@@ -128,7 +128,7 @@ pub struct InitializeResponse {
128128
#[serde(default)]
129129
pub agent_capabilities: AgentCapabilities,
130130
/// Authentication methods supported by the agent.
131-
#[serde_as(deserialize_as = "VecSkipError<_>")]
131+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
132132
#[serde(default)]
133133
pub auth_methods: Vec<AuthMethod>,
134134
/// Information about the Agent name and version sent to the Client.
@@ -1024,7 +1024,7 @@ pub struct NewSessionResponse {
10241024
#[serde(default)]
10251025
pub models: Option<SessionModelState>,
10261026
/// Initial session configuration options if supported by the Agent.
1027-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1027+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
10281028
#[serde(default)]
10291029
pub config_options: Option<Vec<SessionConfigOption>>,
10301030
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
@@ -1200,7 +1200,7 @@ pub struct LoadSessionResponse {
12001200
#[serde(default)]
12011201
pub models: Option<SessionModelState>,
12021202
/// Initial session configuration options if supported by the Agent.
1203-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1203+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
12041204
#[serde(default)]
12051205
pub config_options: Option<Vec<SessionConfigOption>>,
12061206
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
@@ -1384,7 +1384,7 @@ pub struct ForkSessionResponse {
13841384
#[serde(default)]
13851385
pub models: Option<SessionModelState>,
13861386
/// Initial session configuration options if supported by the Agent.
1387-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1387+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
13881388
#[serde(default)]
13891389
pub config_options: Option<Vec<SessionConfigOption>>,
13901390
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
@@ -1574,7 +1574,7 @@ pub struct ResumeSessionResponse {
15741574
#[serde(default)]
15751575
pub models: Option<SessionModelState>,
15761576
/// Initial session configuration options if supported by the Agent.
1577-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1577+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
15781578
#[serde(default)]
15791579
pub config_options: Option<Vec<SessionConfigOption>>,
15801580
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
@@ -1817,7 +1817,7 @@ impl ListSessionsRequest {
18171817
#[non_exhaustive]
18181818
pub struct ListSessionsResponse {
18191819
/// Array of session information objects
1820-
#[serde_as(deserialize_as = "VecSkipError<_>")]
1820+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
18211821
pub sessions: Vec<SessionInfo>,
18221822
/// Opaque cursor token. If present, pass this in the next request's cursor parameter
18231823
/// to fetch the next page. If absent, there are no more results.
@@ -1962,7 +1962,7 @@ pub struct SessionModeState {
19621962
/// The current mode the Agent is in.
19631963
pub current_mode_id: SessionModeId,
19641964
/// The set of modes that the Agent can operate in
1965-
#[serde_as(deserialize_as = "VecSkipError<_>")]
1965+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
19661966
pub available_modes: Vec<SessionMode>,
19671967
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
19681968
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at
@@ -2663,7 +2663,7 @@ impl SetSessionConfigOptionRequest {
26632663
#[non_exhaustive]
26642664
pub struct SetSessionConfigOptionResponse {
26652665
/// The full set of configuration options and their current values.
2666-
#[serde_as(deserialize_as = "VecSkipError<_>")]
2666+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
26672667
pub config_options: Vec<SessionConfigOption>,
26682668
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
26692669
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at
@@ -3255,7 +3255,7 @@ pub struct SessionModelState {
32553255
/// The current model the Agent is in.
32563256
pub current_model_id: ModelId,
32573257
/// The set of models that the Agent can use
3258-
#[serde_as(deserialize_as = "VecSkipError<_>")]
3258+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
32593259
pub available_models: Vec<ModelInfo>,
32603260
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
32613261
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at
@@ -3531,7 +3531,7 @@ pub struct ProviderInfo {
35313531
/// Provider identifier, for example "main" or "openai".
35323532
pub id: String,
35333533
/// Supported protocol types for this provider.
3534-
#[serde_as(deserialize_as = "VecSkipError<_>")]
3534+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
35353535
pub supported: Vec<LlmProtocol>,
35363536
/// Whether this provider is mandatory and cannot be disabled via `providers/disable`.
35373537
/// If true, clients must not call `providers/disable` for this id.
@@ -3632,7 +3632,7 @@ impl ListProvidersRequest {
36323632
#[non_exhaustive]
36333633
pub struct ListProvidersResponse {
36343634
/// Configurable providers with current routing info suitable for UI display.
3635-
#[serde_as(deserialize_as = "VecSkipError<_>")]
3635+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
36363636
pub providers: Vec<ProviderInfo>,
36373637
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
36383638
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at

src/client.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::elicitation::{
1717
};
1818
use crate::{
1919
ContentBlock, ExtNotification, ExtRequest, ExtResponse, IntoOption, Meta, Plan,
20-
SessionConfigOption, SessionId, SessionModeId, ToolCall, ToolCallUpdate,
20+
SessionConfigOption, SessionId, SessionModeId, SkipListener, ToolCall, ToolCallUpdate,
2121
};
2222
use crate::{IntoMaybeUndefined, MaybeUndefined};
2323

@@ -162,7 +162,7 @@ impl CurrentModeUpdate {
162162
#[non_exhaustive]
163163
pub struct ConfigOptionUpdate {
164164
/// The full set of configuration options and their current values.
165-
#[serde_as(deserialize_as = "VecSkipError<_>")]
165+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
166166
pub config_options: Vec<SessionConfigOption>,
167167
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
168168
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at
@@ -412,7 +412,7 @@ impl ContentChunk {
412412
#[non_exhaustive]
413413
pub struct AvailableCommandsUpdate {
414414
/// Commands the agent can execute
415-
#[serde_as(deserialize_as = "VecSkipError<_>")]
415+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
416416
pub available_commands: Vec<AvailableCommand>,
417417
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
418418
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at
@@ -1554,7 +1554,7 @@ pub struct ClientCapabilities {
15541554
///
15551555
/// The position encodings supported by the client, in order of preference.
15561556
#[cfg(feature = "unstable_nes")]
1557-
#[serde_as(deserialize_as = "VecSkipError<_>")]
1557+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
15581558
#[serde(default, skip_serializing_if = "Vec::is_empty")]
15591559
pub position_encodings: Vec<PositionEncodingKind>,
15601560

src/content.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use schemars::JsonSchema;
1313
use serde::{Deserialize, Serialize};
1414
use serde_with::{DefaultOnError, VecSkipError, serde_as, skip_serializing_none};
1515

16-
use crate::{IntoOption, Meta};
16+
use crate::{IntoOption, Meta, SkipListener};
1717

1818
/// Content blocks represent displayable information in the Agent Client Protocol.
1919
///
@@ -459,7 +459,7 @@ impl ResourceLink {
459459
#[serde(rename_all = "camelCase")]
460460
#[non_exhaustive]
461461
pub struct Annotations {
462-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
462+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
463463
#[serde(default)]
464464
pub audience: Option<Vec<Role>>,
465465
pub last_modified: Option<String>,

src/nes.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use schemars::JsonSchema;
88
use serde::{Deserialize, Serialize};
99
use serde_with::{DefaultOnError, VecSkipError, serde_as, skip_serializing_none};
1010

11-
use crate::{IntoOption, Meta, SessionId};
11+
use crate::{IntoOption, Meta, SessionId, SkipListener};
1212

1313
// Method name constants
1414

@@ -1099,7 +1099,7 @@ pub struct StartNesRequest {
10991099
/// The root URI of the workspace.
11001100
pub workspace_uri: Option<String>,
11011101
/// The workspace folders.
1102-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1102+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
11031103
#[serde(default)]
11041104
pub workspace_folders: Option<Vec<WorkspaceFolder>>,
11051105
/// Repository metadata, if the workspace is a git repository.
@@ -1436,27 +1436,27 @@ impl SuggestNesRequest {
14361436
#[non_exhaustive]
14371437
pub struct NesSuggestContext {
14381438
/// Recently accessed files.
1439-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1439+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
14401440
#[serde(default)]
14411441
pub recent_files: Option<Vec<NesRecentFile>>,
14421442
/// Related code snippets.
1443-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1443+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
14441444
#[serde(default)]
14451445
pub related_snippets: Option<Vec<NesRelatedSnippet>>,
14461446
/// Recent edit history.
1447-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1447+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
14481448
#[serde(default)]
14491449
pub edit_history: Option<Vec<NesEditHistoryEntry>>,
14501450
/// Recent user actions (typing, navigation, etc.).
1451-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1451+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
14521452
#[serde(default)]
14531453
pub user_actions: Option<Vec<NesUserAction>>,
14541454
/// Currently open files in the editor.
1455-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1455+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
14561456
#[serde(default)]
14571457
pub open_files: Option<Vec<NesOpenFile>>,
14581458
/// Current diagnostics (errors, warnings).
1459-
#[serde_as(deserialize_as = "Option<VecSkipError<_>>")]
1459+
#[serde_as(deserialize_as = "Option<VecSkipError<_, SkipListener>>")]
14601460
#[serde(default)]
14611461
pub diagnostics: Option<Vec<NesDiagnostic>>,
14621462
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
@@ -1757,7 +1757,7 @@ pub enum NesDiagnosticSeverity {
17571757
#[non_exhaustive]
17581758
pub struct SuggestNesResponse {
17591759
/// The list of suggestions.
1760-
#[serde_as(deserialize_as = "VecSkipError<_>")]
1760+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
17611761
pub suggestions: Vec<NesSuggestion>,
17621762
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
17631763
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at

src/plan.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use schemars::JsonSchema;
99
use serde::{Deserialize, Serialize};
1010
use serde_with::{VecSkipError, serde_as, skip_serializing_none};
1111

12-
use crate::{IntoOption, Meta};
12+
use crate::{IntoOption, Meta, SkipListener};
1313

1414
/// An execution plan for accomplishing complex tasks.
1515
///
@@ -28,7 +28,7 @@ pub struct Plan {
2828
///
2929
/// When updating a plan, the agent must send a complete list of all entries
3030
/// with their current status. The client replaces the entire plan with each update.
31-
#[serde_as(deserialize_as = "VecSkipError<_>")]
31+
#[serde_as(deserialize_as = "VecSkipError<_, SkipListener>")]
3232
pub entries: Vec<PlanEntry>,
3333
/// The _meta property is reserved by ACP to allow clients and agents to attach additional
3434
/// metadata to their interactions. Implementations MUST NOT make assumptions about values at

src/serde_util.rs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
//!
55
//! - [`MaybeUndefined<T>`] — three-state: undefined (key absent), null, or value.
66
//! - [`RequiredNullable<T>`] — required-but-nullable: key must be present, value may be null.
7+
//! - [`SkipListener`] — [`serde_with::InspectError`] hook used by every
8+
//! `VecSkipError` call site in the protocol types.
79
//!
810
//! ## Builder traits
911
//!
@@ -22,6 +24,79 @@ use std::{
2224
use schemars::JsonSchema;
2325
use serde::{Deserialize, Deserializer, Serialize, Serializer};
2426

27+
// ---- SkipListener ----
28+
29+
/// Inspector passed to every `VecSkipError<_, SkipListener>` in the protocol
30+
/// types so that malformed list entries dropped during deserialization are
31+
/// surfaced to observability tooling rather than vanishing silently.
32+
///
33+
/// - With the `tracing` feature enabled, this is a zero-sized type whose
34+
/// [`InspectError`](serde_with::InspectError) implementation emits a
35+
/// [`tracing::warn!`] event on every skipped entry.
36+
/// - With the feature disabled (the default), it resolves to `()` — which
37+
/// `serde_with` ships with a no-op `InspectError` implementation — so call
38+
/// sites incur zero runtime cost.
39+
#[cfg(feature = "tracing")]
40+
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)]
41+
#[non_exhaustive]
42+
pub struct SkipListener;
43+
44+
#[cfg(feature = "tracing")]
45+
impl serde_with::InspectError for SkipListener {
46+
fn inspect_error(error: impl serde::de::Error) {
47+
tracing::warn!(
48+
%error,
49+
"skipped malformed list entry during deserialization",
50+
);
51+
}
52+
}
53+
54+
/// Zero-cost stand-in for [`SkipListener`] when the `tracing` feature is
55+
/// disabled. Resolves to `()`, which `serde_with` already ships with a no-op
56+
/// `InspectError` implementation.
57+
#[cfg(not(feature = "tracing"))]
58+
pub type SkipListener = ();
59+
60+
#[cfg(test)]
61+
mod skip_listener_tests {
62+
use std::cell::Cell;
63+
64+
use serde::{Deserialize, Serialize};
65+
use serde_json::json;
66+
use serde_with::{VecSkipError, serde_as};
67+
68+
thread_local! {
69+
static SKIP_COUNT: Cell<u32> = const { Cell::new(0) };
70+
}
71+
72+
/// Test-only inspector that counts skipped entries.
73+
struct CountingListener;
74+
75+
impl serde_with::InspectError for CountingListener {
76+
fn inspect_error(_error: impl serde::de::Error) {
77+
SKIP_COUNT.with(|c| c.set(c.get() + 1));
78+
}
79+
}
80+
81+
#[serde_as]
82+
#[derive(Serialize, Deserialize, Debug, PartialEq)]
83+
struct Wrapper {
84+
#[serde_as(deserialize_as = "VecSkipError<_, CountingListener>")]
85+
values: Vec<u32>,
86+
}
87+
88+
#[test]
89+
fn inspector_runs_for_each_skipped_entry() {
90+
SKIP_COUNT.with(|c| c.set(0));
91+
92+
let input = json!({"values": [1, "oops", 2, {}, 3]});
93+
let wrapper: Wrapper = serde_json::from_value(input).unwrap();
94+
95+
assert_eq!(wrapper.values, vec![1, 2, 3]);
96+
assert_eq!(SKIP_COUNT.with(Cell::get), 2);
97+
}
98+
}
99+
25100
// ---- IntoOption ----
26101

27102
/// Utility trait for builder methods for optional values.

0 commit comments

Comments
 (0)