diff --git a/Cargo.lock b/Cargo.lock index d53fccbfa..267957002 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2063,7 +2063,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -2190,7 +2190,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -5299,7 +5299,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -5864,7 +5864,7 @@ dependencies = [ [[package]] name = "pebble-agent" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/pebble?rev=09b17c1b9e6cf7b2f82657d2080f82c73a0064ef#09b17c1b9e6cf7b2f82657d2080f82c73a0064ef" +source = "git+https://github.com/lithoscomputer/pebble?rev=4db661c44b09048db28d892568382408fbbc6c6e#4db661c44b09048db28d892568382408fbbc6c6e" dependencies = [ "async-trait", "futures-util", @@ -5881,7 +5881,7 @@ dependencies = [ [[package]] name = "pebble-coding-agent" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/pebble?rev=09b17c1b9e6cf7b2f82657d2080f82c73a0064ef#09b17c1b9e6cf7b2f82657d2080f82c73a0064ef" +source = "git+https://github.com/lithoscomputer/pebble?rev=4db661c44b09048db28d892568382408fbbc6c6e#4db661c44b09048db28d892568382408fbbc6c6e" dependencies = [ "async-trait", "futures-util", @@ -6246,7 +6246,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.59.0", + "windows-sys 0.60.2", ] [[package]] @@ -6723,7 +6723,7 @@ dependencies = [ "errno 0.3.14", "libc", "linux-raw-sys", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -6782,7 +6782,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -7426,7 +7426,7 @@ version = "1.4.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4db69cba1110affc0e9f7bcd48bbf87b3f4fc7c61fc9155afd4c469eb3d6c1b" dependencies = [ - "errno 0.3.14", + "errno 0.2.8", "libc", ] @@ -7988,10 +7988,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "82a72c767771b47409d2345987fda8628641887d5466101319899796367354a0" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom 0.4.1", "once_cell", "rustix", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -8026,7 +8026,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" dependencies = [ "rustix", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -9093,7 +9093,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index da9564d49..981294dd4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -108,12 +108,13 @@ futures-util = "0.3" # same rev, read from this file. # pebble: the coding agent loop fabro runs its agent stages, Ask Fabro # sessions, hook evaluators, and `fabro exec` on. Pinned by rev to the pebble -# branch that added the live exec output sink and `max_turns` -# (`fabro-exec-sink-max-turns`); re-pin to the merge commit once it lands. -# Pebble pins the same lithos-llm rev as fabro, and its lockfile policy is -# that every shared crate resolves to the version lithos-llm locks. -pebble-agent = { git = "https://github.com/lithoscomputer/pebble", rev = "09b17c1b9e6cf7b2f82657d2080f82c73a0064ef" } -pebble-coding-agent = { git = "https://github.com/lithoscomputer/pebble", rev = "09b17c1b9e6cf7b2f82657d2080f82c73a0064ef" } +# branch `embedder-concerns`, which merges the live exec output sink and +# `max_turns` into pebble's main (so fabro and petri pin one line); re-pin to +# the merge commit once it lands. Pebble pins the same lithos-llm rev as +# fabro, and its lockfile policy is that every shared crate resolves to the +# version lithos-llm locks. +pebble-agent = { git = "https://github.com/lithoscomputer/pebble", rev = "4db661c44b09048db28d892568382408fbbc6c6e" } +pebble-coding-agent = { git = "https://github.com/lithoscomputer/pebble", rev = "4db661c44b09048db28d892568382408fbbc6c6e" } sandbox-driver = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "23062b6ad62ff4665cbbcb7dce037ec9c4c34318" } sandbox-driver-protocol = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "23062b6ad62ff4665cbbcb7dce037ec9c4c34318" } sandbox-driver-host = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "23062b6ad62ff4665cbbcb7dce037ec9c4c34318" } diff --git a/lib/apps/fabro-cli/src/commands/exec.rs b/lib/apps/fabro-cli/src/commands/exec.rs index dd9f062be..4b9044033 100644 --- a/lib/apps/fabro-cli/src/commands/exec.rs +++ b/lib/apps/fabro-cli/src/commands/exec.rs @@ -19,7 +19,7 @@ use fabro_llm::middleware::{Call, Middleware, Next, Output}; use fabro_llm::{Client, ClientOptions, Error as LlmError, ErrorKind}; use fabro_mcp::config::McpServerSettings; use fabro_mcp::connection_manager::McpConnectionManager; -use fabro_sandbox::{RunSandbox, local_sandbox}; +use fabro_sandbox::{RunSandbox, SecretRedactor, local_sandbox}; use fabro_static::EnvVars; use fabro_types::settings::cli::OutputFormat as SettingsOutputFormat; use fabro_types::settings::run::ResolvedMcpEntry; @@ -633,6 +633,7 @@ async fn run_session( .model(format!("{provider_id}/{model}")) .options(options) .tool_middleware(Arc::new(permission_middleware)) + .redactor(Arc::new(SecretRedactor)) .web_fetch_summarizer(summarizer_model(&catalog, &provider_id, &model)) .subagents(SubagentOptions::enabled()); if let Some(manager) = &mcp { diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs index c511b9ca7..ba85d0e42 100644 --- a/lib/apps/fabro-server/src/server/handler/sessions.rs +++ b/lib/apps/fabro-server/src/server/handler/sessions.rs @@ -16,6 +16,7 @@ use fabro_api::types::{ }; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::{FabroClient, ModelSelectionError, selection}; +use fabro_sandbox::SecretRedactor; use fabro_sandbox::reconnect::reconnect_for_run; use fabro_store::{ EventPayload, ProjectedRunSession, RunDatabase, project_run_session, project_run_sessions, @@ -796,6 +797,7 @@ async fn build_agent( AskFabroToolPolicy, )))) .system_prompt_transform(Arc::new(AskFabroPrompt)) + .redactor(Arc::new(SecretRedactor)) .build() .await .map_err(|err| AskFabroBuildError::Agent(anyhow::Error::new(err))) diff --git a/lib/components/fabro-hooks/src/executor.rs b/lib/components/fabro-hooks/src/executor.rs index 299ce7f5e..a054d7d96 100644 --- a/lib/components/fabro-hooks/src/executor.rs +++ b/lib/components/fabro-hooks/src/executor.rs @@ -8,13 +8,13 @@ use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::{Client, ClientOptions, Request}; use fabro_redact::redacted_url_for_log; -use fabro_sandbox::RunSandbox; +use fabro_sandbox::{RunSandbox, SecretRedactor}; use fabro_types::PermissionLevel; use fabro_types::settings::{InterpString, ResolveCtx, ResolveError}; use pebble_coding_agent::extensions::{ SystemPromptContext, SystemPromptDecision, SystemPromptTransform, }; -use pebble_coding_agent::{CodingAgent, CodingAgentOptions, ShutdownReason}; +use pebble_coding_agent::{CodingAgent, CodingAgentOptions, Error as AgentError, ShutdownReason}; use tokio::process::Command as TokioCommand; use tokio::time::timeout as tokio_timeout; @@ -25,6 +25,9 @@ use crate::types::{ const HOOK_EVALUATOR_SYSTEM_PROMPT: &str = "You are a hook evaluator for a workflow engine. Given context about a workflow event, evaluate the condition."; +/// How many tool rounds an agent hook may run when its definition names none. +const DEFAULT_MAX_TOOL_ROUNDS: u32 = 50; + static HOOK_RESPONSE_SCHEMA: LazyLock = LazyLock::new(|| { serde_json::json!({ "type": "object", @@ -364,8 +367,16 @@ impl HookExecutorImpl { /// /// The agent runs pebble's full tool set at `PermissionLevel::Full`, with /// no memory or skills, the evaluator system prompt in place of the - /// profile's, and `max_tool_rounds` as its turn budget. Exhausting the - /// budget, an LLM failure, or a timeout all fail open. + /// profile's, and `max_tool_rounds` as pebble's tool-round budget. + /// Exhausting the budget, an LLM failure, or a timeout all fail open. + /// + /// Fabro's `max_tool_rounds` names how many model turns the hook may + /// take, executing the tools each asks for, before it proceeds on a turn + /// that still asks for tools. Pebble's budget of `rounds` lets `rounds` + /// tool turns run and refuses the next one without running its tools, so + /// `max_tool_rounds - 1` reaches the same decision at the same turn and + /// spares the last, useless tool execution. Zero is a loop that never + /// asks the model: the hook proceeds without an agent. async fn execute_agent( definition: &HookDefinition, prompt: &InterpString, @@ -388,6 +399,14 @@ impl HookExecutorImpl { let resolved_model = Self::resolve_model(model.as_deref()); let user_msg = Self::build_hook_user_message(&prompt, context); + let Some(rounds) = max_tool_rounds + .unwrap_or(DEFAULT_MAX_TOOL_ROUNDS) + .checked_sub(1) + else { + tracing::warn!("agent hook allows no tool rounds, proceeding"); + return HookDecision::Proceed; + }; + let rounds = usize::try_from(rounds).unwrap_or(usize::MAX); Self::execute_llm_with_timeout(definition.timeout(), "agent", || async move { let client = match Self::build_client(catalog, llm_source).await { @@ -398,14 +417,14 @@ impl HookExecutorImpl { } }; - let max_turns = usize::try_from(max_tool_rounds.unwrap_or(50).max(1)).unwrap_or(50); let options = CodingAgentOptions::default() .with_context_compaction(false) - .with_max_turns(max_turns); + .with_max_tool_rounds(rounds); let mut agent = match CodingAgent::builder(client, sandbox) .model(resolved_model) .permission_level(PermissionLevel::Full) .system_prompt_transform(Arc::new(HookEvaluatorPrompt)) + .redactor(Arc::new(SecretRedactor)) .options(options) .build() .await @@ -420,6 +439,10 @@ impl HookExecutorImpl { let report = agent.prompt(user_msg).await; let decision = match report.result { Ok(output) => Self::parse_prompt_response(output.text.as_deref().unwrap_or("")), + Err(AgentError::ToolRoundsExhausted { .. }) => { + tracing::warn!("agent hook exhausted max tool rounds, proceeding"); + HookDecision::Proceed + } Err(e) => { tracing::warn!(error = %e, "agent hook did not complete, proceeding"); HookDecision::Proceed diff --git a/lib/components/fabro-sandbox/src/lib.rs b/lib/components/fabro-sandbox/src/lib.rs index 29a0d3042..7dc02155a 100644 --- a/lib/components/fabro-sandbox/src/lib.rs +++ b/lib/components/fabro-sandbox/src/lib.rs @@ -61,6 +61,7 @@ pub use push_credentials::RefreshErrorKind; pub use reconnect::{ reconnect, reconnect_driver_for_run, reconnect_for_run, reconnect_for_run_with_events, }; +pub use redact::SecretRedactor; pub use sandbox::{ CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, ExecResult, ExecStreamingRequest, ExecStreamingResult, GitRunInfo, GitSetupIntent, OutputCaptureStats, PushAttempt, PushError, diff --git a/lib/components/fabro-sandbox/src/redact.rs b/lib/components/fabro-sandbox/src/redact.rs index 12afa2fd6..bd94986de 100644 --- a/lib/components/fabro-sandbox/src/redact.rs +++ b/lib/components/fabro-sandbox/src/redact.rs @@ -1,6 +1,53 @@ +//! Fabro's secret scanner on the text seams pebble and the sandbox expose. + +use std::borrow::Cow; + +use pebble_coding_agent::extensions::Redactor; + +/// Strips a specific authenticated URL out of `text`, when one is known. pub fn redact_auth_url(text: &str, auth_url: Option<&fabro_redact::DisplaySafeUrl>) -> String { let Some(auth_url) = auth_url else { return text.to_string(); }; text.replace(&auth_url.raw_string(), &auth_url.redacted_string()) } + +/// Fabro's secret scanner as pebble's [`Redactor`]. +/// +/// Pebble calls it where text a process or the operating system wrote leaves +/// a session: the output tail a shell tool puts on the event stream and the +/// model-facing message of a failed tool call. It runs the same +/// `fabro_redact::redact_string` pass the run's stored events go through, so +/// what the model reads back matches what the log keeps. The final pass over +/// every stored `RunEvent` stays in place: this one covers the text pebble +/// hands the model and does not replace redaction of the stored event. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct SecretRedactor; + +impl Redactor for SecretRedactor { + fn redact<'a>(&self, text: &'a str) -> Cow<'a, str> { + let redacted = fabro_redact::redact_string(text); + if redacted == text { + Cow::Borrowed(text) + } else { + Cow::Owned(redacted) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_secret_redactor_borrows_clean_text_and_masks_secrets() { + let redactor = SecretRedactor; + assert!(matches!( + redactor.redact("plain stderr"), + Cow::Borrowed("plain stderr") + )); + let redacted = redactor.redact("key=AKIAYRWQG5EJLPZLBYNP"); + assert!(matches!(redacted, Cow::Owned(_))); + assert_eq!(redacted, "key=REDACTED"); + } +} diff --git a/lib/components/fabro-workflow/src/agent_memory.rs b/lib/components/fabro-workflow/src/agent_memory.rs index 5779b6e0d..2fa9cb4e5 100644 --- a/lib/components/fabro-workflow/src/agent_memory.rs +++ b/lib/components/fabro-workflow/src/agent_memory.rs @@ -3,17 +3,19 @@ //! Pebble loads memory from explicit paths and looks in no conventional //! location. Fabro supplies the convention: each coding harness reads the //! instruction files its vendor's own agent reads, found in the sandbox -//! working directory. +//! working directory. The loading itself — the 32 KB budget, duplicate text +//! skipped, the file that crosses the budget cut and marked — is pebble's +//! [`ProjectMemory`], the same loader an agent stage runs over +//! `with_memory_files`, so a prompt stage reads by the agent's rules. use fabro_sandbox::RunSandbox; use fabro_types::AgentProfileKind; +use pebble_coding_agent::environment::Environment; +use pebble_coding_agent::{InterruptReason, ProjectMemory}; use tokio_util::sync::CancellationToken; use crate::error::Error; -/// The most memory text a stage loads into its system prompt. -pub const MEMORY_BUDGET_BYTES: usize = 32_768; - /// The instruction filenames a harness reads, in load order. #[must_use] pub fn memory_filenames(profile_kind: AgentProfileKind) -> &'static [&'static str] { @@ -45,55 +47,25 @@ pub fn memory_paths(working_dir: &str, profile_kind: AgentProfileKind) -> Vec Result, Error> { - let mut documents = Vec::new(); - let mut seen = std::collections::HashSet::new(); - let mut budget = MEMORY_BUDGET_BYTES; - for path in memory_paths(working_dir, profile_kind) { - if cancel.is_cancelled() { - return Err(Error::Cancelled); - } - let Ok(content) = sandbox.read_file_text(&path).await else { - continue; - }; - if content.is_empty() || !seen.insert(content.clone()) { - continue; - } - if budget == 0 { - break; - } - if content.len() <= budget { - budget -= content.len(); - documents.push(content); - } else { - documents.push(truncate_to_budget(&content, budget)); - budget = 0; - } - } - if documents.is_empty() { - Ok(None) - } else { - Ok(Some(documents.join("\n\n"))) - } -} - -fn truncate_to_budget(content: &str, budget: usize) -> String { - const MARKER: &str = "[Project instructions truncated at 32KB]"; - if budget <= MARKER.len() { - return MARKER[..budget].to_string(); - } - let keep = content.floor_char_boundary(budget - MARKER.len()); - format!("{}{MARKER}", &content[..keep]) + let paths = memory_paths(working_dir, profile_kind); + let memory = ProjectMemory::load(sandbox as &dyn Environment, &paths, cancel) + .await + .map_err(|error| match error { + pebble_coding_agent::Error::Interrupted(InterruptReason::Cancelled) => Error::Cancelled, + other => Error::handler_with_source("Failed to load project memory", other), + })?; + Ok((!memory.is_empty()).then(|| memory.text())) } #[cfg(test)] diff --git a/lib/components/fabro-workflow/src/handler/llm/pebble.rs b/lib/components/fabro-workflow/src/handler/llm/pebble.rs index 1b80ef57c..63040474c 100644 --- a/lib/components/fabro-workflow/src/handler/llm/pebble.rs +++ b/lib/components/fabro-workflow/src/handler/llm/pebble.rs @@ -19,7 +19,7 @@ use fabro_llm::types::ResponseFormat; use fabro_llm::{Client, ClientOptions, ErrorData, Request, Response}; use fabro_mcp::config::McpServerSettings; use fabro_mcp::connection_manager::McpConnectionManager; -use fabro_sandbox::RunSandbox; +use fabro_sandbox::{RunSandbox, SecretRedactor}; use fabro_types::settings::run::RunModelControls; use fabro_types::{ AgentProfileKind, ModelRef, PermissionLevel, Principal, SessionCapability, StageId, @@ -68,6 +68,15 @@ use crate::services::FabroRunToolServices; use crate::steering_hub::{ActiveControlHandle, SteeringHub, SteeringItem}; use crate::web_search::{SearchBackend, SearchSecrets}; +/// The share of the model's context window at which an agent stage compacts +/// its conversation. Fabro's own agent loop used this value; pebble's default +/// is the same, and it is set here so the stage's policy is fabro's to state. +pub const COMPACTION_THRESHOLD_PERCENT: usize = 80; + +/// How many recent turns compaction leaves verbatim, as fabro's agent loop +/// did. +pub const COMPACTION_PRESERVE_TURNS: usize = 6; + /// The API backend: pebble coding agents over the workflow's LLM client. pub struct PebbleBackend { model: String, @@ -131,6 +140,13 @@ fn classify_agent_error( "Agent session used every model turn it was allowed".to_string(), )) } + // Stages set no round budget; the arm names the outcome should one + // ever be configured. + pebble_coding_agent::Error::ToolRoundsExhausted { limit } => { + AgentErrorDisposition::Terminal(Error::Precondition(format!( + "Agent session reached its limit of {limit} tool rounds" + ))) + } pebble_coding_agent::Error::EventSink(sink) => AgentErrorDisposition::Terminal(Error::Io( format!("Failed to persist agent events: {sink:#}"), )), @@ -619,6 +635,9 @@ impl PebbleBackend { )) .with_skill_dirs(self.skill_dirs(sandbox)) .with_recorded_permission_level(PermissionLevel::Full) + .with_context_compaction(true) + .with_compaction_threshold_percent(COMPACTION_THRESHOLD_PERCENT) + .with_compaction_preserve_turns(COMPACTION_PRESERVE_TURNS) } /// Start the stage's MCP servers, reporting each as a run event. @@ -691,6 +710,7 @@ impl PebbleBackend { scope: bindings.stage_scope.clone(), file_tracking: Arc::clone(bindings.file_tracking), })) + .redactor(Arc::new(SecretRedactor)) .subagents(SubagentOptions::enabled()); if let Some(provider) = &self.tool_env { builder = builder.tool_env_provider(Arc::clone(provider));