Commit Graph
53 Commits
Author SHA1 Message Date
Zaki ManianandGitHub 269b3f462f test(html_to_markdown): refresh golden files after renderer bump (#1016) 2026-03-11 20:28:38 -07:00
Zaki ManianandGitHub c37b64124c fix(setup): preserve model selection on provider re-run (#679) (#987) 2026-03-12 01:14:25 +00:00
Zaki ManianandGitHub f31cd13135 fix(mcp): attach session manager for non-OAuth HTTP clients (#793) (#986)
* fix(mcp): attach session manager for non-OAuth HTTP clients (#793)

* style(mcp): format factory regression test (#986)
2026-03-12 01:12:01 +00:00
Zaki ManianandGitHub 195ff44b1a fix(security): migrate webhook auth to HMAC-SHA256 signature header (#970) 2026-03-12 01:10:26 +00:00
a9821ac20f fix(security): make unsafe env::set_var calls safe with explicit invariants (#968)
* fix(security): make unsafe env::set_var calls safe with explicit invariants

`std::env::set_var` is unsafe in Rust 1.82+ because concurrent calls
from multiple threads cause undefined behavior. This commit addresses
the two production-code call sites:

1. `bootstrap.rs:load_ironclaw_env()` -- called before the Tokio
   runtime starts (genuinely single-threaded). Added a `debug_assert!`
   that verifies no Tokio runtime is active, making the safety
   invariant machine-checkable rather than relying on a comment.

2. `llm/session.rs:api_key_login()` -- was calling `set_var` mid-
   execution inside the multi-threaded Tokio runtime (UB risk).
   Replaced with `set_runtime_env()`, a new thread-safe overlay
   backed by `OnceLock<Mutex<HashMap>>`. The overlay integrates with
   the existing `optional_env()` config resolution and a new
   `env_or_override()` reader function.

All call sites that read `NEARAI_API_KEY` via raw `std::env::var()`
(wizard.rs, main.rs, doctor.rs) are updated to use the thread-safe
`env_or_override()` helper instead, so the value set during
interactive login is visible without mutating the process environment.

Test code `set_var`/`remove_var` calls (bootstrap tests, config tests,
shell tests, oauth tests, wizard tests) are left as-is since they run
under `ENV_MUTEX` serialization and are not production paths.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix: address review feedback on thread-safe env overlay PR

- Replace debug_assert! with runtime check in bootstrap.rs so release
  builds skip unsafe set_var when a Tokio runtime is active
- Recover from mutex poison in set_runtime_env instead of silently
  dropping writes (poisoned HashMap is still usable)
- Skip empty override values in env_or_override and optional_env for
  consistency with real env var handling
- Fix doc comment on env_or_override (real env checked first, not
  runtime overrides)
- Update api_key_login doc to describe runtime overlay instead of
  env var mutation

[skip-regression-check]

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix: use LazyLock::lock() for INJECTED_VARS; use set_runtime_env() in bootstrap fallback

- helpers.rs: fix env_or_override() to call INJECTED_VARS.lock() instead
  of .get() — INJECTED_VARS was changed upstream from OnceLock<HashMap>
  to LazyLock<Mutex<HashMap>>; calling .get() caused a compile error
  (E0599: no method named 'get' for LazyLock)

- bootstrap.rs: when load_ironclaw_env() is called with an active Tokio
  runtime, use set_runtime_env("DATABASE_BACKEND", "libsql") instead of
  silently dropping the write. This ensures DATABASE_BACKEND is always
  set regardless of thread context (addresses ilblackdragon review item 1).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

---------

Co-authored-by: Gabe Hamilton <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
2026-03-12 00:59:58 +00:00
8bbb43da52 fix(security): require explicit SANDBOX_ALLOW_FULL_ACCESS to enable FullAccess policy (#967)
* fix(security): require explicit SANDBOX_ALLOW_FULL_ACCESS to enable FullAccess policy

FullAccess policy bypasses Docker entirely and runs commands via sh -c
directly on the host. Previously, setting SANDBOX_POLICY=full_access
alone was sufficient to enable this, which could be triggered
accidentally or via prompt injection if tool approval is bypassed.

This adds a double opt-in guard:

- New SANDBOX_ALLOW_FULL_ACCESS=true env var must ALSO be set for
  FullAccess to take effect. Without it, the policy is downgraded to
  WorkspaceWrite with a tracing::error! log.

- At execution time, every FullAccess command emits a tracing::warn!
  with the command and working directory for audit visibility.

- The FullAccess variant now documents its blast radius (host shell,
  unrestricted filesystem/network/environment).

- SandboxConfig and SandboxModeConfig gain an allow_full_access field,
  wired through from_env() and the builder.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(sandbox): address review feedback on FullAccess double opt-in

- Add doc comment on builder .policy() warning that FullAccess requires
  .allow_full_access(true) or execution will return SandboxError::Config
- Sanitize audit log: log only binary name instead of full command to
  prevent secret leakage; add [FullAccess] prefix for grep-ability
- Add test_builder_full_access_without_allow_returns_error test covering
  the builder path without explicit allow_full_access(true)
- Fix doc comment mismatch: config.rs and SandboxPolicy::FullAccess docs
  said "will downgrade to WorkspaceWrite" but runtime returns
  SandboxError::Config -- aligned docs with actual behavior

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix: merge duplicate mod tests; add allow_full_access to struct initializers

After upstream merge, src/config/sandbox.rs had two issues:
- Duplicate mod tests block (upstream's original tests at line 271 + our
  new FullAccess guard tests at line 478) caused E0428 compile error
- Upstream test struct literals for SandboxModeConfig were missing the
  new allow_full_access field (E0063)

Fixes: merge the two mod tests into one; add allow_full_access: false to
the sandbox_mode_config_custom_values and sandbox_mode_to_sandbox_config
test struct initializers.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

---------

Co-authored-by: Gabe Hamilton <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
2026-03-12 00:57:05 +00:00
f48fe95ac4 fix(security): add Content-Security-Policy header to web gateway (#966)
* fix(security): add Content-Security-Policy header to web gateway

The web gateway set X-Frame-Options and X-Content-Type-Options but had
no Content-Security-Policy header. Without CSP, there is no browser-
enforced mitigation against XSS attacks. This adds a tailored CSP that
matches the resources the frontend actually loads.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(security): address CSP review feedback

- Remove cdnjs.cloudflare.com from script-src (not used in codebase)
- Add explicit object-src 'none' per security best practice
- Add regression test asserting CSP header presence and directives

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

---------

Co-authored-by: Gabe Hamilton <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
2026-03-12 00:55:38 +00:00
88f4894a18 merge: resolve conflicts for PR #800 and #822 into staging (#881)
* fix(ci): secrets can't be used in step if conditions [skip-regression-check] (#787)

GitHub Actions step-level `if:` doesn't have access to `secrets` context.
Replace `if: secrets.X != ''` with `continue-on-error: true` and let
the Set token step handle the fallback.

Co-authored-by: Claude Sonnet 4.6 <[email protected]>

* fix(ci): clean up staging pipeline — remove hacks, skip redundant checks [skip-regression-check] (#794)

- Remove continue-on-error from staging-ci.yml app token steps (secrets are configured)
- Skip test.yml and code_style.yml on PRs targeting staging (staging-ci.yml
  already runs tests before promoting, promotion PR gets full CI on main)
- Allow ironclaw-ci[bot] in Claude Code review for bot-created promotion PRs

Co-authored-by: Claude Opus 4.6 <[email protected]>

* fix(ci): run fmt + clippy on staging PRs, skip Windows clippy [skip-regression-check] (#802)

- Remove branches:[main] filter from code_style.yml so it runs on all PRs
- Gate clippy-windows with `if: github.base_ref == 'main'` (skip on staging PRs)
- Update rollup job to allow skipped clippy-windows
- Simplify claude-review.yml to only trigger on labeled event (avoids duplicate runs)

Co-authored-by: Claude Opus 4.6 <[email protected]>

* feat: persist user_id in save_job and expose job_id on routine runs (#709)

* feat: persist worker events to DB and fix activity tab rendering

In-process Worker (used by Scheduler::dispatch_job) now persists events
via save_job_event at key execution points: plan creation, LLM
responses, tool_use, tool_result, and job completion/failure/stuck.
Event data shapes match the container worker format so the gateway
activity tab renders them correctly.

Frontend: tool_result errors now show a red X icon with danger styling
instead of a silent empty output. The result event falls back to the
error field when message is absent.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: wire RoutineEngine into gateway for direct manual trigger firing

Replace the message-channel hack in routines_trigger_handler with a
direct call to RoutineEngine::fire_manual(), ensuring FullJob routines
dispatch correctly when triggered from the web UI. Inject the engine
into GatewayState from Agent::run after construction.

Also persists user_id in save_job for both PG and libSQL backends,
removes the source='sandbox' filter so all jobs are visible, and
exposes job_id on RoutineRunInfo for the frontend job link.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: remove stale gateway_state argument from Agent::new test call sites

The gateway_state parameter was removed from Agent::new during rebase
(replaced by post-construction set_routine_engine_slot), but three test
call sites still passed the extra None argument.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review — restore sandbox source filter, remove blank lines

- Revert removal of `source = 'sandbox'` filter in all SandboxStore
  queries (8 sites across PG and libSQL). Sandbox-specific APIs should
  stay scoped to sandbox jobs; unified job listing for the Jobs tab
  should use a separate query path.
- Remove extra blank lines in agent_loop.rs and worker.rs that caused
  formatting CI failure.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review — regenerate Cargo.lock, add user_id regression test

- Regenerate Cargo.lock from main's lockfile to eliminate dependency
  version downgrades (anyhow, syn, etc.) that were churn from rebase.
- Add regression test verifying user_id round-trips through save_job
  and get_job in the libSQL backend.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: remove trailing blank line in libsql jobs.rs

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add Postgres-side regression test for user_id persistence in save_job

Mirrors the existing libSQL test (test_save_job_persists_user_id) for the
Postgres backend. Gated behind #[cfg(feature = "postgres")] + #[ignore]
since it requires a running PostgreSQL instance (integration tier).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>

* refactor: unify three agentic loops into single AgenticLoop engine (#654)

Replace three independent copy-pasted agentic loops (dispatcher, worker,
container runtime) with a single shared engine in `agentic_loop.rs` that
all consumers customize via the `LoopDelegate` trait.

Phase 1 — Shared engine (`src/agent/agentic_loop.rs`, 205 lines):
  - `run_agentic_loop()` owns the core LLM → tool exec → repeat cycle
  - `LoopDelegate` trait (Send + Sync, &dyn dispatch) with 6 hook points
  - Tool intent nudge logic consolidated (was duplicated in 3 files)
  - Iteration limit + force-text behavior preserved

Phase 2 — Three delegate implementations:
  - `ChatDelegate` (dispatcher.rs): 3-phase approval flow, hooks, cost
    guard, context compaction, skill attenuation, interruption
  - `JobDelegate` (worker/job.rs): planning pre-loop phase, parallel
    JoinSet exec, mark_completed/stuck/failed, SSE streaming, self-repair
  - `ContainerDelegate` (worker/container.rs): sequential tool exec,
    HTTP-proxied LLM, container-safe tools, credential injection

Phase 3 — File moves and cleanup:
  - Delete `src/agent/worker.rs` — job logic moved to `src/worker/job.rs`
  - Rename `src/worker/runtime.rs` → `src/worker/container.rs`
  - Re-export `Worker`/`WorkerDeps` from `crate::worker` in `agent/mod.rs`
  - Update `scheduler.rs` imports to new worker location

Shared helpers (`src/tools/execute.rs`):
  - `execute_tool_with_safety()` replaces 4 copies of validate → timeout
    → execute → serialize
  - `process_tool_result()` replaces 3 copies of sanitize → wrap →
    ChatMessage (also used by thread_ops.rs approval resume paths)

Net result: -2,408 lines, zero duplicated loop logic, single code path
for tool intent nudge and completion detection.

Closes #654

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review feedback from Copilot

1. scheduler.rs: Replace `unwrap_or` fallback with proper error
   propagation when parsing tool output JSON — surfaces bugs instead
   of silently changing the output type.

2. worker/job.rs: Drop MutexGuard before the cancellation `.await` in
   `check_signals()` to avoid holding a lock across an async I/O call
   (prevents `await_holding_lock` lint).

3. worker/job.rs: Restore consecutive rate-limit counter
   (MAX_CONSECUTIVE_RATE_LIMITS = 10) so sustained rate limiting marks
   the job stuck with "Persistent rate limiting" instead of silently
   burning through max_iterations.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: incorporate staging changes — token budget tracking + mark_failed

Merge staging's changes into the refactored JobDelegate:
- Add token budget tracking in call_llm (update_context/add_tokens)
- mark_stuck → mark_failed for iteration cap and rate-limit exhaustion
  (aligns with staging's #788 fix)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address zmanian's PR review — eliminate type erasure, clean up

Address all 6 review points from zmanian on PR #800:

1. Replace LoopOutcome::Custom(Box<dyn Any>) with typed
   LoopOutcome::NeedApproval(Box<PendingApproval>) — eliminates
   type erasure and downcast, resolves clippy large_enum_variant.

2. Remove dead max_tool_iterations field from ChatDelegate struct.

3. Add on_tool_intent_nudge() hook to LoopDelegate trait with
   implementations in Job and Container delegates for observability.

4. Fix SSE events in job worker to emit raw sanitized content
   instead of XML-wrapped <tool_output> tags.

5. Remove 4 duplicate completion tests from job.rs that were
   already covered by the shared util module.

6. Avoid logging full tool results — use result_size_bytes in
   debug logs (execute.rs, job.rs).

Also updates path references in CLAUDE.md, COVERAGE_PLAN.md,
and add-sse-event.md command.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(doctor): expand diagnostics from 7 to 16 health checks

* test: add unit tests for agentic_loop and execute shared modules

Add 16 tests covering the two new critical shared modules:

agentic_loop.rs (10 tests):
- Text response exits loop immediately
- Tool call → text response continuation
- LoopSignal::Stop exits before LLM call
- LoopSignal::InjectMessage adds user message to context
- Max iterations terminates with LoopOutcome::MaxIterations
- Tool intent nudge fires twice then caps
- before_llm_call early exit bypasses LLM
- truncate_for_preview: short string, long string, multibyte safety

execute.rs (6 tests):
- execute_tool_with_safety success path
- Missing tool returns ToolError::NotFound
- Tool execution failure propagates
- Per-tool timeout enforcement (50ms)
- process_tool_result XML wrapping on success
- process_tool_result error formatting

All 2,777 unit tests pass, 0 clippy warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address code review — 9 issues across agentic loop, job worker, container

CRITICAL fixes:
- Rate-limit exhaustion now returns Err(LlmError::RateLimited) instead of
  Ok(Text("")), stopping the loop immediately with no ghost iteration.
  Below-threshold retries still use Text("") with an explicit empty-string
  guard in handle_text_response to skip injection.
- check_signals drains the entire message channel before returning,
  prioritizing Stop over UserMessage. Previously returned early on first
  UserMessage, silently dropping any queued Stop or additional messages.
- check_signals now detects all non-progressing job states (Cancelled,
  Failed, Stuck, Completed, Submitted, Accepted) instead of only
  Cancelled and Failed.

HIGH fixes:
- Error path in process_tool_result_job applies truncate_for_preview to
  bound error strings in SSE/DB events (was unbounded).
- Document Send+Sync lifetime constraint on LoopDelegate trait.
- Test mock before_llm_call refactored from double-lock to single lock
  acquisition, eliminating deadlock risk on refactor.

MEDIUM fixes:
- CompletionReport includes actual iteration count via shared
  Arc<Mutex<u32>> tracker (was hardcoded 0).
- process_tool_result_job return type changed from Result<bool> to
  Result<()> — the bool was always false (dead API).
- Deduplicate truncate in container.rs; now uses truncate_for_preview
  from agentic_loop.

Verified: 0 clippy warnings, 2781 tests pass, cargo fmt clean.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Henry Park <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
Co-authored-by: Umesh Kumar Singh <[email protected]>
Co-authored-by: reidliu41 <[email protected]>
2026-03-10 11:19:23 -07:00
56b7218897 fix(setup): preserve model name when re-running onboarding with same provider (#600) (#694)
Each provider setup function unconditionally cleared selected_model,
so re-running the wizard with "Keep current provider? Yes" would lose
the model name, forcing the user to re-select it every time.

Now only clears selected_model when the backend actually changes
(old model may be invalid for the new provider). When keeping the
same provider, the model is preserved and Step 4 shows the
"Keep current model" prompt.

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-08 08:32:02 +00:00
200aed16cd feat: configurable LLM request timeout via LLM_REQUEST_TIMEOUT_SECS (#615) (#630)
Add LLM_REQUEST_TIMEOUT_SECS env var (default: 120) to configure the
HTTP request timeout for LLM API calls. Primarily useful for local
models (Ollama, vLLM, LM Studio) that need more time for prompt
evaluation on consumer hardware.

The timeout is applied to the NearAI provider's HTTP client. Other
providers (Anthropic, OpenAI) use rig-core's default client.

- Add request_timeout_secs field to LlmConfig
- Thread timeout through create_llm_provider -> NearAiChatProvider
- Add NearAiChatProvider::new_with_timeout constructor
- Add .env.example documentation
- 2 regression tests for default and custom timeout values

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-08 08:30:52 +00:00
4c0275bcdc fix(setup): initialize secrets crypto for env-var security option (#666) (#706)
The "Environment variable" option in the setup wizard's security step
generated a master key but never initialized `secrets_crypto`, causing
subsequent API key saves to fail silently. Fix by:

1. Creating SecretsCrypto from the generated key (matching keychain path)
2. Storing the key hex in settings for write_bootstrap_env to persist
3. Auto-writing SECRETS_MASTER_KEY to ~/.ironclaw/.env
4. Using inject_single_var for thread-safe env overlay
5. Fixing misleading message (shell profiles don't work, only .env)

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-08 08:30:02 +00:00
272d31797e chore: remove dead code (#648) (#703)
* chore: remove dead code (LlmEvaluator, chunk_by_paragraphs, bundled channel installer, Reasoning::safety)

Delete unused code flagged in #648:
- evaluation/success.rs: delete LlmEvaluator struct/impl, remove #[allow(dead_code)] from RuleBasedEvaluator methods
- workspace/chunker.rs: delete chunk_by_paragraphs() and its tests (zero production callers)
- extensions/manager.rs: delete install_bundled_channel_from_artifacts() (hot-activation never shipped)
- llm/reasoning.rs: remove unused safety field from Reasoning struct; cascade removal through ContextCompactor, HeartbeatRunner, LlmSoftwareBuilder, and all callers

Closes #648

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: move RuleBasedEvaluator into test module to fix dead_code warning

RuleBasedEvaluator has no production callers -- it was only used in
tests of itself. Moving it into #[cfg(test)] eliminates the clippy
dead_code error that broke CI.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-08 08:26:04 +00:00
45ec691f4c Improve test infrastructure: StubChannel, gateway helpers, security tests, search edge cases (#623)
* feat(testing): add StubChannel test double for Channel trait

Adds StubChannel to src/testing.rs alongside StubLlm. Supports message
injection via mpsc sender, response/status capture, and configurable
health check toggling. Includes handle methods for use after ownership
transfer to ChannelManager.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(testing): wire StubChannel into TestHarnessBuilder

Add with_stub_channel() builder method that creates a StubChannel
pre-registered in a ChannelManager. Tests can inject messages via
the sender and verify routing through the manager. The channel field
on TestHarness is Optional, defaulting to None for backward compat.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: gate external-service tests behind integration feature flag

Replace silent try_connect() skip pattern with explicit feature gating.
cargo test now runs only self-contained tests.
cargo test --features integration runs tests requiring PostgreSQL.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(channels): add ChannelManager unit tests using StubChannel

Cover add/start_all stream merging, respond routing, unknown channel
errors, health_check_all with mixed health, empty-channels error path,
and injection channel merging -- all via StubChannel test double.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* docs: document test tier separation (unit/integration/live)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* ci: add architecture boundary check script

Grep-based checks for three architecture boundaries:
- Direct database driver usage (tokio_postgres/libsql) outside src/db/
- .unwrap()/.expect() in production code (warning only)
- Direct std::env::var reads outside config layer (warning only)

The DB driver check is a hard violation; the other two are warnings
for gradual cleanup. Run with: bash scripts/check-boundaries.sh

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(search): add RRF edge case tests for empty inputs, limits, and config modes

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(security): add regression tests for skill installer ZIP and SSRF protections

Add 11 regression tests covering the security controls in skill_tools:

ZIP extraction safety:
- Valid SKILL.md extraction works correctly
- Non-SKILL.md entries are ignored (returns error)
- Path traversal entries (../../SKILL.md) do not match
- Nested path entries (subdir/SKILL.md) do not match
- Oversized entries (>1MB uncompressed) are rejected

SSRF prevention:
- Loopback addresses (127.0.0.1) are blocked
- Private ranges (10.x, 172.16.x, 192.168.x) are blocked
- Link-local addresses (169.254.x) are blocked
- Public IPs (8.8.8.8, 1.1.1.1) are allowed
- IPv4-mapped IPv6 unwrapping logic works correctly
- Metadata endpoints and .internal/.local hostnames are blocked
- Normal hostnames (github.com, clawhub.dev) are allowed

Also documents a known gap: url::Url::host_str() returns bracketed
IPv6 addresses that std::net::IpAddr cannot parse, so IPv4-mapped
IPv6 URLs currently bypass IP-based checks in validate_fetch_url.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(testing): extract TestGatewayBuilder to eliminate gateway test duplication

Both ws_gateway_integration.rs and openai_compat_integration.rs manually
constructed GatewayState with 19+ fields. Extracted to a shared builder in
src/channels/web/test_helpers.rs that provides sensible defaults and lets
tests override only what they need.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* docs: add implementation plans for testing batches 1 and 2

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(security): close IPv6 SSRF bypass in validate_fetch_url

validate_fetch_url used host_str() which returns bracketed IPv6
(e.g. "[::ffff:7f00:1]") that IpAddr::parse() cannot handle,
silently skipping IP-based SSRF checks for all IPv6 URLs.

Switch to url::Host enum matching to extract proper IpAddr values
without string parsing. IPv4-mapped IPv6 addresses like
::ffff:127.0.0.1 are now correctly unwrapped and blocked.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(skills): add activation criteria limits enforcement tests

Adds test_activation_criteria_enforce_limits to verify that
enforce_limits() correctly trims excess patterns (>5), keywords (>20),
and tags (>10), and filters out short keywords/tags (<3 chars).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(wasm): add security regression tests for WASM tool loader

Add 6 tests covering: tool name path separator rejection, empty name
rejection, nonexistent file handling, invalid WASM bytes rejection,
dotfile discovery behavior, and subdirectory non-recursion.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: address PR review feedback

- Remove plan files from repo (ilblackdragon review)
- Replace CLAUDE.md test tier rules with pointer to check-boundaries.sh
- Add Check 4 to check-boundaries.sh: enforces integration tests are
  gated behind the 'integration' feature flag

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* ci: add try_connect silent-skip pattern check to check-boundaries.sh

Check 5 catches try_connect() and similar silent-skip patterns in
integration tests. Tests should use feature gates to fail loudly
when prerequisites are missing, not silently return.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(security): harden skill fetch SSRF checks

* fix(scripts): use bash arrays in check-boundaries.sh tier violation check

Refactor Check 4 in check-boundaries.sh to use bash arrays and printf
instead of string concatenation with echo -e. This is more robust with
special characters in filenames and avoids portability concerns with
echo -e. [skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-07 08:30:47 +00:00
4ac78a5b1f fix: reliable network tests and improved tool error messages (#626)
* fix(wasm): use per-engine cache dirs on Windows to avoid file lock error (#448)

On Windows, multiple wasmtime Engine instances sharing the default
compilation cache directory hit OS error 33 (ERROR_LOCK_VIOLATION)
because Windows holds exclusive file locks on memory-mapped cache
files. This is especially triggered when the Telegram channel WASM
module is loaded at startup and then hot-activated via the Extensions
UI.

Fix by giving each engine its own cache subdirectory on Windows
(~/.cache/ironclaw/wasmtime-tools/ and wasmtime-channels/). On
Unix the shared default cache continues to work as before.

Also adds Windows CI jobs (cargo check + clippy across all feature
flag combinations) to catch Windows-specific issues going forward.

Closes #448

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: silence Windows clippy warnings for platform-gated code

Gate PathBuf import behind #[cfg(unix)] in container.rs (only used
in Unix socket path), suppress unused_mut on conflicts Vec in
channels.rs (mutations are platform-gated), and add cfg gates on
keychain constants and hex_to_bytes that are only used on macOS/Linux.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: escape directory path in TOML cache config to prevent injection

Use double-quoted TOML strings with backslash and double-quote
escaping for the cache directory path, preventing breakage or
injection when paths contain special characters (e.g. single
quotes on Unix, backslashes on Windows).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve cargo fmt formatting errors

Fix import ordering in container.rs and line wrapping in runtime.rs
to pass the CI formatting check.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(ci): restore Path import for all platforms, keep PathBuf unix-only

Path is used in non-cfg-gated functions (lines 148, 244) so it must
be available on all platforms. Only PathBuf is unix-specific.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: use RFC 5737 TEST-NET-1 IPs for reliable network failure tests

Replace localhost/loopback addresses with 192.0.2.1 (TEST-NET-1) in
network failure tests so they work consistently behind HTTP proxies.
Tighten the catalog.rs error assertion to avoid matching any string
containing "error".

Closes #444 (takeover from hobostay)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: include tool name in error messages sent to LLM

Format tool errors as "Tool '<name>' failed: <reason>" instead of the
bare "Error: <reason>" so the LLM can identify which tool failed and
reason about alternatives. Does not short-circuit the agent loop --
errors still flow back to the LLM for reasoning.

Closes #487 (takeover from lustsazeus-lab, PR #530)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve cargo fmt formatting in dispatcher

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-07 05:54:12 +00:00
13e000dc20 fix(wasm): use per-engine cache dirs on Windows to avoid file lock error (#624)
* fix(wasm): use per-engine cache dirs on Windows to avoid file lock error (#448)

On Windows, multiple wasmtime Engine instances sharing the default
compilation cache directory hit OS error 33 (ERROR_LOCK_VIOLATION)
because Windows holds exclusive file locks on memory-mapped cache
files. This is especially triggered when the Telegram channel WASM
module is loaded at startup and then hot-activated via the Extensions
UI.

Fix by giving each engine its own cache subdirectory on Windows
(~/.cache/ironclaw/wasmtime-tools/ and wasmtime-channels/). On
Unix the shared default cache continues to work as before.

Also adds Windows CI jobs (cargo check + clippy across all feature
flag combinations) to catch Windows-specific issues going forward.

Closes #448

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: silence Windows clippy warnings for platform-gated code

Gate PathBuf import behind #[cfg(unix)] in container.rs (only used
in Unix socket path), suppress unused_mut on conflicts Vec in
channels.rs (mutations are platform-gated), and add cfg gates on
keychain constants and hex_to_bytes that are only used on macOS/Linux.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: escape directory path in TOML cache config to prevent injection

Use double-quoted TOML strings with backslash and double-quote
escaping for the cache directory path, preventing breakage or
injection when paths contain special characters (e.g. single
quotes on Unix, backslashes on Windows).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve cargo fmt formatting errors

Fix import ordering in container.rs and line wrapping in runtime.rs
to pass the CI formatting check.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(ci): restore Path import for all platforms, keep PathBuf unix-only

Path is used in non-cfg-gated functions (lines 148, 244) so it must
be available on all platforms. Only PathBuf is unix-specific.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-06 23:31:58 +00:00
ce5961b1ec fix(libsql): support flexible embedding dimensions (#534)
* fix(libsql): support flexible embedding dimensions (#494)

The libSQL schema hardcoded F32_BLOB(1536) for the embedding column,
preventing use of models with other dimensions (e.g. 768-dim
nomic-embed-text). This adds incremental migration support to the
libSQL backend and a V9 migration that rebuilds the memory_chunks
table with a plain BLOB column accepting any dimension.

- Add incremental migration infrastructure (INCREMENTAL_MIGRATIONS
  array + run_incremental() runner tracked via _migrations table)
- V9 migration rebuilds memory_chunks with BLOB column, drops the
  vector index (which requires fixed-dimension F32_BLOB)
- Update base schema for fresh installs (BLOB, no vector index)
- Vector search gracefully falls back to FTS-only when the index
  is absent (matches PostgreSQL behavior after its V9 migration)
- Remove now-incorrect "dimension is not 1536" warnings

Existing embeddings are preserved during migration. Users only need
to re-embed if they change their embedding model/dimension.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: wrap incremental migrations in transaction for atomicity

Address PR review feedback: if the process crashes after executing
migration SQL but before recording it in _migrations, the migration
would be applied but not marked complete. Wrapping both operations
in a transaction ensures they succeed or fail together.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* chore: merge main and fix formatting drift

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-06 23:29:32 +00:00
Zaki ManianGitHubClaude Opus 4.6gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
ffb9978ec6 test(workspace): regression test for document_path in search results (#509)
* test(workspace): add regression test for document_path propagation through RRF

Verifies that search results carry the source document's file path
through the RRF fusion pipeline, not the document UUID. Covers the
bug fixed in PR #503 / issue #481.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Update src/workspace/search.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* chore: merge main and fix formatting

Co-Authored-By: Claude Opus 4.6 <[email protected]>
[skip-regression-check]

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2026-03-06 23:27:45 +00:00
06c84a5c77 test: add 26 tests for multi-thread safety, db CRUD, concurrency, errors (#442)
* fix: use std::sync::RwLock in MessageTool to avoid runtime panic

The `requires_approval` method is synchronous but was using
`tokio::sync::RwLock` with `.await` which requires blocking the
runtime. This caused a panic:
"Cannot block the current thread from within a runtime"

Changes:
- Replace `tokio::sync::RwLock` with `std::sync::RwLock` for
  `default_channel` and `default_target` fields
- Use `unwrap_or_else(|e| e.into_inner())` to gracefully handle
  poisoned locks (recovers instead of panicking)
- Update all usages from `.read().await` to `.read().unwrap_or_else()`

The locks are short-held (just cloning strings), making std::sync::RwLock
appropriate for sync methods called from async contexts.

Fixes: "Cannot block the current thread from within a runtime" panic
when the LLM tries to send a message via the message tool.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: comprehensive testing improvements and fix MessageTool blocking_read panic

Fix tokio::sync::RwLock::blocking_read() panic in MessageTool::requires_approval()
under multi-threaded tokio runtimes by switching to std::sync::RwLock with poison
recovery. Add 26 new tests across 4 tiers:

Tier 1 - Multi-thread runtime safety:
- Fix MessageTool to use std::sync::RwLock instead of tokio::sync::RwLock
- 4 multi-thread tests for MessageTool::requires_approval() scenarios
- 1 multi-thread test for HttpTool credential-dependent approval
- 1 structural test exercising all core tool sync trait methods under multi-thread runtime

Tier 2 - Database CRUD coverage:
- Settings lifecycle (CRUD, bulk ops)
- Tool failure tracking (record, broken list, repair)
- Routine lifecycle (create, get, list, update, delete, runs)
- LLM call recording
- Sandbox job lifecycle (create, get, update, list, mode)
- Job events (save, list, limit)
- Estimation snapshot round-trip

Tier 3 - Concurrency:
- ToolRegistry concurrent register + read under 4-worker runtime

Tier 4 - Error coverage:
- Display tests for all 8 error variants
- From conversion tests for top-level Error enum

Supersedes the fix in PR #411 with the same bug fix plus comprehensive test coverage.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: remove trailing whitespace in registry.rs

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Jerome Revillard <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
2026-03-06 04:39:04 +00:00
df49b17d0f fix: prevent concurrent memory hygiene passes and Windows file lock errors (#535)
* fix: prevent concurrent memory hygiene passes and Windows file lock errors (#495)

The heartbeat system spawns hygiene passes via tokio::spawn on every
tick, creating a TOCTOU race where multiple tasks read the state file
before any saves, causing all to execute concurrently. On Windows this
also triggers OS error 1224 (file locked by memory-mapped section)
when multiple tasks call std::fs::write on the same file.

Three fixes:
- AtomicBool guard (RUNNING + RunningGuard RAII) ensures only one
  hygiene pass runs at a time
- State file is saved before cleanup (not after) to claim the cadence
  window early and close the TOCTOU race
- Atomic file write (write to .tmp then rename) avoids Windows
  file-locking errors from concurrent writers

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: add Mutex to serialize tests touching global RUNNING AtomicBool

Address PR review feedback: the running_guard_prevents_reentry test
manipulates a global static AtomicBool, which could cause flaky
failures if future tests also touch it and run in parallel. A test-only
Mutex ensures serialization.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-06 02:23:11 +00:00
b4b19738a8 Trajectory benchmarks and e2e trace test rig (#553)
* refactor: extract shared assertion helpers to support/assertions.rs

Move 5 assertion helpers from e2e_spot_checks.rs to a shared module.
Add assert_all_tools_succeeded and assert_tool_succeeded for eliminating
false positives in E2E tests.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add tool output capture via tool_results() accessor

Extract (name, preview) from ToolResult status events in TestChannel
and TestRig, enabling content assertions on tool outputs.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: correct tool parameters in 3 broken trace fixtures

- tool_time.json: add missing "operation": "now" for time tool
- robust_correct_tool.json: same fix
- memory_full_cycle.json: change "path" to "target" for memory_write

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: add tool success and output assertions to eliminate false positives

Every E2E test that exercises tools now calls assert_all_tools_succeeded.
Added tool output content assertions where tool results are predictable
(time year, read_file content, memory_read content).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: capture per-tool timing from ToolStarted/ToolCompleted events

Record Instant on ToolStarted and compute elapsed duration on
ToolCompleted, wiring real timing data into collect_metrics() instead
of hardcoded zeros.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: add RAII CleanupGuard for temp file/dir cleanup in tests

Replace manual cleanup_test_dir() calls and inline remove_file() with
Drop-based CleanupGuard that ensures cleanup even if a test panics.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: add Drop impl and graceful shutdown for TestRig

Wrap agent_handle in Option so Drop can abort leaked tasks. Signal
the channel shutdown before aborting for future cooperative shutdown.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: replace agent startup sleep with oneshot ready signal

Use a oneshot channel fired in Channel::start() instead of a fixed
100ms sleep, eliminating the race condition on slow systems.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: replace fragile string-matching iteration limit with count-based detection

Use tool completion count vs max_tool_iterations instead of scanning
status messages for "iteration"/"limit" substrings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: use assert_all_tools_succeeded for memory_full_cycle test

Remove incorrect comment about memory_tree failing with empty path
(it actually succeeds). Omit empty path from fixture and use the
standard assert_all_tools_succeeded instead of per-tool assertions.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: promote benchmark metrics types to library code

Move TraceMetrics, ScenarioResult, RunResult, MetricDelta, and
compare_runs() from tests/support/metrics.rs to src/benchmark/metrics.rs.
Existing tests use re-export for backward compatibility.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add Scenario and Criterion types for agent benchmarking

Scenario defines a task with input, success criteria, and resource
limits. Criterion is an enum of programmatic checks (tool_used,
response_contains, etc.) evaluated without LLM judgment.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add initial benchmark scenario suite (12 scenarios across 5 categories)

Scenarios cover tool_selection, tool_chaining, error_recovery,
efficiency, and memory_operations. All loaded from JSON with
deserialization validation test.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add benchmark runner with BenchChannel and InstrumentedLlm

BenchChannel is a minimal Channel implementation for benchmarks.
InstrumentedLlm wraps any LlmProvider to capture per-call metrics.
Runner creates a fresh agent per scenario, evaluates success criteria,
and produces RunResult with timing, token, and cost metrics.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add baseline management, reports, and benchmark entry point

- baseline.rs: load/save/promote benchmark results
- report.rs: format comparison reports with regression detection
- benchmark_runner.rs: integration test with real LLM (feature-gated)
- Add benchmark feature flag to Cargo.toml

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: apply cargo fmt to benchmark module

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): add multi-turn scenario types with setup, judge, ResponseNotContains

Add BenchScenario, Turn, TurnAssertions, JudgeConfig, ScenarioSetup,
WorkspaceSetup, SeedDocument types for multi-turn benchmark scenarios.
Add ResponseNotContains criterion variant. Add TurnAssertions::to_criteria()
converter for backward compat with existing evaluation engine.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): add JSON scenario loader with recursive discovery and tag filter

Add load_bench_scenarios() for the new BenchScenario format with recursive
directory traversal and tag-based filtering. Create 4 initial trajectory
scenarios across tool-selection, multi-turn, and efficiency categories.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): multi-turn runner with workspace seeding and per-turn metrics

Add run_bench_scenario() that loops over BenchScenario turns, seeds workspace
documents, collects per-turn metrics (tokens, tool calls, wall time), and
evaluates per-turn assertions. Add TurnMetrics to metrics.rs and
clear_for_next_turn() to BenchChannel.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): add LLM-as-judge scoring with prompt formatting and score parsing

Create judge.rs with format_judge_prompt, parse_judge_score, and judge_turn.
Wire into run_bench_scenario for turns with judge config -- scores below
min_score fail the turn.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): add CLI subcommand (ironclaw benchmark)

Add BenchmarkCommand with --tags, --scenario, --no-judge, --timeout,
--update-baseline flags. Wire into Command enum and main.rs dispatch.
Feature-gated behind benchmark flag.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): per-scenario JSON output with full trajectory

Add save_scenario_results() that writes per-scenario JSON files alongside
the run summary. Each scenario gets its own file with turn_metrics trajectory.
Update CLI to use new output format.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): add ToolRegistry::retain_only and wire tool filtering in scenarios

Add a retain_only() method to ToolRegistry that filters tools down to a
given allowlist. Wire this into run_bench_scenario() so that when a
scenario specifies a tools list in its setup, only those tools are
available during the benchmark run. Includes two tests for the new
method: one verifying filtering works and one verifying empty input
is a no-op.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): wire identity overrides into workspace before agent start

Add seed_identity() helper that writes identity files (IDENTITY.md,
USER.md, etc.) into the workspace before the agent starts, so that
workspace.system_prompt() picks them up. Wire it into
run_bench_scenario() after workspace seeding. Include a test that
verifies identity files are written and readable.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): add --parallel and --max-cost CLI flags

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(benchmark): use feature-conditional snapshot names for CLI help tests

Prevents snapshot conflicts between default (no benchmark) and
all-features (with benchmark) builds by using separate snapshot names
per feature set.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): parallel execution with JoinSet and budget cap enforcement

Replace sequential loop in run_all_bench() with parallel execution using
JoinSet + semaphore when config.parallel > 1. Add budget cap enforcement
that skips remaining scenarios when max_total_cost_usd is exceeded.
Track skipped count in RunResult.skipped_scenarios and display it in
format_report().

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): add tool restriction and identity override test scenarios

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* chore: fix formatting for Phase 3

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): add SkillRegistry::retain_only and wire skill filtering in scenarios

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(benchmark): add --json flag for machine-readable output

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* ci: add GitHub Actions benchmark workflow (manual trigger)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(benchmark): remove in-tree benchmark harness, keep retain_only utilities

Move benchmark-specific code out of ironclaw in preparation for the
nearai/benchmarks trajectory adapter. This removes:

- src/benchmark/ (runner, scenarios, metrics, judge, report, etc.)
- src/cli/benchmark.rs and the Benchmark CLI subcommand
- benchmarks/ data directory (scenarios + trajectories)
- .github/workflows/benchmark.yml
- The "benchmark" Cargo feature flag

What remains:
- ToolRegistry::retain_only() and SkillRegistry::retain_only()
- Test support types (TraceMetrics, InstrumentedLlm) inlined into
  tests/support/ instead of re-exporting from the deleted module

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* docs: add README for LLM trace fixture format

Documents the trajectory JSON format, response types, request hints,
directory structure, and how to write new traces.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(test): unify trace format around turns, add multi-turn support

Introduce TraceTurn type that groups user_input with LLM response steps,
making traces self-contained conversation trajectories. Add run_trace()
to TestRig for automatic multi-turn replay. Backward-compatible: flat
"steps" JSON is deserialized as a single turn transparently.

Includes all trace fixtures (spot, coverage, advanced), plan docs, and
new e2e tests for steering, error recovery, long chains, memory, and
prompt injection resilience.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(test): fix CI failures after merging main

- Fix tool_json fixture: use "data" parameter (not "input") to match
  JsonTool schema
- Fix status_events test: remove assertion for "time" tool that isn't
  in the fixture (only "echo" calls are used)
- Allow dead_code in test support metrics/instrumented_llm modules
  (utilities for future benchmark tests)

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Working on recording traces and testing them

* feat(test): add declarative expects to trace fixtures, split infra tests

Add TraceExpects struct with 9 optional assertion fields (response_contains,
tools_used, all_tools_succeeded, etc.) that can be declared in fixture JSON
instead of hand-written Rust. Add verify_expects() and run_recorded_trace()
so recorded trace tests become one-liners.

Split trace infra tests (deserialization, backward compat) into
tests/trace_format.rs which doesn't require the libsql feature gate.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(test): add expects to all trace fixtures, simplify e2e tests

Add declarative expects blocks to all 19 trace fixture JSONs across
spot/, coverage/, advanced/, and root directories. Update all 8 e2e
test files to use verify_trace_expects() / run_and_verify_trace(),
replacing ~270 lines of hand-written assertions with fixture-driven
verification.

Tests that check things beyond expects (file content on disk, metrics,
event ordering) keep those extra assertions alongside the declarative
ones.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(test): adapt tests to AppBuilder refactor, fix formatting

Update test files to work with refactored TestRigBuilder that uses
AppBuilder::build_all() (removing with_tools/with_workspace methods).
Update telegram_check fixture to use tool_list instead of echo.
Fix cargo fmt issues in src/llm/mod.rs and src/llm/recording.rs.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(test): deduplicate support unit tests into single binary

Support modules (assertions, cleanup, test_channel, test_rig, trace_llm)
had #[cfg(test)] mod tests blocks that were compiled and run 12 times —
once per e2e test binary that declares `mod support;`. Extracted all 29
support unit tests into a dedicated `tests/support_unit_tests.rs` so they
run exactly once.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: fix trailing newlines in support files

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(test): unify trace types and fix recorded multi-turn replay

Import shared types (TraceStep, TraceResponse, TraceToolCall, RequestHint,
ExpectedToolResult, MemorySnapshotEntry, HttpExchange*) from
ironclaw::llm::recording instead of redefining them in trace_llm.rs.

Fix the flat-steps deserializer to split at UserInput boundaries into
multiple turns, instead of filtering them out and wrapping everything
into a single turn. This enables recorded multi-turn traces to be
replayed as proper multi-turn conversations via run_trace().

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(test): fix CI failures - unused imports and missing struct fields

- Add #[allow(unused_imports)] on pub use re-exports in trace_llm.rs
  (types are re-exported for downstream test files, not used locally)
- Add `..` to ToolCompleted pattern in test_channel.rs to match new
  `error` and `parameters` fields

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(test): fix CI failures after merging main

- Add missing `error` and `parameters` fields to ToolCompleted
  constructors in support_unit_tests.rs
- Add `..` to ToolCompleted pattern match in support_unit_tests.rs
- Add #[allow(dead_code)] to CleanupGuard, LlmTrace impl, and
  TraceLlm impl (only used behind #[cfg(feature = "libsql")])

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Adding coverage running script

* fix(test): address review feedback on E2E test infrastructure

- Increase wait_for_responses polling to exponential backoff (50ms-500ms)
  and raise default timeout from 15s to 30s to reduce CI flakiness (#1)
- Strengthen prompt_injection_resilience test with positive safety layer
  assertion via has_safety_warnings(), enable injection_check (#2)
- Add assert_tool_order() helper and tools_order field in TraceExpects
  for verifying tool execution ordering in multi-step traces (#3)
- Document TraceLlm sequential-call assumption for concurrency (#6)
- Clean up CleanupGuard with PathKind enum instead of shotgun
  remove_file + remove_dir_all on every path (#8)
- Fix coverage.sh: default to --lib only, fix multi-filter syntax,
  add COV_ALL_TARGETS option
- Add coverage/ to .gitignore
- Remove planning docs from PR

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review - use HashSet in retain_only, improve skill test

- Use HashSet for O(N+M) lookup in SkillRegistry::retain_only and
  ToolRegistry::retain_only instead of linear scan
- Strengthen test_retain_only_empty_is_noop in SkillRegistry to
  pre-populate with a skill before asserting the no-op behavior

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(test): revert incorrect safety layer assertion in injection test

The safety layer sanitizes tool output, not user input. The injection
test sends a malicious user message with no tools called, so the safety
layer never fires. Reverted to the original test which correctly
validates the LLM refuses via trace expects. Also fixed case-sensitive
request hint ("ignore" -> "Ignore") to suppress noisy warning.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: clean stale profdata before coverage run

Adds `cargo llvm-cov clean` before each run to prevent
"mismatched data" warnings from stale instrumentation profiles.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: fix formatting in retain_only test

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
2026-03-05 09:13:09 +00:00
f99991d27b fix(wasm): coerce string parameters to schema-declared types (#498)
* fix(wasm): coerce string parameters to schema-declared types

LLMs frequently pass numeric values as JSON strings ("5" instead of 5)
or booleans as strings ("true" instead of true). The WASM module's
serde deserializer rejects these type mismatches. This adds a
coerce_params_to_schema() helper that walks the params JSON object
and converts string values to their schema-declared types (number,
integer, boolean) before passing to the WASM module.

Adds 5 unit tests covering number, integer, boolean coercion,
already-correct types, and unparseable strings.

Closes #486

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: use in-place mutation and case-insensitive boolean coercion

Address review feedback:
- Use get_mut instead of clone+insert to avoid allocations
- Make boolean coercion case-insensitive (handles "True", "FALSE", etc.)
- Expand boolean test to cover false and mixed-case values

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: collapse nested if-let to satisfy clippy collapsible_if lint

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-04 18:19:01 +00:00
89600e2b5c fix(agent): strip leaked [Called tool ...] text from responses (#497)
* fix(agent): strip leaked [Called tool ...] text from agent responses

When the NEAR AI provider flattens tool_call messages to plain text,
markers like [Called tool ...] and [Tool ... returned: ...] can leak
into the user-visible response if the LLM echoes them back. This adds
a sanitization step in the agentic loop's text response path that
strips these internal markers before returning. If stripping leaves
the response empty, a generic fallback message is returned instead.

Closes #487

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: use fold instead of collect+join to avoid heap allocation

Address review feedback: replace Vec collect + join with fold to build
the filtered string directly, avoiding an intermediate heap allocation.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Pierre LE GUEN <[email protected]>
2026-03-04 18:16:26 +00:00
e4e78d8a87 fix(web): reset job list UI on restart failure (#499)
* fix(web): reset job list UI on restart failure

The restartJob() catch handler was missing a loadJobs() call, so the
job row stayed in a stale highlighted state after a failed restart
attempt. Add loadJobs() to match the success path behavior.

Closes #485

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: use .finally() for loadJobs() instead of duplicating

Move loadJobs() to a .finally() block so it runs on both success and
failure without duplication.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-04 10:05:42 -08:00
35a79caf87 fix(web): assign unique thread_id to manual routine triggers (#500)
* fix(web): assign unique thread_id to manual routine triggers

Manual routine triggers via the web API created an IncomingMessage
without a thread_id, causing session_manager.resolve_thread() to
route the output to whatever thread was last associated with the
(user, "gateway", None) key. This sets a unique thread_id of the
form "routine-{id}-{timestamp}" so each manual trigger gets its own
dedicated thread.

Closes #484

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: add ownership check to routine trigger handler (IDOR)

Address review feedback: verify routine.user_id matches the
authenticated user before allowing the trigger, preventing
unauthorized cross-user routine execution.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-04 00:41:41 +00:00
85999b25a8 fix(web): refresh routine UI after Run Now trigger (#501)
* fix(web): refresh routine UI after "Run Now" trigger

triggerRoutine() only showed a toast but did not refresh the routine
data after triggering. This adds openRoutineDetail() / loadRoutines()
calls after the toast, matching the pattern used by toggleRoutine().

Closes #483

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: only refresh detail view if triggered routine matches current view

Check currentRoutineId === id before refreshing the detail panel to
avoid refreshing the wrong routine's view.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-04 00:38:48 +00:00
b60e5e907a fix(skills): use slug for skill download URL from ClawHub (#502)
* fix(web): use slug for skill download URL from ClawHub

The skill install handler was using req.name (display name like
"Markdown Converter") instead of the slug (like "owner/markdown-converter")
when constructing the download URL. The registry endpoint expects a slug,
so display names caused 502 errors.

- Add optional `slug` field to SkillInstallRequest
- Prefer slug over name when building the download URL
- JS installSkill() now sends slug from search results

Closes #482

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: guard against empty slug string in skill download URL

Filter out empty slug strings so we fall back to name instead of
constructing an invalid download URL.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-03 23:53:16 +00:00
d562dc8d90 fix(workspace): thread document path through search results (#503)
* fix(workspace): thread document path through search results

Memory search results were showing chunk UUIDs instead of source file
paths. Thread document_path through RankedResult, SearchResult, and the
RRF fusion pipeline so handlers can display the actual file path.

Fixes #481

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: use into_iter to move values instead of cloning

Address review feedback: consume results with into_iter() to move
String fields directly instead of cloning them.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-03 23:31:30 +00:00
dc7d9cce34 fix(channels): add host-based credential injection to WASM channel wrapper (#421)
* fix(channels): add host-based credential injection to WASM channel wrapper

The channel WASM wrapper was missing the host-based credential injection
that the tools wrapper implements. The `credentials` block in channel
capabilities files was dead code: Slack's `on_respond` sends requests
with no Authorization header, expecting the host to inject the bot token
based on `host_patterns`, but the host never did.

This caused Slack (and any channel relying on capabilities-declared
credentials) to fail all outbound API calls with `not_authed`.

Changes:
- Add `ResolvedHostCredential` struct mirroring the tools wrapper
- Add `host_credentials` field to `ChannelStoreData`
- Add `inject_host_credentials()` method on `ChannelStoreData`
- Update `redact_credentials()` to also scrub host-injected secret values
- Add `secrets_store` field to `WasmChannel` + `with_secrets_store()` builder
- Add `resolve_channel_host_credentials()` async helper that decrypts
  capabilities-declared credentials before each WASM callback
- Update `create_store()` and all `call_on_*` / `execute_status` /
  `execute_poll` call sites to pre-resolve and pass host credentials
- Fix leak scan ordering: scan runs on WASM-provided values BEFORE host
  credential injection, preventing false-positive blocks on injected
  Bearer tokens (e.g. xoxb- Slack tokens)
- Make `credential_injector` module pub(crate) so channels can reuse
  `inject_credential` and `host_matches_pattern`

Fixes #389, root cause of #413

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(wasm): redact URL-encoded credentials, use url::Url, derive Clone

Address review feedback on PR #421:

1. Security: redact_credentials now scrubs URL-encoded forms of secrets
   in addition to raw values, preventing exfiltration via encoded
   representations in error strings from reqwest
2. Use url::Url::query_pairs_mut() for query parameter injection instead
   of manual string manipulation, improving robustness with malformed URLs
3. Derive Clone on ResolvedHostCredential and simplify the per-tick
   clone in the status repeater loop

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Sprite <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
2026-03-01 19:04:37 -08:00
a21dba0ac1 refactor: rename WasmBuildable::repo_url to source_dir (#445)
* refactor: rename WasmBuildable::repo_url to source_dir

The field receives a local directory path (e.g. "tools-src/gmail"), not a
URL. Rename to source_dir to accurately reflect its purpose.

Adds #[serde(alias = "repo_url")] for backwards compatibility with any
previously serialized data.

Closes #329

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: rename extract_url to extract_source

The function can return a local directory path, not just a URL.
Addresses review feedback on PR #445.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 16:36:46 -08:00
bb279ad822 fix: pre-validate Cloudflare tunnel token by spawning cloudflared (#446)
* fix: pre-validate Cloudflare tunnel token by spawning cloudflared

After format validation passes, spawn `cloudflared tunnel run` briefly
with a dummy URL and watch stderr for up to 10s. If an error appears
before a connection URL, report it and offer "Save anyway?". This
catches bad tokens during setup instead of at runtime 30s later.

Closes #440

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: tighten cloudflared output matching in live validation

- Check for cfargotunnel.com/trycloudflare.com in success detection
- Use starts_with("err") instead of contains("err") to avoid false
  positives on words like "stderr"

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 16:36:35 -08:00
293a700b69 fix: prevent Telegram 409 Conflict on webhook re-registration (#447)
* fix: prevent Telegram 409 Conflict on webhook re-registration

Delete any existing webhook before calling setWebhook in on_start(),
matching the defensive cleanup that polling mode already does. As a
safety net, register_webhook() now retries once on 409 after calling
delete_webhook().

Closes #440

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: deduplicate 409 retry logic in register_webhook

Restructure the match block so the initial request and retry share
a single response-handling code path.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 16:36:26 -08:00
7481aea083 fix: batch of quick fixes (#417, #338, #330, #358, #419, #344) (#428)
- #417: Add Docker auto-start login item hint for macOS in setup wizard
- #338: Add clippy.toml with complexity thresholds for AI-assisted dev
- #330: Add structured FallbackFailed error variant to ExtensionError
- #358: Revoke credential mappings on extension removal (SharedCredentialRegistry)
- #419: Detect conflicting cloudflared services during tunnel setup
- #344: Improve embedding auth failure warning with configuration hint

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 09:01:07 +00:00
Zaki ManianGitHubClaude Opus 4.6Illia Polosukhingemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
fa52df593d fix: persist channel activation state across restarts (#432)
* fix: persist channel activation state across restarts (#392)

Channels activated via the web UI were lost on restart because
active_channel_names was only in memory. Now persist activation state
to the settings store under "activated_channels" and auto-activate
persisted channels on startup.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: log warnings for channel activation load failures

Replace silent catch-all with explicit error logging when
database queries or deserialization fails for activated channels.

Addresses Gemini review feedback on PR #432.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Apply suggestions from code review

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2026-03-01 08:53:32 +00:00
3362081192 fix: add TLS support for PostgreSQL connections (#363) (#427)
All PostgreSQL connection sites hardcoded NoTls, preventing connections
to managed providers that require TLS (AWS RDS, Neon, Supabase, etc.).

- Add tokio-postgres-rustls with rustls + system root certificates
- Add SslMode enum (disable/prefer/require) via DATABASE_SSLMODE env var
- Replace NoTls at all 4 production call sites with TLS-aware pool creation
- Add SslMode::from_env() helper for lightweight CLI tools
- Log native cert loading errors and warn on empty root store

Default mode is Prefer (attempts TLS, matching most managed providers).

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 08:49:09 +00:00
1f2e8c3b72 fix: scan inbound messages for leaked secrets (#433)
* fix: scan inbound messages for leaked secrets before LLM processing (#393)

Add scan_inbound_for_secrets() to SafetyLayer that reuses the existing
leak detector on user input. Wire it into thread_ops.rs after the policy
check so messages containing API keys or tokens are rejected early,
preventing the LLM from echoing them back and triggering outbound
leak-detection error loops.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: unify inbound secret scan warning messages

Both the detected-secret and error branches now show the same
actionable message guiding users to remove secrets and use the
config system instead.

Addresses Gemini review feedback on PR #433.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 08:45:27 +00:00
dbf3406bf5 fix: use tailscale funnel --bg for proper tunnel setup (#430)
* fix: use tailscale funnel --bg for proper tunnel setup (#394)

The old command `tailscale funnel http://127.0.0.1:3000` would hang
without establishing a tunnel. The correct invocation is
`tailscale funnel --bg <port>` which configures the tunnel as a
background daemon and exits.

Changes:
- Use `--bg` flag with just the port number
- Run as a one-shot command instead of spawning a child process
- Use `tailscale <cmd> off` to tear down (matches --bg semantics)
- health_check uses stored URL instead of non-existent child PID

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: use local_host parameter and verify tailscale health

Pass full http://host:port URL to tailscale instead of ignoring
the local_host parameter. Health check now verifies tailscale is
actually running via 'tailscale status --json'.

Addresses Gemini review feedback on PR #430.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 08:44:08 +00:00
2052cddf1d fix: add missing build.sh for Discord and WhatsApp channels (#429)
* fix: add missing build.sh for Discord and WhatsApp channels (#406)

Both channels had full source code in channels-src/ but no build.sh,
so their WASM binaries were never compiled and they didn't appear in
the setup wizard's channel selection list.

Modeled after the existing channels-src/telegram/build.sh.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: guard wasm-tools availability in WASM build scripts

Add command existence check before invoking wasm-tools in discord
and whatsapp build scripts. Prints actionable error message if missing.

Addresses Gemini review feedback on PR #429.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 08:41:20 +00:00
ec31e83a7d fix: normalize secret names to lowercase for case-insensitive matching (#413) (#431)
The Slack channel capabilities.json declares secret names in lowercase
(slack_bot_token) but the web UI stored them in UPPERCASE
(SLACK_BOT_TOKEN), causing credential injection to fail with
"not_authed".

Changes:
- CreateSecretParams::new() normalizes name to lowercase on creation
- All SecretsStore lookups (get, exists, delete, is_accessible) now
  lowercase the name parameter before querying
- Applied to all three backends: PostgreSQL, libSQL, InMemory
- CredentialInjector::is_secret_allowed() uses case-insensitive
  comparison

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 08:33:58 +00:00
f62937d482 fix: persist model name to .env so dotted names survive restart (#426)
* fix: persist model name to .env so dotted names survive restart (#400)

The setup wizard saved selected_model to the DB but not to .env.
Since Config::from_env_with_toml() runs before the DB connects, the
model name was lost on restart -- backends fell back to hardcoded
defaults, truncating names like "llama3.2" to "llama3".

- Add LlmBackend::model_env_var() as single source of truth for the
  backend-to-env-var mapping
- Write the model env var in write_bootstrap_env() using the new method
- Add selected_model fallback to all 6 backends (was missing from
  OpenAI, Anthropic, Ollama, and Tinfoil)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: extract resolve_model() helper to reduce duplication

Address review feedback: the env → settings → default model resolution
pattern was repeated across all 6 backends.  Centralise it in a single
LlmConfig::resolve_model() helper.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 08:32:59 +00:00
914f3cd075 fix(setup): check cloudflared binary and validate tunnel token (#424)
* fix(setup): check cloudflared binary and validate tunnel token (#418)

The Cloudflare tunnel setup accepted tokens blindly without checking if
cloudflared was installed or if the token was valid. Now:

- Checks for cloudflared on PATH before accepting a token, with install
  instructions if missing (user can continue anyway)
- Validates token format (base64-decoded JSON with account/tunnel fields)
  with a warning if malformed (user can override)
- Replaces misleading "will start automatically at boot" with honest
  instructions for starting the tunnel and installing as a service
- Reuses binary_exists() from skills::gating (promoted to pub(crate))
  for cross-platform PATH lookup

Closes #418

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: reuse cloudflared_found instead of redundant binary_exists call

Address review feedback: the binary check result was already stored
in cloudflared_found from earlier in the function.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 08:31:43 +00:00
e794f39726 fix(setup): validate PostgreSQL version and pgvector availability before migrations (#423)
* fix(setup): validate PostgreSQL version and pgvector before migrations

The setup wizard accepted any DATABASE_URL without checking the server
version or pgvector availability. Users who installed PostgreSQL 14
(or any version < 15) got opaque migration failures. Users without
pgvector installed hit CREATE EXTENSION errors at runtime.

After a successful connection, the wizard now:
1. Queries SHOW server_version and rejects versions below 15
2. Checks pg_available_extensions for the vector extension

Both checks provide actionable error messages with platform-specific
install guidance.

Closes #415
Closes #416

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: extract version constant, fix hex escapes in pgvector message

- Extract MIN_PG_MAJOR_VERSION constant to avoid magic number
- Replace \x20 hex escapes with regular spaces in install guidance

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(setup): use detected PG version in pgvector install instructions

The pgvector install hints were hardcoded for PG 16. Since we already
parse major_version from SHOW server_version, use it dynamically so
users on PG 15 or 17 get correct package names.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 08:29:49 +00:00
c6bfd18401 fix: guard zsh compdef call to prevent error before compinit (#422)
* fix: guard zsh compdef call to prevent error before compinit

The generated ironclaw.zsh completions file calls compdef without
checking if it exists. Users who source this file before compinit
runs in their .zshrc get "compdef: command not found" on every
terminal open.

Wrap the call with the standard (( $+functions[compdef] )) guard.

Closes #420

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(completions): apply compdef guard during zsh generation

Instead of hand-patching the generated ironclaw.zsh file (which is
fragile and lost on regeneration), patch the compdef call in the
generation code itself. The Zsh output is post-processed to wrap
`compdef _ironclaw ironclaw` with a `$+functions[compdef]` guard.

Regenerated ironclaw.zsh from the patched code to stay in sync.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-01 08:28:32 +00:00
a24fd3e8a3 Add automated QA: schema validator, CI matrix, Docker build, and P1 test coverage (#353)
* Add automated QA: tool schema validator, feature-flag CI matrix, Docker build

P0 items from the automated QA plan (#352):

- Add validate_tool_schema() that checks OpenAI strict-mode rules
  (type: object, required keys in properties, nested object/array
  recursion) with 10 unit tests and 6 integration tests covering
  all core built-in tools

- CI test matrix now runs with --all-features, default features, and
  --no-default-features --features libsql to catch dead code behind
  wrong cfg gates

- CI clippy now runs the same 3-feature matrix with --all flags

- Docker build job added to catch missing files in Dockerfile

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Add P1 automated QA tests and fix LeakDetector prefix shadowing bug

P1 test coverage: config round-trip (settings + bootstrap), shell tool
arg handling, safety adversarial tests (sanitizer, leak detector,
allowlist), turn persistence (conversations, metadata, pagination, jobs),
and a clippy fix for libsql-only builds.

Fixed a real bug where AhoCorasick non-overlapping prefix iteration
caused shorter prefixes (e.g. "sk-") to shadow longer ones
(e.g. "sk-ant-api"), preventing Anthropic API key and SSH private key
detection.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Add P2 automated QA tests: chaos, lifecycle, collision, and recovery

Cover all P2 items from the automated QA plan:
- Circuit breaker chaos tests (hanging provider, rapid cycles, mixed errors)
- Failover chaos tests (hanging failover, all-fail, tools path, single provider)
- Value estimator boundary tests (negative cost, zero price, zero earnings)
- Context length recovery test (ContextLengthExceeded -> compact -> retry)
- WASM channel lifecycle tests (write/commit/read round-trip, namespace isolation)
- Extension registry collision tests (same-name different-kind coexistence)
- Extension filesystem collision tests (separate dirs, detect_kind priority)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Add P3 concurrent stress tests for ContextManager and SessionManager

Tests verify thread safety of double-checked locking, TOCTOU
prevention, and RwLock-based concurrent access patterns under load.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Add dispatcher loop guard and self-repair stuck job tests

Dispatcher: test force_text mechanism prevents infinite tool call loops,
verify iteration bound arithmetic guarantees termination for all configs.

Self-repair: test stuck job detection, recovery within attempt limits,
manual escalation when limit exceeded, graceful degradation without
store/builder dependencies.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Add E2E testing infrastructure design doc

Python + Playwright framework with mock LLM server for deterministic
browser-level testing of the web gateway. Covers connection/auth,
chat round-trip with SSE streaming, and skills lifecycle scenarios.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Add E2E testing infrastructure implementation plan

10-task plan covering: scaffolding, mock LLM server, helpers,
conftest fixtures, connection/chat/skills test scenarios,
CI workflow, README, and integration run.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* scaffold: E2E test project with pyproject.toml

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: E2E helpers with DOM selectors and port discovery

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: mock OpenAI-compat LLM server for E2E tests

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: E2E conftest with session fixtures for mock LLM and ironclaw

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: E2E scenario 1 -- connection and tab navigation tests

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: E2E scenario 2 -- chat message round-trip tests

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: E2E scenario 3 -- skills search, install, remove tests

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* ci: add weekly E2E test workflow with Playwright

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* docs: E2E test README with setup and usage instructions

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: E2E test integration fixes from first run

- Use temp file DB instead of :memory: (libSQL :memory: doesn't persist
  tables across execute_batch)
- Fix installed skills selector: #skills-list not #installed-skills
- Add pytest-timeout to dependencies
- Improve skills install/remove test with wait_for instead of fixed sleeps

8 passed, 1 skipped (skills install depends on ClawHub availability)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add OpenAI strict-mode schema validator for all built-in tools (QA 1.1)

Add src/tools/schema_validator.rs with validate_strict_schema() that checks
tool parameter schemas against OpenAI function calling strict-mode rules:
type object at top level, required keys in properties, enum type consistency,
array items definitions, nested object recursion, and additionalProperties.

17 tests validate all 34+ built-in tool schemas across 5 test groups:
- 9 simple tools (echo, time, json, http, shell, file read/write/list/patch)
- 4 job tools (create, list, status, cancel)
- 4 skill tools (list, search, install, remove)
- 13 inline schemas for extension, routine, and complex job tools
- 4 memory tool schemas

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add E2E scenarios for SSE reconnect, HTML injection, and tool approval (QA 3.3/5/6)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: E2E test reliability for HTML injection and SSE reconnect

- HTML injection: test sanitization directly via JS injection instead of
  depending on full LLM round-trip (avoids intermittent 404 from mock)
- SSE reconnect: increase wait times for DB persistence and relax
  assertion to check total message count after history reload

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: cargo fmt formatting

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add WASM and MCP tool schema validation tests (QA 1.1)

Extends the schema validator with representative WASM tool schemas
(weather, HTTP client, batch processor, status), MCP tool schemas
(default, file read, SQL query, strict mode), and defect detection
tests for common external schema issues (missing type, typo in
required, array without items, enum type mismatch).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add auth middleware and compaction module tests

Auth middleware (8 new tests): valid/invalid bearer tokens, query param
fallback, case sensitivity, empty tokens, whitespace handling.

Compaction module (16 new tests): truncation strategy, summarize strategy
with mock LLM, workspace fallback, format_turns helper, sequential
compactions, coherence after compaction, token decrease verification.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add config round-trip integration tests (QA 1.2)

Test the full bootstrap .env lifecycle: write via the same format
as save_bootstrap_env/upsert_bootstrap_var, read back via dotenvy,
and assert values match. Covers LLM backend selection, embedding
disable flag, onboard completion flag, session token keys, multi-key
preservation across upsert, and special characters (spaces, equals,
quotes, backslashes, hashes).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add value estimator boundary tests and dispatcher loop guard (QA 4.3/4.4)

Value estimator (14 new tests): zero/negative prices, large values,
negative cost, exact margin boundaries, custom margin configuration.

Dispatcher loop guard (2 new tests): verifies the dispatch loop terminates
when all tool calls fail (regression guard for PR #252 infinite loop)
and when max iterations are reached.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add failover edge cases and provider chaos tests (QA 2.6/4.1)

Failover edge cases (4 new tests): cooldown at zero nanos, half-open
failure reopens circuit, all providers fail gracefully (no panic),
single failing provider with cooldown.

Provider chaos tests (15 new tests): flakey provider with retries,
hanging provider with timeout, garbage provider, circuit breaker
trip/recover, failover chain cascading, non-transient error stops
chain, full stack integration (retry + failover + circuit breaker).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review feedback on QA tests

- Fix Bearer auth case-sensitivity per RFC 6750 (auth.rs)
- Refactor bootstrap.rs to expose path-parameterized variants so
  config_round_trip tests call real code instead of reimplementations
- Remove deprecated event_loop fixture, use dynamic ports, minimal env,
  session-scoped browser, and wire HEADED=1 in E2E conftest
- Add cross-referencing doc comments between schema validators
- Simplify array validation logic in tool.rs
- Bump e2e.yml checkout@v4 to @v6

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: cargo fmt and fix clippy warning in signal.rs

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: improve E2E fixture error reporting and prevent stdin blocking

- Add --no-onboard flag to prevent wizard from blocking in CI
- Pipe /dev/null to stdin to prevent any stdin reads from hanging
- Add RUST_BACKTRACE=1 for crash diagnostics
- On server startup timeout, dump stderr to pytest output so CI
  logs show why the server failed to start

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: set session-scoped event loop for E2E async fixtures

pytest-asyncio 1.3.0 defaults asyncio_default_fixture_loop_scope to
None (function scope), causing session-scoped async fixtures to be
re-evaluated per test function with independent event loops. Each test
then independently attempts to start the ironclaw server, times out
at 120s, and wastes ~24 minutes of CI before the job is cancelled.

Setting asyncio_default_fixture_loop_scope = "session" ensures all
session-scoped async fixtures share a single event loop, so the server
starts once and is reused across all tests.

Also adds -x flag to pytest in CI to stop on first failure instead of
running all 19 tests when the fixture is broken.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: set test loop scope to session to match fixture loop scope

With asyncio_default_fixture_loop_scope=session but
asyncio_default_test_loop_scope=function (the default), tests run on
a per-function event loop while fixtures produce objects (Playwright
pages, browser contexts) on the session event loop. This event loop
mismatch causes the test to hang indefinitely awaiting Playwright
operations that are bound to the wrong loop.

Setting both scopes to "session" ensures a single event loop is shared
across all fixtures and tests, eliminating the deadlock.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* ci: add roll-up jobs to match branch protection required checks

Branch protection expects "Code Style (fmt + clippy)" and "Run Tests"
status checks, but only individual job names were reported. Add
roll-up jobs that aggregate results and report the expected names.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-27 09:09:45 +04:00
4e2dd76ae5 Fix skills system: enable by default, fix registry and install (#300)
* feat: add Docker detection module with platform guidance

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add Docker sandbox step to setup wizard

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: show Docker status in boot screen

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: check Docker availability at startup

When SANDBOX_ENABLED=true, proactively detect whether Docker is
installed and running before creating the ContainerJobManager.
If Docker is unavailable, log a warning with platform-specific
guidance and disable the sandbox for the session.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: enable sandbox by default, improve wizard explanation, document detection limits

- SandboxConfig defaults to enabled=true (startup check disables
  gracefully if Docker is unavailable)
- Wizard step explains why Docker matters: isolation for LLM-generated
  code vs running directly on the host
- Document detection confidence per platform in detect.rs module docs:
  high on macOS/Linux, medium on Windows (named pipe edge cases)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: cargo fmt + update test_builder_defaults for enabled-by-default

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: deduplicate wizard Docker status handling per review

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: fix skills system - enable by default, fix registry connectivity and install

- Enable skills system by default (SKILLS_ENABLED no longer required)
- Bypass Vercel TLS fingerprint blocking by pointing DEFAULT_REGISTRY_URL
  directly at the Convex backend (wry-manatee-359.convex.site)
- Handle ZIP archives from ClawHub download API - the registry returns
  ZIP files containing SKILL.md, not raw text. Uses flate2 (existing dep)
  to extract SKILL.md from the archive.
- Surface catalog search errors in the UI with a yellow warning banner
  instead of silently returning empty results
- Handle both {"results":[...]} envelope and bare [...] array JSON formats
  from the search API
- Add ClawHub links and metadata to search result cards (clickable skill
  names linking to clawhub.ai, relevance score, "updated X ago" recency)
- Fix 3 pre-existing clippy warnings in tests/html_to_markdown.rs

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address security review feedback on ZIP extraction and SSRF

- Cap download size to 10 MB before reading response body
- Guard against ZIP bombs: cap uncompressed_size at 1 MB, wrap
  DeflateDecoder with .take() read limit
- Use checked_add for ZIP header offset arithmetic to prevent overflow
- Remove .unwrap() on try_into() -- use direct array construction
- Handle IPv4-mapped IPv6 addresses (::ffff:192.168.x.x) in SSRF checks
- Don't leak internal registry URLs in user-facing catalog_error messages
- Fix non-ASCII panic in catalog response debug logging (use .get() instead
  of byte slicing)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add /skills command and enrich search results with ClawHub metadata

- Parse /skills and /skills search <query> as SystemCommands in submission.rs
- Add skill_catalog to AgentDeps and wire it through main.rs
- Handle "skills" command in commands.rs: list installed skills and search ClawHub
- Add /skills and /skills search <q> entries to /help output
- Add SkillDetail, SkillStats, SkillOwner structs to catalog.rs
- Add fetch_skill_detail() calling GET /api/v1/skills/{slug} on Convex backend
- Add enrich_search_results() to fetch stars/downloads/owner for top 5 results in parallel
- Fix SkillDetailResponse wrapper struct to match actual API shape: {"skill":{...},"owner":{...}}
- Surface stars, downloads, owner in web UI skill search cards (app.js)
- Surface enriched data in skills web handler and skill_search tool output

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix: cargo fmt after merge conflict resolution

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix: separate installed_skills dir for correct trust on restart, remove duplicate handlers

Trust level bug: skills installed from ClawHub were written to user_dir
(~/.ironclaw/skills/) which is discovered as Trusted on restart. Now installs
go to ~/.ironclaw/installed_skills/ which is discovered as Installed, matching
the documented skill directory layout.

Changes:
- SkillsConfig: add installed_dir field (SKILLS_INSTALLED_DIR env var,
  default ~/.ironclaw/installed_skills/)
- SkillRegistry: add with_installed_dir() builder, installed_dir()/
  install_target_dir() accessors, and discover installed_dir with
  SkillTrust::Installed in discover_all()
- All install paths (web handler, skill tool) use install_target_dir()
  instead of user_dir() so new installs land in the correct directory
- 3 new registry tests: test_installed_dir_uses_installed_trust,
  test_install_target_dir_prefers_installed_dir,
  test_user_dir_stays_trusted_with_installed_dir

Duplicate handler cleanup: handlers/skills.rs was the canonical implementation
but the handlers module was never compiled (not declared in web/mod.rs), so
server.rs had its own duplicate inline definitions that the router used.
Wire up the handlers module, delete the 260-line duplicate in server.rs, and
have server.rs import skills handlers from handlers::skills. Fix pre-existing
compile error in handlers/extensions.rs (missing needs_setup field). Add
#[allow(dead_code)] on not-yet-migrated handler modules to suppress warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: probe more Docker socket paths on macOS

Docker Desktop 4.13+ (stabilised in 4.18) no longer creates the
/var/run/docker.sock symlink by default. The API socket lives at
~/.docker/run/docker.sock, which bollard's connect_with_local_defaults()
does not try.

Add a fallback probe list covering the common macOS container runtimes:
- ~/.docker/run/docker.sock   — Docker Desktop 4.13+
- ~/.colima/default/docker.sock — Colima
- ~/.rd/docker.sock             — Rancher Desktop

Remove the bogus ~/.docker/desktop/docker.sock path that was added
previously; it is not an API socket on any known Docker installation.

Fixes the false-negative "Docker is installed but not running" warning
reported by Illia on macOS with Docker Desktop 4.18+.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* Harden Docker detection for rootless Linux and Windows fallback

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-23 10:04:02 -08:00
2544df1c4a feat: add web UI test skill for Chrome extension (#302)
* feat: add web UI test skill for Chrome extension testing

Add a SKILL.md checklist for manually testing the IronClaw web gateway
UI using the Claude for Chrome browser extension. Covers connection,
chat, skills tab (search, install by search, install by URL, remove),
and smoke tests for other tabs.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: use placeholder token and correct cleanup path per review

- Replace hardcoded test123 token with <your-token> placeholder
- Fix cleanup path: ~/.ironclaw/installed_skills/ (not skills/)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-22 08:49:35 +00:00
a320f265b3 Fix tool schema OpenAI compatibility (#301)
* fix: remove union type arrays from tool schemas for OpenAI compatibility

OpenAI rejects JSON Schema union types containing "array" without an
"items" subschema. The http tool's "body" and json tool's "data" params
used union types to accept any value. Replace with freeform (untyped)
schemas which OpenAI treats as accepting any JSON value.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: update schema tests to assert type is absent, fix missed json.rs test

- http.rs test: assert body has no "type" (not just has description)
- json.rs test: update to match the freeform schema change (was still
  asserting type is present)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-22 07:52:34 +00:00
356f56f77c docs: update CLAUDE.md for recently merged features (#183)
* docs: update CLAUDE.md for recently merged features

Document skills system, sandbox network proxy, leak detector,
Tinfoil private inference, setup wizard, and shell env scrubbing
that were merged but not reflected in CLAUDE.md.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* docs: fix SKILL.md format example and scoring description

Align SKILL.md frontmatter example with actual SkillManifest struct:
activation block with patterns/keywords/max_context_tokens, requires
nested under metadata.openclaw. Fix scoring pipeline description to
mention keywords, tags, and regex patterns instead of triggers/intents.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* docs: optimize CLAUDE.md structure and reduce from 959 to 671 lines

- Update llm/ directory tree (4 -> 12 files to match actual codebase)
- Fix "NEAR AI (required)" -> "NEAR AI (when LLM_BACKEND=nearai)"
- Remove 28-item Completed changelog list (no actionable value)
- Deduplicate 3 config blocks with cross-references
- Extract Workspace deep-dive to src/workspace/README.md
- Extract Tool Architecture deep-dive to src/tools/README.md
- Consolidate Code Style and Review Discipline under Key Patterns
- Add workspace and tools to Module Specifications table

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-20 01:04:39 +00:00
bac2d75713 feat: Secure prompt-based skills system (Phases 1-4) (#51)
* feat: Add secure prompt-based skills system (Phase 1 MVP)

Implement a skills system that extends the agent with prompt-level
instructions from local directories. Skills declare activation criteria,
tool permissions, and trust tiers that determine authority attenuation.

Core security model: the minimum trust level of any active skill
determines a tool ceiling -- tools above the ceiling are removed from
the LLM's tool list entirely at the API level, preventing prompt-based
manipulation.

New modules:
- skills/mod.rs: Core types (SkillTrust, SkillManifest, LoadedSkill)
- skills/scanner.rs: Content scanner for manipulation detection
- skills/registry.rs: Filesystem discovery and manifest parsing
- skills/selector.rs: Deterministic two-phase prefilter (no LLM)
- skills/attenuation.rs: Trust-based tool filtering

Integration:
- Agent loop selects skills per-turn and applies tool attenuation
- Reasoning engine injects skill context with structural isolation
- Config supports SKILLS_ENABLED, SKILLS_DIR, SKILLS_MAX_ACTIVE,
  SKILLS_MAX_CONTEXT_TOKENS environment variables
- Disabled by default (SKILLS_ENABLED=false)

41 new tests covering all modules.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Address all adversarial review findings for skills system

Security fixes:
- Escape skill name/version in XML attributes to prevent trust spoofing
- Escape prompt content to prevent </skill> tag breakout
- Require integrity hash for Verified/Community tier skills
- Validate skill names against [a-zA-Z0-9][a-zA-Z0-9._-]{0,63}
- Add 64 KiB file size limit on prompt.md

Bug fixes:
- Use actual SkillsConfig from AgentDeps instead of SkillsConfig::default()
- Add skills_config field to AgentDeps, wired through from main.rs

Performance:
- Pre-compile regex patterns at load time (cached on LoadedSkill)
- Selector uses pre-compiled patterns instead of recompiling per message
- Switch all std::fs to tokio::fs for non-blocking async I/O

Hardening:
- Cap keyword score at 30 points to prevent keyword stuffing attacks
- Enforce max 20 keywords and 5 patterns per skill
- Normalize line endings (CRLF/CR to LF) before hashing
- Also includes cargo fmt formatting fixes for adjacent code

Tests: 54 skills tests pass (up from 41), zero new clippy warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Address medium/low severity findings from adversarial review

Fixes all 18 medium/low severity findings identified by the security review:

- mod.rs: Add MAX_TAGS_PER_SKILL cap (10) in enforce_limits(); use
  RegexBuilder with 64 KiB size_limit to prevent ReDoS; replace
  case-enumerated escape_skill_content with regex matching all case
  variants plus whitespace/null byte injection between </ and skill;
  document allowed_patterns as unenforced until Phase 2; document
  Marketplace URL validation as Phase 3 concern

- registry.rs: Add MAX_MANIFEST_FILE_SIZE (16 KiB) check before reading;
  add symlink detection via symlink_metadata to reject symlinks in
  discover_local; add MAX_DISCOVERED_SKILLS (100) cap; validate
  prompt_hash format (sha256: + 64 hex chars); warn on name collision
  before overwriting; accept SkillSource parameter in load_skill instead
  of always using Local; add InvalidHashFormat, ManifestTooLarge,
  SymlinkDetected error variants

- selector.rs: Add MAX_TAG_SCORE (15) cap parallel to keyword cap; warn
  when declared max_context_tokens diverges >2x from actual prompt size

- scanner.rs: Add mixed-script homoglyph detection (Cyrillic, Greek,
  Armenian unicode ranges); document token-boundary bypass and semantic
  paraphrasing as known limitations

- attenuation.rs: Document READ_ONLY_TOOLS maintenance requirements

- agent_loop.rs: Surface scan warnings via structured tracing; add
  structured audit events for skill activation and tool attenuation

61 tests pass, 0 new clippy warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Address 12 findings from second adversarial security review

HIGH:
- Escape opening <skill tags in prompt content (prevents fake skill block injection)
- Scan manifest metadata fields (description, author, tags, reasons) not just prompt
- Block trust downgrade on name collision (existing Local can't be replaced by Community)

MEDIUM:
- Eliminate TOCTOU gap: read files then check size instead of metadata-then-read
- Reject file-level symlinks in load_skill (prompt.md, skill.toml)
- Truncate and filter manifest.skill.tags (prevent unlimited tag scoring)
- Cap regex pattern score at 40 (prevent 5x20=100 dominating keyword+tag)
- Add doc comment about skill_list tool exposing metadata (sanitization required)
- Move Community disclaimer inside <skill> tags (not outside structural boundary)
- Filter keywords/tags shorter than 3 chars (prevent broad matching)

LOW:
- Enforce minimum token_cost of 1 (max_context_tokens=0 can't bypass budget)
- Remove redundant try_exists checks in discover_local (let load_skill handle errors)

70 skills tests passing.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: Add HTTP endpoint scoping for skills (Phase 1)

Skills that declare an [http] section in skill.toml now have their HTTP
requests constrained to declared endpoints at runtime. This addresses
the gap where allowed_patterns was parsed but never enforced -- once the
http tool was visible via attenuation, the LLM could reach any URL.

Enforcement reuses EndpointPattern/AllowlistValidator from the WASM
capability system. Semantics: if no active skill declares [http], all
requests pass through (backward compat). If any skill declares [http],
URLs must match at least one skill's allowlist (union). Community skills'
[http] declarations are silently ignored (defense in depth).

Shell commands using curl/wget are also validated against scopes.

Scanner gains detection for known exfiltration domains (webhook.site,
ngrok.io, etc.), overly broad wildcards, and credential/host mismatches.

Closes #38

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: Apply cargo fmt to http_scoping.rs

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: Apply cargo fmt across codebase

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: Add parameter-level permission enforcement for skills (Phase 2)

Activates enforcement of `allowed_patterns` in skill.toml permissions.
Previously these patterns were parsed but not enforced -- a Verified skill
declaring `permissions.shell` with `allowed_patterns = [{command = "cargo *"}]`
could still run any shell command. Now the enforcer validates tool parameters
against declared glob patterns before execution.

Key changes:
- New `enforcer.rs` module with `SkillPermissionEnforcer`, `glob_to_regex()`,
  and `validate_tool_call()` with union semantics across active skills
- Typed pattern enums (`ShellPattern`, `FilePathPattern`, `MemoryTargetPattern`)
  replace the previous `Vec<serde_json::Value>` in `ToolPermissionDeclaration`
- Scanner gains `scan_permission_patterns()` detecting dangerous patterns
  (rm, sudo, curl, bare wildcards, command chaining, sensitive paths, identity files)
- Registry blocks non-Local skills with critical permission pattern warnings
- Agent loop threads enforcer into `execute_chat_tool` alongside HTTP scoping

Trust interaction: Community patterns ignored, Verified enforced, Local without
patterns unrestricted, Local with patterns enforced as guidance. Union semantics
across skills -- tool call allowed if ANY skill's patterns permit it.

34 new tests. All 818 library tests pass.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: Add worker permission enforcement and LLM behavioral analysis (Phase 3+4)

Phase 3 - Worker-side permission enforcement:
- Add SerializedToolPermission/SerializedPattern DTOs for HTTP boundary crossing
- Extend JobDescription, ContainerHandle, and orchestrator API to carry permissions
- CreateJobTool snapshots and forwards skill permissions to spawned workers
- Worker runtime builds SkillPermissionEnforcer and checks before tool execution
- Load-time token budget enforcement rejects prompts exceeding 2x declared budget
- Deduplicate enforcer construction: from_active_skills() delegates to from_serialized()

Phase 4 - LLM behavioral analysis:
- BehavioralAnalyzer with cached, LLM-based semantic content analysis
- Structured output parsing (FINDING|CATEGORY|SEVERITY|DESCRIPTION or CLEAN)
- Content-hash caching with bounded size (MAX_CACHE_ENTRIES=256)
- Graceful degradation when LLM unavailable
- Integrated into load_skill() for non-Local skills; critical findings block loading

Review fixes:
- Real cache tests with CountingLlm mock (test_cache_hit, test_cache_miss, test_cache_bounded)
- UTF-8-safe truncate() in worker runtime
- Few-shot examples in behavioral analysis prompt
- Documented max_context_tokens=0 opt-out and create_job() permission gap

848 tests passing, no new clippy warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Address review feedback from serrrfirat on skills-phase2

- Fix truncate_cmd UTF-8 panic: use char-boundary-aware slicing
- Remove redundant effective_tools branching in reasoning.rs
- Document cache eviction as known limitation (arbitrary, not LRU)
- Add safety comment on SkillTrust enum ordering (security-critical)
- Simplify active_skills selection (prefilter_skills handles empty input)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address remaining skills review feedback

* refactor: replace skills system with OpenClaw SKILL.md format + 2-state trust

Replace the 5-gate, 3-tier trust hierarchy (scanner, behavioral analyzer,
parameter-level enforcer, HTTP endpoint scoping) with a simplified 3-layer
security model: gating -> attenuation -> Docker confinement.

Key changes:
- SKILL.md format (YAML frontmatter + markdown prompt) replaces skill.toml + prompt.md
- 2-state trust (Installed/Trusted) replaces 3-tier (Community/Verified/Local)
- New parser.rs for SKILL.md parsing with serde_yaml
- New gating.rs for requirements checking (bins/env/config)
- Simplified registry with 2-location discovery (workspace + user dirs)
- Removed scanner, behavioral_analyzer, enforcer, http_scoping (~4,100 lines)
- Removed skill_permissions propagation through job/orchestrator/worker pipeline
- Added serde_yaml dependency for YAML frontmatter parsing

Net: -5,298 lines, 59 skills tests pass, 907 total tests pass.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add in-app skill management tools and ClawHub catalog integration

Add 4 chat-callable tools (skill_list, skill_search, skill_install,
skill_remove) plus matching web gateway endpoints for managing skills
at runtime. The catalog fetches from ClawHub's public registry API
at runtime rather than bundling entries at compile time.

Key changes:
- SkillRegistry gains mutation methods (install_skill, remove_skill,
  reload, find_by_name) with Arc<RwLock> for concurrent access
- New catalog module queries ClawHub /api/v1/search with in-memory
  caching (5-min TTL, configurable via CLAWHUB_REGISTRY env var)
- skill_list and skill_search added to READ_ONLY_TOOLS for safe use
  under Installed trust ceiling
- Web gateway gets /api/skills, /api/skills/search, /api/skills/install,
  and /api/skills/{name} DELETE endpoints

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR #51 review feedback from ilblackdragon

Security:
- Add SSRF protection to fetch_skill_content: require HTTPS, reject
  private/loopback/link-local IPs and internal hostnames, disable
  redirects. Gateway install handler now reuses the same validation.
- URL-encode slug in skill_download_url to prevent query injection.
- Require X-Confirm-Action header on gateway skill install/remove
  endpoints (equivalent to chat tool requires_approval gate).

Correctness:
- Eliminate all block_in_place/block_on usage in skill tools and
  gateway handlers. Split install into prepare_install_to_disk (static
  async, no lock) + commit_install (sync, brief write lock). Same
  pattern for remove: validate_remove + delete_skill_files + commit_remove.
- Write normalized content to disk in install_skill (was writing
  original un-normalized content, causing hash mismatch on re-read).
- Fix token estimation from 0.75 to 0.25 tokens/byte (~4 chars per
  token) in registry.rs, selector.rs, and standalone loader.

Dependencies:
- Replace deprecated serde_yaml 0.9 with serde_yml 0.0.12.
- Remove unused toml dependency.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-18 00:28:38 +00:00
Zaki ManianGitHubClaude Opus 4.6Illia Polosukhingemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
d9ff86d7e0 docs: Add review discipline guidelines to CLAUDE.md (#68)
* docs: Add review discipline guidelines to CLAUDE.md

Codifies lessons learned from Illia's review fixes on the libSQL
backend PR -- patterns we missed that should be caught systematically
going forward.

- Ban .expect() alongside .unwrap() in production code
- Add mechanical grep checks before committing
- New "Review & Fix Discipline" section covering:
  - Fix all instances of a pattern, not just the one flagged
  - Propagate architectural changes to satellite types
  - Schema translation must include indexes and seed data
  - Feature flag testing with each feature in isolation

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Apply suggestions from code review

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2026-02-14 04:25:53 +00:00
e843c18141 feat: add libSQL/Turso embedded database backend (#47)
* feat: add libSQL/Turso database backend with full feature parity

Introduce a Database trait abstraction (~60 async methods) enabling
compile-time backend selection between PostgreSQL and libSQL/Turso.
Convert all modules from concrete Store to Arc<dyn Database>, add
LibSqlSecretsStore and LibSqlWasmToolStore implementations, wire
libsql stores throughout CLI and main entry points, and make the
setup wizard backend-agnostic.

Key changes:
- src/db/: Database trait, PostgresDatabase adapter, LibSqlBackend
  with native SQLite-dialect SQL, and idempotent migration system
- src/secrets/store.rs: LibSqlSecretsStore (all 8 trait methods)
- src/tools/wasm/storage.rs: LibSqlWasmToolStore (all 7 trait methods)
- src/main.rs, cli/tool.rs, cli/mcp.rs: backend-conditional wiring
- src/setup/channels.rs: SecretsContext uses Arc<dyn SecretsStore>
- Feature-gate postgres-only tests and examples

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: enable onboarding wizard for libSQL builds

Refactor the setup wizard to work with both postgres and libsql feature
flags. Previously the wizard was gated behind #[cfg(feature = "postgres")]
only, so libsql-only builds would print an error on `ironclaw onboard`.

- Add libsql fields to Settings (database_backend, libsql_path, libsql_url)
- Split wizard database/migration/secrets methods into feature-gated variants
- Add step_database_libsql() with local path and Turso remote replica prompts
- Update setup/mod.rs and main.rs feature gates to any(postgres, libsql)
- Extend check_onboard_needed() to detect libsql database presence

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review feedback for libSQL backend

- P0: Switch libsql_backend to connection-per-operation pattern to fix
  shared Connection concurrency issue across tokio tasks
- P0: Wrap secrets store INSERT+SELECT in transaction to fix TOCTOU race
- P0: Document encryption-at-rest limitations and json_patch divergence
- P1: Fix get_opt_text removing .filter(|s| !s.is_empty()) that conflated
  empty strings with NULL
- P1: Replace datetime('now') with fmt_ts(&Utc::now()) for consistent
  RFC 3339 timestamps across all queries
- P2: Use explicit _rowid column in FTS5 triggers and joins for stability
  across VACUUM operations
- P2: Add tracing::warn when embedding provided but vector search disabled
  in hybrid_search
- Extract shared connect_from_config() helper to deduplicate DB connection
  logic across main.rs, cli/config.rs, and cli/mcp.rs

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: add missing JobContext fields and resolve fmt/clippy warnings

Add total_tokens_used and max_tokens fields to JobContext in
libsql_backend.rs, apply cargo fmt, and fix clippy warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: review fixes for libSQL backend (shared connections, panics, indexes)

- Replace .expect() with proper error propagation in 3 call sites
- Share Arc<Database> between backend and stores instead of single Connection
- Add connect-per-operation pattern to LibSqlSecretsStore and LibSqlWasmToolStore
- Wrap store() INSERT + SELECT-back in a transaction
- Add ~22 missing indexes for parity with PostgreSQL schema
- Add 18 leak_detection_patterns seed rows matching PostgreSQL V2 migration
- Fix super:: import to use crate:: style
- Gate mask_password_in_url behind #[cfg(feature = "postgres")]
- Rewrite secrets store init with or_else chain for runtime backend selection

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Resolve clippy lints (collapsible_if, too_many_arguments)

Collapse nested if blocks into let_chains to satisfy clippy's
collapsible_if lint (CI uses -D warnings). Suppress too_many_arguments
on libsql_row_to_tool_at since refactoring the positional index
pattern would be a larger change.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
2026-02-14 02:05:05 +00:00
5df0d13b59 Bump MSRV to 1.92, add GCP deployment files (#40)
* Bump MSRV to 1.92 and add GCP deployment files

rig-core 0.30 uses let_chains (stabilized post-1.87), which breaks
builds on Rust 1.85. Bump rust-version in Cargo.toml and both
Dockerfiles to 1.92 (verified working).

Add cloud deployment scaffolding:
- Dockerfile: multi-stage build for the main agent container
- deploy/cloud-sql-proxy.service: systemd unit for Cloud SQL Auth Proxy
- deploy/ironclaw.service: systemd unit for the IronClaw container
- deploy/setup.sh: VM bootstrap script (Docker, proxy, services)
- deploy/env.example: reference environment configuration

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Address review feedback: harden deploy scaffolding

- Add comment explaining GATEWAY_HOST=0.0.0.0 and when to use 127.0.0.1
- Document /opt/ironclaw ownership model (root-owned, Docker reads as root)
- Switch cloud-sql-proxy service from User=root to DynamicUser=yes

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Resolve clippy lints (Rust 1.93) and fix CI test workflow

- Fix 97 collapsible_if warnings using let-chains syntax (auto-fixed)
- Fix ptr_arg: change &PathBuf to &Path in pairing store functions
- Fix suspicious_open_options: add .truncate(false) to OpenOptions
- Fix too_many_arguments: add clippy allow on execute_status
- Fix unnecessary_unwrap: use if-let in repository.rs hybrid_search
- Gate unused EchoTool with #[cfg(test)]
- Add PairingStore argument to ChannelStoreData::new() test call sites
- Add skip guard for bundled channel test when WASM artifacts unavailable
- Split CI test workflow to exclude PostgreSQL-dependent integration tests

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Address review feedback from ilblackdragon

- Add root check to setup.sh (exits with error if not root)
- Add warning comment to env.example about placeholder passwords
- Dockerfile.worker already uses rust:1.92 (no change needed)
- PR #41 overlap noted; will rebase after #41 merges

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve 47 collapsible_if clippy warnings

Collapse nested if statements across the codebase to satisfy
clippy::collapsible_if on Rust 1.93.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-13 22:21:50 +04:00
b3dee13954 fix: flatten tool messages for NEAR AI cloud-api compatibility (#41)
* fix: flatten tool messages for NEAR AI cloud-api compatibility

NEAR AI cloud-api does not support the OpenAI multi-turn tool-calling
protocol (role:"tool" messages cause HTTP 400). This adds a
flatten_tool_messages() pass in NearAiChatProvider that rewrites
assistant tool_call messages and tool result messages into plain
assistant/user text before sending to the API. The model still sees
the tool execution history, just in a text format it can process.

Also includes a minor fix to telegram channel send_pairing_reply
for updated WASM host function signature.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve CI failures in fmt, rate limiting, and test configuration

- Apply cargo fmt to nearai_chat.rs formatting violations
- Fix truncate(true) bug in record_failed_approve that cleared the
  attempts file before reading, preventing rate limit from ever
  triggering
- Skip bundled channel test when WASM build artifacts are unavailable
  (CI lacks wasm32-wasip2 target)
- Split CI test workflow to exclude workspace_integration tests that
  require PostgreSQL

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve clippy unnecessary_unwrap lint (Rust 1.93)

Replace is_some() + unwrap() pattern with if-let binding to satisfy
clippy::unnecessary_unwrap which is now deny-by-default.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: firat.sertgoz <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
2026-02-13 06:18:43 +00:00
bb228f6315 feat: Add multi-provider LLM support via rig-core adapter (#36)
Add support for OpenAI, Anthropic, Ollama, and OpenAI-compatible
endpoints alongside the existing NEAR AI backend. Users can now
bring their own API keys via environment variables (LLM_BACKEND,
OPENAI_API_KEY, ANTHROPIC_API_KEY, etc.) while NEAR AI remains
the default.

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-12 00:37:51 +00:00