Skip to content

chore: promote staging to staging-promote/e5b82fc6-24240316990 (2026-04-10 14:22 UTC) - #2264

Merged
henrypark133 merged 17 commits into
staging-promote/e5b82fc6-24240316990from
staging-promote/a8e6533a-24247643750
Apr 10, 2026
Merged

henrypark133 merged 17 commits into
staging-promote/e5b82fc6-24240316990from
staging-promote/a8e6533a-24247643750

Conversation

@ironclaw-ci

@ironclaw-ci ironclaw-ci Bot commented Apr 10, 2026 •

Copy link
Copy Markdown
Contributor

Auto-promotion from staging CI

Batch range: 13c458e30cff5c437dfe3d19ddee0522f0c6e4fa..a8e6533a1f76502e65ff75b19914af516407a815
Promotion branch: staging-promote/a8e6533a-24247643750
Base: staging-promote/e5b82fc6-24240316990
Triggered by: Staging CI batch at 2026-04-10 14:22 UTC

Commits in this batch (19):

Current commits in this promotion (0)

Current base: staging-promote/e5b82fc6-24240316990
Current head: staging-promote/a8e6533a-24247643750
Current range: origin/staging-promote/e5b82fc6-24240316990..origin/staging-promote/a8e6533a-24247643750

  • (no non-merge commits in range)

Auto-updated by staging promotion metadata workflow

Waiting for gates:

  • Tests: pending
  • E2E: pending
  • Claude Code review: pending (will post comments on this PR)

Auto-created by staging-ci workflow

matiasbenary and others added 2 commits April 10, 2026 16:14
* feat: add amazon tutorial

* chore: apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Guille <gagdiez.c@gmail.com>

* chore: apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Guille <gagdiez.c@gmail.com>

---------

Co-authored-by: Guille <gagdiez.c@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix(oauth): use localhost for redirect URI when bound to 0.0.0.0

Google rejects redirect_uri=http://0.0.0.0:<port>/oauth/callback as
invalid. When no tunnel public_url is configured, the gateway base URL
was built directly from GATEWAY_HOST, which is a bind address — not a
routable hostname. Map unspecified addresses (0.0.0.0, ::, [::]) to
localhost, matching the social-login flow's existing fallback.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(oauth): use IpAddr::is_unspecified for robust wildcard detection

Address PR feedback: replace string matching with std::net::IpAddr
parsing so all representations of unspecified addresses (including
0:0:0:0:0:0:0:0) are caught. Add regression tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use is_ok_and instead of map_or(false, ..) to satisfy clippy

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added scope: docs Documentation size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Apr 10, 2026
@claude

claude Bot commented Apr 10, 2026

Copy link
Copy Markdown

Code review

Found 8 issues:

  1. [CRITICAL:85] IPv6 address URL format bug in oauth_base_url() — when called with bare IPv6 address like "::1", returns invalid URL http://::1:3000. Per RFC 3986, IPv6 addresses in URLs must be bracketed: http://[::1]:3000. The test at line 1483 expects the wrong format. This breaks OAuth redirects on IPv6 deployments.

ironclaw/src/main.rs

Lines 1480 to 1484 in a8e6533

oauth_base_url("my-server.example.com", 8080),
"http://my-server.example.com:8080"
);
assert_eq!(oauth_base_url("::1", 3000), "http://::1:3000");
}

  1. [HIGH:80] Curl pipe-to-shell installation pattern in documentation — Line 133 of amazon.mdx recommends piping untrusted shell script directly to sh. This is a security anti-pattern. Should recommend: download script separately, verify SHA256 hash, review before execution, then run explicitly.

```bash
curl --proto '=https' --tlsv1.2 -LsSf https://github.com/nearai/ironclaw/releases/latest/download/ironclaw-installer.sh | sh
```

  1. [HIGH:85] Documentation content parity violation — Chinese version (docs/zh/infrastructure/amazon.mdx) omits significant sections from English version, particularly the "Configure Your Instance" hardening steps. Chinese version has ~90 lines vs English's 154 lines, creating inconsistent user experience and potentially leaving Chinese-speaking users with incomplete setup guidance.

https://github.com/nearai/ironclaw/blob/a8e6533a1f76502e65ff75b19914af516407a815/docs/zh/infrastructure/amazon.mdx

  1. [MEDIUM:75] SSH key management guidance incomplete — Documentation shows chmod 400 but doesn't warn about shell history exposure (key path in bash history) or recommend using ssh-agent/SSH config to avoid exposing keys in process listings.

```bash
# Restrict permissions on the downloaded key (required by SSH)
chmod 400 ~/.ssh/your-key.pem
```

  1. [MEDIUM:75] Resource guidance for t2.micro incomplete — No documentation of CPU, memory, storage limits. t2 instances have limited burst CPU (5-15% baseline) that could throttle LLM inference. No mention that 8GB default EBS volume may fill quickly with vector embeddings. Free tier only lasts 12 months. Contrast with DigitalOcean docs which specify "$4/month" explicitly.

- **Name**: choose a descriptive name (e.g. `ironclaw`)
- **AMI**: Ubuntu Server 24.04 LTS (free tier eligible)
- **Instance type**: `t2.micro` or `t3.micro` (free tier eligible) — sufficient for most use cases
- **Key pair**: click **Create new key pair**, give it a name, choose RSA and `.pem` format, then download the file. Keep it in a safe place — you will not be able to download it again.

  1. [MEDIUM:75] Missing integration test for oauth_base_url — Per CLAUDE.md ("Test Through the Caller, Not Just the Helper"), the oauth_base_url() helper gates a side effect (OAuth flow via extension manager). Unit tests on the helper exist (lines 1473-1484), but missing integration test that drives the caller (app startup with mocked config) and verifies correct enable_gateway_mode() behavior. Regression coverage gap.

ironclaw/src/main.rs

Lines 1449 to 1459 in a8e6533

fn oauth_base_url(host: &str, port: u16) -> String {
let trimmed = host.trim_start_matches('[').trim_end_matches(']');
let is_unspecified = trimmed
.parse::<std::net::IpAddr>()
.is_ok_and(|ip| ip.is_unspecified());
if is_unspecified {
format!("http://localhost:{}", port)
} else {
format!("http://{}:{}", host, port)
}
}

  1. [MEDIUM:70] Insufficient sudo privilege management guidance — Documentation enables passwordless sudo for new user without mentioning security implications. Should recommend configuring sudoers to require password for sensitive operations.

sudo adduser ironclaw
sudo usermod -aG sudo ironclaw
```

  1. [LOW:65] Chinese documentation references missing security guide — Chinese version defers hardening to /zh/security guide (line 76-80) that may not exist or match EC2-specific steps, creating a documentation gap.

## 安全加固与安装 IronClaw
[安全指南](/zh/security) 同样适用于您的 EC2 实例,但有一点不同:由于您已以 `ubuntu`(非 root 的 sudo 用户)身份登录,可以 **跳过"创建新用户"步骤** ——`ubuntu` 已承担了该角色。
其余所有步骤——更新系统、加固 SSH、安装 Fail2Ban、配置防火墙以及安装 IronClaw——均完全适用。

vutran1710 and others added 15 commits April 10, 2026 23:53
* feat: add Composio WASM tool for third-party app integrations

Add Composio integration as a WASM tool (tools-src/composio/), providing
a single multiplexed tool with 4 actions: list, execute, connect, and
connected_accounts. Supports 250+ third-party apps via Composio's REST
API with WASM sandbox security (fuel metering, memory limits, network
allowlisting, host-injected credentials).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review — retry safety, dead code, registry manifest

Code fixes (tools-src/composio/src/lib.rs):
- Only retry GET requests (idempotent); POST executes once to prevent
  duplicate side effects on execute/connect actions
- Remove dead parse_json_response status check (already handled by
  caller); use serde_json::from_slice to avoid extra allocation
- Remove misleading secret_exists pre-flight (only checks capability
  allowlist, not actual presence); instead surface helpful error on
  401/403 from the API
- Extract entity_id logic into extract_entity_id() helper with 6 unit
  tests covering precedence chain and edge cases

Registry:
- Add registry/tools/composio.json manifest (matches format of other
  tools like web-search, github, gmail)
- Add composio to the default bundle in _bundles.json

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: restore secret pre-flight, enforce schema, remove default tag

- Restore secret_exists pre-flight as best-effort check (avoids wasting
  rate-limited API calls when clearly misconfigured)
- Add #[serde(deny_unknown_fields)] to Params to match the schema's
  additionalProperties: false contract
- Remove "default" tag from registry manifest and remove from default
  bundle until WASM artifacts are published

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: simplify entity_id fallback, add params type to schema

- Remove requester_id fallback from extract_entity_id (user_id is
  always present in JobContext, so requester_id was dead code)
- Add "type": "object" to params field in both tool schema and
  capabilities.json to prevent schema-driven callers from sending
  non-object values

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: align with Composio v3 API contract + fixture tests

Address serrrfirat's review — update all response parsing and request
fields to match the current Composio v3 API:

- Add unwrap_items() helper for paginated { "items": [...] } envelopes,
  with bare-array fallback for backward compatibility
- connect_app: parse auth_configs from paginated response via
  extract_auth_config_id()
- execute_action: use v3 fields `user_id` + `arguments` (not deprecated
  `entity_id` + `input`)
- list_accounts/resolve_account: use plural query params `user_ids`,
  `toolkit_slugs` (v3 contract)
- lookup_app_for_tool: look for nested `toolkit.slug` (v3), falling
  back to `toolkit_slug` and `appName`
- find_active_account: sort by `updated_at` (v3), falling back to
  `updatedAt`

Add 15 fixture-style tests covering:
- Paginated envelope parsing (envelope, bare array, empty, non-array)
- Auth config extraction (paginated, bare, empty)
- Toolkit slug extraction (v3 nested, legacy flat, appName fallback,
  case-insensitive, not-found)
- Active account selection (v3 timestamps, legacy timestamps, no active)

Total: 25 tests (5 URL, 5 entity_id, 15 v3 contract fixtures)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address PR review — remove duplicate parameters, validate params, fix ordering

- Remove `parameters` section from capabilities JSON (duplicates SCHEMA const,
  runtime ignores it, creates drift risk)
- Fix Cargo.toml exclude ordering: tools-src/composio before tools-src/github
- Validate `params` is a JSON object when provided, reject non-object values early
- Remove 429 from retry logic (WASM has no sleep/backoff, immediate retry wastes
  rate-limit budget) — only retry on transient 5xx
- Add tests for params validation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address maintainer review — retry convention, numeric IDs, slug validation

- Revert 429 retry to align with github/web-search tool convention (sub-second
  sliding-window resets can make immediate retries worthwhile)
- Handle numeric entity_id/user_id in context JSON (as_u64/as_i64 fallback)
- Add validate_tool_slug() defense-in-depth against path traversal (same
  pattern as github tool)
- Add tests for numeric entity IDs and slug validation (32 total)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address 4 unresolved audit issues — pagination, direct lookup, array params

1. list_tools: expose cursor/limit params in schema, preserve next_cursor
   and total in response for multi-page browsing, add toolkit_versions=latest
2. lookup_app_for_tool: use direct GET /tools/{slug} endpoint instead of
   fuzzy search (avoids false negatives from search pagination/ranking),
   add toolkit_versions=latest
3. connected_accounts queries: encode user_ids and toolkit_slugs as array
   params (user_ids[], toolkit_slugs[]) per v3 API contract
4. toolkit_versions=latest added to both list and lookup endpoints

Adds 5 new tests (37 total): cursor/limit params, array query encoding,
direct tool response parsing (v3 nested, legacy, missing).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: ilblackdragon@gmail.com <ilblackdragon@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: firat.sertgoz <f@nuff.tech>
…stant (#1736)

* v2 architecture phase 1

* feat(engine): Phase 2 — execution loop, capability system, thread runtime

Add the core execution engine to ironclaw_engine crate:

- CapabilityRegistry: register/get/list capabilities and actions
- LeaseManager: async lease lifecycle (grant, check, consume, revoke, expire)
- PolicyEngine: deterministic effect-level allow/deny/approve
- ThreadTree: parent-child relationship tracking
- ThreadSignal/ThreadOutcome: inter-thread messaging via mpsc
- ThreadManager: spawn threads as tokio tasks, stop, inject messages, join
- ExecutionLoop: core loop replacing run_agentic_loop() with signals,
  context building, LLM calls, action execution, and event recording
- Structured executor (Tier 0): lease lookup → policy check → effect execution
- Tool intent nudge detection
- MemoryStore + RetrievalEngine stubs for Phase 4
- Full 8-phase architecture plan in docs/plans/
- CLAUDE.md spec for the engine crate

74 tests passing, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): Phase 3 — Monty Python executor with RLM pattern

Add CodeAct execution (Tier 1) using the Monty embedded Python
interpreter, following the Recursive Language Model (RLM) pattern
from arXiv:2512.24601.

Key additions:
- executor/scripting.rs: Monty integration with FunctionCall-based
  tool dispatch, catch_unwind panic safety, resource limits (30s,
  64MB, 1M allocs)
- LlmResponse::Code variant + ExecutionTier::Scripting
- Context-as-variables (RLM 3.4): thread messages, goal, step_number,
  previous_results injected as Python variables — LLM context stays
  lean while code accesses data selectively
- llm_query(prompt, context) (RLM 3.5): recursive subagent calls
  from within Python code — results stored as variables, not injected
  into parent's attention window (symbolic composition)
- Compact output metadata between code steps instead of full stdout
- MontyObject ↔ serde_json::Value bidirectional conversion
- Updated architecture plan with RLM design principles

74 tests passing, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): RLM best-practices enhancements from cross-reference analysis

Cross-referenced our implementation against the official RLM (alexzhang13/rlm),
fast-rlm (avbiswas/fast-rlm), and Prime Intellect's verifiers implementation.
Key enhancements:

- FINAL(answer) / FINAL_VAR(name): explicit termination pattern matching
  all three reference implementations. Code can signal completion at any
  point, not just via return value.
- llm_query_batched(prompts): parallel recursive sub-calls via tokio::spawn,
  matching fast-rlm's asyncio.gather pattern and Prime Intellect's llm_batch.
- Output truncation increased to 8000 chars (from 120), matching Prime
  Intellect's 8192 default. Shows [TRUNCATED: last N chars] or [FULL OUTPUT].
- Step 0 orientation preamble: auto-injects context metadata (message count,
  total chars, goal, last user message preview) before first code step,
  matching fast-rlm's auto-print pattern.
- Error-to-LLM flow: Python parse errors, runtime errors, NameErrors,
  OS errors, and async errors now flow back as stdout content instead of
  terminating the step, enabling LLM self-correction on next iteration.
  Only VM panics (catch_unwind) terminate as EngineError.

74 tests passing, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(engine): update architecture plan with RLM cross-reference learnings

Comprehensive update after cross-referencing against official RLM
(alexzhang13/rlm), fast-rlm (avbiswas/fast-rlm), Prime Intellect
(verifiers/RLMEnv), rlm-rs (zircote/rlm-rs), and Google ADK RLM.

Changes:
- Mark Phases 1-3 as DONE with commit refs and test counts
- Add "Key Influences" section documenting all reference implementations
- Phase 3: full table of implemented RLM features with sources
- Phase 3: "Remaining gaps" table with which phase addresses each
- Phase 4: expanded with compaction (85% context), rlm_query() (full
  recursive sub-agent), dual model routing, budget controls (USD,
  timeout, tokens, consecutive errors), lazy loading, pass-by-reference
- Add "RLM Execution Model" cross-cutting section
- Add "Implementation Progress" tracking table
- Remove stale "TO IMPLEMENT" markers (all Phase 3 work is done)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): Phase 4 — budget controls, compaction, reflection pipeline

Budget enforcement in ExecutionLoop:
- max_tokens_total: cumulative token limit, checked before each iteration
- max_duration: wall-clock timeout for entire thread
- max_consecutive_errors: consecutive error steps threshold (resets on
  success, matching official RLM behavior)
- All produce ThreadOutcome::Failed with descriptive messages

Context compaction (from RLM paper, 85% threshold):
- estimate_tokens(): char-based estimation (chars/4, matching RLM)
- should_compact(): triggers when tokens >= threshold_pct * context_limit
- compact_messages(): asks LLM to summarize progress, replaces history
  with [system, summary, continuation_note], preserves intermediate results
- Configurable via ThreadConfig: model_context_limit, compaction_threshold

Dual model routing:
- LlmCallConfig gains depth field (0=root, 1+=sub-call)
- Implementations can route to cheaper models for sub-calls
- ExecutionLoop passes thread depth to every LLM call

Reflection pipeline (reflection/pipeline.rs):
- reflect(thread, llm): analyzes completed thread via LLM
- Produces Summary doc (always), Lesson doc (if errors), Issue doc (if failed)
- Builds transcript from thread messages + error events
- Returns ReflectionResult with docs + token usage

ThreadConfig extended with: max_tokens_total, max_consecutive_errors,
model_context_limit, enable_compaction, compaction_threshold, depth, max_depth.

78 tests passing, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): Phase 5 — conversation surface separated from execution

Conversation is now a UI layer, not an execution boundary. Multiple
threads can run concurrently within one conversation; threads can
outlive their originating conversation.

New types (types/conversation.rs):
- ConversationSurface: channel + user + entries + active_threads
- ConversationEntry: sender (User/Agent/System) + content + origin_thread_id
- ConversationId, EntryId (UUID newtypes)
- EntrySender enum (User, Agent{thread_id}, System)

ConversationManager (runtime/conversation.rs):
- get_or_create_conversation(channel, user) — indexed by (channel, user)
- handle_user_message() — injects into active foreground thread or spawns new
- record_thread_outcome() — adds agent/system entries, untracks completed threads
- get_conversation(), list_conversations()

This enables the key architectural insight: a user can ask "what's the
weather?" while a deployment thread is still running. Both produce entries
in the same conversation.

85 tests passing, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(engine): simplify execution tiers — Monty-only for CodeAct/RLM

Restructure phases 6-8 to clarify execution model:

- Monty is the sole Python executor for CodeAct/RLM. No WASM or Docker
  Python runtimes for LLM-generated code.
- WASM sandbox is for third-party tool isolation (existing infra, Phase 8)
- Docker containers are for thread-level isolation of high-risk work (Phase 8)
- Two-phase commit moves to Phase 6 (integration) at the adapter boundary

Phase renumbering:
- Old Phase 6 (Tier 2-3) → removed as separate phase
- Old Phase 7 (integration) → Phase 6
- Old Phase 8 (cleanup) → Phase 7
- New Phase 8: WASM tools + Docker thread isolation (infra integration)

Updated progress table: Phases 1-5 marked DONE with test counts and commits.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): Phase 6 — bridge adapters for main crate integration

Strategy C parallel deployment: when ENGINE_V2=true env var is set,
user messages route through the engine instead of the existing agentic
loop. All existing behavior is unchanged when the flag is off.

Bridge module (src/bridge/):
- LlmBridgeAdapter: wraps LlmProvider as engine LlmBackend, converts
  ThreadMessage↔ChatMessage, ActionDef↔ToolDefinition, depth-based
  model routing (primary vs cheap_llm)
- EffectBridgeAdapter: wraps ToolRegistry+SafetyLayer as EffectExecutor,
  routes tool calls through existing execute_tool_with_safety pipeline
- InMemoryStore: HashMap-backed Store impl (no DB tables needed yet)
- EngineRouter: is_engine_v2_enabled() + handle_with_engine() that
  builds engine from Agent deps and processes messages end-to-end

Integration touchpoint (4 lines in agent_loop.rs):
  After hook processing, before session resolution, check ENGINE_V2
  flag and route UserInput through the engine path.

Accessor visibility widened: llm(), cheap_llm(), safety(), tools()
changed from pub(super) to pub(crate) for bridge access.

85 engine tests + main crate clippy clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): add user message and system prompt to thread before execution

The ExecutionLoop was sending empty messages to the LLM because the
thread was spawned with the user's input as the goal but no messages.

Fixes:
- ThreadManager.spawn_thread() now adds the goal as an initial user
  message before starting the execution loop
- ExecutionLoop.run() injects a default system prompt if none exists

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(bridge): match existing LLM request format to prevent 400 errors

The LLM bridge was missing several defaults that the existing
Reasoning.respond_with_tools() sets:

- tool_choice: "auto" when tools are present (required by some providers)
- max_tokens: 4096 (default)
- temperature: 0.7 (default)
- When no tools (force_text): use plain complete() instead of
  complete_with_tools() with empty tools array — matches existing
  no-tools fallback path

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): persist conversation context across messages

The engine was creating a fresh ThreadManager and InMemoryStore per
message, losing all context between turns. A follow-up question like
"what are the latest 10 issues?" had no memory of the prior "how many
issues" response.

Fixes:
- EngineState (ThreadManager, ConversationManager, InMemoryStore) now
  persists across messages via OnceLock, initialized on first use
- ConversationManager builds message history from prior conversation
  entries (user messages + agent responses) and passes it to new threads
- ThreadManager.spawn_thread_with_history() accepts initial_messages
  that are prepended before the current user message
- System notifications (thread started/completed) are filtered out of
  the history (not useful as LLM context)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): enable CodeAct/RLM mode with code block detection

The engine now operates in CodeAct/RLM mode:

System prompt (executor/prompt.rs):
- Instructs LLM to write Python in ```repl fenced blocks
- Documents available tools as callable Python functions
- Documents llm_query(), llm_query_batched(), FINAL()
- Documents context variables (context, goal, step_number, previous_results)
- Strategy guidance: examine context, break into steps, use tools, call FINAL()

Code block detection (bridge/llm_adapter.rs):
- extract_code_block() scans LLM text responses for ```repl or ```python blocks
- When detected, returns LlmResponse::Code instead of LlmResponse::Text
- The ExecutionLoop routes Code responses through Monty for execution

No structured tool definitions sent to LLM:
- Tools are described in the system prompt as Python functions
- The LLM call sends empty actions array, forcing text-mode responses
- This ensures the LLM writes code blocks (CodeAct) instead of
  structured tool calls (which would bypass the REPL)

85 tests passing, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(engine): add 8 CodeAct/RLM E2E tests with mock LLM

Comprehensive test coverage for the Monty Python execution path:

- codeact_simple_final: Python code calls FINAL('answer') → thread completes
- codeact_tool_call_then_final: code calls test_tool() → FunctionCall
  suspends VM → MockEffects returns result → code resumes → FINAL()
- codeact_pure_python_computation: sum([1,2,3,4,5]) → FINAL('Sum is 15')
  with no tool calls — pure Python in Monty
- codeact_multi_step: first step prints output (no FINAL), second step
  sees output metadata and calls FINAL — tests iterative REPL flow
- codeact_error_recovery: first step has NameError → error flows to LLM
  as stdout → second step recovers with FINAL — tests error transparency
- codeact_context_variables_available: code accesses `goal` and `context`
  variables injected by the RLM context builder
- codeact_multiple_tool_calls_in_loop: for loop calls test_tool() 3 times
  → 3 FunctionCall suspensions → all results collected → FINAL
- codeact_llm_query_recursive: code calls llm_query('prompt') → VM
  suspends → MockLlm provides sub-agent response → result returned as
  Python string variable

93 tests passing (85 prior + 8 new), zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(bridge): detect code blocks in plain completion path + multi-block support

Two bugs fixed:

1. The no-tools completion path (used by CodeAct since we send empty
   actions) returned LlmResponse::Text without checking for code blocks.
   Code blocks were rendered as markdown text instead of being executed.

2. extract_code_block now:
   - Handles bare ``` fences (skips non-Python languages)
   - Collects ALL code blocks in the response and concatenates them
     (models often split code across multiple blocks with explanation)
   - Tries markers in order: ```repl, ```python, ```py, then bare ```

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(bridge): add 11 regression tests for code block extraction

Covers the exact failure modes discovered during live testing:

- extract_repl_block: standard ```repl fenced block
- extract_python_block: ```python marker
- extract_py_block: ```py shorthand
- extract_bare_backtick_block: bare ``` with Python content
- skip_non_python_language: ```json should NOT be extracted
- no_code_blocks_returns_none: plain text, no fences
- multiple_code_blocks_concatenated: two ```repl blocks with
  explanation between them → concatenated with \n\n
- mixed_thinking_and_code: model outputs explanation + two
  ```python blocks (the Hyperliquid case) → both extracted
- repl_preferred_over_bare: ```repl takes priority over bare ```
- empty_code_block_skipped: empty fenced block returns None
- unclosed_block_returns_none: no closing ``` returns None

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): detect FINAL() in text responses + regression tests

Models sometimes write FINAL() outside code blocks — as plain text
after an explanation. The Hyperliquid case: model outputs a long
analysis then FINAL("""...""") at the end, not inside ```repl fences.

Fixes:
- extract_final_from_text(): regex-based FINAL detection in text
  responses, matching the official RLM's find_final_answer() fallback
- Handles: double-quoted, single-quoted, triple-quoted, unquoted,
  nested parens
- Checked in LlmResponse::Text handler BEFORE tool intent nudge
  (FINAL takes priority)

9 new tests:
- codeact_final_in_text_response: FINAL("answer") in plain text
- codeact_final_triple_quoted_in_text: FINAL("""multi\nline""") in text
- final_double_quoted, final_single_quoted, final_triple_quoted,
  final_unquoted, final_with_nested_parens, final_after_long_text,
  no_final_returns_none

102 tests passing (93 + 9 new), zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add crate extraction & cleanup roadmap

Documents architectural recommendations from the engine v2 design
process for future reference:

- Root directory consolidation (channels-src + tools-src → extensions/)
- Crate extraction tiers: zero-coupling (estimation, observability,
  tunnel), trivial-coupling (document_extraction, pairing, hooks),
  medium-coupling (secrets, MCP, db, workspace, llm, skills),
  heavy-coupling (web gateway, agent, extensions)
- src/ module reorganization into logical groups (core, persistence,
  infra, media, support)
- main.rs/app.rs slimming targets (100/500 lines after migration)
- WASM module candidates (document_extraction) and non-candidates
  (REPL, web gateway → separate crates instead)
- Priority ordering for extraction work
- Tracks completed items (ironclaw_safety, ironclaw_engine,
  transcription move)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): live progress status updates via event broadcast

Engine v2 now shows live progress in the CLI (and any channel):
- "Thinking..." when a step starts
- Tool name + success/error when actions execute
- "Processing results..." when a step completes

Implementation:
- ThreadManager holds a broadcast::Sender<ThreadEvent> (capacity 256)
- ExecutionLoop.emit_event() writes to thread.events AND broadcasts
- ThreadManager.subscribe_events() returns a receiver
- Router uses tokio::select! to listen for events while waiting for
  thread completion, forwarding them as StatusUpdate to the channel

This replaces the polling approach with zero-latency event streaming.
Agent.channels visibility widened to pub(crate) for bridge access.

102 tests passing, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): include tool results in code step output for LLM context

The LLM was ignoring tool results and answering from training data
because the compact output metadata didn't include what tools returned.
Tool results lived only as ActionResult messages (role: Tool) which
some providers flatten or the model ignores.

Now the code step output includes:
- stdout from Python print() statements
- [tool_name result] with the actual output (truncated to 4K per tool)
- [tool_name error] for failed tools
- [return] for the code's return value
- Total output truncated to 8K chars to prevent context bloat

This ensures the model sees web_search results, API responses, etc.
in the next iteration and can reason about them instead of hallucinating.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): add debug/trace logging for CodeAct execution

Three verbosity levels for debugging the engine:

RUST_LOG=ironclaw_engine=debug:
- LLM call: message count, iteration, force_text
- LLM response: type (text/code/action_calls), token usage
- Code execution: code length, action count, had_error, final_answer
- Text response: length, FINAL() detection

RUST_LOG=ironclaw_engine=trace:
- Full message list sent to LLM (role, length, first 200 chars each)
- Full code block being executed
- stdout preview (first 500 chars)
- Per-tool results (name, success, first 300 chars of output)
- Text response preview (first 500 chars)

Usage:
  ENGINE_V2=true RUST_LOG=ironclaw_engine=debug cargo run
  ENGINE_V2=true RUST_LOG=ironclaw_engine=trace cargo run

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): execution trace recording + retrospective analysis

Enable with ENGINE_V2_TRACE=1 to get full execution traces and
automatic issue detection after each thread completes.

Trace recording (executor/trace.rs):
- build_trace(): captures full thread state — messages (with full
  content), events, step count, token usage, detected issues
- write_trace(): writes JSON to engine_trace_{timestamp}.json
- log_trace_summary(): logs summary + issues at info/warn level

Retrospective analyzer detects 8 issue categories:
- thread_failure: thread ended in Failed state
- no_response: no assistant message generated
- tool_error: specific tool failures with error details
- code_error: Python errors (NameError, SyntaxError, etc.) in output
- missing_tool_output: tool results exist but not in system messages
- excessive_steps: >10 steps (may be stuck in loop)
- no_tools_used: single-step answer without tools (hallucination risk)
- mixed_mode: text responses without code blocks (prompt not followed)

Thread state now saved to store after execution completes (for trace
access after join_thread).

Usage:
  ENGINE_V2=true ENGINE_V2_TRACE=1 cargo run
  # After each message: trace JSON + issue log in terminal

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): wire reflection pipeline + trace analysis into thread lifecycle

After every thread completes, ThreadManager now automatically runs:

1. Retrospective trace analysis (non-LLM, always):
   - Detects 8 issue categories (tool errors, code errors, missing
     outputs, excessive steps, hallucination risk, etc.)
   - Logs issues at warn level when found

2. Trace file recording (when ENGINE_V2_TRACE=1):
   - Writes full JSON trace to engine_trace_{timestamp}.json

3. LLM reflection (when enable_reflection=true):
   - Calls reflection pipeline to produce Summary, Lesson, Issue docs
   - Saves docs to store for future context retrieval
   - Enabled by default in the bridge router

All three run inside the spawned tokio task after exec.run() completes,
before saving the final thread state. No external wiring needed.

Removed duplicate trace recording from the router — it's now handled
by ThreadManager automatically.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(bridge): convert tool name hyphens to underscores for Python compatibility

Root cause from trace analysis: the LLM writes `web_search()` (valid
Python identifier) but the tool registry has `web-search` (with hyphen).
The EffectBridgeAdapter couldn't find the tool → "Tool not found" error
→ model fabricated fake data instead.

Fixes:
- available_actions(): converts tool names from hyphens to underscores
  (web-search → web_search) so the system prompt lists valid Python names
- execute_action(): tries the original name first, then falls back to
  hyphenated form (web_search → web-search) for tool registry lookup
- Same conversion in router's capability registry builder

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(bridge): parse JSON tool output to prevent double-serialization

From trace analysis: web_search returned a JSON string, which was
wrapped as serde_json::json!(string) creating a Value::String containing
JSON. When Monty got this as MontyObject::String, the Python code
couldn't index it with result['title'] → TypeError.

Fix: try parsing the tool output string as JSON first. If valid, use the
parsed Value (becomes a Python dict/list). If not valid JSON, keep as
string. This means web_search results are directly indexable in Python:
  results = web_search(query="...")
  print(results["results"][0]["title"])  # works now

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): persist variables across code steps via `state` dict

Monty creates a fresh runtime per code step, so variables are lost
between steps. This caused the model to re-paste tool results from
system messages, wasting tokens.

Fix: maintain a `persisted_state` JSON dict in the ExecutionLoop that
accumulates across steps:
- Tool results stored by tool name: state["web_search"] = {results...}
- Return values stored: state["last_return"], state["step_0_return"]
- Injected as a `state` Python variable in each new MontyRun

Now the model can do:
  Step 1: results = web_search(query="...")  # tool result saved in state
  Step 2: data = state["web_search"]         # access previous result
          summary = llm_query("summarize", str(data))
          FINAL(summary)

System prompt updated to document the `state` variable.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): add state hint on code errors + retrieval engine integration

When code fails with NameError/UnboundLocalError (model trying to
access variables from a previous step), the error output now includes:

  [HINT] Variables don't persist between code blocks. Use the `state`
  dict to access data from previous steps. Available keys: ["web_search",
  "last_return"]

This teaches the model to use `state["web_search"]` instead of `result`
after a NameError, reducing wasted steps from 3-4 to 1.

Also integrates RetrievalEngine into context building and ThreadManager:
- build_step_context() now accepts optional RetrievalEngine to inject
  relevant memory docs (Lessons, Specs, Playbooks) into LLM context
- RetrievalEngine uses keyword matching with doc-type priority scoring
- Memory docs from reflection (Phase 4) now feed back into future threads

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: remove trace files and add to .gitignore

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): replace web_fetch example with web_search in CodeAct prompt

The system prompt example used web_fetch(url="...") which doesn't exist
as a tool. The model learned from the example and tried web_fetch,
getting "Tool not found". Changed to web_search(query="...") which is
an actual registered tool.

Found via trace analysis — reflection pipeline correctly identified
this as a "Tool Name Correction" spec doc.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(engine): extract prompt templates to markdown files

Prompt templates moved from inline Rust strings to plain markdown files
at crates/ironclaw_engine/prompts/ for easy inspection and iteration:

- prompts/codeact_preamble.md — main instructions, special functions,
  context variables, rules
- prompts/codeact_postamble.md — strategy section

Loaded at compile time via include_str!(), so no runtime file I/O.
Edit the .md files and rebuild to iterate on prompts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): replace byte-index slicing with char-safe truncation

Panic: 'byte index 80 is not a char boundary; it is inside ''' when
tool output contained multi-byte UTF-8 characters (smart quotes from
web search results).

Fixed 4 unsafe byte-index slices:
- thread.rs:281: message preview &content[..80] → chars().take(80)
- loop_engine.rs:556: tool output &str[..4000] → chars().take(4000)
- loop_engine.rs:579: output tail &str[len-8000..] → chars().skip()
- scripting.rs:82: stdout tail &str[len-N..] → chars().skip()

All now use .chars().take() or .chars().skip() which respect character
boundaries. Follows CLAUDE.md rule: "Never use byte-index slicing on
user-supplied or external strings."

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): fix false positive missing_tool_output warning in trace analyzer

The check was looking for "[" + "result]" in System-role messages only,
but tool output metadata is added with patterns like "[shell result]"
and may appear in messages with any role. Changed to scan all messages
for " result]" or " error]" patterns regardless of role.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(engine): update architecture plan with Phase 6 status and approval flow design

Phase 6 updated to reflect what was actually built:
- Bridge adapters (LLM, Effect, InMemoryStore, Router) — all done
- Integration touchpoint (4 lines in handle_message) — done
- Live progress via broadcast events — done
- Conversation persistence across messages — done
- Trace recording + retrospective analysis — done
- 8 bugs found and fixed via trace analysis — documented

Phase 6 remaining work documented:
- Approval flow: detailed 5-step design (send to channel, pause thread,
  route response, resume execution, always handling) with v1 reference
- Database persistence (InMemoryStore → real DB tables)
- Acceptance testing (TestRig + TraceLlm fixtures)
- Two-phase commit for high-stakes effects

Progress table updated: Phase 6 marked as DONE (partial), 134 tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add self-improving engine design plan

Designs a system where the engine debugs and improves itself, based on
the pattern observed in the last session: 5 consecutive bug fixes all
followed trace → read → identify → edit → test, using tools the engine
already has access to.

Three levels of self-improvement:
- Level 1 (Prompt): edit prompts/*.md to prevent LLM mistakes. Auto-apply.
- Level 2 (Config): adjust defaults/mappings. Branch + test + PR.
- Level 3 (Code): Rust patches for engine bugs. Branch + test + clippy + PR.

Architecture: Self-improvement Mission spawns a Reflection thread that
reads traces, reads source, proposes fixes, validates via cargo test,
and either auto-applies (Level 1) or creates a PR (Level 2-3).

Includes: fix pattern database (seeded from our 8 debugging session
fixes), feedback loop diagram, safety model, implementation phases
(A through D), and what exists vs what's new.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add engine v2 security model and audit

Comprehensive security analysis of engine v2 covering:

Threat model: 4 attacker profiles (malicious input, prompt injection
via tools, poisoned memory, supply chain).

Current state audit: 9 controls working (Monty sandbox, safety layer,
policy engine, leases, provenance, events) and 9 gaps identified.

Critical finding: ALL tools granted by default — CodeAct code can call
shell, write_file, apply_patch without approval. Proposed fix: 3-tier
tool classification (auto/approve-once/always-approve).

CodeAct-specific threats: tool call amplification, prompt injection via
search results, data exfiltration via tool chains, Monty escape.

Self-improvement security: poisoned trace attacks, memory poisoning via
reflection. Mitigations: edit validation, frequency caps, audit trail,
auto-rollback, reflection output scanning.

6-layer security architecture proposed: input validation, capability
gating, output sanitization, execution sandboxing, self-improvement
controls, observability.

Prioritized implementation plan with severity/effort ratings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(security): cross-reference v1 controls — use, don't reinvent

Updated security plan with detailed audit of ALL existing v1 security
controls and how they map to engine v2 bridge gaps:

Key finding: v1 already has solutions for every security gap identified.
The bridge just needs to wire them in:

- Tool::requires_approval() exists but bridge doesn't call it
- safety.wrap_for_llm() exists but tool results enter context unwrapped
- RateLimiter exists but bridge doesn't check rate limits
- BeforeToolCall hooks exist but bridge doesn't run them
- redact_params() exists but bridge doesn't redact sensitive params
- Shell risk classification (Low/Medium/High) is inherited but ignored

Revised priority: most fixes are small wiring tasks in EffectBridgeAdapter,
not new security infrastructure. The bridge is the security boundary.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): add missions, reliability tracker, reflection executor, and provenance-aware policy

- Add Mission type and MissionManager for recurring thread scheduling
- Add ReliabilityTracker for per-capability success/failure/latency tracking
- Add reflection executor that spawns CodeAct threads for post-completion reflection
- Extend PolicyEngine with provenance-aware taint checking (LLM-generated data
  requires approval for financial/external-write effects)
- Extend Store trait with mission CRUD methods
- Add conversation surface tracking, compaction token fix, context memory injection
- Wire new modules through lib.rs re-exports and bridge adapters

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(bridge): wire v1 security controls into engine v2 adapter

Zero engine crate changes. All security controls enforced at the bridge
boundary in EffectBridgeAdapter:

1. Tool approval (v1: Tool::requires_approval):
   - Checks each tool's approval requirement with actual params
   - Always → returns EngineError::LeaseDenied (blocks execution)
   - UnlessAutoApproved → checks auto_approved set, blocks if not approved
   - Never → proceeds
   - Per-session auto_approved HashSet (for future "always" handling)

2. Hook interception (v1: BeforeToolCall):
   - Runs HookEvent::ToolCall before every execution
   - HookOutcome::Reject → blocks with reason
   - HookError::Rejected → blocks with reason
   - Hook errors → fail-open (logged, execution continues)

3. Output sanitization (v1: sanitize_tool_output + wrap_for_llm):
   - Leak detection: API keys in tool output are redacted
   - Policy enforcement: content policy rules applied
   - Length truncation: output capped at 100KB
   - XML boundary protection: prevents injection via tool output

4. Sensitive param redaction (v1: redact_params):
   - Tool's sensitive_params() consulted before hooks see parameters
   - Redacted params sent to hooks, original params used for execution

5. available_actions() now sets requires_approval based on each tool's
   default approval requirement, so the engine's PolicyEngine can
   gate tools it hasn't seen before.

6. Actual execution timing measured via Instant::now() (replaces
   placeholder Duration::from_millis(1)).

Accessor visibility: hooks() widened to pub(crate).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(bridge): implement tool approval flow for engine v2

Adds a complete approval flow that mirrors v1 behavior, using the
existing v1 security controls (Tool::requires_approval, auto-approve
sets, StatusUpdate::ApprovalNeeded).

## How it works

### Step 1: Tool blocked at execution
When the LLM's code calls a tool (e.g., `shell("ls")`):
1. EffectBridgeAdapter.execute_action() looks up the Tool object
2. Calls tool.requires_approval(&params) — returns ApprovalRequirement
3. If Always → EngineError::LeaseDenied (always blocks)
4. If UnlessAutoApproved → checks auto_approved HashSet → if not in set,
   returns EngineError::LeaseDenied
5. If Never → proceeds to execution

### Step 2: Engine returns NeedApproval
The LeaseDenied error propagates through:
- CodeAct path: becomes Python RuntimeError, code halts, thread returns
  NeedApproval with action_name + parameters
- Structured path: same via ActionResult.is_error

### Step 3: Router stores pending approval
- PendingApproval { action_name, original_content } stored on EngineState
- StatusUpdate::ApprovalNeeded sent to channel (shows approval card in
  CLI/web with tool name, parameters, yes/always/no buttons)
- Returns text: "Tool 'shell' requires approval. Reply yes/always/no."

### Step 4: User responds
handle_message() intercepts Submission::ApprovalResponse when ENGINE_V2:
- 'yes' → auto_approve_tool(name) on EffectBridgeAdapter, re-processes
  original message (tool now passes the approval check on second run)
- 'always' → same + logs for session persistence
- 'no' → returns "Denied: tool was not executed."

### Key design choice
Instead of pausing/resuming mid-execution (which needs engine changes
to freeze/restore the Monty VM state), we auto-approve the tool and
re-run the full message. The EffectBridgeAdapter's auto_approved set
persists across runs, so the second execution passes immediately.

This trades one extra LLM call for zero engine modifications.

## Files changed
- src/bridge/router.rs: PendingApproval struct, handle_approval(),
  NeedApproval → StatusUpdate::ApprovalNeeded conversion
- src/bridge/mod.rs: export handle_approval
- src/agent/agent_loop.rs: intercept ApprovalResponse for engine v2
- src/bridge/effect_adapter.rs: fmt fixes

151 tests passing, clippy + fmt clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): demote trace/reflection logging from info to debug

INFO-level log output from background tasks (trace analysis, reflection)
corrupts the REPL terminal UI. The trace summary, issue warnings, and
reflection doc previews were printing mid-approval-card, breaking the
interactive display.

Fix: all logging in trace.rs changed from info!/warn! to debug!/warn!.
Trace analysis and reflection results now only show when
RUST_LOG=ironclaw_engine=debug is set.

Also added logging discipline rule to global CLAUDE.md:
- info! → user-facing status the REPL intentionally renders
- debug! → internal diagnostics (traces, reflection, engine internals)
- Background tasks must NEVER use info! — it breaks the TUI

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(bridge): demote all router info! logging to debug!

"engine v2: initializing" and "engine v2: handling message" were
printing at INFO level, corrupting the REPL UI. All router logging
now uses debug! — only visible with RUST_LOG=ironclaw=debug.

Zero info! calls remain in crates/ironclaw_engine/ or src/bridge/.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): demote leak detector warn-action logs from warn! to debug!

The leak detector's Warn-action matches (high_entropy_hex pattern on
web search results containing commit SHAs, CSS colors, URL hashes)
were logging at warn! level, corrupting the REPL UI with lines like:
  WARN Potential secret leak detected pattern=high_entropy_hex preview=a96f********cee5

These are informational false positives — real leaks use LeakAction::Redact
which silently modifies the content. Warn-action matches only log for
debugging purposes and should not appear in production output.

Changed to debug! level — visible with RUST_LOG=ironclaw_safety=debug.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): strengthen CodeAct prompt to prevent shallow text answers

The model was answering "Suggested 45 improvements" as a brief text
summary from training data without actually searching or listing them.
The trace showed: no code block, no tool calls, no FINAL().

Prompt changes:
- Rule 1: "ALWAYS respond with a ```repl code block. NEVER answer with
  plain text only." (was: "Always write code... plain text for brief
  explanations")
- Rule 2 (NEW): "NEVER answer from memory or training data alone.
  Always use tools to get real, current information before answering."
- Rule 3: FINAL answer "should be detailed and complete — not just a
  summary like 'found 45 items'"
- Rule 8 (NEW): "Include the actual content in your FINAL() answer,
  not just a count or summary. Users want to see the details."

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(bridge): persist reflection docs to workspace for cross-session learning

Replaces InMemoryStore with HybridStore:
- Ephemeral data (threads, steps, events, leases) stays in-memory
- MemoryDocs (lessons, specs, playbooks from reflection) persist to
  the workspace at engine/docs/{type}/{id}.json

On engine init, load_docs_from_workspace() reads existing docs back
into the in-memory cache. This means:
- Lessons learned in session 1 are available in session 2
- The RetrievalEngine injects relevant past lessons into new threads
- The engine genuinely improves over time as reflection accumulates

Workspace paths:
  engine/docs/lessons/{uuid}.json
  engine/docs/specs/{uuid}.json
  engine/docs/playbooks/{uuid}.json
  engine/docs/summaries/{uuid}.json
  engine/docs/issues/{uuid}.json

No new database tables. Uses existing workspace write/read/list.
workspace() accessor widened to pub(crate).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(bridge): adapt to execute_tool_with_safety params-by-value change

Staging merge changed execute_tool_with_safety to take params by value
instead of by reference (perf optimization from PR #926). Updated
bridge adapter to clone params before passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(engine): add web gateway integration plan to Phase 6

Documents three gaps between engine v2 and the web gateway:
1. No SSE streaming (engine emits ThreadEvent, gateway expects SseEvent)
2. No conversation persistence (engine uses HybridStore, gateway reads v1 DB)
3. No cross-channel visibility (REPL ↔ web messages invisible to each other)

Implementation plan: bridge ThreadEvent→AppEvent, write messages to v1
conversation tables after thread completion. Prerequisite: AppEvent
extraction PR (in progress separately).

Also updated DB persistence status: HybridStore with workspace-backed
MemoryDocs is now implemented (partial persistence).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs(engine): document routine/job gap and SIGKILL crash scenario

Routines are entirely v1 — not hooked up to engine v2. When a user
asks "create a routine" as natural language, engine v2 tries to call
routine_create via CodeAct, but the tool needs RoutineEngine + Database
refs that the bridge's minimal JobContext doesn't provide. This caused
a SIGKILL crash during testing.

Options documented: block routine tools in v2 (short term), pass refs
through context (medium), replace with Mission system (long term).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: extract AppEvent to crates/ironclaw_common

SseEvent was defined in src/channels/web/types.rs but imported by 12+
modules across agent, orchestrator, worker, tools, and extensions — it
had become the application-wide event protocol, not a web transport
concern.

Create crates/ironclaw_common as a shared workspace crate and move the
enum there as AppEvent.  Also move the truncate_preview utility which
was similarly leaked from the web gateway into agent modules.

- New crate: crates/ironclaw_common (AppEvent, truncate_preview)
- Rename SseEvent → AppEvent, from_sse_event → from_app_event
- web/types.rs re-exports AppEvent for internal gateway use
- web/util.rs re-exports truncate_preview
- Wire format unchanged (serde renames are on variants, not the enum)

Aligned with the event bus direction on refactor/architectural-hardening
where DomainEvent (≡ AppEvent) is wrapped in a SystemEvent envelope.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(bridge): integrate with web gateway via AppEvent + v1 conversation DB

Three changes to make engine v2 visible in the web gateway:

1. SSE event streaming (AppEvent broadcast):
   - ThreadEvent → AppEvent conversion via thread_event_to_app_event()
   - Events broadcast to SseManager during the poll loop
   - Covers: Thinking, ToolCompleted (success/error), Status, Response
   - Web gateway receives real-time progress without any gateway changes

2. Conversation persistence to v1 database:
   - After thread completes, writes user message + agent response to
     v1 ConversationStore via add_conversation_message()
   - Uses get_or_create_assistant_conversation() for per-user per-channel
   - Web gateway reads from DB as usual — chat history appears

3. Final response broadcast:
   - AppEvent::Response with full text + thread_id sent via SSE
   - Web gateway renders the response in the chat UI

New EngineState fields: sse (Option<Arc<SseManager>>),
db (Option<Arc<dyn Database>>). Both populated from Agent.deps.

Agent.deps visibility widened to pub(crate).

Depends on: ironclaw_common crate with AppEvent type (PR #1615).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(bridge): complete Phase 6 — v1-only tool blocking, rate limiting, call limits

Three security/stability improvements in EffectBridgeAdapter:

1. V1-only tool blocking:
   - routine_create, create_job, build_software (and hyphenated variants)
     return helpful error: "use the slash command instead"
   - Filtered out of available_actions() so system prompt doesn't list them
   - Prevents crash from tools needing RoutineEngine/Scheduler refs

2. Per-step tool call limit:
   - Max 50 tool calls per code block (AtomicU32 counter)
   - Prevents amplification: `for i in range(10000): shell(...)`
   - Returns "call limit reached, break into multiple steps"

3. Rate limiting:
   - Per-user per-tool sliding window via RateLimiter
   - Checks tool.rate_limit_config() before every execution
   - Returns "rate limited, try again in Ns"

Architecture plan updated:
- Gateway integration: DONE
- Routines: BLOCKED (gracefully, with slash command fallback)
- Rate limiting: DONE
- Call limit: DONE
- Phase 6 status: DONE (remaining: acceptance tests, two-phase commit)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add Mission system design — goal-oriented autonomous threads

Missions replace routines with evolving, knowledge-accumulating
autonomous agents. Unlike routines (fixed prompt, stateless), Missions:

- Generate prompts from accumulated Project knowledge (lessons,
  playbooks, issues from prior threads)
- Adapt approach when something fails repeatedly
- Track progress toward a goal with success criteria
- Self-manage: pause when stuck, complete when goal achieved

Architecture: MissionManager with cron ticker spawns threads via
ThreadManager. Meta-prompt built from mission goal + Project MemoryDocs
via RetrievalEngine. Reflection feeds back automatically.

6-step implementation plan: cron trigger, meta-prompt builder, bridge
wiring, CodeAct tools, progress tracking, persistence.

Includes two worked examples: daily tech news briefing (ongoing) and
test coverage improvement (goal-driven, self-completing).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): extend Mission types with webhook/event triggers + evolving strategy

Mission types updated to support external activation sources:

MissionCadence expanded:
- Cron { expression, timezone } — timezone-aware scheduling
- OnEvent { event_pattern } — channel message pattern matching
- OnSystemEvent { source, event_type } — structured events from tools
- Webhook { path, secret } — external HTTP triggers (GitHub, email, etc.)
- Manual — explicit triggering only

The engine defines trigger TYPES. The bridge implements infrastructure
(cron ticker, webhook endpoints, event matchers). GitHub issues, PRs,
email, Slack events all use the generic Webhook cadence — no
special-casing in the engine. Webhook payload injected as
state["trigger_payload"] in the thread's Python context.

Mission struct extended:
- current_focus: what the next thread should work on (evolving)
- approach_history: what we've tried (for adaptation)
- max_threads_per_day / threads_today: daily budget
- last_trigger_payload: webhook/event data for thread context

Plan updated with trigger type table and webhook integration design.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): implement MissionManager execution with meta-prompts

The MissionManager now builds evolving meta-prompts and processes
thread outcomes for continuous learning:

fire_mission() upgraded:
- Loads Project MemoryDocs via RetrievalEngine for context
- Builds meta-prompt from: goal, current_focus, approach_history,
  project knowledge docs, trigger payload, thread count
- Spawns thread with meta-prompt as user message
- Background task waits for completion and processes outcome
- Daily thread budget enforcement (max_threads_per_day)

Meta-prompt structure:
  # Mission: {name}
  Goal: {goal}
  ## Current Focus (evolves between threads)
  ## Previous Approaches (what we've tried)
  ## Knowledge from Prior Threads (lessons, playbooks, issues)
  ## Trigger Payload (webhook/event data if applicable)
  ## Instructions (accomplish step, report next focus, check goal)

Outcome processing:
- Extracts "next focus:" from FINAL() response → updates current_focus
- Detects "goal achieved: yes" → completes mission
- Records accomplishment in approach_history
- Failed threads recorded as "FAILED: {error}"

Cron ticker:
- start_cron_ticker() spawns tokio task, ticks every 60s
- Checks active Cron missions, fires those past next_fire_at

151 tests passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(bridge): wire MissionManager into engine v2 for CodeAct access

Missions are now callable from CodeAct Python code:

```python
# Create a daily briefing mission
result = mission_create(
    name="Tech News",
    goal="Daily AI/crypto/software news briefing",
    cadence="0 9 * * *"
)

# List all missions
missions = mission_list()

# Manually fire a mission
mission_fire(id="...")

# Pause/resume
mission_pause(id="...")
mission_resume(id="...")
```

Implementation:
- MissionManager created on engine init, cron ticker started
- EffectBridgeAdapter intercepts mission_* function calls before tool
  lookup and routes to MissionManager
- parse_cadence() handles: "manual", cron expressions, "event:pattern",
  "webhook:path"
- Mission functions documented in CodeAct system prompt
- MissionManager set on adapter via set_mission_manager() after init
  (avoids circular dependency)

System prompt updated with mission_create, mission_list, mission_fire,
mission_pause, mission_resume documentation.

151 tests passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(bridge): map routine_* calls to mission operations in v2

When the model calls routine_create, routine_list, routine_fire,
routine_pause, routine_resume, or routine_delete, the bridge now
routes them to the MissionManager instead of blocking with an error.

Mapping:
  routine_create → mission_create (with cadence parsing)
  routine_list   → mission_list
  routine_fire   → mission_fire
  routine_pause  → mission_pause
  routine_resume → mission_resume
  routine_update → mission_pause/resume (based on params)
  routine_delete → mission_complete (marks as done)

Routine tools removed from v1-only blocklist and restored in
available_actions(). The model can use either "routine" or "mission"
vocabulary — both work.

Still blocked: create_job, cancel_job, build_software (need v1
Scheduler/ContainerJobManager refs).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(engine): add E2E mission flow tests — 7 new tests

Comprehensive mission lifecycle tests:

- fire_mission_builds_meta_prompt_with_goal: verifies thread spawned
  with project context and recorded in history
- outcome_processing_extracts_next_focus: "Next focus: X" in FINAL()
  response → mission.current_focus updated
- outcome_processing_detects_goal_achieved: "Goal achieved: yes" →
  mission status transitions to Completed
- mission_evolves_via_direct_outcome_processing: 3-step evolution:
  step 1 sets focus to "db module", step 2 evolves to "tools module",
  step 3 detects goal achieved → mission completes. Tests the full
  learning loop without background task timing dependencies.
- fire_with_trigger_payload: webhook payload stored on mission and
  threads_today counter incremented
- daily_budget_enforced: max_threads_per_day=1 → first fire succeeds,
  second returns None

157 tests passing (151 prior + 6 new mission E2E).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): self-improving engine via Mission system

Wire the self-improvement loop as a Mission with OnSystemEvent cadence,
inspired by karpathy/autoresearch's program.md approach. The mission
fires when threads complete with issues, receives trace data as trigger
payload, and uses tools directly to diagnose and fix problems.

Key changes:

Engine self-improvement (Phase A+B from design doc):
- Add fire_on_system_event() to MissionManager for OnSystemEvent cadence
- Add start_event_listener() that subscribes to thread events and fires
  matching missions when non-Mission threads complete with trace issues
- Add ensure_self_improvement_mission() with autoresearch-style goal
  prompt (concrete loop steps, not vague instructions)
- Add process_self_improvement_output() for structured JSON fallback
- Seed fix pattern database with 8 known patterns from debugging
- Runtime prompt overlay via MemoryDoc (build_codeact_system_prompt now
  async + Store-aware, appends learned rules from prompt_overlay docs)
- Pass Store to ExecutionLoop for overlay loading

Bridge review fixes (P1/P2):
- Scope engine v2 SSE events to requesting user (broadcast_for_user)
- Per-user pending approvals via HashMap instead of global Option
- Reset tool-call limit counter before each thread execution
- Only persist auto-approval when user chose "always", not one-off "yes"
- Remove dead store/mission_manager fields from EngineState

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Add checkpoint-based engine thread recovery

* feat(engine): add Python orchestrator module and host functions

Add the orchestrator infrastructure for replacing the Rust execution
loop with versioned Python code. This commit adds the module and host
functions without switching over — the existing Rust loop is unchanged.

New files:
- orchestrator/default.py: v0 Python orchestrator (run_loop + helpers)
- executor/orchestrator.rs: host function dispatch, orchestrator
  loading from Store with version selection, OrchestratorResult parsing

Host functions exposed to orchestrator Python via Monty suspension:
  __llm_complete__, __execute_code_step__ (nested Monty VM),
  __execute_action__, __check_signals__, __emit_event__,
  __add_message__, __save_checkpoint__, __transition_to__,
  __retrieve_docs__, __check_budget__, __get_actions__

Also makes json_to_monty, monty_to_json, monty_to_string pub(crate)
in scripting.rs for cross-module use.

Design doc: docs/plans/2026-03-25-python-orchestrator.md

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): switch ExecutionLoop::run() to Python orchestrator

Replace the 900-line Rust execution loop with a ~80-line bootstrap
that loads and runs the versioned Python orchestrator via Monty VM.

The orchestrator Python code (orchestrator/default.py) is the v0
compiled-in version. Runtime versions can override it via MemoryDoc
storage (orchestrator:main with tag orchestrator_code).

Key fixes during switchover:
- Use ExtFunctionResult::NotFound for unknown functions so Monty
  falls through to Python-defined functions (extract_final, etc.)
- Move helper function definitions above run_loop for Monty scoping
- Use FINAL result value (not VM return value) in Complete handler
- Rename 'final' variable to 'final_answer' to avoid Python keyword

Status: 171/177 tests pass. 6 remaining failures are step_count and
token tracking bookkeeping — the orchestrator manages these internally
but doesn't yet update the thread's counters via host functions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): all 177 tests pass with Python orchestrator

- Increment step_count and track tokens in __emit_event__("step_completed")
  so thread bookkeeping matches the old Rust loop behavior
- Remove double-counting of tokens in bootstrap (orchestrator handles it)
- Match nudge text to existing TOOL_INTENT_NUDGE constant
- Fix FINAL result propagation (use stored final_result, not VM return)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): orchestrator versioning, auto-rollback, and tests

Add version lifecycle for the Python orchestrator:
- Failure tracking via MemoryDoc (orchestrator:failures)
- Auto-rollback: after 3 consecutive failures, skip the latest version
  and fall back to previous (or compiled-in v0)
- Success resets the failure counter
- OrchestratorRollback event for observability

Update self-improvement Mission goal with Level 1.5 instructions for
orchestrator patches — the agent can now modify the execution loop
itself via memory_write with versioned orchestrator docs.

12 new tests: version selection (highest wins), rollback after failures,
rollback to default, failure counting/resetting, outcome parsing for
all 5 ThreadOutcome variants.

189 tests pass, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add engine v2 architecture, self-improvement, and dev history

Three new docs for contributors:

- engine-v2-architecture.md: Two-layer architecture (Rust kernel +
  Python orchestrator), five primitives, execution model with nested
  Monty VMs, bridge layer, memory/reflection, missions, capabilities

- self-improvement.md: Three improvement levels (prompt/orchestrator/
  config/code), autoresearch-inspired Mission loop, versioned
  orchestrator with auto-rollback, fix pattern database, safety model

- development-history.md: Summary of 6 Claude Code sessions that
  built the system, key design decisions and debugging moments,
  architecture evolution from 900-line Rust loop to Python orchestrator

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): complete v2 side-by-side integration with gateway API

Wire engine v2 into the full submission pipeline and expose threads,
projects, and missions through the web gateway REST API.

Bridge routing — route ExecApproval, Interrupt, NewThread, and Clear
submissions to engine v2 when ENGINE_V2=true. Previously only UserInput
and ApprovalResponse were handled; all other control commands fell
through to disconnected v1 sessions.

Bridge query layer — add 11 read-only query functions and 6 DTO types
so gateway handlers can inspect engine state (threads, steps, events,
projects, missions) without direct access to the EngineState singleton.

Gateway endpoints — new /api/engine/* routes:
  GET  /threads, /threads/{id}, /threads/{id}/steps, /threads/{id}/events
  GET  /projects, /projects/{id}
  GET  /missions, /missions/{id}
  POST /missions/{id}/fire, /missions/{id}/pause, /missions/{id}/resume

SSE events — add ThreadStateChanged, ChildThreadSpawned, and
MissionThreadSpawned AppEvent variants. Expand the bridge event mapper
to forward StateChanged and ChildSpawned engine events to the browser.

Engine crate — add ConversationManager::clear_conversation() for /new
and /clear commands.

Code quality — replace 10 .expect() calls with proper error returns,
remove dead AgentConfig.engine_v2 field, log silent init errors, fix
duplicate doc comment, improve fallthrough documentation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): empty call_id on ActionResult and trace analyzer false positives

Fix structured executor not stamping call_id onto ActionResult — the
EffectExecutor trait doesn't receive call_id, so the structured executor
must copy it from the original ActionCall after execution. Empty call_id
caused OpenAI-compatible providers to reject the next LLM request with
"Invalid 'input[2].call_id': empty string".

Fix trace analyzer false positives:
- code_error check now only scans User-role code output messages
  (prefixed with [stdout]/[stderr]/[code ]/Traceback), not System
  prompt which contains example error text
- missing_tool_output check now recognizes ActionResult messages as
  valid tool output (Tier 0 structured path)
- Add NotImplementedError to detected code error patterns

New trace checks:
- empty_call_id: detect ActionResult messages with missing/empty
  call_id before they reach the LLM API (severity: Error)
- llm_error: extract LLM provider errors from Failed state reason
- orchestrator_error: extract orchestrator errors from Failed state

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(web): add Missions tab to gateway UI

Add a full Missions page to the web gateway with list view, detail view,
and action buttons (Fire, Pause, Resume).

Backend: add /api/engine/missions/summary endpoint returning counts by
status (active/paused/completed/failed).

Frontend:
- New "Missions" tab between Jobs and Routines
- Summary cards showing mission counts by status
- Table with name, goal, cadence type, thread count, status, actions
- Detail view with goal, cadence, current focus, success criteria,
  approach history, spawned thread list, and action buttons
- Fire/Pause/Resume actions with toast notifications
- i18n support (English + Chinese)
- CSS following the existing routines/jobs patterns

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): eagerly initialize engine v2 at startup

The gateway API endpoints (/api/engine/missions, etc.) call bridge
query functions that return empty results when the engine state hasn't
been initialized yet. Previously, initialization only happened lazily
on the first chat message via handle_with_engine().

Now when ENGINE_V2=true, the engine is initialized in Agent::run()
before channels start, so the self-improvement mission and other
engine state is available to gateway API endpoints immediately.

Also rename get_or_init_engine → init_engine and make it public so
it can be called from agent_loop.rs at startup.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(web): improve mission detail with markdown goal and thread table

- Goal rendered as full-width markdown block instead of plain-text
  meta item (uses existing renderMarkdown/marked)
- Current focus and success criteria also rendered as markdown
- Spawned threads shown as a clickable table with goal, type, state,
  steps, tokens, and created date instead of a UUID list
- Clicking a thread row opens an inline thread detail view showing
  metadata grid and full message history with markdown rendering
- Back button returns to the mission detail view
- Backend: mission detail now returns full thread summaries (goal,
  state, step_count, tokens) instead of just thread IDs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(web): close SSE connections on page unload to prevent connection starvation

The browser limits concurrent HTTP/1.1 connections per origin to 6.
Without cleanup, SSE connections from prior page loads linger after
refresh/navigation, eating into the pool. After 2-3 refreshes, all 6
slots are consumed by stale SSE streams and new API fetch calls queue
indefinitely — the UI shows "connected" (SSE works) but data never
loads.

Add a beforeunload handler that closes both eventSource (chat events)
and logEventSource (log stream) so the browser can reuse connections
immediately on page reload.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(web): support multiple gateway tabs by reducing SSE connections

Each browser tab opened 2 SSE connections (chat events + log events).
With the HTTP/1.1 per-origin limit of 6, the 3rd tab exhausted the
pool and couldn't load any data.

Three changes:

1. Lazy log SSE — only connect when the logs tab is active, disconnect
   when switching away. Most users rarely view logs, so this saves a
   connection slot per tab.

2. Visibility API — close SSE when the browser tab goes to background
   (user switches to another tab), reconnect when it becomes visible.
   Background tabs don't need real-time events.

3. Combined with the existing beforeunload cleanup, this means:
   - Active foreground tab: 1 connection (chat SSE only, +1 if logs tab)
   - Background tabs: 0 connections
   - Closed/refreshed tabs: 0 connections (beforeunload cleanup)

This allows many gateway tabs to coexist within the 6-connection limit.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): route messages to correct conversation by thread scope

Messages sent from a new conversation in the gateway always appeared in
the default assistant conversation because handle_with_engine ignored
the thread_id from the frontend.

Two fixes:

1. Engine conversation scoping — when the message carries a thread_id
   (from the frontend's conversation picker), use it as part of the
   engine conversation key: "gateway:<thread_id>" instead of just
   "gateway". This creates a distinct engine conversation per v1
   thread, so messages don't cross-contaminate.

2. V1 dual-write targeting — write user messages and assistant
   responses to the v1 conversation matching the thread_id (via
   ensure_conversation), not the hardcoded assistant conversation.
   Falls back to the assistant conversation when no thread_id is
   present (e.g., default chat).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(web): richer activity indicators for engine v2 execution

The gateway UI showed only generic "Thinking..." during engine v2
execution with no visibility into CodeAct code execution, tool calls,
or reflection. Now the event mapping produces detailed status updates:

Step lifecycle:
- "Calling LLM..." when a step starts (was "Thinki…
…1957)

* fix(engine): compute next_fire_at for cron missions (#1944)

MissionCadence::Cron missions never fired automatically because
next_fire_at was initialized to None and never computed from the cron
expression. The ticker checked next_fire_at <= now which was always
false.

- Add next_cron_fire() helper that parses cron expressions (5/6/7-field)
  and computes the next fire time, with optional timezone support
- Compute next_fire_at in create_mission() for Cron cadence
- Advance next_fire_at in fire_mission() after each successful fire
- Recompute next_fire_at in resume_mission() for stale cron missions
- Add regression tests covering create, fire, tick, and resume flows

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(engine): propagate user timezone to missions and CodeAct scripts (#1944)

The LLM had to ask users for timezone because it wasn't available in
context. Now:

- Add user_timezone to ThreadExecutionContext (from thread metadata)
- Store user timezone in thread metadata when received from channel
- Auto-inject timezone into mission_create cron cadence from context
- Expose user_timezone as a Monty/CodeAct context variable
- Document user_timezone in the CodeAct preamble prompt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: introduce ValidTimezone strict type in ironclaw_common (#1944)

Address PR review feedback: timezone strings were stored and propagated
without validation. Now:

- Add ValidTimezone newtype in ironclaw_common that validates IANA
  timezone strings at construction (parse returns None for empty/invalid)
- MissionCadence::Cron.timezone is now Option<ValidTimezone>
- ThreadExecutionContext.user_timezone is now Option<ValidTimezone>
- Bridge router validates timezone before storing in thread metadata
- next_cron_fire() takes Option<&ValidTimezone> — no silent fallback
- CodeAct scripting validates on read, falls back to "UTC" for missing
- Fix orchestrator doc comment (bridge router, not ConversationManager)
- Tighten test assertion to require strictly future next_fire_at
- Add ValidTimezone unit tests (parse, serde roundtrip, empty/invalid)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove clone_on_copy for ValidTimezone (clippy)

ValidTimezone is Copy, so .clone() is unnecessary. Clippy CI caught this.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: lenient timezone deserialization + bootstrap backfill (#1944)

Address second round of PR review feedback:

- Add deserialize_option_lenient() in ironclaw_common so persisted
  missions with invalid timezone strings deserialize as None instead
  of failing the whole record
- Apply lenient deserializer to MissionCadence::Cron.timezone field
- Backfill next_fire_at in bootstrap_project() for legacy cron missions
  that predate the scheduling fix (next_fire_at was None)
- Remove unused chrono-tz direct dep from ironclaw_engine (now via
  ironclaw_common)
- Add tests for lenient deserialization (valid, invalid, null, missing)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: collapse nested if in bootstrap_project (clippy)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): address PR review — pre-spawn timezone, update_mission scheduling, polish

Addresses 6 unanswered review comments on #1957:

- (high, serrrfirat) router.rs: user_timezone was set after start_thread,
  so the executor's in-memory thread never saw it on the first turn.
  Threaded user_timezone through handle_user_message ->
  spawn_thread_with_history via a new initial_metadata param so it lands
  on the thread before the background task starts. source_channel is
  routed through the same path (had the same latent bug).

- (high, serrrfirat / Copilot) update_mission: Manual -> Cron left
  next_fire_at = None and the mission stayed inert; cron expression
  edits kept firing on the old schedule. update_mission now recomputes
  next_fire_at for Cron and clears it for non-cron cadences.

- (low, Copilot) parse_cadence: dropped trimmed.contains(' ') so cron
  expressions with tab/newline separators are detected.

- (medium, Copilot) bootstrap_project: backfill save_mission failure now
  logs at debug! instead of being silently swallowed.

- (low, Copilot) codeact_preamble: cron timezone is a default, not
  automatic — explicit timezone param overrides.

Plus self-review polish:
- normalize_cron_expression rejects 4-field (and other off-count) input
  up front with a clear error instead of falling through to the cron
  parser.
- fire_mission has a comment explaining the catch-up semantics
  (next_fire_at recomputed from now(), missed windows coalesced).
- New unit tests for next_cron_fire with an explicit America/New_York
  timezone, plus 3 update_mission regression tests for Manual->Cron,
  Cron->Manual, and cron expression change.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): DST/timezone tests, InvalidCadence error, lenient drop log

Addresses the latest review on #1957:

- Add EngineError::InvalidCadence and switch next_cron_fire to use it.
  Cron parse failures are validation errors, not store errors — callers
  can now map them to user-facing messages without misclassification.

- Add DST regression tests in types/mission.rs covering the two tricky
  cases the PR exists to enable:
    * spring-forward gap (30 2 * * * on a missing-local-hour day must
      not land in the [02:00, 03:00) window)
    * fall-back overlap (30 1 * * * on a doubled-hour day must
      consistently round-trip)
    * 30-day window straddling spring-forward must contain both
      13:00 and 14:00 UTC fires for an "9am NY" schedule
  Plus normalize_seven_field_cron and an InvalidCadence error path test.

- Add a tz-positive test in runtime/mission.rs that creates a cron
  mission with America/New_York and asserts the resulting next_fire_at
  lands at UTC 13/14 (NY 09:00) rather than UTC 09 — exercising the
  full MissionManager → next_cron_fire chain end-to-end.

- ironclaw_common::deserialize_option_lenient now logs at debug! when
  it drops an invalid IANA timezone string to None, so a typo in fresh
  user config is observable in logs even though the record loads.
  Adds tracing as a direct dep of ironclaw_common.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): suppress no-panics check on DST test helper

The check_no_panics.py CI script has a pre-existing lexer bug where a
lifetime apostrophe in this file (`Formatter<'_>` on line 38) puts its
char-state lexer into character mode permanently, breaking brace
tracking and so failing to detect that the new `schedule_after` test
helper lives inside `#[cfg(test)] mod tests`. The script normally only
checks added lines so the latent bug is invisible — my new helper
exposed it.

Use the script's documented `// safety:` per-line escape hatch to
suppress the false positives. The helper is unambiguously a test-only
helper and the panics are intentional in test context.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): log all next_cron_fire failure modes in bootstrap backfill

PR review (Copilot): bootstrap_project only patched the legacy mission
when next_cron_fire returned Ok(Some(next)). The Ok(None) case (cron
expression with no upcoming fire times — e.g. a year-restricted
expression in the past) and the Err(_) case (invalid expression) both
fell through silently, leaving the mission Active with next_fire_at =
None and no log signal — exactly the silent-stuck-mission scenario
this PR is supposed to prevent.

Match all three branches and emit a debug! log on each path so an
operator can see which legacy missions need attention.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): close resume_mission TOCTOU window and stop fire_mission from orphaning threads

Two medium-severity issues from PR review (serrrfirat):

1. **resume_mission TOCTOU.** The previous flow was
   `update_mission_status(Active)` followed by a separate `load + mutate
   next_fire_at + save` round-trip — leaving an extra interleave window
   where a concurrent `update_mission`/`fire_mission` write could be
   silently clobbered by the second save's stale reload. Use the mission
   already loaded for the ownership check, set `status = Active` and
   `next_fire_at`, and do a single `save_mission`.

2. **fire_mission orphan thread.** The thread was spawned, then
   `next_cron_fire(...)?` and `save_mission(...)?` ran. Both could
   propagate Err *after* the thread was already running, leaving:
   - no entry in `thread_history`
   - `threads_today` not incremented (budget bypassed)
   - no outcome watcher installed

   Two narrow fixes:
   - Install `spawn_mission_outcome_watcher` *before* `save_mission`.
     The watcher only depends on `thread_id` (it joins via
     ThreadManager and reloads the mission record itself), so a
     transient store error no longer abandons the running thread.
   - Replace `next_cron_fire(...)?` with match-and-log: a parse error
     on a corrupt persisted expression now preserves the existing
     `next_fire_at` and emits a `debug!` instead of aborting fire and
     unwinding past the spawn.

Regression tests:
- `fire_mission_with_corrupt_cron_expression_does_not_orphan_thread`
- `resume_mission_preserves_concurrent_field_changes`

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): Paranoid Architect review — orphan/re-fire, TOCTOU, cron docs, resume tz

Addresses 4 of 5 findings from serrrfirat's Paranoid Architect review on
#1957 (the 5th is declined with rationale in the PR thread).

1. (HIGH) fire_mission save_mission failure non-fatal + tick() per-mission
   isolation. The previous code propagated `save_mission(...)?` after the
   thread was spawned, leaving an orphan thread *and* — for cron cadences —
   leaving next_fire_at un-advanced, causing the next tick to re-fire the
   same mission in a runaway loop. Now save is best-effort: log a `debug!`
   on failure and still return Ok(Some(thread_id)). The watcher is already
   installed (from the earlier round). Separately, tick() now catches
   per-mission load and fire failures with match-and-continue so a single
   bad mission doesn't abort the entire tick cycle.

2. (MED) bootstrap_project backfill TOCTOU: previously list_all_missions
   then a deferred save_mission could clobber concurrent writes with the
   stale snapshot. Now re-load the mission immediately before save and
   only patch if next_fire_at is still None — narrows the window
   significantly without adding a new Store trait method. Residual race
   documented inline.

3. (MED) 6-field cron ambiguity: the `0 9 * * * 2027` form could be read
   as either "sec min hr dom mon dow" (our normalizer's interpretation,
   matching the `cron` crate's native format) or Quartz-style "min hr dom
   mon dow year". Documented the assumed format in the doc comment, added
   a regression test pinning the interpretation, and noted it in the
   CodeAct preamble so users know to use the explicit 7-field form for
   year-bounded schedules.

4. (MED) user_timezone propagation on inject/resume: only set on new
   thread spawn before. Now the resume path writes the fresh tz to
   thread metadata before resume_thread() reloads from store, so the
   resumed execution sees the up-to-date value. The inject path is
   documented as a known limitation — updating live in-memory state in
   a running ExecutionLoop requires a new signal type (out of scope).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): close latent next_fire_at bypass in ensure_* mission helpers

PR review (serrrfirat): two private mission-creation helpers built a
Mission via `Mission::new() + save_mission()` directly, skipping the
`next_fire_at` computation that `create_mission` performs. Today every
caller passes `OnSystemEvent` so the bug never bites — but a future
caller passing `Cron` would silently re-introduce the original
`next_fire_at = None` bug that #1944 fixes, and the bootstrap backfill
wouldn't help (it only triggers for Active+Cron with `next_fire_at =
None` *after* a process restart).

Add the same Cron-cadence guard to both helpers:
- `ensure_self_improvement_mission`
- `ensure_mission_by_metadata`

Regression test `ensure_mission_by_metadata_with_cron_cadence_computes_next_fire_at`
exercises the cron path through the private helper and asserts the
computed `next_fire_at` is set and in the future.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): final PR #1957 review pass — Ok(None) cron + tick cooldown

Addresses the remaining open review threads on PR #1957:

1. types/mission.rs:391 — DST test reference comment now matches the
   actual `2027-03-13 00:00 UTC` anchor instead of the stale `22:00 UTC`
   wording (Copilot 3050772614 / 3055762609).

2. runtime/manager.rs::set_thread_metadata — was best-effort and silently
   swallowed store errors, while the conversation.rs:250 caller comment
   claimed the resumed thread was guaranteed to see the new value. Now
   returns Result<(), EngineError>; conversation.rs logs on failure and
   the comment is honest about the contract (Copilot 3051471863 +
   serrrfirat 3056444137).

3. types/mission.rs::next_cron_fire_required — new helper that maps
   Ok(None) to EngineError::InvalidCadence. Used by create_mission,
   update_mission cadence-change, resume_mission, and the two
   ensure_*_mission helpers. fire_mission and bootstrap_project keep
   the existing grace (logged) since the thread/data is already in
   flight (Copilot 3051471912/47/85 + serrrfirat 3056443657).

4. runtime/mission.rs::tick — when save_mission fails after a successful
   spawn, the persisted next_fire_at stays in the past and every
   subsequent 60s tick re-fires the same mission, spawning duplicates
   up to the daily budget. Added an in-memory `last_fire_attempt` map
   armed by fire_mission (regardless of save outcome) and consulted by
   tick to enforce a 90s per-mission cooldown (serrrfirat 3056443083).

Regression tests:
 - create_mission_rejects_unschedulable_cron
 - update_mission_rejects_switch_to_unschedulable_cron
 - resume_mission_rejects_unschedulable_cron
 - tick_cooldown_suppresses_re_fire_on_save_failure

All four use a 7-field year-locked cron (`0 0 0 1 1 * 2020`) to exercise
the Ok(None) path deterministically. Mission test count: 49 → 53.

Verified: cargo fmt clean, cargo clippy --all-targets --all-features
zero warnings, full `cargo test` green.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): prune fire cooldown map + reject Quartz-style 6-field cron

Addresses two follow-up review threads on PR #1957:

1. `last_fire_attempt` was insert-only — completed/paused missions left
   stale entries that accumulated over the process lifetime. Now:
   - `pause_mission` and `complete_mission` drop the entry explicitly.
   - `tick` opportunistically prunes any entry whose cooldown window has
     elapsed, catching stragglers from non-graceful transitions (crash
     recovery, direct store edits) so the map can never grow unbounded.
   Regression test: `pause_and_complete_drop_cooldown_entry`.
   (serrrfirat 3057568698)

2. 6-field cron with a year-shaped trailing field (e.g. `0 9 * * * 2027`)
   was silently misinterpreted as `sec min hr dom mon dow=2027` instead
   of the Quartz-style "9am daily in 2027" the caller almost certainly
   meant. `normalize_cron_expression` now rejects this pattern with a
   clear `InvalidCadence` error pointing at the explicit 7-field form
   (`0 0 9 * * * 2027`). The 4-digit year heuristic is bounded to
   1970-2099 so out-of-range numerics fall through unchanged. The
   existing pinning test for `* `-terminated 6-field input is unaffected.
   Regression test: `six_field_cron_with_year_shaped_last_field_is_rejected`.
   (serrrfirat 3057569413)

Verified: cargo fmt clean, cargo clippy --all-targets --all-features
zero warnings, full `cargo test` green (308 engine tests pass).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(bridge,engine): parse_cadence prefix ordering + year-stable cron tests

Addresses three Copilot review threads on PR #1957:

1. `parse_cadence` (src/bridge/effect_adapter.rs) checked the cron
   heuristic (`split_whitespace().count() >= 5`) BEFORE the explicit
   `event:` / `webhook:` prefixes. An input like `event: a b c d e`
   silently became a `Cron` cadence with a parse-error downstream rather
   than the `OnEvent` the user requested. Reordered to check explicit
   prefixes first, falling through to cron only when none match.
   Regression test `parse_cadence_event_prefix_with_multi_token_pattern`
   covers `event:` + `webhook:` + a real cron control case.
   (Copilot 3057828107)

2. `next_cron_fire_respects_timezone` asserted `in_ny.year() >= 2026`,
   which is time-dependent (fails before 2026, tautology after). Replaced
   with `assert!(in_ny > Utc::now())` so the test stays stable across
   calendar years. (Copilot 3057828177)

3. `update_mission_cron_expression_change_recomputes_next_fire_at` used
   `0 0 1 1 *` ("once a year on Jan 1") as `before` and asserted
   `after < before`. Race around New Year's: the yearly schedule's next
   fire could land within seconds and invert the ordering. Switched to a
   year-locked 7-field cron (`0 0 0 1 1 * 2099`) so the next fire is
   deterministically far in the future regardless of run date.
   (Copilot 3057828212)

Verified: cargo fmt clean, cargo clippy --all-targets --all-features
zero warnings, full `cargo test --lib` green.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): outcome processor reconciles fire accounting on save failure

Addresses Copilot review thread on PR #1957:

When `fire_mission`'s post-spawn `save_mission` fails, the persisted
mission is left missing the new `thread_id` (in `thread_history`), the
`threads_today` increment, the `last_fire_at` stamp, and — for cron
cadences — the advanced `next_fire_at`. The in-memory `last_fire_attempt`
cooldown holds the runaway re-fire path closed for ~90 s, but once it
elapses tick would re-fire against the still-stale persisted state.
Worse, when the spawned thread later completes, the outcome watcher
loaded the **stale** mission, mutated only `approach_history`/`status`,
and saved — permanently overwriting any chance to record the missing
fields.

`process_mission_outcome_and_notify` now reconciles those fields the
first time it sees a `thread_id` that isn't in `thread_history`:

  - Append `thread_id` to `thread_history`
  - Bump `threads_today` (saturating)
  - Stamp `last_fire_at = now` as a conservative approximation
  - For cron missions, recompute `next_fire_at` if it's None-or-past

The reconcile is idempotent: a replay with the same `thread_id` is a
no-op. Achieves eventual consistency for transient store failures even
after retries are exhausted, and handles the permanent-failure case
that a save-side retry loop alone could not.

Regression test: `outcome_processor_reconciles_missing_fire_accounting`
covers both the first-time reconcile path (history append, budget bump,
last_fire_at stamp, cron next_fire_at advance) and idempotent replay
(no double-count, no duplicate history entry).

Verified: cargo fmt clean, cargo clippy --all-targets --all-features
zero warnings, full `cargo test --lib` green, ironclaw_engine 87/87
mission tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): cooldown only on save failure + reconcile to original fire instant

Self-review pass on the #1944 work — addresses three findings from a fresh
read of the cooldown / reconcile interaction:

1. **Tick cooldown was throttling high-frequency cron schedules.** The
   in-memory `last_fire_attempt` map was checked unconditionally, with
   the 90 s cooldown enforced even for normally-firing cron missions.
   For `* * * * *` (every minute) the 60 s tick interval falls inside
   the 90 s window, so half the events were silently dropped after every
   successful fire. The cooldown was only ever needed to detect a
   `save_mission` failure, not to enforce a global rate floor.

   Fix: `fire_mission` now uses a single `fire_instant` for both the
   persisted `Mission.last_fire_at` and the in-memory cooldown entry.
   Tick arms the cooldown only when the persisted `last_fire_at`
   does NOT equal the in-memory value — the equality is the proof
   that `save_mission` succeeded. On the success path the two match
   and the cooldown is transparent regardless of cron frequency. On
   the failure path the persisted side still holds the OLD value
   (or `None`), the mismatch fires, and re-fire is suppressed until
   reconcile.

   Regression test:
   `tick_does_not_throttle_high_frequency_cron_after_successful_fire`
   creates a `* * * * *` mission, fires it, advances `next_fire_at`
   into the past WITHOUT clobbering `last_fire_at`, calls tick, and
   asserts a new thread is spawned. The pre-existing failure-mode
   test was updated to explicitly clobber `last_fire_at` so it
   continues to model the failed-save state under the new logic.

2. **Reconcile drifted `last_fire_at` to the outcome time.** When
   `process_mission_outcome_and_notify` reconciled fields after a
   failed save, it stamped `last_fire_at = now`, which can be many
   seconds (or hours, for long-running mission threads) later than the
   actual fire instant. For users with a configured `cooldown_secs`,
   that drift gradually extended the cooldown window beyond what the
   user asked for.

   Fix: thread the original `fire_instant` from `fire_mission` through
   `spawn_mission_outcome_watcher` and `process_mission_outcome_and_notify`
   as `original_fire_at: Option<DateTime<Utc>>`. The reconcile path
   uses it to set `last_fire_at` back to the moment of the original
   spawn, falling back to `now` only when the watcher path is unknown
   (test helpers, callers without an original instant). The watcher's
   `original_fire_at` ALSO matches the in-memory `last_fire_attempt[mid]`
   value, so the cooldown's mismatch detector resolves immediately
   after reconcile — even before the 90 s window elapses.

   Regression test: `outcome_processor_reconciles_missing_fire_accounting`
   now passes an explicit `original_fire_at` and asserts the
   reconciled `last_fire_at` equals that exact instant (not `now`).

3. **Reconcile bypassed `mission.record_thread`.** It pushed directly
   into `thread_history` and missed the `updated_at` bump.
   `process_mission_outcome_and_notify` re-stamps `updated_at` later
   so there was no functional impact, but two paths diverging on the
   same field-mutation pattern is a future-bug invitation.

   Fix: use `mission.record_thread(thread_id)` in the reconcile branch
   for parity with `fire_mission`.

Polish:
 - Documented the lenient `next_cron_fire` choice at all three call
   sites (`bootstrap_project` backfill, `fire_mission` post-spawn
   advance, reconcile path) to make the lenient/strict split easy to
   spot during review.
 - Added a clarifying comment on `update_mission`'s save-after-validate
   sequencing — `save_mission` is the only persistence boundary in the
   function, so `next_cron_fire_required`'s `Err` leaves the store
   untouched even though the in-memory `mission` was already mutated.

Verified: cargo fmt clean, cargo clippy --all-targets --all-features
zero warnings, full `cargo test --lib` green, ironclaw_engine 88/88
mission tests pass (87 → 88).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): close cooldown race + corrupt-cron runaway

Self-review pass on the cooldown rework — addresses the two failure
modes a fresh read of the success/failure detection turned up plus a
documentation gap on the equality check.

1. **TOCTOU race between save_mission success and cooldown insert.**
   The previous order was `save_mission(...)` → `last_fire_attempt.insert(...)`.
   A concurrent tick observing the gap saw the freshly-persisted
   `last_fire_at = fire_instant` AND no in-memory entry, evaluated the
   mismatch check to false, and (if `next_fire_at` was still in the past
   from any other code path) re-fired immediately.

   Fix: insert into `last_fire_attempt` BEFORE calling `save_mission`.
   While save is in flight, the in-memory map has `fire_instant` and
   the persisted record still has the OLD `last_fire_at`, so a concurrent
   tick sees a mismatch and arms the cooldown. Once save lands the
   values match (success) or stay mismatched (failure) — both correct.

   Regression test: `fire_mission_arms_cooldown_before_save_mission`.
   Adds an optional save-mission gate to `TestStore` (via oneshot
   channel + Notify), spawns `fire_mission` in a task, waits for save
   to enter the gate, and asserts `last_fire_attempt[mid]` is already
   populated while save is parked.

2. **Corrupted cron expression bypassed the cooldown.** When
   `next_cron_fire(expression)` returned `Err` (corrupt persisted
   expression), the previous code stamped `last_fire_at = fire_instant`
   anyway. Save then succeeded with `last_fire_at` matching the
   in-memory value, so tick's mismatch detector saw "save succeeded"
   and the cooldown was never armed. With `next_fire_at` still in the
   past (preserved because the cron crate couldn't compute a new one),
   every tick re-fired the same mission until `max_threads_per_day`
   was exhausted — same root cause shape as #1944.

   Fix: track `cron_advanced: bool` in `fire_mission`. When the cron
   advance fails, deliberately leave `last_fire_at` at its OLD value.
   The in-memory `last_fire_attempt[mid]` is still set to `fire_instant`,
   so the in-memory vs persisted mismatch arms the cooldown via the
   exact same code path as a save failure — no new signal needed.
   After 90 s the cooldown elapses and tick can retry, bounded by
   `max_threads_per_day`, in case the corruption resolves.

   Regression test: `tick_does_not_re_fire_corrupted_cron_within_cooldown_window`.
   Creates a cron mission, corrupts the persisted expression, sets
   `next_fire_at` to the past, fires once, then asserts a subsequent
   tick returns no new spawn AND that the persisted `last_fire_at`
   was deliberately left unset (proving the mismatch-arming path).

3. **Documented the equality check's precision requirement.** The
   `mission.last_fire_at != Some(*in_mem_last)` comparison is
   load-bearing and assumes the `Store` round-trips `DateTime<Utc>`
   without precision loss. The bridge's in-memory cache and JSON
   persistence both preserve nanoseconds; a future Postgres-backed
   store using `TIMESTAMPTZ` would silently break the success-path
   detection (microsecond truncation). Added a long comment at the
   tick check pointing future store implementers at the requirement
   so the next backend addition can either preserve precision or
   relax the comparison to "within one microsecond" before landing.

Verified: cargo fmt clean, cargo clippy --all-targets --all-features
zero warnings, full `cargo test --lib` green, ironclaw_engine 90/90
mission tests pass (88 → 90).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: add extensible deployment profiles (IRONCLAW_PROFILE)

Add a profile system that lets users select a deployment shape with a
single env var. Profiles are partial Settings TOML files merged onto
defaults before config.toml and DB overlays.

Built-in profiles: local, local-sandbox, server, server-multitenant.
Users can create custom profiles in ~/.ironclaw/profiles/.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review comments — path traversal, case normalization, override order, formatting

- Sanitize IRONCLAW_PROFILE to reject path traversal attempts (/, \, ..)
- Normalize profile name to lowercase for both user-path and built-in lookups
- Fix load_bootstrap_settings to use Settings::default() matching from_env() pattern
- Collapse .unwrap_or_default() onto one line to satisfy rustfmt
- Improve user_profile_overrides_builtin test to exercise actual merge logic
- Add path_traversal_rejected test with 5 malicious name patterns

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…2025)

* feat(tools): add production-grade coding tools, file history, and coding skills

Add dedicated coding tools inspired by Claude Code's architecture to make
IronClaw a more effective coding assistant:

New tools:
- GlobTool: fast file pattern matching via `glob` crate, sorted by mtime,
  with default exclusions (.git, node_modules, target, etc.)
- GrepTool: content search wrapping ripgrep with 3 output modes
  (content, files_with_matches, count), pagination, and context lines
- FileUndoTool: restore files to pre-modification state using in-memory
  file history snapshots

Enhanced tools:
- ReadFileTool: 10MB limit, 2000-line default, binary detection, device
  path blocking (/dev/zero, /proc/*/fd/*)
- ApplyPatchTool: uniqueness validation (error on ambiguous matches),
  workspace path rejection, 10MB size limit, file history integration
- WriteFileTool: file history integration for undo support

Updated tool descriptions to guide LLM behavior (prefer apply_patch over
write_file, always read before editing, use glob/grep instead of shell).

New skills:
- coding: best practices for code editing, search, and file operations
- commit: git commit message generation workflow
- review: code review workflow with structured checklist

Shared infrastructure:
- DEFAULT_EXCLUDED_DIRS constant in path_utils.rs
- FileHistory module with SharedFileHistory for cross-tool snapshots

66 new tests covering all tools, edge cases, and regression scenarios.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: apply cargo fmt formatting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): address PR review — security, correctness, and robustness fixes

- Move device path blocking after validate_path() to prevent traversal bypass
- Add /proc/kcore, /proc/kmem to blocked paths
- Reject absolute patterns and '..' in glob tool, add strip_prefix defense
- Wrap glob sync I/O in spawn_blocking to avoid blocking tokio executor
- Sort files_with_matches globally before pagination in grep tool
- Add default exclusions for node_modules/target in grep tool
- Inject ctx.extra_env into rg environment matching ShellTool policy
- Use per-line strip_prefix for content mode path relativization
- Change FileSnapshot.content_before to Vec<u8> for binary file support
- Log snapshot errors with tracing::debug instead of silently discarding
- Fix skill name mismatch: code-review → review to match directory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(skills): rename review skill directory to code-review

Aligns the directory name with the manifest name (code-review) to prevent
incorrect override/dedup behavior in the bundled-skill loader. The name
stays "code-review" since other domains may also need review-type skills.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(tools): add file edit guards — staleness detection, fuzzy matching, encoding preservation

Add file_edit_guard module with production-grade safeguards for file editing:
- ReadFileState tracks file reads with mtime for staleness detection
- 4-level fuzzy matching fallback (exact → whitespace-normalized → quote-normalized → both)
- UTF-16LE BOM detection and line ending style preservation (LF/CRLF/CR)
- Read-before-edit enforcement for ApplyPatch and WriteFile tools
- No-op edit rejection (old_string == new_string)
- Shared state injection via Arc<RwLock<>> across ReadFile, WriteFile, ApplyPatch

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): address all PR review comments — session scoping, parallelism, security

- Session-scoped state: ReadFileState and FileHistory now keyed by job_id
  so concurrent sessions sharing the same registry don't leak state (#2025)
- Parallel metadata: grep files_with_matches uses JoinSet (max 64 concurrency)
  instead of sequential await per file for mtime sorting
- Shared env allowlist: grep_tool imports SAFE_ENV_VARS from shell.rs
  (made pub(crate)) instead of maintaining a divergent copy
- Glob traversal: uses Component::ParentDir check instead of substring ".."
  match, so patterns like "foo..bar" are no longer falsely rejected
- UTF-16LE in read_file: binary detection skips null-byte check for files
  with UTF-16LE BOM; read_file uses encoding-aware read path
- Partial flag: default 2000-line truncation now marks read as partial,
  preventing edits against unseen content
- write_file guard softened: staleness check logs warning instead of
  hard error (full-file replacement has lower risk than apply_patch)
- Updated e2e trace to include read_file before apply_patch
- Updated expected tool list in schema validation tests

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): use async metadata instead of blocking path.exists() in write_file

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): fix false-positive panic detection for lifetimes in char lexer

The check_no_panics.py lexer misinterpreted Rust lifetimes ('static) as
char literal starts, causing in_char state to persist across lines and
hide all subsequent brace-delimited blocks — including #[cfg(test)] mod
tests. Reset in_char at line boundaries since Rust char literals cannot
span lines.

https://claude.ai/code/session_012bJjER6L5zSAFd9BqYMUmC

* test: verify MCP push works

* test

* chore: remove test file

* style: apply cargo fmt to file.rs

Collapse multi-line method chain to single line per rustfmt.

https://claude.ai/code/session_012bJjER6L5zSAFd9BqYMUmC

* style: apply cargo fmt to file.rs

Collapse multi-line method chain to single line per rustfmt.

https://claude.ai/code/session_012bJjER6L5zSAFd9BqYMUmC

* fix(file-tools): harden fuzzy patch matching and undo

* fix(ci): formatting + wasmtime 43 cache config compatibility

After merging latest staging, cargo fmt had diffs in file tools and the
wasmtime cache TOML format changed (v43 dropped the `enabled` field
under `[cache]`). Also removes accidental .fmt-test artifact.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(file-tools): simplify strip_trailing_whitespace

Remove redundant double-pass through .lines() — the first
collect+join was a no-op since .lines() already handles line endings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): address PR review comments — security, correctness, tests

- Add is_sensitive_path checks to GlobTool and GrepTool, matching the
  defense-in-depth posture of ReadFileTool/WriteFileTool/ListDirTool
- Fix UTF-8 panicking byte-index slice in apply_patch error preview
  (old_string[..200] → chars().take(200))
- Add 10MB size guard on file_history snapshots to prevent memory
  exhaustion from snapshotting large files
- Replace dead turn_number field with auto-incrementing sequence_number
  in FileHistory — callers no longer pass a hardcoded 0
- Fix glob mtime test flakiness by increasing sleep to 1100ms (above
  1s filesystem granularity)
- Fix emoji test to actually include emoji/non-ASCII content

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Zaki Manian <zaki@iqlusion.io>
* fix(docker): copy profiles/ directory into build stages

The deployment profiles feature (152e8b0) added include_str!() references
to profiles/*.toml but never updated the Dockerfile, breaking Docker builds.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(docker): remove profiles/ from planner stage

cargo chef prepare only scans manifests, not compile-time macros.
Having profiles/ in the planner stage needlessly invalidates the
dependency cache when a profile TOML changes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(ci): resolve 4 staging test failures (#2224, #2178)

1. **wasmtime cache config**: Remove `enabled = true` from generated
   TOML — wasmtime 43.x dropped this field, causing both
   `test_enable_compilation_cache_*` tests to panic with
   "unknown field `enabled`".

2. **bare approval keyword routing**: The `pending_approval.is_none()`
   guard at thread resolution rejected `ApprovalResponse` submissions
   (bare "yes"/"no") before reaching the `should_route_as_approval`
   downgrade added in e0e0fcd. Narrow the early rejection to
   `ExecApproval` only so bare keywords fall through to `UserInput`
   processing when no approval is pending.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(test): tighten WASM cache TOML assertion to check key-value format

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(agent): gate approval auth check on pending_approval.is_some()

Bare keywords falling through as UserInput should not be blocked by the
cross-channel authorization guard. Threads with source_channel=None
(e.g. hydrated legacy threads) fail closed in is_approval_authorized,
which would reject bare "yes"/"no" even though they are about to be
downgraded to regular user input.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(agent): update stale security comment and add guard regression test

Address review feedback from @serrrfirat:
1. Update the comment at the approval guard to reflect that only
   ExecApproval is early-rejected (ApprovalResponse falls through).
2. Add unit test verifying the guard narrowing: ApprovalResponse +
   no pending approval passes, ExecApproval is rejected, and
   ExecApproval with pending approval also passes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ce race (#2209)

* fix(workspace): collapse reindex delete+insert into one transaction to close TOCTOU race

Two concurrent reindexers for the same document could both DELETE the
existing chunks, then both try to INSERT chunk_index 0, hitting the
UNIQUE (document_id, chunk_index) constraint and failing with
"database is locked" or constraint violation. The delete and inserts
were separate libsql transactions with async points between them.

Add `WorkspaceStore::replace_chunks(document_id, &[ChunkWrite])` that
runs DELETE + N INSERTs inside a single BEGIN IMMEDIATE transaction
(not the default DEFERRED — DEFERRED bypasses busy_timeout on the
first write contention). The libsql impl, postgres impl, and the
in-memory storage variant all go through the new method, and
`Workspace::reindex_document` builds the `ChunkWrite` Vec (with
embeddings) up front so nothing async happens between the delete and
the insert loop.

Regression test in `workspace::versioning_tests` spawns 4 concurrent
writers against the same document on a multi-thread runtime and
asserts last-writer-wins without UNIQUE collisions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(auth): resolve display name + extension target from action when surfacing auth gates

The engine's `ResumeKind::Authentication` only carries `credential_name`
(e.g. `google_oauth_token`), which was being used as both the
user-facing display string AND as the first argument to
`submit_auth_token`. Two failure modes:

1. Display: users saw "google_oauth_token" in the auth-required prompt
   instead of the friendly extension name "google-drive-tool".
2. Routing: `submit_auth_token` expects an *extension* name and walks
   the extension's capabilities file to find the actual secret. Passing
   `google_oauth_token` directly fails closed with "Extension not
   installed: google_oauth_token", trapping the user in a re-auth loop
   on every paste.

`bridge/router.rs` now resolves the actual extension via
`tools.provider_extension_for_tool(action_name)` for both the gate
display path and the `submit_auth_token` call. Built-in tools, HTTP,
and skill credentials still fall back to the credential name (the
existing behaviour for those callers).

`extensions/manager.rs` fixes three more auth-readiness traps surfaced
by the v2 Drive trace:

- All capabilities lookups (`auth_wasm_tool`, channel activate, setup
  schema, configure, explicit secret query, upgrader) now go through
  `load_tool_capabilities` / `load_channel_capabilities` so a tool
  installed under the legacy hyphen filename
  (`google-drive-tool.capabilities.json`) is still resolved when
  looked up by canonical underscore name. The pre-v0.23 layout
  silently reported `no_auth_required` and bypassed the gate
  entirely.

- `activate_wasm_tool` now uses `existing_extension_file_path` for
  both the `.wasm` and `.capabilities.json` lookups so the legacy
  hyphen filename is resolved here as well. Without this, the
  upstream `determine_installed_kind` happily reported the extension
  as installed via its own alias check, but `activate_wasm_tool`
  then failed with `NotInstalled` — the readiness probe fell back to
  "treat as ready" and the agent ended up calling a tool that
  couldn't activate, hit a 401/403, and looped trying to recover.

- `configure()` post-activation OAuth cleanup now skips deletion when
  the caller is *also* providing a fresh credential in the same
  `secrets` map. The previous behaviour wrote the user's pasted token
  then immediately deleted it (along with `_scopes` /
  `_refresh_token` siblings), causing the resume to hit the wrapper
  with `token_exists=false` and re-fire the gate forever. Explicit
  Reconfigure (empty secrets map) still wipes the records to kick
  off a fresh OAuth dance.

`config/mod.rs` test config now seeds a deterministic 32-byte master
key so replay-mode tests that touch credentials get a working secrets
store out of the box without each test having to build its own.

Three regression tests in `extensions::manager::tests`:
- `test_activate_wasm_tool_finds_legacy_hyphen_alias`
- `test_auth_wasm_tool_finds_legacy_hyphen_alias`
- `test_configure_preserves_oauth_token_when_caller_provides_it`
- `test_configure_clears_oauth_token_for_reconfigure_flow`

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(mcp): canonicalize MCP tool identifiers to snake_case at registration

MCP tool names commonly contain dashes (e.g. Notion's `notion-search`),
and so do user-supplied server names (`my-server`). The runtime
converges on snake_case identifiers per `ToolRegistry::resolve_name`,
and LLMs (Codex / GPT-5 in particular) silently normalize tool names
to valid Python identifiers by converting dashes to underscores. The
old code built the registry key as `format!("{server}_{tool}")` and
preserved any dashes from the original tool name, so the registry got
`notion_notion-search` while the LLM emitted `notion_notion_search` —
direct lookup missed and the legacy alias fallback (which only goes
underscores → dashes) couldn't reconstruct the mixed-separator form
either, leaving every Notion MCP tool unreachable.

Add `mcp_tool_id(server, tool)` in `tools/mcp/client.rs` that does
`format!(...).replace('-', "_")`, re-export it from `tools/mcp/mod.rs`,
and use it in:

- `McpClient::create_tools` — the prefixed_name on every wrapped MCP
  tool now agrees with what the LLM will emit
- `ExtensionManager::activate_mcp` — `tool_names` is now sourced from
  `tool_impls.iter().map(|t| t.name())` instead of being independently
  rebuilt from the raw McpTool list, eliminating drift between
  registered names and reported names
- `ExtensionManager::latent_actions_for_mcp_server` — latent provider
  actions surfaced before activation use the same canonical form

The original (possibly hyphenated) `t.name` is still preserved on the
wrapper's inner `McpTool` and used verbatim when forwarding the
`tools/call` request to the MCP server, so MCP protocol compatibility
is unchanged — the canonicalization is internal-only.

5 regression tests in `tools::mcp::client::tests`:
- `test_mcp_tool_id_canonicalizes_dashed_tool_name`
- `test_mcp_tool_id_canonicalizes_dashed_server_name`
- `test_mcp_tool_id_passthrough_for_already_canonical_names`
- `test_create_tools_canonicalizes_dashed_mcp_tool_names` (drives
  `create_tools` end-to-end via MockTransport)
- `test_create_tools_round_trips_through_registry_resolve_name`
  (caller-level test per `.claude/rules/testing.md` — registers the
  wrapped tools in a real `ToolRegistry` and asserts that
  `resolve_name("notion_notion_search")` returns the registered tool
  via the direct HashMap path, not via the legacy alias fallback)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(llm): flatten top-level schema unions for OpenAI + symmetric Python/Rust action_calls round-trip

Two related bugs in the LLM ↔ engine boundary, both surfaced by the
GitHub Copilot MCP and the v2 orchestrator:

1. **Top-level schema flatten.** OpenAI's tool API rejects schemas
   whose top level isn't `type: "object"` or that contain top-level
   `oneOf`/`anyOf`/`allOf`/`enum`/`not`, with HTTP 400:

       Invalid schema for function '<name>': schema must have type
       'object' and not have 'oneOf'/'anyOf'/'allOf'/'enum'/'not' at
       the top level.

   The GitHub Copilot MCP's `github` tool uses top-level `oneOf` for
   action dispatch, so the agent was 400-ing the moment it tried to
   enumerate tools. `normalize_schema_strict` (already shared between
   the OpenAI Codex provider and `RigAdapter::convert_tools`) now
   short-circuits when it sees a forbidden top-level construct,
   replacing `parameters` with a permissive object envelope
   (`{type: "object", properties: {}, additionalProperties: true,
   required: []}`) and appending the original schema to the tool
   description as advisory text (truncated on a char boundary at 1500
   bytes). The MCP server still validates the actual shape on its
   end, so the tool keeps working — we just lose API-level schema
   enforcement and the LLM has to read variant structure from the
   description.

   The function signature changes to take `&mut String` for the
   description (to append the hint). Both call sites
   (`openai_codex_provider::convert_tool_definition` and
   `rig_adapter::convert_tools`) pass an owned clone through.

   This is slightly lossy for Anthropic users on tools with top-level
   unions (Claude could have handled the union natively), but
   keeping a single normalizer for all rig-based providers is simpler
   than threading per-provider flags through the adapter, and the
   description hint preserves the variant info Claude needs.

2. **Python ↔ Rust `action_calls` field-name mismatch.** The Python
   orchestrator (`default.py`) appends assistant messages with
   `action_calls=calls` where each call is shaped
   `{name, call_id, params}` (the friendly Python names produced by
   `orchestrator.rs:handle_llm_complete`). The reverse parser
   `json_to_thread_messages` tried to deserialize via
   `serde_json::from_value::<Vec<ActionCall>>`, which expects the
   canonical Rust field names `{action_name, id, parameters}`. The
   deserialize fails, but `.ok()` swallows the error and the
   assistant message comes back with `action_calls = None`. Every
   subsequent tool result then looks orphaned to
   `sanitize_tool_messages` and gets rewritten as a user message,
   losing the model's ability to reason about prior tool calls.

   Introduce a private `PythonActionCall` interchange struct as the
   single source of truth for the field naming convention, with
   bidirectional `From` conversions. Both call sites
   (Rust → Python serialization + Python → Rust deserialization)
   now go through `action_calls_to_python_json` /
   `python_json_to_action_calls`, so any future field addition only
   needs to touch one struct definition.

   `ActionCall` itself is unchanged — adding `#[serde(rename = ...)]`
   would have invalidated every persisted Step record and ThreadEvent.

11 regression tests:
- `rig_adapter::tests::test_normalize_schema_strict_*` (6 tests)
  covering pass-through, top-level oneOf flatten, anyOf/allOf/enum/not
  flatten, non-object replacement, nested-oneOf preservation, and
  char-boundary truncation
- `rig_adapter::tests::test_convert_tools_handles_top_level_oneof_dispatcher`
  (caller-level test driving `convert_tools` end to end)
- `openai_codex_provider::tests::test_convert_tool_definition_handles_top_level_oneof_dispatcher`
  (caller-level test driving the codex provider path)
- `executor::orchestrator::tests::python_action_call_round_trips_through_serde`
- `executor::orchestrator::tests::action_calls_to_python_json_uses_python_field_names`
- `executor::orchestrator::tests::python_json_to_action_calls_parses_python_field_names`
- `executor::orchestrator::tests::python_json_to_action_calls_rejects_canonical_field_names`
  (guards against silent shape drift)
- `executor::orchestrator::tests::json_to_thread_messages_preserves_action_calls_from_python_orchestrator`
  (caller-level test feeding the literal JSON shape `default.py`
  produces, asserting the assistant message's `action_calls` survive
  the round-trip with matching call_ids)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(live): add Drive auth-gate round-trip live test + supporting harness pieces

End-to-end smoke test for the post-flight auth gate path that the
recent auth-postflight commits stitched together. Two phases:

  Phase A: delete the developer's real `google_oauth_token` (and the
  refresh token) from the test rig's libsql DB while keeping the
  `_scopes` companion record so `auth_wasm_tool`'s scope expansion
  check doesn't fire on the re-store. Send a Drive search prompt and
  assert the agent emits `StatusUpdate::AuthRequired` within one
  iteration. The expected path:
    1. agent calls `google-drive-tool { action: "list_files" }`
    2. wrapper's `resolve_host_credentials` reports
       `missing_required = ["google_oauth_token"]`
    3. wrapper fails closed with the
       "requires credentials that are not configured" message
    4. effect adapter's post-flight branch fires
       `auth::postflight::detect_post_call_auth_failure`
    5. matcher hits the `requires credentials + not configured` pair
       (commit cd8b68de)
    6. detector calls `ensure_extension_ready(.., ExplicitAuth)` →
       `EnsureReadyOutcome::NeedsAuth`
    7. `EngineError::GatePaused { resume_kind: Authentication }`
       bubbles to the orchestrator
    8. router stores it in `pending_gates` and emits
       `StatusUpdate::AuthRequired`

  Phase B: re-insert the captured token via `secrets_store()`, send
  the synthetic value as a follow-up message. The v2 router treats the
  next user message after an auth gate as
  `GateResolution::CredentialProvided`, which calls `submit_auth_token`
  (idempotent overwrite of what we just inserted) then
  `execute_pending_gate_action` → `execute_resolved_pending_action`,
  and the original Drive call replays. The test asserts the resume
  ran (additional tool activity + a follow-up response).

Live-tier only (`#[ignore]`); skipped outside `IRONCLAW_LIVE_TEST=1`.
The test deliberately does NOT commit a recorded trace fixture: any
trace would inevitably capture the bearer token, real Drive file
metadata, and HTTP headers — all PII that's hard to scrub safely.
Hermetic regression coverage for the underlying alias-aware
capabilities bug lives in
`test_auth_wasm_tool_finds_legacy_hyphen_alias`.

Supporting harness changes:

- `LiveTestHarnessBuilder::with_no_trace_recording()` — opt-out flag
  for tests that touch real credentials. Live mode still runs against
  the real LLM but no fixture is committed; replay mode builds a stub
  harness so the test can detect the mode and skip gracefully without
  panicking on a missing fixture.

- `LiveTestHarness::finish_turns(&[(user_input, responses)])` —
  multi-turn variant of `finish` for tests that span an auth-gate
  round-trip (prompt → AuthRequired → token → resume). The session
  log renders all turns in order so a reader can follow the full
  conversation, not just the first prompt. Status events are still
  rendered once at the top because the rig doesn't tag them with a
  turn boundary.

- Session log formatter now renders `StatusUpdate::AuthRequired` and
  `StatusUpdate::AuthCompleted` so the gate is visible in the log.

- `TestRig::secrets_store()` and `TestRig::owner_id()` accessors so
  live tests can manipulate credentials directly under the same scope
  the agent loop uses.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(llm): log on python_json_to_action_calls deserialize failure

Address PR #2209 review: the helper used `serde_json::from_value(...).ok()?`
which is the exact `.ok()` swallow pattern the parent commit set out to
fix. If a future Python orchestrator patch ever drifts the action_calls
shape (extra required field, rename, partial migration), the helper would
silently return None and every subsequent tool result would look orphaned
to `sanitize_tool_messages` again — with no operator-visible signal at all.

Replace with an explicit match that emits a `warn!` (with the parse error
and the offending JSON value) on the failure path so the breadcrumb is
visible the moment any drift happens. The `None` return is preserved so
existing callers and the
`python_json_to_action_calls_rejects_canonical_field_names` test still
hold.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(test): generate random master key per Config::for_testing call

Address PR #2209 review: the previous fix hardcoded
`0123456789abcdef...0123456789abcdef` as the AES-256-GCM master key
inside `pub fn for_testing`. The function is `pub` (gated only behind
`#[cfg(feature = "libsql")]`, not `#[cfg(test)]`, because integration
tests in `tests/*.rs` are separate crates compiled against the lib's
non-test surface), which meant every developer building with libsql
had a publicly-known master key sitting in their process — and the
constant was now baked into Git history forever.

Replace with `generate_test_master_key()`, a private helper that pulls
32 bytes from `rand::thread_rng()` and hex-encodes them. Each call
returns a fresh key. Tests don't need cross-process determinism: each
test creates its own temp DB and the secrets store is born fresh on
every call anyway. `rand 0.8` is already a direct workspace dependency
so no Cargo changes are needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(mcp): normalize all non-identifier characters in mcp_tool_id

Address PR #2209 review: `mcp_tool_id` only handled `-` → `_`, but the
MCP spec doesn't actually constrain tool names to OpenAI's
`^[a-zA-Z0-9_-]{1,64}$` regex — a server could legally return
`notion.search`, `notion:create_issue`, `files/read`, or names with
spaces or non-ASCII characters. The same LLM normalization that bites on
`-` will bite on `.` and `:` too, and `extract_server_name` only strips
`.` from the host portion of a URL, leaving the tool portion of the
prefixed name unprotected.

Replace the single `.replace('-', "_")` with a `chars().map()` pass that
sends every non-`[A-Za-z0-9_]` character to `_`. This handles dashes,
dots, colons, slashes, spaces, and unicode in one shot — and since the
chars iterator yields one Rust char per code point, multi-byte
characters become a single `_` rather than splitting weirdly.

New regression test `test_mcp_tool_id_normalizes_non_identifier_chars`
covers dot, colon, slash, space, and multi-byte unicode inputs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(llm): make flatten_top_level hint keyword-aware

Address PR #2209 review: the description hint appended by
`flatten_top_level` was a one-size-fits-all "pick one variant and pass
its fields as a flat object". That's correct for top-level `oneOf` and
`anyOf`, but actively misleading for the other forbidden constructs:

- `allOf` — the LLM should pass fields from ALL variants combined,
  not pick one
- `enum` — the LLM should pass one of the listed literal values,
  not "fields"
- `not` — the LLM should pass any object that does NOT match the
  constraint

Extract `FORBIDDEN_TOP_LEVEL` to a module-level constant (now shared
between `needs_top_level_flatten` and a new `detect_forbidden_top_level`
helper) and add `schema_flatten_hint_intro(detected)` which branches on
the actual keyword that triggered the flatten and returns a precise
intro string. Falls back to a "free-form object" message when the
schema wasn't an object at all (no recognized forbidden keyword, just
the wrong top-level type).

New regression test `test_normalize_schema_strict_hint_is_keyword_aware`
asserts that each of the 5 keywords produces a hint containing the
expected discriminating phrase.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(workspace): serialize postgres replace_chunks via FOR UPDATE on parent doc

Address PR #2209 review (Copilot, src/workspace/repository.rs:350):
the libsql `replace_chunks` is fine because `BEGIN IMMEDIATE` acquires
the writer lock at transaction start, but the postgres path used the
default-isolation `BEGIN` which is not equivalent. Two concurrent
reindexers running under separate snapshots can both DELETE (each sees
its own pre-delete state, neither sees the other's), then race to
INSERT chunk_index 0 and hit the `UNIQUE (document_id, chunk_index)`
constraint.

Add `SELECT 1 FROM memory_documents WHERE id = $1 FOR UPDATE` at the
top of the transaction. The `FOR UPDATE` row lock is per-document,
ties to the existing parent row (FK already in place from
`memory_chunks.document_id`), and is released automatically on
commit/rollback. Concurrent reindexers for the same doc now serialize
on the parent row and last-writer-wins cleanly.

Picked `FOR UPDATE` over `pg_advisory_xact_lock` because it's the
row-locking primitive PG operators expect when reading the code, and
it doesn't introduce a hash function dependency for the lock key.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(llm): merge top-level union variants into flatten_top_level envelope

Address PR #2209 review (gemini-code-assist, src/llm/rig_adapter.rs:296):
after flattening a top-level oneOf/anyOf/allOf, the LLM was left with
`properties: {}` and could only read variant fields from the description
hint. That works but it's lossy — the LLM can't do schema-based
reasoning about which fields exist, and the description hint is
truncated to 1500 bytes so deeply-nested schemas are unreadable.

Add `merge_top_level_variant_properties` which walks the union variants,
collects every property they declare, and returns a single map. The
flatten envelope now uses that map instead of empty `{}`, so the LLM
sees structured field hints. `additionalProperties: true` and
`required: []` are preserved, so strict-mode validation stays disabled
and the LLM is free to mix fields across variants — the upstream MCP
server enforces the actual constraints on its end.

First-write wins on conflicting types: if two variants declare the
same field with different schemas, the first variant's schema is kept.
The full original schema still goes into the description hint, so the
ambiguous case is recoverable from there.

Two new regression tests:
- `test_normalize_schema_strict_merges_variant_properties` exercises a
  GitHub-Copilot-shaped tool with two variants that share a
  discriminator and asserts every field from every variant survives.
- `test_normalize_schema_strict_merge_first_write_wins_on_conflict`
  pins the documented conflict-resolution behaviour.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(router): extract resolve_extension_for_action helper for 3 dup sites

Address PR #2209 review (henrypark133, src/bridge/router.rs:1606): the
`provider_extension_for_tool + unwrap_or_else(credential_name)` pattern
was implemented in three places — once via the
`resolve_auth_gate_display_name` helper at line ~64, and twice inlined
in `resolve_gate` (line ~1603) and `await_thread_outcome` (line ~2779).
The inline sites couldn't use the helper because they'd already
destructured `credential_name` from the `ResumeKind` match and needed
the result for `submit_auth_token`, not just display.

Extract the core into `async fn resolve_extension_for_action(tools,
action_name, credential_fallback) -> String`. Make
`resolve_auth_gate_display_name` a thin wrapper that handles the
non-Authentication ResumeKind variants. Both inline sites now call the
helper directly with the destructured `credential_name`. The two
inline-site comment blocks that explained the rationale are collapsed
into shorter "see helper for full rationale" pointers since the doc
on `resolve_extension_for_action` carries the full explanation now.

Three sites collapse to one implementation. The auth display + routing
logic now has a single source of truth.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(mcp): warn on post-normalization tool name collisions in create_tools

Address PR #2209 review (serrrfirat, src/tools/mcp/client.rs:682):
after the broader `mcp_tool_id` char normalization (commit 18d4ce48),
two MCP tools whose names differ only by `-` vs `_` (e.g. `search-all`
and `search_all`) collide on the same registry key. The second
`ToolRegistry::register` call silently shadows the first with no
signal at all — operators debugging an unreachable tool would have
zero breadcrumb to discover the collision.

Add collision detection in `McpClient::create_tools` itself, where we
still have both the original tool name and the normalized id. Build a
`HashMap<normalized_id, original_name>` while iterating, and emit a
`tracing::warn!` when two distinct originals collide on the same id.
The warn carries the normalized id, both colliding original names,
and the server name, so an operator can immediately see which upstream
tools to rename. Behaviour is unchanged — the second tool still wins
on register, matching what the LLM would emit anyway since it
normalizes both names to the same string.

The collision detection is scoped to a single MCP server's tool list
because cross-server collisions have different registry-key prefixes
(`server_a_foo` vs `server_b_foo`) and can't actually shadow each
other. This is the right level — `ToolRegistry::register` itself
doesn't have access to the pre-normalization name and couldn't emit
this signal even if we wanted it there.

New regression test
`test_create_tools_handles_post_normalization_collision` drives a
MockTransport that lists `search-all` and `search_all`, asserts both
wrappers are produced with the same `Tool::name()`, registers them in
a real `ToolRegistry`, and asserts last-write-wins on shadow.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(engine): drop entries from action_calls_to_python_json on failure instead of injecting null

Address PR #2209 review (serrrfirat,
crates/ironclaw_engine/src/executor/orchestrator.rs:1920): the previous
helper used `unwrap_or_else(|_| Value::Null)` which silently corrupts
the array when serialization fails. The Python orchestrator
(`default.py`) accesses `c.get("name")` / `c.get("call_id")` /
`c.get("params")` on each entry, so a `null` would crash with a Python
`AttributeError` and lose the entire LLM step — and the fallback
contradicts this PR's own stated goal of not silently swallowing
errors.

Replace with `filter_map` so a failed entry is dropped from the output
rather than corrupting it. The warn log on the failure path is
preserved (and now also includes `action_name` for easier
correlation). Python's tool-result loop iterates
`range(len(results))` against the same shortened call list so a
missing entry is benign.

Note: the failure path is essentially unreachable for the
`PythonActionCall` shape (`String + String + Value` all
infallible-to-serialize) but the contract should still be safe — the
helper will be touched again when the Python interchange shape
evolves and we don't want a future maintainer to discover this trap
the hard way.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(engine): summarize action_calls in warn log to avoid leaking PII

Address PR #2209 review (serrrfirat,
crates/ironclaw_engine/src/executor/orchestrator.rs:1999): the
`python_json_to_action_calls` warn log emitted `value = %value` which
dumps the full action_calls JSON array on parse failure. Tool params
can carry user PII (search queries, file names, email content,
conversation text), and the warn fires precisely when the Python ↔
Rust shape drifts — exactly the moment operators will be grepping
logs and shipping output to log aggregators (Datadog, CloudWatch,
Sentry).

Add `summarize_action_calls_for_log` which builds a structural-only
summary: array length and the keys of the first entry. The keys
themselves are static field names (`name`, `call_id`, `params`), not
user data. The shape summary is enough to debug a drift (operator can
see whether the shape is roughly right and which fields are missing)
without exposing any of the actual parameter contents.

Edge cases handled:
- empty array → "empty array"
- non-array value (Python passed wrong shape) → "non-array value of
  type <kind>" via a small `json_value_type_name` helper
- entries that aren't objects → "<not an object>" rather than
  attempting to walk them

Two regression tests:
- `summarize_action_calls_for_log_does_not_leak_user_pii` builds an
  intentionally PII-laden value with salary spreadsheet queries,
  credentials, and "private message about layoffs" content, asserts
  none of the user-content strings appear in the summary, AND that
  even the upstream tool name doesn't leak (operator-level intent
  signal).
- `summarize_action_calls_for_log_handles_edge_cases` pins the
  empty/string/object/null fallback paths.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* incorporate #2227: factory server_name normalization, registry bidirectional alias, WASM/channel loader normalization, legacy token fallback

Merge the non-overlapping changes from PR #2227
(fix-tool-name-hyphen-normalization) so this PR supersedes it. Our PR
already fixed the core issue (mcp_tool_id canonicalization + stricter
non-identifier-char normalization), but #2227 adds valuable
defense-in-depth and compatibility layers that we didn't cover:

- `tools/mcp/factory.rs` — normalize `server.name` at the factory
  boundary (before any branch including the OAuth early-return). This
  ensures the secret name, session key, and tool prefix all use the
  same underscore-only form. Our PR normalized only at the tool-id
  level which left the server_name itself hyphenated in the session
  manager and token secret store.

- `tools/mcp/client.rs` — normalize `new_with_name` so callers that
  pass a hyphenated server name get consistent behavior even when
  bypassing the factory.

- `tools/mcp/config.rs` + `tools/mcp/auth.rs` — legacy token secret
  name fallback for pre-normalization tokens. When checking if an MCP
  server is authenticated, if the canonical (underscore) secret name
  doesn't exist, try the legacy (hyphenated) form. This prevents
  forcing re-auth on existing users who stored tokens under the old
  hyphenated server name before upgrade.

- `tools/registry.rs` — bidirectional `resolve_key` helper that tries
  exact → hyphen→underscore → underscore→hyphen aliases in `get`,
  `has`, `unregister`, `resolve_name`, `get_resolved`,
  `provider_extension_for_tool`, and `tool_definitions_for_actions`.
  Defense-in-depth: even if a tool somehow ends up registered with a
  mixed-separator name (edge case, stale DB, manual insertion), the
  registry will still find it. 4 new regression tests cover both
  alias directions, get_resolved, and unregister via alias.

- `tools/wasm/loader.rs` + `channels/wasm/loader.rs` — `load_from_dir`
  and `discover_*` functions normalize hyphenated filenames (file stem
  → replace('-', "_")). Dev tool install name changed from
  `{name}-tool` to `{name}_tool`.

Conflict resolution: manager.rs (kept our version with comment),
client.rs create_tools (kept our `mcp_tool_id` which is strictly
better — handles ALL non-identifier chars, not just dashes), client.rs
tests (kept our comprehensive MockTransport-based test suite, dropped
#2227's simpler duplicate). All other hunks from #2227 applied cleanly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(manager): normalize server name prefix in starts_with tool-list filters

Address PR #2209 review (henrypark133, Critical C1): the 3
`starts_with(&format!("{}_", name))` filters in `activate_mcp`
(already-active fast path), `list()`, and `remove()` used the raw
(possibly hyphenated) server name, while `mcp_tool_id` normalizes the
tool registry keys to underscores-only. A hyphenated server name like
`my-server` produced a prefix `my-server_` that matched zero tools
(they're all `my_server_*`), returning empty tool lists and failing to
unregister on extension removal.

Fix: use `crate::tools::mcp::mcp_tool_id(name, "")` as the prefix.
This produces `my_server_` from `my-server`, matching the registered
keys exactly. All 3 sites now use the same normalization as tool
registration.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(llm): accept array type containing "object" in needs_top_level_flatten

Address PR #2209 review (henrypark133, Critical C2):
`needs_top_level_flatten` only matched `JsonValue::String("object")`
for the type check. A top-level `"type": ["object", "null"]` (valid
JSON Schema for a nullable object, produced by some upstream providers
and `make_nullable`) triggered `bad_type = true` and the schema was
flattened, silently discarding all its properties.

Extend the check to also accept `JsonValue::Array` when any element is
the string `"object"`. This prevents unnecessary flattening of schemas
that are semantically object-typed but use the array form for
nullability.

New regression test:
`test_normalize_schema_strict_does_not_flatten_nullable_object_type`

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(mcp): update seen_ids on collision so 3rd collision reports against 2nd

Address PR #2209 review (henrypark133, Nit N1): the collision detection
in create_tools skipped the `seen_ids.insert` on the collision branch,
so a 3rd colliding tool would report against the 1st original name
instead of the 2nd (the actual shadow). Added the insert inside the
warning branch so subsequent collisions report the correct chain.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(wasm): fix CI type mismatch + add string-matching fallback for wrapped traps

Address PR #2209 CI failure and Copilot review comments:

1. **CI fix (E0308):** `classify_trap_error` took `anyhow::Error` but
   wasmtime 43's `call_execute` returns `wasmtime::Error` (a distinct
   type in `wasmtime_internal_core`). Changed the signature to accept
   `wasmtime::Error` directly. This is also more correct — accepting
   the native error type preserves type information that a lossy
   `.into()` conversion would strip, making the structured `Trap`
   downcast more reliable.

2. **String-matching fallback (Copilot, wrapper.rs:1107):** The doc
   claimed "falls back to string matching" but the implementation only
   did the structured downcast. Added a string-matching fallback that
   checks the full Display chain for "all fuel consumed", "out of fuel",
   "OutOfFuel", and "unreachable" when the downcast fails. This covers
   the case where component-model glue or host wrappers bury the Trap
   inside layers that `downcast_ref` can't see through. New regression
   test `trap_classification_fuel_via_string_fallback` exercises this
   path using a plain `wasmtime::Error::msg` wrapper.

3. **Stale doc comment (Copilot, test_rig.rs:1282):** Updated
   `secrets_store()` doc to reflect that most test rigs now have a
   working secrets store because `Config::for_testing()` generates a
   random master key per call. `None` only occurs with a config
   override that explicitly disables secrets.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review: tighten unreachable trap match, cap schema serialization, document legacy auth

Address PR #2209 review (serrrfirat, 4 comments):

1. `wrapper.rs:1131` — tightened the string fallback from bare
   `contains("unreachable")` to `contains("unreachable code")` /
   `"UnreachableCodeReached"` / `"wasm trap: unreachable"`. The old
   match would false-positive on HTTP errors like "endpoint was
   unreachable" or "server unreachable: connection refused", replacing
   the real diagnostic chain with a generic message.

2. `rig_adapter.rs:326` — added `count_json_nodes` pre-check (cheap
   recursive walk, no alloc) before calling `serde_json::to_string`.
   A malicious MCP server with a many-MB schema would have triggered
   a proportional allocation even though we only keep 1500 bytes.
   Schemas over 5000 nodes skip serialization entirely and get a
   "(schema too large to inline)" placeholder instead.

3. `auth.rs:1202` — documented that the legacy token fallback
   intentionally uses bare `get_decrypted` (no refresh). The path is
   transitional: users re-auth once and get migrated to the canonical
   naming scheme. Wiring refresh through the legacy path adds
   complexity for a self-healing compat layer.

4. `rig_adapter.rs:171` — Anthropic lossiness was already documented
   in the `normalize_schema_strict` doc comment (lines 164-170).
   Reply-only; per-provider flag is a follow-up.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(llm): ensure array items is a JSON Schema object for OpenAI strict mode

OpenAI rejects array-typed properties whose `items` field is missing,
boolean (`true`), or any non-object value with:

  "array schema items is not an object"

Schema generators like schemars produce `{"type": "array"}` (no items)
or `{"type": "array", "items": true}` for `Vec<serde_json::Value>`,
which is valid JSON Schema but violates OpenAI's strict-mode rules.
The google_docs_tool's `requests: Vec<serde_json::Value>` field
triggered this on every tool enumeration.

In `normalize_schema_recursive`, detect array-typed properties and
ensure `items` is a JSON Schema object before recursing. Missing or
non-object `items` are replaced with `{}` (accept any item). Object
`items` are left untouched and recursed into as before.

Regression test covers all three cases: missing items, boolean items
(`true`), and well-formed items (must not be clobbered).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(llm): add post-normalization validation to catch schema rules the normalizer misses

The schema_validator module already knew the "array items must be an
object" rule (Rule 8, line 218), had a test for it
(test_array_missing_items_fails, line 315), and would have caught the
google_docs_tool 400 — but it was only wired into CI tests against
built-in tools, never applied to WASM/MCP tool schemas or to the
output of normalize_schema_strict.

The root cause pattern: we're playing whack-a-mole with OpenAI's
undocumented strict-mode rules, adding fixes one at a time when a new
tool exposes a schema shape the normalizer doesn't handle. Each time,
the fix is a runtime 400 in production that takes a PR cycle to fix.

The structural fix: run validate_strict_schema as a debug-level
post-check after normalization. If the normalizer missed something,
the diagnostic appears in local logs immediately (before the schema
even reaches the LLM provider), giving developers a local breadcrumb
instead of a runtime 400 from OpenAI. The schema still goes through
(the tool remains usable), and the LLM provider surfaces the 400 if
OpenAI actually rejects it — but now the cause is instantly visible
in `RUST_LOG=ironclaw::llm=debug` output.

This also means that any future normalizer rule we add gets automatic
regression coverage: if the normalizer introduces a bug that violates
a rule the validator knows about, the debug log fires on every tool
call in dev mode.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(llm): normalize merged properties on flatten path + silence null action_calls warn

Two runtime issues from the latest deploy:

1. The flatten path in `normalize_schema_strict` short-circuited with
   `return schema`, skipping BOTH the recursive normalizer AND the
   post-normalization validator. The merged properties copied from
   union variants were raw — a `Vec<serde_json::Value>` field like
   google_docs_tool's `requests` kept its bare `{"type": "array"}`
   without `items`, and OpenAI rejected it with "array schema items
   is not an object" on every tool-using call.

   Fix: the flatten path now normalizes each merged property
   individually via `normalize_schema_recursive(prop_schema)` before
   returning the envelope. The top-level envelope stays permissive
   (`additionalProperties: true`, `required: []`) so the LLM can
   mix fields across variants, but each property's internal schema
   gets the full treatment (array items, nested objects, etc.).

   The post-normalization validator also runs on both paths now
   (no early return before it).

   Regression test:
   `test_normalize_schema_strict_flatten_normalizes_merged_array_items`
   mimics the google_docs_tool shape (tagged enum with `oneOf`, one
   variant containing an items-less array) and asserts the merged
   `requests` property has `items` as an object after normalization.

2. `python_json_to_action_calls` warn log fired on every text-only
   assistant message with "invalid type: null, expected a sequence"
   because Python's `action_calls: null` (legitimate "no tool calls"
   signal) was passed to the parser. Added a `.filter(|v| !v.is_null())`
   before the parser call in `json_to_thread_messages` so null is
   treated the same as "key absent" — no parse attempt, no false
   alarm. The warn only fires for genuinely malformed data now.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(llm): replace node-counting DoS guard with size-capped serializer

Address PR #2209 Copilot review (rig_adapter.rs:347 x2, :406):

1. `count_json_nodes` doc claimed "returns early once it exceeds the
   caller's budget" but always fully traversed. The approach also
   missed the case a reviewer flagged: few-node schemas with multi-MB
   string values would pass the node check but still allocate
   proportionally during `serde_json::to_string`.

Replace both the node counter and the `to_string` + truncate pattern
with `serialize_json_capped(value, max_bytes)`: a `serde_json::to_writer`
call through a `CappedWriter` that silently discards bytes past the
budget. This bounds the actual heap allocation to `max_bytes` regardless
of schema shape — many-node deep recursion AND multi-MB string values
are both capped. The writer returns `Ok(data.len())` after the cap so
serde_json thinks all bytes were consumed and continues (minimal
remaining work since the output is being discarded). The output is
guaranteed valid UTF-8 because serde_json only emits ASCII structural
characters and JSON-escaped unicode.

The `count_json_nodes` function and `MAX_SCHEMA_NODES` constant are
removed — the capped serializer subsumes them entirely.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): serialize bootstrap context action_calls through PythonActionCall

The bootstrap context builder (`build_orchestrator_inputs`) serialized
`m.action_calls` directly via the canonical `ActionCall` serde format
(`{action_name, id, parameters}`), but the Python orchestrator passes
these back verbatim in `working_messages` on the next `__llm_complete__`
call, where `python_json_to_action_calls` expects the interchange format
(`{name, call_id, params}`). The mismatch surfaced as "missing field
\`name\`" on every thread resume after a gate pause (approval, auth),
orphaning all subsequent tool results.

This is the SECOND code path (after `handle_llm_complete`) that feeds
action_calls into the Python working transcript. Both must use the same
shape — `action_calls_to_python_json` is the single source of truth.

Triggered by: user approves `tool_upgrade` → thread resumes → bootstrap
rebuilds context from `internal_messages` (which stores canonical
`ActionCall`s from the DB) → Python reads `{action_name, id, parameters}`
→ echoes them back on next LLM call → `python_json_to_action_calls`
fails → assistant message loses tool_call linkage → all tool results
orphaned.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(engine): add bootstrap-path round-trip test to guard against future action_calls serialization drift

The gate-resume bug (08e47209) happened because `build_orchestrator_inputs`
serialized `action_calls` with canonical `ActionCall` field names instead
of the `PythonActionCall` interchange format. The existing round-trip
test only covered the `__llm_complete__` path (within a single Python
orchestrator run), not the bootstrap path (thread resume after gate
pause). Anyone adding a THIRD serialization path in the future would
have no test guardrail.

Two new tests:

1. `bootstrap_context_action_calls_round_trip_through_python_interchange`:
   Builds a `ThreadMessage` with `action_calls` in canonical format
   (the shape stored in the DB), serializes through the EXACT pattern
   `build_orchestrator_inputs` uses, parses back through
   `json_to_thread_messages`, and asserts the calls survive. This is
   the test that would have caught the gate-resume bug on the first
   attempt.

2. `canonical_action_call_field_names_do_not_round_trip`:
   Negative test that verifies canonical names (`{action_name, id,
   parameters}`) are REJECTED by the parser. Documents the current
   contract: if this test ever passes, the `PythonActionCall`
   interchange type can be removed because the formats unified. Serves
   as a tripwire for anyone who adds `#[serde(rename)]` to `ActionCall`
   or changes the parser to accept both formats.

Together these two tests cover every known serialization boundary into
the Python transcript and make the failure mode instantly visible in
`cargo test` rather than as a runtime warn log after a gate pause.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(llm): skip strict-mode post-validator on flatten path to eliminate false-positive noise

The post-normalization validator (added in ce96c2a5 as a safety net)
was firing on every flattened schema with "additionalProperties should
be false" — a false positive because the flatten envelope deliberately
uses `additionalProperties: true` so the LLM can mix variant fields.
With 12 flattened tools loaded, this produced 12 debug log lines PER
LLM CALL, drowning real signals.

The flatten path's output is intentionally non-strict — running a
strict-mode validator on it is semantically wrong. Move the validator
behind the non-flatten branch and add an early `return schema` for the
flatten path (after normalizing individual properties). The validator
still catches issues on normal strict-mode schemas (the non-flatten
path), which is where it has value.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: close 5 coverage gaps across schema normalization, action_calls, and MCP naming

Systematic test audit of all 27 PR commits identified gaps where
production bugs had no hermetic regression test or where important
code paths had only helper-level (not caller-level) coverage.

New tests:

1. **test_realistic_wasm_schema_survives_normalize_flatten_pipeline**
   (rig_adapter.rs) — End-to-end test using the google_docs_tool's
   actual schema shape: tagged enum with 4 variants, one containing
   `requests: Vec<Value>` (bare array, no items), one with a nested
   object (text_style). Drives through normalize_schema_strict AND
   convert_tools. Asserts oneOf flattened, all variant properties
   merged, array items is object, nested objects get strict-mode.
   This single test would have caught BOTH production bugs (flatten
   path short-circuit + array items unreachable on flatten path).

2. **test_normalize_schema_strict_fixes_deeply_nested_array_items**
   (rig_adapter.rs) — 3-level nesting: object → array → object →
   array → object → array. Verifies the recursive normalizer walks
   the full depth and fixes every array items at every level.

3. **json_to_thread_messages_handles_null_action_calls_gracefully**
   + **handles_absent_action_calls** + **handles_empty_action_calls_array**
   (orchestrator.rs) — Three edge cases for the Python ↔ Rust
   message round-trip: null (was a false alarm), absent (baseline),
   and empty array (valid, produces Some(vec![])). The null case
   would have caught the "invalid type: null, expected a sequence"
   false alarm before it hit production.

4. **latent_provider_actions_normalize_hyphenated_server_names**
   (manager.rs) — Registers an MCP server with hyphenated name
   (`my-mcp-server`) and two tools (one with dashes, one without).
   Asserts latent action_names use all-underscore form
   (`my_mcp_server_search_all`) and the old hyphenated form doesn't
   survive. Exercises the mcp_tool_id normalization at the
   ExtensionManager layer.

5. **test_serialize_json_capped_boundary_conditions** +
   **test_serialize_json_capped_large_string_values** (rig_adapter.rs)
   — Size-capped serializer edge cases: under cap (full output),
   exactly at cap, over cap (truncated), zero cap (empty), and the
   multi-MB-string-in-few-nodes case the old node counter missed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review: address all 6 review items — UTF-8 safety, FOR UPDATE row check, MCP config alias, legacy token request path, stale reindex guard

1. **serialize_json_capped UTF-8 safety** (Copilot, rig_adapter.rs:399):
   serde_json v1 emits raw UTF-8 for non-ASCII chars (e.g. CJK in
   property descriptions), so byte-capped truncation can cut
   mid-codepoint. `String::from_utf8` now falls back to
   `e.valid_up_to()` to trim to the last complete codepoint instead
   of dropping the entire hint on a UTF-8 error.

2. **FOR UPDATE row count check** (Copilot x2, repository.rs:377):
   `SELECT 1 ... FOR UPDATE` returns 0 rows if the document doesn't
   exist, silently acquiring no lock. Now checks the row count and
   returns a clear `ChunkingFailed` error when it's 0.

3. **MCP config lookup alias-aware** (serrrfirat, factory.rs:40):
   `get_mcp_server` now tries exact name → hyphen alias
   (underscores→hyphens) → underscore alias (hyphens→underscores).
   After factory normalizes `server.name` to underscores,
   `provider_extension_for_tool` returns `my_server`, but the
   persisted config is keyed as `my-server`. Without alias lookup,
   `activate_mcp("my_server")` failed with `ServerNotFound`.

4. **Legacy token request-time fallback** (serrrfirat, auth.rs:1208):
   `get_access_token` now falls back to the legacy (pre-normalization)
   secret name when the canonical name returns no token. Without this,
   `is_authenticated` reported true (it has its own fallback) but the
   actual MCP request sent no Authorization header — the server
   appeared ready but tool execution 401'd until re-auth.

5. **Stale reindex guard** (serrrfirat, workspace/mod.rs:2181):
   `reindex_document_with_metadata` now captures the content hash at
   read time and re-checks it after computing embeddings. If another
   writer updated the document content during the embedding window,
   the reindexer skips chunk replacement — the other writer's reindex
   call will produce correct chunks for the new content. This closes
   the content-vs-chunks skew where writer B wins the document UPDATE
   but writer A wins the later replace_chunks transaction.

6. **Log summary PII concern** (Copilot, orchestrator.rs:2020): false
   positive — the keys logged are JSON Schema property names from
   the PythonActionCall interchange dict, not user tool parameters.
   Reply-only.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review(workspace): narrow reindex concurrency check error handling

Address Copilot review (workspace/mod.rs:2208): the optimistic
concurrency check caught all `Err(_)` as "document deleted" which
would silently swallow real DB errors (transient connection issues),
leaving chunks stale with no signal. Now only catches
`DocumentNotFound` for the deleted case; other errors propagate.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): canonicalize paths in file_history to fix macOS symlink mismatch

The file_history snapshot/restore tests were failing on macOS because
`/var` is a symlink to `/private/var`. `snapshot()` stored the
original path (`/var/folders/.../code.rs`), but `execute()` called
`validate_path()` which canonicalizes to `/private/var/folders/...`.
The path comparison in `restore_latest` mismatched, returning "No
file history found" even though the snapshot existed.

Fix: canonicalize paths consistently at both the storage boundary
(`snapshot()`) and the lookup boundary (`latest_snapshot_for()`,
`snapshots_for()`, `restore_latest()`). A shared `canonical()` helper
handles the non-existent-file case (write_file's "new file" path)
by canonicalizing the parent directory and joining the filename —
the parent always exists even when the file itself doesn't yet.

This was a pre-existing staging failure from PR #2025 that affected
all macOS developers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): update e2e_live_personas to match refactored test harness APIs

The PR changed `live_harness.rs` and `test_rig.rs` APIs without updating
`e2e_live_personas.rs`, causing Clippy compilation failures across all
feature sets. Also replaces `.expect()` in `rig_adapter.rs` with
`from_utf8_unchecked` (sound per `valid_up_to` invariant) to fix the
no-panics CI check.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): deduplicate path canonicalization in file_history::snapshot

Replace inline canonicalization logic in `snapshot()` with a call to the
existing `Self::canonical()` helper to eliminate duplication.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Henry Park <henrypark133@gmail.com>
…2292)

step_matches() used case-sensitive String::contains() for hint matching.
Fixture hints use lowercase ("write", "save") but test messages use
title case ("Write...", "Save..."), causing hinted steps to get stuck
at the queue head while unhinted steps were consumed out of order.

This broke tool_error_recovery (1 vs 2 write_file) and
workspace_semantic_search (2 vs 3 memory_write).

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…9968

chore: promote staging to staging-promote/b4502cf9-24258118364 (2026-04-10 20:57 UTC)
…8364

chore: promote staging to staging-promote/2cc55460-24254981568 (2026-04-10 18:33 UTC)
…1568

chore: promote staging to staging-promote/f37a26f7-24252590260 (2026-04-10 17:16 UTC)
…0260

chore: promote staging to staging-promote/56eb0adf-24250005637 (2026-04-10 16:16 UTC)
…5637

chore: promote staging to staging-promote/a8e6533a-24247643750 (2026-04-10 15:15 UTC)
@henrypark133
henrypark133 merged commit f9639af into staging-promote/e5b82fc6-24240316990 Apr 10, 2026
13 checks passed
@github-actions github-actions Bot added scope: agent Agent core (agent loop, router, scheduler) scope: channel/cli TUI / CLI channel scope: channel/wasm WASM channel runtime scope: tool Tool infrastructure scope: tool/builtin Built-in tools scope: tool/wasm WASM tool sandbox labels Apr 10, 2026
@github-actions github-actions Bot added scope: tool/mcp MCP client scope: db Database trait / abstraction scope: db/postgres PostgreSQL backend scope: llm LLM integration scope: workspace Persistent memory / workspace scope: config Configuration scope: extensions Extension management scope: sandbox Docker sandbox scope: ci CI/CD workflows scope: dependencies Dependency updates labels Apr 10, 2026
@henrypark133
henrypark133 deleted the staging-promote/a8e6533a-24247643750 branch April 10, 2026 21:26
@github-actions github-actions Bot added size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules and removed size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules labels Apr 10, 2026
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…4247643750

chore: promote staging to staging-promote/1192f12e-24240316990 (2026-04-10 14:22 UTC)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: agent Agent core (agent loop, router, scheduler) scope: channel/cli TUI / CLI channel scope: channel/wasm WASM channel runtime scope: ci CI/CD workflows scope: config Configuration scope: db/postgres PostgreSQL backend scope: db Database trait / abstraction scope: dependencies Dependency updates scope: docs Documentation scope: extensions Extension management scope: llm LLM integration scope: sandbox Docker sandbox scope: tool/builtin Built-in tools scope: tool/mcp MCP client scope: tool/wasm WASM tool sandbox scope: tool Tool infrastructure scope: workspace Persistent memory / workspace size: XL 500+ changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants