-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(reborn): Context management — progressive tool disclosure (flag-gated, default off) #5149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 28 commits
5dfd0cd
8f56d75
db9d099
dda1e59
3c3cc9d
2dc0127
336aa16
7aff964
5d8f068
41bb5ea
aa12134
ecaeb06
58a5140
4aebf24
12a1861
e719896
9908836
2e1e1ac
04e8bc7
ca69d08
44c6183
ba77135
2cf4f00
f52546c
aaadd04
116f226
f9a436a
0c34188
d167e71
c701c33
83ae258
519dbc7
48fd811
7086bf6
7ae4fef
5b4e9be
fca0548
3e97236
81c79b9
1359687
71e5923
1f7d99b
fa1fac2
f501039
cff1055
0e2af9d
622fbc1
75d77f6
78de2bb
69c231b
4b36574
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,25 @@ impl LoopCapabilityPort for CapabilitySurfaceVisibleFilter { | |
| Ok(definitions) | ||
| } | ||
|
|
||
| fn provider_tool_call_capability_ids( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Root-cause / code-judo (blocker #2). This same "delegate to inner, then apply my scope check" body is added ~verbatim to Visible/Deny/Profile filters (and a delegate-only variant in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Took the "at minimum" path in |
||
| &self, | ||
| tool_call: &ProviderToolCall, | ||
| ) -> Result<ProviderToolCallCapabilityIds, AgentLoopHostError> { | ||
| // Delegate resolution to the inner port (e.g. the tool-disclosure | ||
| // forgiving path) rather than the LoopCapabilityPort default, which only | ||
| // searches THIS filter's already-filtered `tool_definitions` and so | ||
| // rejects every deferred/disclosed tool before the inner port can resolve | ||
| // it. Apply the same visible-scope check as `validate`/`register` so a | ||
| // genuinely non-visible call is still rejected. | ||
| let capability_ids = self.inner.provider_tool_call_capability_ids(tool_call)?; | ||
| validate_provider_tool_call_capability_scope( | ||
| capability_ids.clone(), | ||
| |capability_id| self.permits(capability_id), | ||
| "provider tool call is outside the model-visible capability view", | ||
| )?; | ||
| Ok(capability_ids) | ||
| } | ||
|
|
||
| fn validate_provider_tool_call( | ||
| &self, | ||
| tool_call: &ProviderToolCall, | ||
|
|
@@ -161,6 +180,24 @@ impl LoopCapabilityPort for CapabilitySurfaceProfileFilter { | |
| Ok(definitions) | ||
| } | ||
|
|
||
| fn provider_tool_call_capability_ids( | ||
| &self, | ||
| tool_call: &ProviderToolCall, | ||
| ) -> Result<ProviderToolCallCapabilityIds, AgentLoopHostError> { | ||
| // Delegate to the inner port instead of the LoopCapabilityPort default | ||
| // (which only searches this filter's filtered `tool_definitions`), then | ||
| // apply the run-profile scope — mirroring `validate`/`register`. | ||
| let capability_ids = self.inner.provider_tool_call_capability_ids(tool_call)?; | ||
| if !matches!(self.allow_set.as_ref(), CapabilityAllowSet::All) { | ||
| validate_provider_tool_call_capability_scope( | ||
| capability_ids.clone(), | ||
| |capability_id| self.allow_set.permits(capability_id), | ||
| "provider tool call is outside the run-profile surface", | ||
| )?; | ||
| } | ||
| Ok(capability_ids) | ||
| } | ||
|
|
||
| fn validate_provider_tool_call( | ||
| &self, | ||
| tool_call: &ProviderToolCall, | ||
|
|
@@ -1384,4 +1421,66 @@ mod tests { | |
|
|
||
| assert_eq!(surface.version.as_str(), "surface-v1"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn visible_filter_delegates_provider_tool_call_resolution_to_inner() { | ||
| // Regression: the model gateway pre-check calls | ||
| // `provider_tool_call_capability_ids` on whatever capability port it holds | ||
| // — here a CapabilitySurfaceVisibleFilter. If the filter fell back to the | ||
| // LoopCapabilityPort default (which only searches its OWN already-filtered | ||
| // `tool_definitions`), every deferred/disclosed tool would be rejected | ||
| // before the inner forgiving port could resolve it — which is exactly the | ||
| // "outside the visible capability surface" failure that killed every | ||
| // deferred extension-tool call. The filter must delegate to inner, then | ||
| // apply the visible-scope check. | ||
| let inner = Arc::new(SpyPort::default()); | ||
| // Only the advertised tool is in tool_definitions (the filtered surface)... | ||
| *inner | ||
| .tool_definitions | ||
| .lock() | ||
| .expect("tool definitions lock") = | ||
| vec![provider_definition("demo.advertised", "demo__advertised")]; | ||
| // ...but the inner port can still RESOLVE a deferred tool (mimicking the | ||
| // tool-disclosure forgiving path) that is NOT in tool_definitions. | ||
| inner | ||
| .provider_call_capability_ids | ||
| .lock() | ||
| .expect("provider call capability ids lock") | ||
| .insert( | ||
| "demo__deferred".to_string(), | ||
| provider_call_capability_ids(&["demo.deferred"]), | ||
| ); | ||
| // The deferred tool's capability id IS in the model-visible view (it was | ||
| // disclosed via tool_search), so the call must resolve. | ||
| let filter = CapabilitySurfaceVisibleFilter::new( | ||
| Arc::clone(&inner) as Arc<dyn LoopCapabilityPort>, | ||
| [ | ||
| capability_id("demo.advertised"), | ||
| capability_id("demo.deferred"), | ||
| ], | ||
| ); | ||
|
|
||
| let resolved = filter | ||
| .provider_tool_call_capability_ids(&provider_call("demo__deferred")) | ||
| .expect("deferred-but-visible tool resolves through the inner forgiving path"); | ||
| assert_eq!( | ||
| resolved.provider_capability_id, | ||
| capability_id("demo.deferred") | ||
| ); | ||
|
|
||
| // A resolvable tool whose capability id is NOT in the visible view is still | ||
| // rejected by the scope check. | ||
| inner | ||
| .provider_call_capability_ids | ||
| .lock() | ||
| .expect("provider call capability ids lock") | ||
| .insert( | ||
| "demo__hidden".to_string(), | ||
| provider_call_capability_ids(&["demo.hidden"]), | ||
| ); | ||
| let error = filter | ||
| .provider_tool_call_capability_ids(&provider_call("demo__hidden")) | ||
| .expect_err("non-visible tool is rejected"); | ||
| assert_eq!(error.kind, AgentLoopHostErrorKind::InvalidInvocation); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /// Deterministic shadow token estimator for prompt-surface diagnostics. | ||
| #[allow(clippy::manual_div_ceil)] | ||
| pub(crate) fn estimate_tokens(s: &str) -> u32 { | ||
| let char_count = s.chars().count(); | ||
| ((char_count + 3) / 4).min(u32::MAX as usize) as u32 | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::estimate_tokens; | ||
|
|
||
| #[test] | ||
| fn estimate_tokens_uses_ceiling_char_count_over_four() { | ||
| assert_eq!(estimate_tokens(""), 0); | ||
| assert_eq!(estimate_tokens("a"), 1); | ||
| assert_eq!(estimate_tokens("abcd"), 1); | ||
| assert_eq!(estimate_tokens("abcde"), 2); | ||
| } | ||
|
|
||
| #[test] | ||
| fn estimate_tokens_counts_chars_not_bytes() { | ||
| assert_eq!(estimate_tokens("ééééé"), 2); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.