Skip to content

Commit 7e861af

Browse files
RoyLinRoyLin
authored andcommitted
fix(core,node-sdk): harden tool schemas and restore slash command output
1 parent 474fb62 commit 7e861af

35 files changed

Lines changed: 870 additions & 155 deletions

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "a3s-code-core"
3-
version = "1.5.3"
3+
version = "1.5.4"
44
edition = "2021"
55
authors = ["A3S Lab Team"]
66
license = "MIT"

core/src/agent.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,12 @@ pub enum AgentEvent {
227227

228228
/// Agent completed
229229
#[serde(rename = "agent_end")]
230-
End { text: String, usage: TokenUsage },
230+
End {
231+
text: String,
232+
usage: TokenUsage,
233+
#[serde(skip_serializing_if = "Option::is_none")]
234+
meta: Option<crate::llm::LlmResponseMeta>,
235+
},
231236

232237
/// Error occurred
233238
#[serde(rename = "error")]
@@ -1856,7 +1861,6 @@ impl AgentLoop {
18561861
total_usage.prompt_tokens += response.usage.prompt_tokens;
18571862
total_usage.completion_tokens += response.usage.completion_tokens;
18581863
total_usage.total_tokens += response.usage.total_tokens;
1859-
18601864
// Record LLM completion telemetry
18611865
let llm_duration = llm_start.elapsed();
18621866
tracing::info!(
@@ -2002,6 +2006,7 @@ impl AgentLoop {
20022006
tx.send(AgentEvent::End {
20032007
text: final_text.clone(),
20042008
usage: total_usage.clone(),
2009+
meta: response.meta.clone(),
20052010
})
20062011
.await
20072012
.ok();
@@ -3113,6 +3118,7 @@ mod tests {
31133118
cache_write_tokens: None,
31143119
},
31153120
stop_reason: Some("end_turn".to_string()),
3121+
meta: None,
31163122
}
31173123
}
31183124

@@ -3140,6 +3146,7 @@ mod tests {
31403146
cache_write_tokens: None,
31413147
},
31423148
stop_reason: Some("tool_use".to_string()),
3149+
meta: None,
31433150
}
31443151
}
31453152
}
@@ -3887,6 +3894,7 @@ mod tests {
38873894
cache_write_tokens: None,
38883895
},
38893896
stop_reason: Some("tool_use".to_string()),
3897+
meta: None,
38903898
},
38913899
MockLlmClient::text_response("Both executed!"),
38923900
]));
@@ -3959,6 +3967,7 @@ mod tests {
39593967
cache_write_tokens: None,
39603968
},
39613969
stop_reason: Some("tool_use".to_string()),
3970+
meta: None,
39623971
},
39633972
MockLlmClient::text_response("First worked, second rejected."),
39643973
]));
@@ -4611,6 +4620,7 @@ mod tests {
46114620
cache_write_tokens: None,
46124621
},
46134622
stop_reason: Some("tool_use".to_string()),
4623+
meta: None,
46144624
},
46154625
MockLlmClient::text_response("Both commands ran"),
46164626
]));
@@ -4747,7 +4757,7 @@ mod tests {
47474757
let end_event = events.iter().find(|e| matches!(e, AgentEvent::End { .. }));
47484758
assert!(end_event.is_some());
47494759

4750-
if let AgentEvent::End { text, usage } = end_event.unwrap() {
4760+
if let AgentEvent::End { text, usage, .. } = end_event.unwrap() {
47514761
assert_eq!(text, "Final answer here");
47524762
assert_eq!(usage.total_tokens, 15);
47534763
}
@@ -4985,6 +4995,7 @@ mod extra_agent_tests {
49854995
cache_read_tokens: None,
49864996
cache_write_tokens: None,
49874997
},
4998+
meta: None,
49884999
};
49895000
let json = serde_json::to_string(&event).unwrap();
49905001
assert!(json.contains("agent_end"));

core/src/agent_api.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,6 +2104,7 @@ impl AgentSession {
21042104
.send(AgentEvent::End {
21052105
text: answer,
21062106
usage: crate::llm::TokenUsage::default(),
2107+
meta: None,
21072108
})
21082109
.await;
21092110
}
@@ -2129,6 +2130,7 @@ impl AgentSession {
21292130
.send(AgentEvent::End {
21302131
text: output.text.clone(),
21312132
usage: crate::llm::TokenUsage::default(),
2133+
meta: None,
21322134
})
21332135
.await;
21342136
});

core/src/llm/tests.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,7 @@ mod tests {
383383
cache_write_tokens: None,
384384
},
385385
stop_reason: Some("end_turn".to_string()),
386+
meta: None,
386387
};
387388
assert_eq!(response.text(), "Hello!");
388389
assert!(response.tool_calls().is_empty());
@@ -404,6 +405,7 @@ mod tests {
404405
},
405406
usage: TokenUsage::default(),
406407
stop_reason: Some("tool_use".to_string()),
408+
meta: None,
407409
};
408410
let calls = response.tool_calls();
409411
assert_eq!(calls.len(), 1);
@@ -618,6 +620,7 @@ mod extra_llm_tests {
618620
},
619621
usage: TokenUsage::default(),
620622
stop_reason: None,
623+
meta: None,
621624
};
622625
assert_eq!(r.text(), "resp");
623626
}
@@ -636,6 +639,7 @@ mod extra_llm_tests {
636639
},
637640
usage: TokenUsage::default(),
638641
stop_reason: None,
642+
meta: None,
639643
};
640644
assert_eq!(r.tool_calls().len(), 1);
641645
}
@@ -1529,6 +1533,7 @@ mod extra_llm_tests2 {
15291533
},
15301534
usage: TokenUsage::default(),
15311535
stop_reason: None,
1536+
meta: None,
15321537
};
15331538
assert_eq!(response.text(), "response text");
15341539
}
@@ -1547,6 +1552,7 @@ mod extra_llm_tests2 {
15471552
},
15481553
usage: TokenUsage::default(),
15491554
stop_reason: Some("tool_use".to_string()),
1555+
meta: None,
15501556
};
15511557
let calls = response.tool_calls();
15521558
assert_eq!(calls.len(), 1);
@@ -1941,6 +1947,7 @@ mod extra_llm_tests2 {
19411947
message: Message::user("test"),
19421948
usage: TokenUsage::default(),
19431949
stop_reason: Some("end_turn".to_string()),
1950+
meta: None,
19441951
};
19451952
let json = serde_json::to_string(&response).unwrap();
19461953
assert!(json.contains("\"stop_reason\":\"end_turn\""));
@@ -2170,6 +2177,17 @@ mod extra_llm_tests3 {
21702177
assert_eq!(tool_calls[0].function.arguments, "invalid json");
21712178
}
21722179

2180+
#[test]
2181+
fn test_openai_parse_tool_arguments_preserves_parse_error() {
2182+
let parsed = OpenAiClient::parse_tool_arguments("Skill", "invalid json");
2183+
let err = parsed
2184+
.get("__parse_error")
2185+
.and_then(|v| v.as_str())
2186+
.unwrap_or_default();
2187+
assert!(err.contains("Malformed tool arguments"));
2188+
assert!(err.contains("invalid json"));
2189+
}
2190+
21732191
// ========================================================================
21742192
// Anthropic Response Parsing
21752193
// ========================================================================

core/src/llm/types.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,11 +357,33 @@ impl Message {
357357
}
358358

359359
/// LLM response
360+
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
361+
pub struct LlmResponseMeta {
362+
#[serde(default, skip_serializing_if = "Option::is_none")]
363+
pub provider: Option<String>,
364+
#[serde(default, skip_serializing_if = "Option::is_none")]
365+
pub request_model: Option<String>,
366+
#[serde(default, skip_serializing_if = "Option::is_none")]
367+
pub request_url: Option<String>,
368+
#[serde(default, skip_serializing_if = "Option::is_none")]
369+
pub response_id: Option<String>,
370+
#[serde(default, skip_serializing_if = "Option::is_none")]
371+
pub response_model: Option<String>,
372+
#[serde(default, skip_serializing_if = "Option::is_none")]
373+
pub response_object: Option<String>,
374+
#[serde(default, skip_serializing_if = "Option::is_none")]
375+
pub first_token_ms: Option<u64>,
376+
#[serde(default, skip_serializing_if = "Option::is_none")]
377+
pub duration_ms: Option<u64>,
378+
}
379+
360380
#[derive(Debug, Clone, Serialize, Deserialize)]
361381
pub struct LlmResponse {
362382
pub message: Message,
363383
pub usage: TokenUsage,
364384
pub stop_reason: Option<String>,
385+
#[serde(default, skip_serializing_if = "Option::is_none")]
386+
pub meta: Option<LlmResponseMeta>,
365387
}
366388

367389
impl LlmResponse {
@@ -395,6 +417,7 @@ pub struct ToolCall {
395417
}
396418

397419
/// Streaming event from LLM
420+
#[allow(clippy::large_enum_variant)]
398421
#[derive(Debug, Clone)]
399422
pub enum StreamEvent {
400423
/// Text content delta

core/src/plugin.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -569,8 +569,6 @@ mod tests {
569569

570570
#[test]
571571
fn plugin_skills_not_registered_when_no_skill_registry_in_ctx() {
572-
use crate::skills::SkillRegistry;
573-
574572
let mut mgr = PluginManager::new();
575573
mgr.register(AgenticSearchPlugin::new());
576574

core/src/session/tests.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ mod tests {
4040
},
4141
usage: crate::llm::TokenUsage::default(),
4242
stop_reason: Some("end_turn".to_string()),
43+
meta: None,
4344
})
4445
}
4546

@@ -1903,6 +1904,7 @@ mod extra_session_tests {
19031904
},
19041905
usage: crate::llm::TokenUsage::default(),
19051906
stop_reason: None,
1907+
meta: None,
19061908
})
19071909
}
19081910
async fn complete_streaming(

core/src/skills/manage.rs

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,50 +47,77 @@ impl Tool for ManageSkillTool {
4747
}
4848

4949
fn description(&self) -> &str {
50-
"Create, list, remove, or get skills at runtime. Provide feedback on skill effectiveness to improve future behavior. Skills are instruction sets injected into the system prompt. Created skills persist across sessions."
50+
"Create, list, remove, get, or score skills at runtime. \
51+
Use a single JSON object with the canonical field names defined in this schema. \
52+
Always provide the exact 'action' string first, then only the fields relevant to that action. \
53+
Do not invent alias fields or wrapper objects. Skills are instruction sets injected into the system prompt and created skills persist across sessions."
5154
}
5255

5356
fn parameters(&self) -> serde_json::Value {
5457
serde_json::json!({
5558
"type": "object",
59+
"additionalProperties": false,
5660
"properties": {
5761
"action": {
5862
"type": "string",
5963
"enum": ["create", "list", "remove", "get", "feedback", "scores"],
60-
"description": "Action to perform"
64+
"description": "Required. Action to perform. Use exactly one of: create, list, remove, get, feedback, scores."
6165
},
6266
"name": {
6367
"type": "string",
64-
"description": "Skill name (kebab-case, required for create/remove/get/feedback)"
68+
"description": "Canonical skill name in kebab-case. Required for create, remove, get, and feedback."
6569
},
6670
"description": {
6771
"type": "string",
68-
"description": "Skill description (required for create)"
72+
"description": "Skill description. Required only when action='create'."
6973
},
7074
"content": {
7175
"type": "string",
72-
"description": "Skill instructions in markdown (required for create)"
76+
"description": "Skill instructions in markdown. Required only when action='create'."
7377
},
7478
"tags": {
7579
"type": "array",
7680
"items": { "type": "string" },
77-
"description": "Optional tags for categorization"
81+
"description": "Optional tags for categorization when action='create'."
7882
},
7983
"outcome": {
8084
"type": "string",
8185
"enum": ["success", "failure", "partial"],
82-
"description": "Skill usage outcome (required for feedback)"
86+
"description": "Skill usage outcome. Required only when action='feedback'."
8387
},
8488
"score_delta": {
8589
"type": "number",
86-
"description": "Score adjustment from -1.0 to 1.0 (required for feedback)"
90+
"description": "Score adjustment from -1.0 to 1.0. Required only when action='feedback'."
8791
},
8892
"reason": {
8993
"type": "string",
90-
"description": "Reason for the feedback (required for feedback)"
94+
"description": "Reason for the feedback. Required only when action='feedback'."
9195
}
9296
},
93-
"required": ["action"]
97+
"required": ["action"],
98+
"examples": [
99+
{
100+
"action": "list"
101+
},
102+
{
103+
"action": "get",
104+
"name": "code-review"
105+
},
106+
{
107+
"action": "create",
108+
"name": "code-review",
109+
"description": "Review code changes for bugs and regressions.",
110+
"content": "# Code Review\n\nReview the supplied patch for correctness and regressions.",
111+
"tags": ["review", "quality"]
112+
},
113+
{
114+
"action": "feedback",
115+
"name": "code-review",
116+
"outcome": "success",
117+
"score_delta": 0.5,
118+
"reason": "The skill found the regression quickly."
119+
}
120+
]
94121
})
95122
}
96123

@@ -404,11 +431,21 @@ mod tests {
404431
assert!(!tool.description().is_empty());
405432
let params = tool.parameters();
406433
assert_eq!(params["type"], "object");
434+
assert_eq!(params["additionalProperties"], false);
407435
assert!(params["properties"]["action"].is_object());
408436
// Verify new actions are in enum
409437
let actions = params["properties"]["action"]["enum"].as_array().unwrap();
410438
assert!(actions.iter().any(|a| a == "feedback"));
411439
assert!(actions.iter().any(|a| a == "scores"));
440+
441+
let examples = params["examples"].as_array().unwrap();
442+
assert_eq!(examples[0]["action"], "list");
443+
assert_eq!(examples[1]["action"], "get");
444+
assert_eq!(examples[1]["name"], "code-review");
445+
assert_eq!(examples[2]["action"], "create");
446+
assert!(examples[2].get("skill_name").is_none());
447+
assert_eq!(examples[3]["action"], "feedback");
448+
assert_eq!(examples[3]["outcome"], "success");
412449
}
413450

414451
#[tokio::test]

0 commit comments

Comments
 (0)