diff --git a/docs/plans/2026-03-20-engine-v2-architecture.md b/docs/plans/2026-03-20-engine-v2-architecture.md index 21f19c55..46d4dc03 100644 --- a/docs/plans/2026-03-20-engine-v2-architecture.md +++ b/docs/plans/2026-03-20-engine-v2-architecture.md @@ -383,46 +383,25 @@ Engine broadcasts `ThreadEvent`s via `tokio::broadcast`. Router subscribes and f - Loaded on startup via `load_docs_from_workspace()` - Full DB persistence (engine_* tables) deferred — workspace persistence is sufficient for learning across sessions -#### Web gateway integration (NOT YET IMPLEMENTED) +#### Web gateway integration — DONE +- SSE streaming via AppEvent: `ThreadEvent` → `AppEvent` conversion + `SseManager.broadcast()` +- V1 conversation DB persistence: user messages + agent responses written via `add_conversation_message()` +- Depends on `ironclaw_common` crate with `AppEvent` type (PR #1615, merged into branch) -The web gateway is completely disconnected from engine v2. Three gaps: +#### Routines / Jobs — BLOCKED (gracefully) +- V1-only tools (`routine_create`, `create_job`, `build_software`, etc.) are blocked in engine v2 with a helpful error: "use the slash command instead" +- Filtered out of `available_actions()` so the system prompt doesn't list them +- Routines still work via `/routine` slash commands (fall through to v1) +- Long term: replace with engine v2 Mission system -**1. No SSE event streaming** -- Gateway expects `SseEvent` variants via `SseManager.broadcast()` -- Engine v2 emits `ThreadEvent` via `tokio::broadcast` (forwarded to channel as `StatusUpdate` — works for REPL but not web) -- **Fix**: Bridge `ThreadEvent` → `SseEvent` (or common `AppEvent` type) and broadcast to `SseManager` -- **Prerequisite**: SseEvent extraction to common AppEvent type (separate PR in progress) -- Events to bridge: `StepStarted` → `Thinking`, `ActionExecuted` → `ToolCompleted`, `ActionFailed` → `ToolCompleted(error)`, thread completion → `Response` +#### Rate limiting — DONE +- Per-user per-tool sliding window via `RateLimiter` in `EffectBridgeAdapter` +- Checks `tool.rate_limit_config()` before every execution +- Returns "rate limited, try again in Ns" error -**2. No conversation persistence for gateway** -- Gateway reads chat history from v1 `ConversationStore` DB tables -- Engine v2 writes to `HybridStore` (in-memory + workspace) which gateway doesn't query -- **Fix**: After engine thread completes, write user message + response to v1 conversation tables via `ConversationStore::add_conversation_message()` -- This gives the gateway history without changing any gateway code - -**3. No cross-channel message visibility** -- REPL messages processed by engine v2 don't appear in web gateway -- Web gateway messages processed by engine v2 don't appear in REPL history -- **Fix**: Same as #2 — writing to v1 conversation tables makes messages visible everywhere - -**Implementation approach** (minimal, after AppEvent PR): -1. Router accepts `sse_tx: Option>` from `Agent.deps` -2. Forward `ThreadEvent` → `SseEvent` during the event polling loop (alongside channel `StatusUpdate`) -3. After thread completion, write user message + response to v1 DB via `store.add_conversation_message()` -4. Gateway reads from DB as usual — no gateway code changes needed - -#### Routines / Jobs (NOT HOOKED UP) - -Routines are entirely v1 — `RoutineEngine` fires via cron/event triggers and runs through `run_agentic_loop()` with its own delegate. Engine v2 routing only intercepts `UserInput` and `ApprovalResponse` in `handle_message`. Routine execution doesn't go through `handle_message`. - -**Known issue:** When a user asks "create a routine for..." as natural language, engine v2 processes it via CodeAct. The model calls `routine_create(...)` which needs `Arc` + `Arc` — these exist on the tool but the `JobContext` built by the bridge has minimal fields. This can cause crashes (observed: SIGKILL during routine creation attempt). - -**Options:** -- Short term: block routine/job tools in engine v2 (return "use /routine command instead") -- Medium term: pass `RoutineEngine` + DB refs through `JobContext.metadata` or a dedicated context field -- Long term: replace routines with engine v2 `Mission` system - -Engine v2 has `Mission` types (`MissionManager`, `MissionCadence`, `MissionStatus`) defined but not wired to trigger infrastructure. +#### Per-step tool call limit — DONE +- Max 50 tool calls per code step (prevents amplification loops in CodeAct) +- Atomic counter in `EffectBridgeAdapter`, error on exceed #### Acceptance testing (NOT YET IMPLEMENTED) - Drive engine via TestRig + TraceLlm fixtures @@ -533,11 +512,11 @@ Once boundaries stabilize, split if beneficial: | **3** | CodeAct (Monty + RLM pattern) | **DONE** | 74 | `b59a0b9`, `9538332` | | **4** | Budget controls + compaction + reflection | **DONE** | 78 | `4bc7ffd` | | **5** | Conversation surface | **DONE** | 85 | `0827235` | -| **6** | Main crate bridge (Strategy C) | **DONE** (partial) | 134 | `ac4ced0`→`7afeaa9c` | +| **6** | Main crate bridge (Strategy C) | **DONE** | 151 | `ac4ced0`→`ccec1917` | | **7** | Cleanup + migration | Planned | — | — | | **8** | WASM tools + Docker isolation | Planned | — | — | -**Phase 6 remaining:** approval flow, DB persistence, acceptance tests, two-phase commit. +**Phase 6 remaining:** acceptance tests (TestRig fixtures), two-phase commit. Phase 7 depends on Phase 6 approval + DB being complete. Phase 8 is infrastructure integration. --- diff --git a/src/bridge/effect_adapter.rs b/src/bridge/effect_adapter.rs index ae4d955d..0bb13e94 100644 --- a/src/bridge/effect_adapter.rs +++ b/src/bridge/effect_adapter.rs @@ -22,18 +22,23 @@ use ironclaw_engine::{ use crate::context::JobContext; use crate::hooks::{HookEvent, HookOutcome, HookRegistry}; use crate::safety::SafetyLayer; +use crate::tools::rate_limiter::RateLimiter; use crate::tools::{ApprovalRequirement, ToolRegistry}; /// Wraps the existing tool pipeline to implement the engine's `EffectExecutor`. /// /// Enforces all v1 security controls at the adapter boundary: -/// tool approval, output sanitization, hooks, and rate limiting. +/// tool approval, output sanitization, hooks, rate limiting, and call limits. pub struct EffectBridgeAdapter { tools: Arc, safety: Arc, hooks: Arc, /// Tools the user has approved with "always" (persists within session). auto_approved: RwLock>, + /// Per-step tool call counter (reset externally between steps). + call_count: std::sync::atomic::AtomicU32, + /// Per-user per-tool sliding window rate limiter. + rate_limiter: RateLimiter, } impl EffectBridgeAdapter { @@ -47,17 +52,25 @@ impl EffectBridgeAdapter { safety, hooks, auto_approved: RwLock::new(HashSet::new()), + call_count: std::sync::atomic::AtomicU32::new(0), + rate_limiter: RateLimiter::new(), } } /// Mark a tool as auto-approved (user said "always"). - #[allow(dead_code)] pub async fn auto_approve_tool(&self, tool_name: &str) { self.auto_approved .write() .await .insert(tool_name.to_string()); } + + /// Reset the per-step call counter (called between code steps). + #[allow(dead_code)] + pub fn reset_call_count(&self) { + self.call_count + .store(0, std::sync::atomic::Ordering::Relaxed); + } } #[async_trait::async_trait] @@ -79,6 +92,31 @@ impl EffectExecutor for EffectBridgeAdapter { &hyphenated }; + // ── Per-step call limit (prevent amplification loops) ── + const MAX_CALLS_PER_STEP: u32 = 50; + let count = self + .call_count + .fetch_add(1, std::sync::atomic::Ordering::Relaxed); + if count >= MAX_CALLS_PER_STEP { + return Err(EngineError::Effect { + reason: format!( + "Tool call limit reached ({MAX_CALLS_PER_STEP} per code step). \ + Break your task into multiple steps." + ), + }); + } + + // ── 0. Block tools that need v1 runtime deps (RoutineEngine, Scheduler) ── + if is_v1_only_tool(lookup_name) { + return Err(EngineError::Effect { + reason: format!( + "Tool '{}' is not available in engine v2. \ + Tell the user to use the slash command instead (e.g. /routine, /job).", + action_name + ), + }); + } + // ── 1. Check tool approval (v1: Tool::requires_approval) ── if let Some(tool) = self.tools.get(lookup_name).await { @@ -109,6 +147,29 @@ impl EffectExecutor for EffectBridgeAdapter { } } + // ── 1.5. Check rate limit (v1: RateLimiter) ── + + if let Some(tool) = self.tools.get(lookup_name).await + && let Some(rl_config) = tool.rate_limit_config() + { + let result = self + .rate_limiter + .check_and_record(&context.user_id, lookup_name, &rl_config) + .await; + if let crate::tools::rate_limiter::RateLimitResult::Limited { + retry_after, .. + } = result + { + return Err(EngineError::Effect { + reason: format!( + "Tool '{}' is rate limited. Try again in {:.0}s.", + action_name, + retry_after.as_secs_f64() + ), + }); + } + } + // ── 2. Run BeforeToolCall hook (v1: hooks.run) ── let redacted_params = if let Some(tool) = self.tools.get(lookup_name).await { @@ -204,9 +265,14 @@ impl EffectExecutor for EffectBridgeAdapter { ) -> Result, EngineError> { let tool_defs = self.tools.tool_definitions().await; - // Build action defs with approval info from each tool + // Build action defs, excluding v1-only tools let mut actions = Vec::with_capacity(tool_defs.len()); for td in tool_defs { + // Skip tools that can't work in engine v2 + if is_v1_only_tool(&td.name) { + continue; + } + let python_name = td.name.replace('-', "_"); // Check default approval requirement (with empty params) @@ -231,3 +297,25 @@ impl EffectExecutor for EffectBridgeAdapter { Ok(actions) } } + +/// Tools that depend on v1 runtime components (RoutineEngine, Scheduler, +/// ContainerJobManager) and cannot work in engine v2's minimal JobContext. +fn is_v1_only_tool(name: &str) -> bool { + matches!( + name, + "routine_create" + | "routine-create" + | "routine_update" + | "routine-update" + | "routine_delete" + | "routine-delete" + | "routine_fire" + | "routine-fire" + | "create_job" + | "create-job" + | "cancel_job" + | "cancel-job" + | "build_software" + | "build-software" + ) +}