Skip to content

Commit 1f58a79

Browse files
authored
Merge pull request GCWing#3238 from GCWing/codex/fix-deferred-spec-and-cron-badge
fix: handle compacted tool specs and paused cron badges
2 parents 100cc84 + e628cad commit 1f58a79

9 files changed

Lines changed: 266 additions & 44 deletions

File tree

‎src/crates/assembly/core/src/agentic/tools/product_runtime/get_tool_spec_tool.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,6 @@ mod tests {
167167
assert!(result_for_assistant
168168
.as_deref()
169169
.unwrap_or_default()
170-
.contains("already loaded in the current conversation"));
170+
.contains("already loaded in the current context"));
171171
}
172172
}

‎src/crates/assembly/core/src/agentic/tools/product_runtime/loaded_spec_state.rs‎

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,12 @@ fn get_tool_spec_load_observation(message: &Message) -> Option<GetToolSpecLoadOb
6767
#[cfg(test)]
6868
mod tests {
6969
use super::{collect_product_loaded_deferred_tool_specs, ProductLoadedDeferredToolSpecs};
70-
use crate::agentic::core::{Message, ToolResult};
70+
use crate::agentic::core::{Message, ToolCall, ToolResult};
71+
use crate::agentic::session::ContextCompressor;
72+
use openbitfun_agent_tools::{
73+
resolve_get_tool_spec_execution_plan, validate_deferred_tool_usage,
74+
GetToolSpecExecutionPlan, GET_TOOL_SPEC_TOOL_NAME,
75+
};
7176
use serde_json::json;
7277

7378
fn loaded_spec(tool_name: &str) -> openbitfun_agent_tools::LoadedDeferredToolSpec {
@@ -77,6 +82,108 @@ mod tests {
7782
}
7883
}
7984

85+
#[test]
86+
fn compaction_requires_reload_only_when_the_full_spec_leaves_the_context() {
87+
let deferred_tools = vec!["Cron".to_string()];
88+
let input = json!({ "tool_name": "Cron" });
89+
let spec_result = Message::tool_result(ToolResult {
90+
tool_id: "load-cron".to_string(),
91+
tool_name: GET_TOOL_SPEC_TOOL_NAME.to_string(),
92+
effective_tool_name: None,
93+
result: json!({
94+
"tool_name": "Cron",
95+
"catalog_generation": 42,
96+
"description": "Manage scheduled jobs.",
97+
"input_schema": { "type": "object", "properties": { "action": { "enum": ["list"] } } },
98+
}),
99+
result_for_assistant: None,
100+
is_error: false,
101+
duration_ms: None,
102+
image_attachments: None,
103+
});
104+
let history = vec![
105+
Message::user("Check the scheduled jobs.".to_string()),
106+
Message::assistant_with_tools(
107+
String::new(),
108+
vec![ToolCall {
109+
tool_id: "load-cron".to_string(),
110+
tool_name: GET_TOOL_SPEC_TOOL_NAME.to_string(),
111+
arguments: input.clone(),
112+
raw_arguments: None,
113+
is_error: false,
114+
parse_error: None,
115+
recovered_from_truncation: false,
116+
repair_kind: Default::default(),
117+
}],
118+
),
119+
spec_result.clone(),
120+
];
121+
let compressor = ContextCompressor::new();
122+
123+
// The same summary may be generated with or without a retained tool
124+
// result. Only the actual retained result is an execution receipt.
125+
for (recent_tokens, retains_spec) in [(0, false), (10_000, true)] {
126+
let plan = compressor
127+
.plan_compression("session", &history, 128_000, recent_tokens)
128+
.unwrap()
129+
.unwrap();
130+
let mut compressed = compressor
131+
.compress_plan_with_contract(
132+
"session",
133+
plan,
134+
None,
135+
"The Cron definition was loaded with GetToolSpec earlier.".to_string(),
136+
)
137+
.unwrap()
138+
.messages;
139+
let loaded = collect_product_loaded_deferred_tool_specs(&compressed, &deferred_tools);
140+
let admission = validate_deferred_tool_usage(
141+
"Cron",
142+
&deferred_tools,
143+
&loaded,
144+
42,
145+
GET_TOOL_SPEC_TOOL_NAME,
146+
);
147+
let names = loaded
148+
.iter()
149+
.map(|spec| spec.tool_name.clone())
150+
.collect::<Vec<_>>();
151+
let reload_plan = resolve_get_tool_spec_execution_plan(&input, &names).unwrap();
152+
153+
if retains_spec {
154+
assert_eq!(loaded, vec![loaded_spec("Cron")]);
155+
assert!(admission.is_ok());
156+
assert!(matches!(
157+
reload_plan,
158+
GetToolSpecExecutionPlan::DuplicateLoad(_)
159+
));
160+
} else {
161+
assert!(loaded.is_empty());
162+
assert!(admission
163+
.unwrap_err()
164+
.to_string()
165+
.contains("reload it even if the summary says it was loaded"));
166+
assert!(matches!(
167+
reload_plan,
168+
GetToolSpecExecutionPlan::LoadDetail { tool_name: "Cron" }
169+
));
170+
171+
// Reading the definition again restores normal admission.
172+
compressed.push(spec_result.clone());
173+
let reloaded =
174+
collect_product_loaded_deferred_tool_specs(&compressed, &deferred_tools);
175+
validate_deferred_tool_usage(
176+
"Cron",
177+
&deferred_tools,
178+
&reloaded,
179+
42,
180+
GET_TOOL_SPEC_TOOL_NAME,
181+
)
182+
.expect("a fresh spec must unlock Cron after compaction");
183+
}
184+
}
185+
}
186+
80187
#[test]
81188
fn product_loaded_spec_state_collects_visible_get_tool_spec_results() {
82189
let visible_get_tool_spec_result = Message::tool_result(ToolResult {

‎src/crates/execution/agent-runtime/src/prompt.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ const DIRECT_TOOL_LISTING_GUIDANCE: &str = r#"Their definitions are already avai
1616
Each entry below is a directly callable tool name."#;
1717
const DEFERRED_TOOL_LISTING_TITLE: &str = "## Deferred tools";
1818
const DEFERRED_TOOL_LISTING_GUIDANCE: &str = r#"Their definitions are not loaded at the start of the conversation.
19-
You must obtain the tool definition using GetToolSpec before you first invoke a deferred tool. Once its definition is available in the conversation, you can call it through CallDeferredTool.
19+
Use GetToolSpec to read a deferred tool's full definition before invoking it through CallDeferredTool. Reuse it while the successful GetToolSpec result remains in the current context. If compaction or truncation removed that result, load it again; a summary or a past call does not keep the definition loaded.
2020
Each entry below is a deferred tool name with an optional short description."#;
2121

2222
pub fn render_direct_tool_listing_body<'a>(

‎src/crates/execution/agent-runtime/tests/agent_definition_contracts/prompt_contracts.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,17 +70,20 @@ fn tool_listing_sections_render_only_present_sections() {
7070
assert!(deferred_tool_listing
7171
.contains("Their definitions are not loaded at the start of the conversation."));
7272
assert!(deferred_tool_listing
73-
.contains("You must obtain the tool definition using GetToolSpec before you first invoke a deferred tool."));
73+
.contains("Use GetToolSpec to read a deferred tool's full definition before invoking it through CallDeferredTool."));
7474
assert!(deferred_tool_listing.contains(
75-
"Once its definition is available in the conversation, you can call it through CallDeferredTool."
75+
"Reuse it while the successful GetToolSpec result remains in the current context."
76+
));
77+
assert!(deferred_tool_listing.contains(
78+
"If compaction or truncation removed that result, load it again; a summary or a past call does not keep the definition loaded."
7679
));
7780
assert!(deferred_tool_listing
7881
.contains("Each entry below is a deferred tool name with an optional short description."));
7982
assert!(deferred_tool_listing.contains(
8083
"## Direct tools\nTheir definitions are already available. You can call them directly.\nEach entry below is a directly callable tool name.\n\n<direct_tools>\n- Read\n- GetToolSpec\n- CallDeferredTool\n</direct_tools>"
8184
));
8285
assert!(deferred_tool_listing.contains(
83-
"## Deferred tools\nTheir definitions are not loaded at the start of the conversation.\nYou must obtain the tool definition using GetToolSpec before you first invoke a deferred tool. Once its definition is available in the conversation, you can call it through CallDeferredTool."
86+
"## Deferred tools\nTheir definitions are not loaded at the start of the conversation.\nUse GetToolSpec"
8487
));
8588
assert!(deferred_tool_listing.ends_with("Search: summary"));
8689
}

‎src/crates/execution/tool-contracts/src/deferred_tool.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ pub fn call_deferred_tool_input_schema() -> Value {
6262
"properties": {
6363
"tool_name": {
6464
"type": "string",
65-
"description": "Exact deferred tool name previously loaded with GetToolSpec."
65+
"description": "Exact deferred tool name whose full GetToolSpec result is still visible in the current context."
6666
},
6767
"args": {
6868
"type": "object",
@@ -80,6 +80,7 @@ pub fn call_deferred_tool_short_description() -> String {
8080
pub fn call_deferred_tool_description() -> String {
8181
r#"Call a deferred tool after reading its full schema with GetToolSpec.
8282
83+
The full GetToolSpec result must still be visible in the current context. If compaction or truncation removed it, reload it with GetToolSpec first; a summary or a past call is not a loaded definition.
8384
Pass the exact deferred tool name in tool_name and put only that tool's arguments inside args.
8485
The order is important. ALWAYS output tool_name first, then args."#
8586
.to_string()

‎src/crates/execution/tool-contracts/src/framework.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ impl fmt::Display for DeferredToolUsageError {
9191
get_tool_spec_tool_name,
9292
} => write!(
9393
formatter,
94-
"Tool '{tool_name}' is deferred. Call {get_tool_spec_tool_name} first with {{\"tool_name\":\"{tool_name}\"}} to read its full usage instructions and input schema before invoking it."
94+
"Tool '{tool_name}' has no loaded definition in the current context. Call {get_tool_spec_tool_name} with {{\"tool_name\":\"{tool_name}\"}} to read its full usage instructions and input schema before invoking it. If compaction removed an earlier definition, reload it even if the summary says it was loaded."
9595
),
9696
Self::StaleSpec {
9797
tool_name,
@@ -371,7 +371,8 @@ pub fn get_tool_spec_short_description() -> String {
371371
pub fn build_get_tool_spec_description() -> String {
372372
r#"Read the full schema before first calling a deferred tool through CallDeferredTool.
373373
374-
Do not call GetToolSpec again for a tool whose definition is already loaded in the current conversation."#
374+
Do not call GetToolSpec again while its successful result with the full definition is still visible in the current context.
375+
If compaction or truncation removed that result, call GetToolSpec again before using CallDeferredTool. A summary mentioning a previously loaded tool or a past successful call does not load its definition. Reload also when the runtime reports a stale definition."#
375376
.to_string()
376377
}
377378

@@ -439,7 +440,7 @@ pub fn validate_get_tool_spec_input(input: &Value) -> ValidationResult {
439440

440441
pub fn build_get_tool_spec_duplicate_load_hint(tool_name: &str) -> String {
441442
format!(
442-
"Tool '{}' is already loaded in the current conversation. Do not call GetToolSpec again for it. Use CallDeferredTool with tool_name '{}' and put the tool arguments inside args.",
443+
"Tool '{}' is already loaded in the current context. Use CallDeferredTool with tool_name '{}' and put the tool arguments inside args. Reload with GetToolSpec only if its full definition leaves the context or the runtime reports it stale.",
443444
tool_name, tool_name
444445
)
445446
}
@@ -2688,6 +2689,10 @@ mod tests {
26882689

26892690
assert!(description.contains("Read the full schema"));
26902691
assert!(description.contains("Do not call GetToolSpec again"));
2692+
assert!(description.contains("full definition is still visible in the current context"));
2693+
assert!(description
2694+
.contains("If compaction or truncation removed that result, call GetToolSpec again"));
2695+
assert!(description.contains("does not load its definition"));
26912696
}
26922697

26932698
#[test]

‎src/crates/execution/tool-contracts/tests/tool_contracts.rs‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ fn call_deferred_tool_contract_uses_nested_object_arguments() {
9797

9898
assert!(call_deferred_tool_description()
9999
.contains("The order is important. ALWAYS output tool_name first, then args."));
100+
assert!(call_deferred_tool_description()
101+
.contains("If compaction or truncation removed it, reload it with GetToolSpec first"));
100102
assert_eq!(schema["additionalProperties"], false);
101103
assert_eq!(schema["required"], json!(["tool_name", "args"]));
102104
assert_eq!(schema["properties"]["args"]["type"], "object");
@@ -1470,7 +1472,7 @@ fn deferred_tool_usage_gate_preserves_get_tool_spec_unlock_contract() {
14701472
.expect_err("deferred tool should require GetToolSpec unlock");
14711473
assert_eq!(
14721474
err.to_string(),
1473-
"Tool 'WebFetch' is deferred. Call GetToolSpec first with {\"tool_name\":\"WebFetch\"} to read its full usage instructions and input schema before invoking it."
1475+
"Tool 'WebFetch' has no loaded definition in the current context. Call GetToolSpec with {\"tool_name\":\"WebFetch\"} to read its full usage instructions and input schema before invoking it. If compaction removed an earlier definition, reload it even if the summary says it was loaded."
14741476
);
14751477

14761478
let loaded_deferred_tool_specs = vec![LoadedDeferredToolSpec {
@@ -2030,7 +2032,7 @@ fn get_tool_spec_contract_escapes_assistant_detail_for_xml_sections() {
20302032
fn get_tool_spec_contract_preserves_duplicate_load_hint() {
20312033
assert_eq!(
20322034
build_get_tool_spec_duplicate_load_hint("WebFetch"),
2033-
"Tool 'WebFetch' is already loaded in the current conversation. Do not call GetToolSpec again for it. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args."
2035+
"Tool 'WebFetch' is already loaded in the current context. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args. Reload with GetToolSpec only if its full definition leaves the context or the runtime reports it stale."
20342036
);
20352037
}
20362038

@@ -2052,7 +2054,7 @@ fn get_tool_spec_contract_builds_duplicate_load_result() {
20522054
assert_eq!(
20532055
result_for_assistant.as_deref(),
20542056
Some(
2055-
"Tool 'WebFetch' is already loaded in the current conversation. Do not call GetToolSpec again for it. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args."
2057+
"Tool 'WebFetch' is already loaded in the current context. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args. Reload with GetToolSpec only if its full definition leaves the context or the runtime reports it stale."
20562058
)
20572059
);
20582060
assert_eq!(image_attachments, None);
@@ -2127,7 +2129,7 @@ fn get_tool_spec_contract_plans_duplicate_load_without_core_context() {
21272129
assert!(result_for_assistant
21282130
.as_deref()
21292131
.unwrap_or_default()
2130-
.contains("already loaded in the current conversation"));
2132+
.contains("already loaded in the current context"));
21312133
assert_eq!(image_attachments, None);
21322134
}
21332135

@@ -3058,7 +3060,7 @@ async fn get_tool_spec_detail_resolver_preserves_contextual_detail_contract() {
30583060
.expect("collapsed WebFetch detail");
30593061

30603062
assert_eq!(detail.tool_name, "WebFetch");
3061-
assert_eq!(detail.description, "WebFetch description for agentic");
3063+
assert_eq!(detail.description, "WebFetch description for Standard");
30623064
assert_eq!(
30633065
detail.input_schema["properties"]["agent"]["const"],
30643066
"Standard"
@@ -3067,7 +3069,7 @@ async fn get_tool_spec_detail_resolver_preserves_contextual_detail_contract() {
30673069
detail.to_value(),
30683070
json!({
30693071
"tool_name": "WebFetch",
3070-
"description": "WebFetch description for agentic",
3072+
"description": "WebFetch description for Standard",
30713073
"input_schema": {
30723074
"type": "object",
30733075
"properties": {
@@ -3118,7 +3120,7 @@ async fn get_tool_spec_catalog_provider_preserves_runtime_catalog_contract() {
31183120
.await
31193121
.expect("provider-backed detail");
31203122
assert_eq!(detail.tool_name, "WebFetch");
3121-
assert_eq!(detail.description, "WebFetch description for agentic");
3123+
assert_eq!(detail.description, "WebFetch description for Standard");
31223124
}
31233125

31243126
#[tokio::test]
@@ -3153,7 +3155,7 @@ async fn get_tool_spec_provider_execution_returns_duplicate_result_without_detai
31533155
assert!(result_for_assistant
31543156
.as_deref()
31553157
.unwrap_or_default()
3156-
.contains("already loaded in the current conversation"));
3158+
.contains("already loaded in the current context"));
31573159
assert_eq!(image_attachments, None);
31583160
}
31593161

@@ -3189,15 +3191,15 @@ async fn get_tool_spec_provider_execution_returns_detail_result_from_provider()
31893191
};
31903192

31913193
assert_eq!(data["tool_name"], "WebFetch");
3192-
assert_eq!(data["description"], "WebFetch description for agentic");
3194+
assert_eq!(data["description"], "WebFetch description for Standard");
31933195
assert_eq!(
31943196
data["input_schema"]["properties"]["agent"]["const"],
31953197
"Standard"
31963198
);
31973199
let assistant = result_for_assistant.expect("assistant detail");
3198-
assert!(assistant.contains("<description>\nWebFetch description for agentic"));
3200+
assert!(assistant.contains("<description>\nWebFetch description for Standard"));
31993201
assert!(assistant.contains("\"agent\""));
3200-
assert!(assistant.contains("\"agentic\""));
3202+
assert!(assistant.contains("\"Standard\""));
32013203
assert_eq!(image_attachments, None);
32023204
}
32033205

@@ -3267,7 +3269,7 @@ async fn get_tool_spec_runtime_facade_owns_execution_path() {
32673269
panic!("expected normal tool result");
32683270
};
32693271
assert_eq!(data["tool_name"], "WebFetch");
3270-
assert_eq!(data["description"], "WebFetch description for agentic");
3272+
assert_eq!(data["description"], "WebFetch description for Standard");
32713273
assert_eq!(
32723274
data["input_schema"]["properties"]["agent"]["const"],
32733275
"Standard"
@@ -3306,7 +3308,7 @@ async fn get_tool_spec_runtime_facade_owns_tool_result_vector_adapter_shape() {
33063308
assert_eq!(data["tool_name"], "WebFetch");
33073309
assert!(result_for_assistant
33083310
.expect("assistant detail")
3309-
.contains("<description>\nWebFetch description for agentic"));
3311+
.contains("<description>\nWebFetch description for Standard"));
33103312
assert_eq!(image_attachments, None);
33113313

33123314
let duplicate_runtime =
@@ -3339,7 +3341,7 @@ async fn get_tool_spec_runtime_facade_owns_tool_result_vector_adapter_shape() {
33393341
assert_eq!(
33403342
result_for_assistant.as_deref(),
33413343
Some(
3342-
"Tool 'WebFetch' is already loaded in the current conversation. Do not call GetToolSpec again for it. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args."
3344+
"Tool 'WebFetch' is already loaded in the current context. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args. Reload with GetToolSpec only if its full definition leaves the context or the runtime reports it stale."
33433345
)
33443346
);
33453347
assert!(image_attachments.is_none());

0 commit comments

Comments
 (0)