feat(prompt): tell the agent what its sandbox can run
Nightly Build / build (push) Successful in 8m6s
Nightly Build / build (push) Successful in 8m6s
The agent had no way to know its container ships ffmpeg, ripgrep or tesseract, so it either declined work it could do or spent a round finding out. This adds a command list to the system prompt as a **discovery hint** — explicitly not an inventory. Every decision follows from it being a hint: - The allowlist (~35 entries, `container/commands.rs`) is the curation; a full PATH dump is 800 entries of coreutils noise. The probe exists so the list cannot *lie*, not so it can discover: `command -v` at login means we never announce something a container recreate threw away. - The rendered prose says the list is partial and names `command -v`, so a tool outside the allowlist costs one check rather than a wrong conclusion. An empty probe renders as an explicit "could not be read", never as silence under a heading promising a list. - Order is the allowlist's own, grouped by kind of work — the grouping is the curation, and the reader is a model, not a grep. - Staleness is cheap both ways, so there is no invalidation machinery: a login-time snapshot on `UserContext`, non-fatal, refreshed at next login. The gate is the tool, not the sentinel. Every AGENT.md carries `common/sandbox.md` — the four system agents included — and the section is emitted iff the turn's model is shown `execute_cmd`, derived from `allow_tools` plus the security group's visibility filter for a root turn and from `child_defs` for a sub-agent: always the same definitions the model will see. `has_execute_cmd` therefore joins the PrefixCache key, since the group is switchable mid-conversation and that switch already rewrites the tool payload in the same provider cache. The fragment holds only the heading and one stable sentence; every conditional claim lives in the renderer, because prose promising `sudo apt-get install` is not the renderer's to retract when the tool is absent. `execute_cmd`'s own description loses `(python + node available)`: its job is steering away from the shell, and a capability advertisement diluted it.
This commit is contained in:
@@ -0,0 +1,159 @@
|
||||
//! What the agent is told its sandbox can run — a **discovery aid, not an
|
||||
//! inventory**.
|
||||
//!
|
||||
//! The failure this closes is upstream of any tool call: an agent that does not
|
||||
//! know `ffmpeg` is installed either declines the job or spends a round finding
|
||||
//! out. So the point is to make the common case answerable without a round-trip,
|
||||
//! and nothing more. It follows that:
|
||||
//!
|
||||
//! - **The list is curated, not discovered.** `ls /usr/bin` is 800 entries of
|
||||
//! coreutils noise; a hint that long is not a hint. [`PROBE_ALLOWLIST`] is the
|
||||
//! curation — the image's own toolbelt plus the handful of things an agent
|
||||
//! plausibly installs — and its **order is meaningful** (grouped by the kind of
|
||||
//! work), which is why nothing here sorts.
|
||||
//! - **The probe exists so the list cannot lie**, not so it can discover. A
|
||||
//! hand-maintained list drifts from the image, and a container recreate throws
|
||||
//! away everything an agent installed with apt; `command -v` at login means we
|
||||
//! never announce something that is not there.
|
||||
//! - **Incompleteness is stated, not hidden.** The rendered section says the list
|
||||
//! is partial and that more can be installed — so a tool outside the allowlist
|
||||
//! costs the agent one `command -v`, which is what it would have paid anyway.
|
||||
//!
|
||||
//! Because it is a hint, staleness is cheap in both directions: a mid-session
|
||||
//! install is known to the agent that performed it, and a container recreate
|
||||
//! costs one `not found` plus an `apt-get install` on a path the agent was
|
||||
//! already walking. Hence a plain login-time snapshot, refreshed at the next
|
||||
//! login, and no invalidation machinery.
|
||||
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
|
||||
/// How long the probe may take before login gives up on it.
|
||||
const PROBE_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
|
||||
/// The commands worth spending prompt tokens on, in the order they are rendered.
|
||||
///
|
||||
/// Grouped by the kind of work, because the reader is a model deciding whether
|
||||
/// it can do a job — related tools next to each other is the whole value of a
|
||||
/// curated list over a sorted one. Two kinds of entry live here: what
|
||||
/// `container/Dockerfile` installs, and what an agent plausibly adds with
|
||||
/// `sudo apt-get install` (`pandoc`, `cargo`, `yt-dlp`…) — the latter appear
|
||||
/// only once actually installed, at the next login.
|
||||
///
|
||||
/// Keep it short. Every addition is paid on every request of every agent that
|
||||
/// can run commands, and a list long enough to skim is a list that stopped
|
||||
/// being a hint.
|
||||
pub const PROBE_ALLOWLIST: &[&str] = &[
|
||||
// Runtimes and package managers.
|
||||
"python3", "pip3", "node", "npm", "cargo", "go", "php", "perl",
|
||||
// Media.
|
||||
"ffmpeg", "ffprobe", "convert", "yt-dlp",
|
||||
// Documents and OCR.
|
||||
"pdftotext", "pdftoppm", "tesseract", "pandoc",
|
||||
// Text, data, search.
|
||||
"jq", "rg", "sqlite3", "file",
|
||||
// Archives.
|
||||
"unzip", "zip", "tar", "xz", "gzip",
|
||||
// Network and source control.
|
||||
"curl", "wget", "git", "ssh", "rsync", "dig",
|
||||
// Build.
|
||||
"make", "gcc", "g++",
|
||||
];
|
||||
|
||||
/// The shell snippet run inside the container: one `command -v` per allowlist
|
||||
/// entry, printing the ones that resolve.
|
||||
///
|
||||
/// `exit 0` is load-bearing — without it the script's status is that of the last
|
||||
/// `command -v`, so a container missing the final entry would look like a failed
|
||||
/// probe. Entries are interpolated rather than passed positionally because they
|
||||
/// are compile-time constants restricted to `[a-z0-9+._-]` (asserted by
|
||||
/// `allowlist_is_shell_safe`), unlike the user-supplied paths in `exec_fs`.
|
||||
pub fn probe_script() -> String {
|
||||
let mut s = String::from("for c in");
|
||||
for c in PROBE_ALLOWLIST {
|
||||
s.push(' ');
|
||||
s.push_str(c);
|
||||
}
|
||||
s.push_str("; do command -v \"$c\" >/dev/null 2>&1 && echo \"$c\"; done; exit 0");
|
||||
s
|
||||
}
|
||||
|
||||
/// Parses the probe's stdout: one command per line, blanks dropped, duplicates
|
||||
/// collapsed, **order preserved** (the script walks the allowlist, so its output
|
||||
/// already carries the curation).
|
||||
pub fn parse_probe_output(stdout: &str) -> Vec<String> {
|
||||
let mut out: Vec<String> = Vec::new();
|
||||
for line in stdout.lines() {
|
||||
let name = line.trim();
|
||||
if name.is_empty() || out.iter().any(|c| c == name) {
|
||||
continue;
|
||||
}
|
||||
out.push(name.to_string());
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// Probes `container` for the allowlisted commands it actually has.
|
||||
///
|
||||
/// One `docker exec`, bounded by [`PROBE_TIMEOUT`]. Callers treat a failure as an
|
||||
/// empty list: this is a hint, and login must never fail for it.
|
||||
pub async fn probe_container_commands(container: &str) -> Result<Vec<String>> {
|
||||
let stdout = tokio::time::timeout(
|
||||
PROBE_TIMEOUT,
|
||||
super::exec_fs::sh(container, &probe_script(), &[]),
|
||||
)
|
||||
.await
|
||||
.map_err(|_| anyhow::anyhow!("sandbox command probe timed out after {PROBE_TIMEOUT:?}"))?
|
||||
.context("sandbox command probe failed")?;
|
||||
|
||||
Ok(parse_probe_output(&String::from_utf8_lossy(&stdout)))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The allowlist is interpolated straight into a shell script, so every entry
|
||||
/// must be inert there. This is the check that lets `probe_script` skip the
|
||||
/// positional-argument dance `exec_fs` needs for user-supplied paths.
|
||||
#[test]
|
||||
fn allowlist_is_shell_safe() {
|
||||
for c in PROBE_ALLOWLIST {
|
||||
assert!(
|
||||
!c.is_empty()
|
||||
&& c.chars()
|
||||
.all(|ch| ch.is_ascii_alphanumeric() || "+._-".contains(ch)),
|
||||
"allowlist entry is not shell-safe: {c:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allowlist_has_no_duplicates() {
|
||||
let mut seen: Vec<&str> = Vec::new();
|
||||
for c in PROBE_ALLOWLIST {
|
||||
assert!(!seen.contains(c), "duplicate allowlist entry: {c}");
|
||||
seen.push(c);
|
||||
}
|
||||
}
|
||||
|
||||
/// A container missing the *last* allowlist entry must not read as a failed
|
||||
/// probe — see the `exit 0` note on `probe_script`.
|
||||
#[test]
|
||||
fn probe_script_always_exits_zero() {
|
||||
assert!(probe_script().ends_with("exit 0"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_drops_blanks_and_duplicates_and_keeps_order() {
|
||||
let out = parse_probe_output("ffmpeg\n\n jq \nffmpeg\ngit\n");
|
||||
assert_eq!(out, vec!["ffmpeg", "jq", "git"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_of_nothing_is_empty() {
|
||||
assert!(parse_probe_output("").is_empty());
|
||||
assert!(parse_probe_output("\n \n").is_empty());
|
||||
}
|
||||
}
|
||||
@@ -27,7 +27,7 @@ use tokio::io::AsyncWriteExt;
|
||||
/// Runs a shell snippet inside `container` with `args` bound to `$1`, `$2`, …
|
||||
/// Returns raw stdout — callers that expect text decode it themselves, so a
|
||||
/// binary `cat` is not mangled on the way through.
|
||||
async fn sh(container: &str, script: &str, args: &[&str]) -> Result<Vec<u8>> {
|
||||
pub(super) async fn sh(container: &str, script: &str, args: &[&str]) -> Result<Vec<u8>> {
|
||||
let mut argv: Vec<&str> = vec!["exec", container, "sh", "-c", script, "_"];
|
||||
argv.extend_from_slice(args);
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@
|
||||
//! a container can be recreated from the image at any time; boot reconciliation
|
||||
//! relies on that.
|
||||
|
||||
pub mod commands;
|
||||
pub mod exec_fs;
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
@@ -126,30 +126,6 @@ impl AgentCatalog for SkaldAgentCatalog {
|
||||
);
|
||||
let model = meta.client.as_deref().map(ModelHint::name);
|
||||
|
||||
// The child's system context: its own prompt, no per-turn extras.
|
||||
let context = Arc::new(AgentSystemContext {
|
||||
agent_id: id.to_string(),
|
||||
extra_static: None,
|
||||
extra_dynamic: None,
|
||||
tail_reminder: None,
|
||||
substitutions: Default::default(),
|
||||
pool: self.pool.clone(),
|
||||
shared_pool: self.shared_pool.clone(),
|
||||
user_id: self.user_id.clone(),
|
||||
mcp: self.mcp.clone(),
|
||||
// A sub-agent sees the same skills its parent does: in a delegation
|
||||
// the one doing the work is the child, so an index injected only in
|
||||
// the parent would leave it knowing a procedure exists and handing
|
||||
// the job to someone who cannot read it.
|
||||
fs: self.fs.clone(),
|
||||
project_root: scope.project_root.clone(),
|
||||
// The scratchpad is the session's blackboard: a sub-agent reads and
|
||||
// writes the SAME one as its parent.
|
||||
scratchpad_sid: scope.scratchpad_sid,
|
||||
datetime: self.config.datetime.clone(),
|
||||
prefix_cache: self.prefix_cache.clone(),
|
||||
});
|
||||
|
||||
// The child's def list: parent's base minus root-only minus the
|
||||
// re-derived augmentations (added back natively below), plus
|
||||
// sub-agents-only tools, through the approval visibility filter.
|
||||
@@ -176,6 +152,42 @@ impl AgentCatalog for SkaldAgentCatalog {
|
||||
});
|
||||
}
|
||||
|
||||
// The child's system context: its own prompt, no per-turn extras.
|
||||
//
|
||||
// Built here rather than before `child_defs` because the sandbox command
|
||||
// hint is gated on the child's own view of `execute_cmd` — which the
|
||||
// visibility filter above may have just removed. A child that cannot run
|
||||
// commands must not be told what it could run with them.
|
||||
let has_execute_cmd = child_defs.iter().any(|d| {
|
||||
d["function"]["name"].as_str() == Some(crate::tools::tool_names::EXECUTE_CMD)
|
||||
});
|
||||
let context = Arc::new(AgentSystemContext {
|
||||
agent_id: id.to_string(),
|
||||
extra_static: None,
|
||||
extra_dynamic: None,
|
||||
tail_reminder: None,
|
||||
substitutions: Default::default(),
|
||||
pool: self.pool.clone(),
|
||||
shared_pool: self.shared_pool.clone(),
|
||||
user_id: self.user_id.clone(),
|
||||
mcp: self.mcp.clone(),
|
||||
// A sub-agent sees the same skills its parent does: in a delegation
|
||||
// the one doing the work is the child, so an index injected only in
|
||||
// the parent would leave it knowing a procedure exists and handing
|
||||
// the job to someone who cannot read it.
|
||||
fs: self.fs.clone(),
|
||||
project_root: scope.project_root.clone(),
|
||||
// The scratchpad is the session's blackboard: a sub-agent reads and
|
||||
// writes the SAME one as its parent.
|
||||
scratchpad_sid: scope.scratchpad_sid,
|
||||
datetime: self.config.datetime.clone(),
|
||||
// Same sandbox as the parent: one container per user, and the child
|
||||
// runs in it.
|
||||
sandbox_commands: self.config.sandbox_commands.clone(),
|
||||
has_execute_cmd,
|
||||
prefix_cache: self.prefix_cache.clone(),
|
||||
});
|
||||
|
||||
// Native child tools: clarification, sub-delegation (depth permitting),
|
||||
// and the frame-scoped activate_tools with a FRESH grant set — a child
|
||||
// never inherits the parent's activations.
|
||||
|
||||
@@ -47,10 +47,13 @@ use agent_loop::ids::ConversationId;
|
||||
/// minutes; OpenAI's automatic prefix cache is fuzzier and can last longer.
|
||||
pub const PREFIX_TTL: Duration = Duration::from_secs(20 * 60);
|
||||
|
||||
/// A conversation plus the agent running in it. Both are needed: a sub-agent
|
||||
/// shares its parent's conversation but has its own prompt, and therefore its
|
||||
/// own cache prefix.
|
||||
type Key = (ConversationId, String);
|
||||
/// A conversation, the agent running in it, and whether that agent is shown
|
||||
/// `execute_cmd`. The first two because a sub-agent shares its parent's
|
||||
/// conversation but has its own prompt; the third because the sandbox command
|
||||
/// hint appears with the tool, and the security group behind it is switchable
|
||||
/// mid-conversation — a switch that already invalidates the provider's cache by
|
||||
/// rewriting the tool payload, so keying on it here costs nothing.
|
||||
type Key = (ConversationId, String, bool);
|
||||
|
||||
struct Entry {
|
||||
base: String,
|
||||
@@ -134,7 +137,7 @@ mod tests {
|
||||
use super::*;
|
||||
|
||||
fn key(conv: &str, agent: &str) -> Key {
|
||||
(ConversationId::new(conv), agent.to_string())
|
||||
(ConversationId::new(conv), agent.to_string(), true)
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -68,6 +68,9 @@ pub struct LoopConfig {
|
||||
/// Automatic compaction bounds the context instead of a message window.
|
||||
pub auto_compaction_enabled: bool,
|
||||
pub datetime: DatetimeConfig,
|
||||
/// Allowlisted commands this user's container actually has, snapshotted at
|
||||
/// login — the prompt's discovery hint. See [`crate::container::commands`].
|
||||
pub sandbox_commands: Arc<Vec<String>>,
|
||||
pub max_agent_depth: u32,
|
||||
}
|
||||
|
||||
@@ -248,6 +251,23 @@ impl UserLoopRuntime {
|
||||
let TurnInputs { scope, config, live_input } = inputs;
|
||||
let frame_agent = config.agent_id.clone();
|
||||
|
||||
// The agent's own declarations. Loaded once here and used three times
|
||||
// below — for the sandbox hint, the tool set, and the selector's
|
||||
// strength floor.
|
||||
let meta = crate::agents::load_meta(&frame_agent).ok();
|
||||
|
||||
// Whether this turn's model is shown `execute_cmd`, which is what gates
|
||||
// the sandbox command hint. Read from the same two things that decide the
|
||||
// tool set below and in that order: an agent declaring `allow_tools:
|
||||
// false` is shown nothing at all, and otherwise `base_tool_defs` has
|
||||
// already been through the security group's visibility filter
|
||||
// (`session/handler/config.rs`). Deriving it from the registry instead
|
||||
// would advertise a sandbox to exactly the agents that cannot reach it.
|
||||
let has_execute_cmd = meta.as_ref().is_none_or(|m| m.allow_tools)
|
||||
&& config.base_tool_defs.iter().any(|d| {
|
||||
d["function"]["name"].as_str() == Some(crate::tools::tool_names::EXECUTE_CMD)
|
||||
});
|
||||
|
||||
// ── System context ──
|
||||
let system = Arc::new(AgentSystemContext {
|
||||
agent_id: frame_agent.clone(),
|
||||
@@ -263,13 +283,11 @@ impl UserLoopRuntime {
|
||||
project_root: scope.project_root.clone(),
|
||||
scratchpad_sid: scope.scratchpad_sid,
|
||||
datetime: self.config.datetime.clone(),
|
||||
sandbox_commands: self.config.sandbox_commands.clone(),
|
||||
has_execute_cmd,
|
||||
prefix_cache: self.prefix_cache.clone(),
|
||||
});
|
||||
|
||||
// The agent's own declarations. Loaded once here and used twice below —
|
||||
// for the tool set and for the selector's strength floor.
|
||||
let meta = crate::agents::load_meta(&frame_agent).ok();
|
||||
|
||||
// ── Tool set: the native tools, then the surface's legacy ones ──
|
||||
//
|
||||
// Unless the agent declares it gets none. An empty set is not the same as
|
||||
|
||||
@@ -48,6 +48,17 @@ pub struct AgentSystemContext {
|
||||
/// sub-task (the blackboard is shared by every agent of a session).
|
||||
pub scratchpad_sid: i64,
|
||||
pub datetime: DatetimeConfig,
|
||||
/// Allowlisted commands this user's sandbox has, snapshotted at login.
|
||||
pub sandbox_commands: Arc<Vec<String>>,
|
||||
/// Whether **this turn's model** is shown `execute_cmd`.
|
||||
///
|
||||
/// The command list is a hint about a tool, so it appears exactly when the
|
||||
/// tool does — under a restrictive security group, or for an agent declaring
|
||||
/// `allow_tools: false`, advertising a sandbox the model cannot reach is
|
||||
/// noise at best. The flag must therefore be derived from the same
|
||||
/// definitions the model will see, never from the agent's type or from an
|
||||
/// unfiltered registry.
|
||||
pub has_execute_cmd: bool,
|
||||
/// The user's frozen prefixes — `base` is assembled once per conversation
|
||||
/// and reused while its provider cache could still be warm.
|
||||
pub prefix_cache: Arc<PrefixCache>,
|
||||
@@ -60,7 +71,12 @@ impl SystemContextSource for AgentSystemContext {
|
||||
// between rounds — which is what an agent editing an injected memory
|
||||
// file used to cause — invalidates the entire request. It is therefore
|
||||
// built once per conversation and held; see [`super::prefix_cache`].
|
||||
let key = (turn.conversation.clone(), self.agent_id.clone());
|
||||
// `has_execute_cmd` is in the key because the security group can change
|
||||
// mid-conversation (the chat's shield pill), which adds or removes the
|
||||
// sandbox section. Keying on it costs nothing: the same switch rewrites
|
||||
// the tool payload, which sits in the provider's cached prefix too, so
|
||||
// the miss is already paid.
|
||||
let key = (turn.conversation.clone(), self.agent_id.clone(), self.has_execute_cmd);
|
||||
let static_content = match self.prefix_cache.get(&key) {
|
||||
Some(base) => base,
|
||||
None => {
|
||||
@@ -167,6 +183,12 @@ impl AgentSystemContext {
|
||||
&crate::skills::render_index(&self.fs.load()),
|
||||
);
|
||||
}
|
||||
if static_content.contains("__SANDBOX_COMMANDS__") {
|
||||
static_content = static_content.replace(
|
||||
"__SANDBOX_COMMANDS__",
|
||||
&render_sandbox_commands(&self.sandbox_commands, self.has_execute_cmd),
|
||||
);
|
||||
}
|
||||
if static_content.contains("__SHARED_FOLDERS__") {
|
||||
static_content = static_content.replace(
|
||||
"__SHARED_FOLDERS__",
|
||||
@@ -348,6 +370,47 @@ impl AgentSystemContext {
|
||||
}
|
||||
}
|
||||
|
||||
/// `__SANDBOX_COMMANDS__` — the sandbox discovery hint.
|
||||
///
|
||||
/// The prose that varies lives here rather than in `agents/common/sandbox.md`,
|
||||
/// which is the one departure from the `__MCP_LIST__` shape it otherwise
|
||||
/// follows. It has to: when the model is not shown `execute_cmd`, a fragment
|
||||
/// promising `sudo apt-get install` is a lie the renderer could not retract,
|
||||
/// because it would not be the renderer's to retract. So the fragment keeps only
|
||||
/// the heading and its one stable sentence, and every conditional claim is made
|
||||
/// here.
|
||||
///
|
||||
/// Three cases, and the middle one is the reason this is not a one-liner: an
|
||||
/// empty list means the probe could not run, **not** that the sandbox is bare —
|
||||
/// rendering nothing under a heading that promises a list is how the MCP section
|
||||
/// once had a model invent a tool to go find one.
|
||||
fn render_sandbox_commands(commands: &[String], has_execute_cmd: bool) -> String {
|
||||
if !has_execute_cmd {
|
||||
return String::from(
|
||||
// No second sentence pointing at the file tools: an agent declaring
|
||||
// `allow_tools: false` has none of those either, and this line must
|
||||
// be true for every way the flag can come out false.
|
||||
"You cannot run shell commands in this session: `execute_cmd` is not available to you.",
|
||||
);
|
||||
}
|
||||
if commands.is_empty() {
|
||||
return String::from(
|
||||
"The list of installed commands could not be read for this session. Assume the \
|
||||
usual Linux toolbelt is present and check a specific one with \
|
||||
`command -v <name>` before relying on it.",
|
||||
);
|
||||
}
|
||||
format!(
|
||||
"Some of the commands it provides: {}.\n\n\
|
||||
**This list is partial**, not an inventory — the sandbox almost certainly has more, \
|
||||
and its absence from the list is not evidence that a command is missing. Check any \
|
||||
other one with `command -v <name>`. You are free to work in there as you see fit, \
|
||||
including installing what you need with `sudo apt-get install …` (which lasts until \
|
||||
the sandbox is recreated).",
|
||||
commands.join(", ")
|
||||
)
|
||||
}
|
||||
|
||||
// ── Prompt sections resolved from the registry ───────────────────────────────
|
||||
|
||||
|
||||
@@ -535,6 +598,64 @@ fn resolve_harness_tag(content: String) -> String {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
// ── The sandbox command hint ─────────────────────────────────────────────
|
||||
|
||||
fn cmds(v: &[&str]) -> Vec<String> {
|
||||
v.iter().map(|s| s.to_string()).collect()
|
||||
}
|
||||
|
||||
/// The hint is about a tool. Without the tool it is noise at best, and at
|
||||
/// worst it has a restricted agent plan around a shell it cannot open.
|
||||
#[test]
|
||||
fn without_execute_cmd_no_command_is_named() {
|
||||
let out = render_sandbox_commands(&cmds(&["ffmpeg", "jq"]), false);
|
||||
assert!(!out.contains("ffmpeg"), "{out}");
|
||||
assert!(out.contains("execute_cmd"), "{out}");
|
||||
}
|
||||
|
||||
/// An empty list means the probe could not run — never that the sandbox is
|
||||
/// bare. Rendering nothing under a heading that promises a list is how the
|
||||
/// MCP section once had a model invent a tool to go find one.
|
||||
#[test]
|
||||
fn an_unreadable_probe_says_so_and_names_the_way_out() {
|
||||
let out = render_sandbox_commands(&[], true);
|
||||
assert!(!out.trim().is_empty());
|
||||
assert!(out.contains("command -v"), "{out}");
|
||||
}
|
||||
|
||||
/// The list is a hint, so it has to say it is one: a model that reads it as
|
||||
/// an inventory concludes that an unlisted command does not exist.
|
||||
#[test]
|
||||
fn the_list_is_rendered_and_announced_as_partial() {
|
||||
let out = render_sandbox_commands(&cmds(&["ffmpeg", "jq", "pandoc"]), true);
|
||||
assert!(out.contains("ffmpeg, jq, pandoc"), "{out}");
|
||||
assert!(out.contains("partial"), "{out}");
|
||||
assert!(out.contains("command -v"), "{out}");
|
||||
assert!(out.contains("apt-get install"), "{out}");
|
||||
}
|
||||
|
||||
/// Unlike the skills index, the sentinel here is **not** the knob — every
|
||||
/// `AGENT.md` carries the fragment and the runtime decides. So the wiring has
|
||||
/// to hold in both directions of the gate: the sentinel must never survive
|
||||
/// into the prompt, and the commands must appear only with the tool.
|
||||
#[tokio::test]
|
||||
async fn the_sentinel_is_substituted_whichever_way_the_gate_falls() {
|
||||
for has_exec in [true, false] {
|
||||
let agent = PromptFixture::new("You are a fixture.\n\n<!-- SANDBOX_COMMANDS -->\n");
|
||||
let tree = SkillsTree::new(&[]);
|
||||
let base = base_of_sandbox(
|
||||
&agent.id,
|
||||
tree.fs.clone(),
|
||||
Arc::new(cmds(&["ffmpeg"])),
|
||||
has_exec,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(!base.contains("__SANDBOX_COMMANDS__"), "sentinel survived: {base}");
|
||||
assert_eq!(base.contains("ffmpeg"), has_exec, "{base}");
|
||||
}
|
||||
}
|
||||
|
||||
// ── The skills index, as it reaches (or does not reach) the prompt ───────
|
||||
//
|
||||
// These exercise `build_base` rather than the renderer, because the failure
|
||||
@@ -628,6 +749,15 @@ mod tests {
|
||||
}
|
||||
|
||||
async fn base_of(agent_id: &str, fs: core_api::user_fs::UserFs) -> String {
|
||||
base_of_sandbox(agent_id, fs, Arc::new(Vec::new()), false).await
|
||||
}
|
||||
|
||||
async fn base_of_sandbox(
|
||||
agent_id: &str,
|
||||
fs: core_api::user_fs::UserFs,
|
||||
sandbox_commands: Arc<Vec<String>>,
|
||||
has_execute_cmd: bool,
|
||||
) -> String {
|
||||
let pool = Arc::new(SqlitePool::connect("sqlite::memory:").await.unwrap());
|
||||
AgentSystemContext {
|
||||
agent_id: agent_id.to_string(),
|
||||
@@ -643,6 +773,8 @@ mod tests {
|
||||
project_root: None,
|
||||
scratchpad_sid: 1,
|
||||
datetime: DatetimeConfig { enabled: false, timezone: None },
|
||||
sandbox_commands,
|
||||
has_execute_cmd,
|
||||
prefix_cache: Arc::new(crate::loop_adapters::prefix_cache::PrefixCache::new()),
|
||||
}
|
||||
.build_base()
|
||||
|
||||
@@ -237,6 +237,8 @@ pub async fn project(db: &Db, agent: &AgentFixture, case: &Case) -> Vec<Value> {
|
||||
datetime: datetime(),
|
||||
// A cache of its own per projection: each case must see a freshly
|
||||
// assembled prefix, never one another case left behind.
|
||||
sandbox_commands: Arc::new(Vec::new()),
|
||||
has_execute_cmd: false,
|
||||
prefix_cache: Arc::new(crate::loop_adapters::prefix_cache::PrefixCache::new()),
|
||||
};
|
||||
let system = system_source
|
||||
|
||||
@@ -68,6 +68,9 @@ impl ChatSessionManager {
|
||||
max_parallel_subagents: usize,
|
||||
max_tool_result_chars: Option<usize>,
|
||||
datetime_config: DatetimeConfig,
|
||||
// The sandbox commands snapshotted at this user's login — see
|
||||
// `crate::container::commands`.
|
||||
sandbox_commands: Arc<Vec<String>>,
|
||||
tools: Arc<ToolRegistry>,
|
||||
mcp: Arc<dyn McpProvider>,
|
||||
approval: Arc<ApprovalManager>,
|
||||
@@ -100,6 +103,7 @@ impl ChatSessionManager {
|
||||
// configured message cap.
|
||||
auto_compaction_enabled: compactor.auto_enabled(),
|
||||
datetime: datetime_config.clone(),
|
||||
sandbox_commands,
|
||||
max_agent_depth: crate::session::handler::MAX_AGENT_DEPTH as u32,
|
||||
},
|
||||
)?;
|
||||
|
||||
@@ -398,6 +398,8 @@ impl Conversation {
|
||||
config.llm.max_parallel_subagents.unwrap_or(DEFAULT_MAX_PARALLEL_SUBAGENTS),
|
||||
config.llm.max_tool_result_chars,
|
||||
DatetimeConfig { timezone: config.timezone.clone(), ..config.llm.datetime },
|
||||
// No container, no probe: this bundle is inert (§19).
|
||||
Arc::new(Vec::new()),
|
||||
Arc::clone(&tools.tools),
|
||||
// Inert ownerless bundle (§19): the global runtime as a provider,
|
||||
// unfiltered — never actually exercised (no loops, no consumers).
|
||||
|
||||
@@ -250,6 +250,23 @@ impl UserContextFactory {
|
||||
if let Err(e) = self.container.ensure(user_id).await {
|
||||
tracing::warn!(user = %user_id, error = %e, "failed to ensure container before per-user MCP start");
|
||||
}
|
||||
|
||||
// Which of the allowlisted commands this user's sandbox actually has, for
|
||||
// the prompt's discovery hint (`__SANDBOX_COMMANDS__`). Snapshotted here
|
||||
// like fs membership and MCP access, and for the same reason: it changes
|
||||
// at login cadence, not turn cadence. **Non-fatal** — a hint must never
|
||||
// cost a login, and an empty list renders as an honest absence rather
|
||||
// than as a claim that the sandbox is bare.
|
||||
let sandbox_commands = {
|
||||
let name = crate::container::container_name(user_id);
|
||||
match crate::container::commands::probe_container_commands(&name).await {
|
||||
Ok(cmds) => Arc::new(cmds),
|
||||
Err(e) => {
|
||||
tracing::warn!(user = %user_id, error = %e, "sandbox command probe failed; the prompt will omit the command list");
|
||||
Arc::new(Vec::new())
|
||||
}
|
||||
}
|
||||
};
|
||||
let user_mcp = Arc::new(McpManager::new(
|
||||
Arc::clone(&pool),
|
||||
user_shutdown.clone(),
|
||||
@@ -345,6 +362,7 @@ impl UserContextFactory {
|
||||
self.max_parallel_subagents,
|
||||
self.max_tool_result_chars,
|
||||
self.datetime_config.clone(),
|
||||
sandbox_commands,
|
||||
Arc::clone(&self.tools),
|
||||
mcp_view,
|
||||
Arc::clone(&approval),
|
||||
|
||||
@@ -31,7 +31,12 @@ impl Tool for ExecuteCmd {
|
||||
fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Shell }
|
||||
|
||||
fn description(&self) -> &str {
|
||||
"Execute a shell command (sh -c) inside your sandbox container (python + node available). \
|
||||
// No capability advertisement here: which commands the sandbox has is the
|
||||
// system prompt's `<!-- SANDBOX_COMMANDS -->` section, which appears
|
||||
// exactly when this tool does. This description's job is the opposite one
|
||||
// — steering the model *away* from the shell for work a file tool does
|
||||
// better — and the two messages dilute each other.
|
||||
"Execute a shell command (sh -c) inside your sandbox container. \
|
||||
Reserve this for: builds, installs, git, tests, scripts, processes, network, package managers. \
|
||||
Runs as a non-root user; prefix system-package or global installs with `sudo` (e.g. `sudo apt-get install …`). \
|
||||
Do NOT use cat/head/tail to read files — use read_file instead. \
|
||||
|
||||
Reference in New Issue
Block a user