Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked.

## [Unreleased]

### Fixed

- **Run-loop capsule tools now appear in `tools/list`.** A pool-less run-loop capsule's tool-describe path returned `NotSupported`, which was captured as a present-but-empty tool set, so the describe fan-out — which fires only on *absent* tools — never consulted the capsule's own `tool.v1.request.describe` responder. The load-time capture now distinguishes "couldn't capture" from "captured, empty" and leaves tools absent for pool-less capsules so the fan-out fires. Closes #1198.
### Removed

- **The unused SurrealDB query wrapper and its dormant dependency graph have
Expand Down
5 changes: 4 additions & 1 deletion crates/astrid-capsule/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ pub use memory_ledger::MemoryLedger;
// `StoreMemoryMeter` is the Wasmtime `ResourceLimiter`; native-only.
#[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))]
pub use memory_ledger::StoreMemoryMeter;
pub use tool_discovery::{ToolDescriptor, describe_loaded_capsule, tools_missing_execute_route};
pub use tool_discovery::{
ToolDescriptor, describe_loaded_capsule, describe_loaded_capsule_status,
tools_missing_execute_route,
};

/// Test-only access to security boundaries that integration tests must drive
/// through real operating-system transports.
Expand Down
105 changes: 99 additions & 6 deletions crates/astrid-capsule/src/tool_discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,63 @@ pub struct ToolDescriptor {
/// Returns an error if the interceptor errors for any reason other than "not
/// implemented", genuinely denies (a reason other than the unknown-action one),
/// or returns a payload that is present but not the expected JSON shape.
///
/// Preserves the original signature: a pool-less run-loop capsule's *unknown*
/// surface collapses to an empty vec here. Callers that must distinguish
/// "unknown" from "empty" — to drive the describe fan-out (#1198) — use
/// [`describe_loaded_capsule_status`] instead.
pub async fn describe_loaded_capsule(capsule: &dyn Capsule) -> anyhow::Result<Vec<ToolDescriptor>> {
let result = match capsule.invoke_interceptor("tool_describe", &[], None).await {
Ok(describe_loaded_capsule_status(capsule)
.await?
.unwrap_or_default())
}

/// Like [`describe_loaded_capsule`], but preserves the "surface unknown" signal
/// the load-time capture needs.
///
/// Returns:
/// - `Ok(Some(tools))` — the describe ran and returned a (possibly empty) tool
/// surface; the caller injects it into the `capsules_loaded` meta.
/// - `Ok(None)` — the describe COULD NOT run because this is a pool-less
/// run-loop capsule (`invoke_interceptor` reports `NotSupported`). The caller
/// must then leave `tools` ABSENT (not `[]`), so the consumer's describe
/// fan-out fires and the capsule's own `tool.v1.request.describe` responder
/// supplies the surface. Injecting `[]` here reads as "0 tools" and suppresses
/// the fan-out — the bug in #1198.
///
/// # Errors
///
/// Same as [`describe_loaded_capsule`].
pub async fn describe_loaded_capsule_status(
capsule: &dyn Capsule,
) -> anyhow::Result<Option<Vec<ToolDescriptor>>> {
interpret_describe_result(capsule.invoke_interceptor("tool_describe", &[], None).await)
}

/// Pure mapping of a `tool_describe` interceptor outcome to a captured tool
/// surface. Split out from [`describe_loaded_capsule`] so the capture semantics
/// — especially the pool-less `NotSupported => None` case that fixes #1198 — are
/// unit-testable without a live capsule.
fn interpret_describe_result(
outcome: Result<InterceptResult, crate::error::CapsuleError>,
) -> anyhow::Result<Option<Vec<ToolDescriptor>>> {
let result = match outcome {
Ok(r) => r,
Err(e) if is_unsupported(&e) => return Ok(Vec::new()),
// Pool-less run-loop capsule: the interceptor path isn't available, so
// the tool surface is UNKNOWN — not empty. Signal absent (`None`) so the
// caller lets the describe fan-out supply it (#1198), rather than
// injecting `[]` and suppressing the fan-out.
Err(e) if is_unsupported(&e) => return Ok(None),
Err(e) => return Err(anyhow::anyhow!("tool_describe interceptor failed: {e}")),
};

let payload = match result {
InterceptResult::Continue(bytes) | InterceptResult::Final(bytes) => bytes,
// No `tool_describe` arm => "no tools", treated like NotSupported above.
// A capsule that CAN run interceptors but has no `#[astrid::tool]` arm
// (e.g. the broker) genuinely has zero static tools: captured-empty
// (`Some([])`), NOT absent — there is nothing for a fan-out to supply.
InterceptResult::Deny { reason } if is_unknown_action(&reason) => {
return Ok(Vec::new());
return Ok(Some(Vec::new()));
},
// Any other deny is a genuine refusal — surface it.
InterceptResult::Deny { reason } => {
Expand All @@ -66,10 +111,10 @@ pub async fn describe_loaded_capsule(capsule: &dyn Capsule) -> anyhow::Result<Ve
};

if payload.is_empty() {
return Ok(Vec::new());
return Ok(Some(Vec::new()));
}

parse_tool_descriptors(&payload)
parse_tool_descriptors(&payload).map(Some)
}

/// Parse the `tools` array out of a `tool_describe` descriptor payload
Expand Down Expand Up @@ -150,6 +195,54 @@ fn is_unknown_action(reason: &str) -> bool {
mod tests {
use super::*;

/// #1198: a pool-less run-loop capsule's `NotSupported` describe maps to
/// `None` (surface UNKNOWN → leave absent so the describe fan-out fires), NOT
/// to a captured-empty `Some([])` which reads as "0 tools" and suppresses it.
#[test]
fn pool_less_notsupported_maps_to_none_for_fan_out() {
let out = interpret_describe_result(Err(crate::error::CapsuleError::NotSupported(
"no interceptors".into(),
)));
assert!(matches!(out, Ok(None)));
}

/// A capsule that CAN run interceptors but has no `#[astrid::tool]` (the
/// broker) is captured-empty (`Some([])`), not absent — nothing to fan out.
#[test]
fn unknown_action_deny_is_captured_empty_not_absent() {
let out = interpret_describe_result(Ok(InterceptResult::Deny {
reason: "unknown hook action: tool_describe".into(),
}));
assert!(matches!(out, Ok(Some(ref v)) if v.is_empty()));
}

#[test]
fn described_tools_are_captured_as_some() {
let payload = serde_json::to_vec(&serde_json::json!({
"tools": [{ "name": "t", "description": "d", "input_schema": { "type": "object" } }]
}))
.unwrap();
let tools = interpret_describe_result(Ok(InterceptResult::Continue(payload)))
.expect("ok")
.expect("some");
assert_eq!(tools.len(), 1);
assert_eq!(tools[0].name, "t");
}

#[test]
fn empty_describe_payload_is_captured_empty() {
let out = interpret_describe_result(Ok(InterceptResult::Continue(Vec::new())));
assert!(matches!(out, Ok(Some(ref v)) if v.is_empty()));
}

#[test]
fn genuine_deny_surfaces_as_error() {
let out = interpret_describe_result(Ok(InterceptResult::Deny {
reason: "capability denied: caps:token:mint".into(),
}));
assert!(out.is_err());
}

#[test]
fn unknown_action_deny_matches_only_tool_describe() {
// A capsule with interceptors but no `#[astrid::tool]` (e.g. the broker)
Expand Down
59 changes: 49 additions & 10 deletions crates/astrid-kernel/src/capsules_loaded.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
//! Helpers for the `astrid.v1.capsules_loaded` broadcast payload.
//!
//! The kernel surfaces, per loaded capsule, its installed `meta.json` plus its
//! tool surface. A surface baked into `meta.json` at build time is forwarded
//! verbatim; one that was not baked is filled in by the kernel probing the live
//! capsule's `tool_describe` and injecting the result ([`inject_tools`]), so an
//! un-rebuilt (or third-party) capsule still contributes a complete surface.
//! Either way the kernel invokes-and-forwards — it does not interpret the
//! descriptors, the way a Linux uevent carries a device's attributes and leaves
//! all interpretation to userspace. A sandboxed consumer (e.g. the sage-mcp
//! broker) derives a deterministic tool surface from this signal, instead of a
//! racy describe fan-out, without itself gaining filesystem access.
//! live tool surface. The reserved `tools` field is removed from installed
//! metadata before the kernel probes the capsule's `tool_describe`; a successful
//! probe injects the current result ([`inject_tools`]), while an unavailable or
//! failed probe leaves the field absent so consumers can fall back to describe
//! fan-out. This also prevents tool surfaces baked by older Astrid releases from
//! suppressing that fallback. The kernel invokes-and-forwards — it does not
//! interpret the descriptors, the way a Linux uevent carries a device's
//! attributes and leaves all interpretation to userspace. A sandboxed consumer
//! (e.g. the sage-mcp broker) derives a deterministic tool surface from this
//! signal without itself gaining filesystem access.
//!
//! These helpers are the pure payload-assembly pieces ([`read_capsule_meta_opaque`],
//! [`meta_has_tools`], [`inject_tools`], [`build_capsules_loaded_payload`]), kept
//! [`without_tools`], [`inject_tools`], [`build_capsules_loaded_payload`]), kept
//! off [`crate::Kernel`] so they are unit-testable without a running kernel; the
//! live `tool_describe` probe itself lives in `Kernel::publish_capsules_loaded`.

Expand All @@ -31,6 +32,23 @@ pub(crate) fn read_capsule_meta_opaque(source_dir: &Path) -> Option<Value> {
serde_json::from_slice(&bytes).ok()
}

/// Remove the reserved live `tools` field from opaque installed metadata.
///
/// Older Astrid releases persisted build-time tool descriptors in `meta.json`.
/// They must not be treated as current: a run-loop capsule needs an absent field
/// to trigger consumer describe fan-out, and a failed live probe must not expose
/// stale descriptors. Other metadata, including non-object values, remains
/// untouched.
pub(crate) fn without_tools(meta: Option<Value>) -> Option<Value> {
meta.map(|value| match value {
Value::Object(mut map) => {
map.remove("tools");
Value::Object(map)
},
other => other,
})
}

/// Inject a freshly-described `tools` array into a capsule's opaque `meta`.
///
/// `meta` is the capsule's `meta.json` value (or `None` if it had none); the
Expand Down Expand Up @@ -94,6 +112,27 @@ mod tests {
assert_eq!(out2["tools"], tools);
}

#[test]
fn without_tools_removes_legacy_surface_and_preserves_other_meta() {
let meta = json!({
"version": "1.0.0",
"tools": [{ "name": "stale_tool" }],
"wasm_hash": "abc"
});

let out = without_tools(Some(meta)).expect("object metadata");

assert!(out.get("tools").is_none());
assert_eq!(out["version"], "1.0.0");
assert_eq!(out["wasm_hash"], "abc");
}

#[test]
fn without_tools_preserves_absent_and_nonobject_meta() {
assert_eq!(without_tools(None), None);
assert_eq!(without_tools(Some(json!("opaque"))), Some(json!("opaque")));
}

#[test]
fn payload_retains_status_and_lists_capsules() {
let meta = json!({ "version": "1.0.0", "tools": [{ "name": "read_file" }] });
Expand Down
20 changes: 18 additions & 2 deletions crates/astrid-kernel/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1963,12 +1963,16 @@ impl Kernel {
self.verify_workspace_capsule_tree(source_dir).ok()?;
meta
});
// `tools` is live-owned data. Strip surfaces persisted by older
// Astrid releases before probing so an unavailable/failed probe
// leaves the field genuinely absent and consumer fan-out can run.
meta = capsules_loaded::without_tools(meta);

// Probe the live instance for its tool surface and inject it. Best-
// effort: a describe (or serialize) failure leaves `tools` absent
// and the consumer falls back to its fan-out for this cycle.
match astrid_capsule::describe_loaded_capsule(capsule.as_ref()).await {
Ok(tools) => {
match astrid_capsule::describe_loaded_capsule_status(capsule.as_ref()).await {
Ok(Some(tools)) => {
// A tool advertises straight from its `#[astrid::tool]`
// annotation, but only EXECUTES if the manifest `[subscribe]`s
// its `tool.v1.execute.<name>` topic (the dispatcher routes
Expand Down Expand Up @@ -2004,6 +2008,18 @@ impl Kernel {
),
}
},
Ok(None) => {
// Pool-less / run-loop capsule: the interceptor describe
// can't run, so leave `tools` ABSENT (not `[]`). The
// consumer's describe fan-out then fires and the capsule's
// own `tool.v1.request.describe` responder supplies its
// surface. Injecting `[]` reads as "0 tools" and suppresses
// the fan-out (#1198).
tracing::debug!(
capsule_id = %name,
"pool-less/run-loop capsule: leaving tools absent so the describe fan-out fires (#1198)"
);
},
Err(e) => tracing::debug!(
capsule_id = %name, error = %e,
"live tool_describe failed; capsule left uncaptured this cycle"
Expand Down
Loading