mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
f1fa303abc75e08f0b81b916b2222eeca502eb42
13
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
f1fa303abc |
feat(config): unify all settings to DB > env > default priority
Previously only LLM settings used DB-first priority while all other subsystems (agent, channels, tunnel, heartbeat, embeddings, sandbox, wasm, safety, builder, transcription, routines, skills, hygiene, search) used env-first. This made web UI settings changes unreliable for non-LLM config — env vars would silently override DB values. Now all subsystems follow the same priority: DB > env > TOML > default. - Add db_first_or_default, db_first_bool, db_first_optional_string, db_first_option helpers to config/helpers.rs with shadow warnings - Flip 10 Group 1 resolvers (agent, channels, tunnel, heartbeat, embeddings, sandbox, wasm, safety, builder, transcription) from parse_optional_env/parse_bool_env to db_first_* equivalents - Add Settings structs for 4 Group 2 resolvers (routines, skills, hygiene, search) that previously had no DB persistence - Update Config::build() call sites and cli/doctor.rs caller - Security-sensitive fields stay env-only: allow_local_tools, allow_full_access, cost/rate limits, auth tokens, API keys - Bootstrap configs (database, secrets) stay env-only Closes #1119 (partial — config unification phases 1-2) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
|
3aa36c8f55 |
fix(tests): eliminate env mutex poison cascade (#1558)
* fix(tests): eliminate env mutex poison cascade and fix test flakiness The shared ENV_MUTEX used by ~68 config tests would cascade a single test panic into failures across every module. Replace all .unwrap() / .expect() lock acquisitions with a poison-recovering lock_env() helper. Consolidate rogue module-local ENV_LOCK instances (workspace, orchestrator, bootstrap) onto the shared global mutex to prevent cross-module races. Also fixes: - gateway user_id fallback was hardcoded to "default" instead of owner_id - test_ironclaw_env_path used LazyLock which is order-dependent Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * test(helpers): add regression test for lock_env poison recovery Satisfies the regression-test-check CI gate by adding a test that intentionally poisons ENV_MUTEX and verifies lock_env() recovers. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(ci): detect test changes inside #[cfg(test)] regions The regression test check relied on git diff -W to expand context to function boundaries, but git doesn't recognize Rust `mod tests {}` as a function boundary. Changes to imports, helpers, or lock calls inside test modules were invisible to the check. Add a line-level fallback: for each changed .rs file, find where #[cfg(test)] starts and check if any diff hunk targets a line at or after that boundary. This catches edits anywhere inside test modules regardless of git's language awareness. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address PR review feedback - Clear ENV_MUTEX poison after regression test so it doesn't leave global state dirty for subsequent tests. - Fix CI regression-test-check to match #[cfg(test)] only when followed by `mod` (the test module pattern), avoiding false positives from standalone #[cfg(test)] items like statics or functions. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
|
d3b69e7be3 |
Fix CI approval flows and stale fixtures (#1478)
* Fix CI approval flows and stale fixtures * Backfill approval thread mapping across channels |
||
|
|
ef3d769742 |
fix(security): validate embedding base URLs to prevent SSRF (#1221)
* fix(security): validate embedding base URLs to prevent SSRF (#1103) User-configurable base URLs (OLLAMA_BASE_URL, EMBEDDING_BASE_URL) were passed directly to reqwest with no validation, allowing SSRF attacks against cloud metadata endpoints, internal services, or file:// URIs. Adds validate_base_url() that rejects: - Non-HTTP(S) schemes (file://, ftp://) - HTTP to non-localhost destinations (prevents credential leakage) - HTTPS to private/loopback/link-local/metadata IPs (169.254.169.254, 10.x, 192.168.x, 172.16-31.x, CGN 100.64/10) - IPv4-mapped IPv6 bypass attempts Validation runs at config resolution time so bad URLs fail at startup. Closes #1103 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(security): add DNS resolution check, ULA blocking, and NEARAI_BASE_URL validation Address review feedback: - Resolve hostnames to IPs and check all resolved addresses against the blocklist (prevents DNS-based SSRF bypass where attacker uses a domain pointing to 169.254.169.254) - Add IPv6 Unique Local Address (fc00::/7) to the blocklist - Validate NEARAI_BASE_URL in llm config (was missing — especially dangerous since bearer tokens are forwarded to the configured URL) - Allow DNS resolution failure gracefully (don't block startup when DNS is temporarily unavailable) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * style: fix formatting Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(security): add SSRF validation to all base URL chokepoints - Add validate_base_url() in resolve_registry_provider() covering all LLM providers (OpenAI, Anthropic, Ollama, openai_compatible, etc.) - Add validate_base_url() for NEARAI_AUTH_URL in LlmConfig::resolve() - Add validate_base_url() for TRANSCRIPTION_BASE_URL in TranscriptionConfig - Add missing SSRF test cases: CGN range, IPv4-mapped IPv6, ULA IPv6, URLs with credentials, empty/invalid URLs Co-Authored-By: Claude Opus 4.6 <[email protected]> * ci: re-trigger CI with latest changes Co-Authored-By: Claude Opus 4.6 <[email protected]> * ci: trigger new run with skip-regression-check label Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(security): validate embedding base URLs to prevent SSRF (#1103) User-configurable base URLs (OLLAMA_BASE_URL, EMBEDDING_BASE_URL) were passed directly to reqwest with no validation, allowing SSRF attacks against cloud metadata endpoints, internal services, or file:// URIs. Adds validate_base_url() that rejects: - Non-HTTP(S) schemes (file://, ftp://) - HTTP to non-localhost destinations (prevents credential leakage) - HTTPS to private/loopback/link-local/metadata IPs (169.254.169.254, 10.x, 192.168.x, 172.16-31.x, CGN 100.64/10) - IPv4-mapped IPv6 bypass attempts Validation runs at config resolution time so bad URLs fail at startup. Closes #1103 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(security): add DNS resolution check, ULA blocking, and NEARAI_BASE_URL validation Address review feedback: - Resolve hostnames to IPs and check all resolved addresses against the blocklist (prevents DNS-based SSRF bypass where attacker uses a domain pointing to 169.254.169.254) - Add IPv6 Unique Local Address (fc00::/7) to the blocklist - Validate NEARAI_BASE_URL in llm config (was missing — especially dangerous since bearer tokens are forwarded to the configured URL) - Allow DNS resolution failure gracefully (don't block startup when DNS is temporarily unavailable) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * style: fix formatting Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(security): add SSRF validation to all base URL chokepoints - Add validate_base_url() in resolve_registry_provider() covering all LLM providers (OpenAI, Anthropic, Ollama, openai_compatible, etc.) - Add validate_base_url() for NEARAI_AUTH_URL in LlmConfig::resolve() - Add validate_base_url() for TRANSCRIPTION_BASE_URL in TranscriptionConfig - Add missing SSRF test cases: CGN range, IPv4-mapped IPv6, ULA IPv6, URLs with credentials, empty/invalid URLs Co-Authored-By: Claude Opus 4.6 <[email protected]> * ci: re-trigger CI with latest changes Co-Authored-By: Claude Opus 4.6 <[email protected]> * ci: trigger new run with skip-regression-check label Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: [email protected] <[email protected]> |
||
|
|
8526cde1be |
fix: restore libSQL vector search with dynamic dimensions (#1393)
* fix: restore libSQL vector search with dynamic embedding dimensions (#655) The V9 migration dropped the libsql_vector_idx and changed memory_chunks.embedding from F32_BLOB(1536) to BLOB, but the documented brute-force cosine fallback was never implemented. hybrid_search silently returned empty vector results — search was FTS5-only on libSQL. Add ensure_vector_index() which dynamically creates the vector index with the correct F32_BLOB(N) dimension, inferred from EMBEDDING_DIMENSION / EMBEDDING_MODEL env vars during run_migrations(). Uses _migrations version=0 as a metadata row to track the current dimension (no-op if unchanged, rebuilds table on dimension change). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * style: move safety comments above multi-line assertions for rustfmt stability Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: remove unnecessary safety comments from test code Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address review comments from PR #1393 [skip-regression-check] - Share model→dimension mapping via config::embeddings::default_dimension_for_model() instead of duplicating the match table (zmanian, Copilot) - Add dimension bounds check (1..=65536) to prevent overflow (zmanian, Copilot) - DROP stale memory_chunks_new before CREATE to handle crashed previous attempts (zmanian, Copilot) - Use plain INSERT instead of INSERT OR IGNORE to surface constraint errors (Copilot) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: add missing builder field to AgentDeps in telegram routing test [skip-regression-check] The self-repair builder field was added to AgentDeps in #712 but this test was not updated. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address zmanian's second review on PR #1393 - Add tracing::info when resolve_embedding_dimension returns None (#2) - Document connection scoping for transaction safety (#1) - Document _rowid preservation for FTS5 consistency (#4) - Document precondition that migrations must run first (#5) - Note F32_BLOB dimension enforcement in insert_chunk (#3) - Add unit tests for resolve_embedding_dimension (#6) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
|
86ae12747b |
feat: LRU embedding cache for workspace search (#1423)
* feat: LRU embedding cache for workspace search (#165) Add CachedEmbeddingProvider that wraps any EmbeddingProvider with an in-memory LRU cache keyed by SHA-256(model_name + text). This avoids redundant HTTP calls when the same text is embedded multiple times (common during reindexing and repeated searches). - Cache uses HashMap + last_accessed tracking with manual LRU eviction (same pattern as llm::response_cache::CachedProvider) - Lock is never held during HTTP calls to prevent blocking - embed_batch() partitions into hits/misses and only fetches misses - Default 10,000 entries (~58 MB for 1536-dim vectors) - Configurable via EMBEDDING_CACHE_SIZE env var - Workspace.with_embeddings() auto-wraps; with_embeddings_uncached() available for tests Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review comments on embedding cache - Validate embed_batch return count matches expected miss count - Replace unwrap_or_default() with proper error propagation - Fix batch eviction: run final eviction pass after insert to enforce cap - Fix test: use different-length inputs to verify ordering correctness - Reject EMBEDDING_CACHE_SIZE=0 in config validation (minimum is 1) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: replace .expect() with proper error handling in embed_batch The all-cache-hits early-return path used .expect("all cache hits") which violates the project convention of no .unwrap()/.expect() in production code. Replaced with the same ok_or_else pattern used in the normal path. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: clarify memory sizing docs and use saturating_add for eviction - Update memory comments in embedding_cache.rs, config/embeddings.rs, and workspace/mod.rs to note the ~58 MB figure is payload-only (actual memory is higher due to HashMap/key/allocation overhead) - Use saturating_add(1) instead of + 1 for eviction threshold to prevent overflow if max_entries is usize::MAX Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address Copilot review on embedding cache - Avoid double-clone per miss in embed_batch: move embedding into results, clone only for the cache entry - Evict per-insert instead of after all inserts to keep peak memory bounded during large batches - Clamp max_entries to at least 1 in constructor to prevent unexpected eviction behavior when set to 0 via the public API Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: reduce embedding_cache module visibility to private Types are already re-exported via `pub use`, so the module itself doesn't need to be public. Reduces unnecessary API surface. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address serrrfirat review feedback on embedding cache - Add TODO comment for O(n) LRU eviction scalability - Add thundering herd note at lock release in embed() - Warn when cache max_entries exceeds 100k - Use with_embeddings_uncached() in integration test - Add tests: error_does_not_pollute_cache, embed_batch_empty_input - Update README with cache-aware with_embeddings() docs Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: prevent u32 wrapping in FailThenSucceedMock failure counter fetch_sub(1) wraps to u32::MAX when called past zero, silently breaking the mock for 3+ calls. Switch to load-then-store to avoid the wrapping bug in both embed() and embed_batch(). Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address Copilot and serrrfirat review findings on embedding cache - Switch tokio::sync::Mutex to std::sync::Mutex (lock never held across .await — cheaper synchronous lock) - Extract DEFAULT_EMBEDDING_CACHE_SIZE constant to avoid 10_000 duplication between EmbeddingCacheConfig and EmbeddingsConfig Co-Authored-By: Claude Opus 4.6 <[email protected]> * test: add all-misses batch test for embedding cache Adds embed_batch_all_misses test covering the case where every text in a batch is a cache miss — fulfilling the commitment from serrrfirat's review. Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: trigger CI re-check after rebase * fix: use raw [u8;32] cache keys and pre-allocate HashMap capacity Address Copilot review findings: - cache_key() now returns [u8; 32] instead of hex String, avoiding a 64-byte allocation per lookup - HashMap::with_capacity(max_entries) avoids incremental reallocation - Fix pre-existing staging compilation error in cli/routines.rs (missing max_tool_rounds/use_tools fields) [skip-regression-check] * fix: make cache accessors sync and update doc for [u8;32] keys Address Copilot review: - len(), is_empty(), clear() are now sync since they only take a std::sync::Mutex lock with no .await points - Update cache_size doc comment to reflect [u8;32] keys instead of String keys [skip-regression-check] * fix: remove clone_on_copy for [u8; 32] cache keys [skip-regression-check] * ci: add safety comments to test code for no-panics check The CI no-panics grep check cannot distinguish test code inside src/ files from production code. Add // safety: test annotations to .unwrap(), .expect(), and assert!() calls in #[cfg(test)] modules. * fix: correct cache doc and demote hit/miss logs to trace - Fix misleading "String keys" in memory comment (cache uses [u8; 32]) - Demote per-request hit/miss logs from debug to trace to reduce noise on hot paths (batch summary stays at trace too) * docs: add missing Arc import in workspace README example * perf: batch eviction in embed_batch to avoid O(n×m) cost Replace per-insert evict_lru call with a single evict_k_oldest pass that computes eviction count upfront and removes the k oldest entries in one O(n) scan. Avoids O(n×m) HashMap iterations while holding the mutex during batch inserts. * fix: cap batch cache inserts at max_entries and use O(n) selection - evict_k_oldest now uses select_nth_unstable_by_key for O(n) average partial selection instead of O(n log n) full sort - embed_batch caps cached entries at max_entries when misses exceed capacity, preventing the cache from growing unbounded - Added test: batch_exceeding_capacity_respects_max_entries * fix: flatten test assert for fmt compatibility Shorten assert message to fit single line so cargo fmt doesn't split the safety annotation onto a separate line. * fix: address review feedback and improve embedding cache (takeover #235) - Fix merge conflict: add missing allow_always field in PendingApproval - Thread EmbeddingCacheConfig through CLI memory commands so they respect EMBEDDING_CACHE_SIZE instead of silently using default (fixes #235 review) - Cap HashMap pre-allocation at min(max_entries, 1024) to avoid upfront memory waste at large cache sizes - Fix FailThenSucceedMock race: replace load+store with atomic fetch_update - Remove noisy '// safety: test' comments (40+ lines of diff noise) - Fix collapsed lines from comment removal - Simplify redundant Ok(...collect()?) to just collect() Co-Authored-By: ztsalexey <[email protected]> Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(embedding-cache): skip eviction on concurrent duplicate insert When the lock is released for the HTTP call, another caller may insert the same key. Re-check under lock and just update the existing entry without evicting, avoiding unnecessary cache churn under concurrency. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: ztsalexey <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: ztsalexey <[email protected]> |
||
|
+12 |
5e7758598f |
chore: periodic sync main into staging (resolved conflicts) (#1098)
* chore: promote staging to main (2026-03-10 15:19 UTC) (#865)
* fix: Channel HTTP: server doesn't start after config change (no hot-r… (#779)
* fix: Channel HTTP: server doesn't start after config change (no hot-reload)
* review fixes
* review fixes
* fix linter
* fix code style
* fix: prevent session lock contention blocking message processing (#783)
* fix: prevent session lock contention blocking message processing
## Problem
After container restart, POST /api/chat/send returns 202 ACCEPTED but messages
don't appear in conversation_messages and agent never responds. Messages get
stuck in "stale state" after restart.
Root cause: Session lock was held for entire duration of chat_threads_handler
and chat_history_handler, including during slow database queries. This blocked
the agent loop from acquiring the session lock to process incoming messages,
causing them to hang indefinitely.
## Solution
1. **Release session lock early in chat_threads_handler**: Only acquire lock
when reading active_thread at response time, not during DB queries for
thread list. DB operations no longer block message processing.
2. **Release session lock early in chat_history_handler**: Only acquire lock
when accessing in-memory thread state, not during paginated DB queries or
thread ownership checks. DB operations no longer block message processing.
3. **Add comprehensive logging**: Track message flow from receipt through
session resolution, thread hydration, and state transitions. Helps diagnose
future issues:
- Message queued to agent loop (chat_send_handler)
- Processing message from channel (handle_message)
- Hydrating thread from DB (maybe_hydrate_thread)
- Resolving session and thread (resolve_thread)
- Checking thread state (process_user_input)
- Persisting user message (persist_user_message)
## Impact
- Message processing no longer blocks on session lock contention
- API response times for thread list/history queries unaffected (DB queries
still happen, but lock is not held)
- Better diagnostics for future debugging
## Testing
- All 2756 tests pass
- Code compiles with zero clippy warnings
- No changes to user-facing API or behavior, only lock timing
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* security: redact PII from info-level logs
Downgrade user_id and channel logging to debug level to prevent exposing
Personally Identifiable Information (PII) in production logs.
The user_id field can contain sensitive information such as phone numbers
(e.g., for Signal messages). Logging PII in cleartext at the info level
creates a security and privacy risk, as these logs may be stored in
persistent storage, indexed by log management systems, or accessible to
unauthorized personnel.
Changes:
- Info level: logs only message_id (UUID) for tracking
- Debug level: logs user_id, channel, thread_id for troubleshooting
This maintains debugging capability for developers while protecting user
privacy in production logs.
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
---------
Co-authored-by: Claude Haiku 4.5 <[email protected]>
* chore: sync main into staging (#855)
* 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]>
* feat(llm): per-provider unsupported parameter filtering (#749, #728) (#809)
Add declarative `unsupported_params` field to provider definitions in
providers.json. Parameters listed are stripped from requests before
sending, preventing 400 errors from providers that reject them (e.g.
gpt-5 family and kimi-k2.5 rejecting custom temperature values).
- Add `unsupported_params` to ProviderDefinition and RegistryProviderConfig
- Propagate from registry through config resolution
- Generic strip helpers handle temperature, max_tokens, stop_sequences
- Apply filtering in RigAdapter and AnthropicOAuthProvider
- Mark openai and tinfoil providers as unsupporting temperature
- Update openai default model to gpt-5-mini
Co-authored-by: Claude Opus 4.6 <[email protected]>
---------
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
* fix: Chat input is hidden in mobile browser mode (#877)
* fix: stop XML-escaping tool output content (#598) (#874)
* 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]>
* feat(llm): per-provider unsupported parameter filtering (#749, #728) (#809)
Add declarative `unsupported_params` field to provider definitions in
providers.json. Parameters listed are stripped from requests before
sending, preventing 400 errors from providers that reject them (e.g.
gpt-5 family and kimi-k2.5 rejecting custom temperature values).
- Add `unsupported_params` to ProviderDefinition and RegistryProviderConfig
- Propagate from registry through config resolution
- Generic strip helpers handle temperature, max_tokens, stop_sequences
- Apply filtering in RigAdapter and AnthropicOAuthProvider
- Mark openai and tinfoil providers as unsupporting temperature
- Update openai default model to gpt-5-mini
Co-authored-by: Claude Opus 4.6 <[email protected]>
* fix: stop XML-escaping tool output content in wrap_for_llm (#598)
Remove content escaping that corrupted JSON in tool output. The
<tool_output> structural boundary is preserved but content now passes
through raw, fixing downstream parse failures.
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]>
* fix(safety): allow empty string tool params (#848)
* fix(safety): allow empty string tool params
* fix(safety): preserve heuristic checks and add path context to tool validation
This follow-up refactor addresses PR review feedback by restoring
heuristic checks (whitespace ratio, character repetition) for tool
parameter validation and improving error reporting.
Changes:
- Restored heuristic warnings in validate_non_empty_input so they apply
to both user input and tool parameters (when non-empty).
- Refactored check_strings to recursively build and pass JSON paths
(e.g., "metadata.tags[1]").
- Updated validation errors to use the specific JSON path as the field
name instead of the generic "input".
- Added regression tests for whitespace/repetition warnings and JSON
path reporting in tool parameters.
This ensures the safety layer remains semantically neutral about empty
strings (fixing the memory_tree path: "" issue) while maintaining
rigorous protection and providing better developer ergonomics.
* style: run cargo fmt
* perf: optimize release and dist build profiles (#843)
* perf: optimize release and dist build profiles
Add [profile.release] with strip=true and panic="abort" for smaller,
faster release binaries. Upgrade [profile.dist] from lto="thin" to
lto="fat" with codegen-units=1 for maximum optimization in CI releases.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: remove panic=abort from release profile
Reviewers (zmanian, Copilot, Gemini) correctly flagged that panic=abort
in the release profile would kill the entire process on any tokio task
panic, breaking fault isolation for the long-running server. Removed
from release profile entirely.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---------
Co-authored-by: Claude Opus 4.6 <[email protected]>
* feat: add PR template with risk assessment (#837)
* feat: add PR template with risk assessment and review tracks
Add a pull request template that includes summary, change type,
validation checklist, security/database impact sections, blast radius,
and rollback plan. Update CONTRIBUTING.md with review track definitions
(A/B/C) based on change risk level.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: expand CONTRIBUTING.md with setup, workflow, and guidelines
Add getting started, development workflow, code style summary,
database change guidance, and dependency management sections.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---------
Co-authored-by: Claude Opus 4.6 <[email protected]>
* feat: add fuzzing targets for untrusted input parsers (#835)
* feat: add fuzzing targets for untrusted input parsers
Add cargo-fuzz infrastructure with 5 fuzz targets exercising
security-critical code paths:
- fuzz_safety_sanitizer: Aho-Corasick + regex injection detection
- fuzz_safety_validator: Input validation (length, encoding, patterns)
- fuzz_leak_detector: Secret leak scanning (API keys, tokens)
- fuzz_tool_params: Tool parameter JSON validation
- fuzz_config_env: TOML/JSON config parsing
Each target exercises real IronClaw business logic with invariant
assertions. Includes corpus directories and setup documentation.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: improve fuzz targets to exercise real IronClaw code paths
- fuzz_config_env: exercise SafetyLayer end-to-end (sanitize, validate,
policy check) instead of generic TOML/JSON parsing
- fuzz_tool_params: add validate_tool_schema coverage alongside
validate_tool_params
- Add "fuzz" to workspace exclude in root Cargo.toml
- Update README descriptions to match actual target behavior
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: replace redundant detect() call with meaningful invariant assertion
Replace the double sanitize()+detect() call with an assertion that
critical severity warnings always trigger content modification.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: rewrite fuzz_config_env to exercise IronClaw safety code directly
Replace SafetyLayer wrapper usage with direct Sanitizer, Validator, and
LeakDetector instantiation and invocation. Adds meaningful consistency
assertions (non-empty output, valid-means-no-errors, scan/clean agreement).
Removes the config construction that was only exercising struct instantiation.
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---------
Co-authored-by: Claude Opus 4.6 <[email protected]>
* fix(wasm): run leak scan before credential injection in tools wrapper (#791)
* fix(wasm): run leak scan before credential injection in tools wrapper
The tools WASM wrapper runs the LeakDetector on HTTP request headers
AFTER inject_host_credentials() has already substituted real secrets
(e.g., xoxb- Slack bot tokens). This causes the leak detector to
flag the tool's own legitimate outbound API calls as secret exfiltration.
Move the scan to run on raw_headers before any credential injection,
matching the fix already applied to the channels wrapper in #421.
Fixes the same class of bug as #421 (which only fixed channels/wasm/wrapper.rs).
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* perf: inline leak scan to avoid Vec allocation on every HTTP request
Address review feedback: instead of cloning all header keys/values into
a Vec to pass to scan_http_request(), iterate over raw_headers directly
using scan_and_clean(). This also provides more specific error messages
(URL vs header vs body).
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* style: fix cargo fmt formatting in leak scan loop
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---------
Co-authored-by: Claude Opus 4.6 <[email protected]>
* fix(setup): drain residual terminal events before secret input (#747) (#849)
* 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]>
* feat(llm): per-provider unsupported parameter filtering (#749, #728) (#809)
Add declarative `unsupported_params` field to provider definitions in
providers.json. Parameters listed are stripped from requests before
sending, preventing 400 errors from providers that reject them (e.g.
gpt-5 family and kimi-k2.5 rejecting custom temperature values).
- Add `unsupported_params` to ProviderDefinition and RegistryProviderConfig
- Propagate from registry through config resolution
- Generic strip helpers handle temperature, max_tokens, stop_sequences
- Apply filtering in RigAdapter and AnthropicOAuthProvider
- Mark openai and tinfoil providers as unsupporting temperature
- Update openai default model to gpt-5-mini
Co-authored-by: Claude Opus 4.6 <[email protected]>
* fix: skip the regression check
[skip-regression-check]
---------
Co-authored-by: Henry Park <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
* feat(agent): add context size logging before LLM prompt (#810)
* 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]>
* feat(agent): add context size logging before LLM prompt
---------
Co-authored-by: Henry Park <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
* fix: preserve text before tool-call XML in forced-text responses (#852)
* fix: preserve text before tool-call XML in forced-text responses (#789)
Local models (Qwen3, DeepSeek, GLM) emit <tool_call> XML even when no
tools are available (force_text mode). The existing strip_xml_tag()
discards everything from an unclosed opening tag onward, producing an
empty string that triggers the "I'm not sure how to respond" fallback.
Add truncate_at_tool_tags() — a code-region-aware pre-processing step
that truncates at the first tool-call XML tag BEFORE clean_response()
runs, preserving all useful text before the tag. Protect all 7
clean_response() call sites. Case-insensitive matching handles models
that emit <TOOL_CALL> or <Tool_Call> variants.
Secondary fix: add has_native_thinking() model detection to skip
<think>/<final> system prompt injection for models with built-in
reasoning (Qwen3, QwQ, DeepSeek-R1, GLM-Z1, etc.), preventing
thinking-only responses that clean to empty.
Wire with_model_name(active_model_name()) at all 9 production sites
that construct Reasoning, so the runtime model name (not static config)
drives system prompt generation.
126 new/updated tests covering truncation edge cases, code-block
awareness, Unicode, case-insensitivity, StubLlm integration for
complete/plan/evaluate_success/respond_with_tools paths, model
detection, and conditional system prompt generation.
Closes #789
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: address Copilot review — unclosed-only truncation, ASCII case folding
- truncate_at_tool_tags() now only truncates at UNCLOSED tool tags;
properly closed tags (e.g. <tool_call>...</tool_call>) are left intact
for clean_response() to strip normally, preserving any text after them
- Switch from to_lowercase() to to_ascii_lowercase() to prevent byte
offset misalignment with non-ASCII characters whose lowercase form
has different byte length (e.g. Kelvin sign U+212A)
- Add closing_tag_for() helper to derive closing tags from open patterns
- Fix doc comment: "fenced markdown code blocks or inline code spans"
(not "indented", which find_code_regions() doesn't detect)
- Add regression tests: closed vs unclosed for each tag variant,
Unicode + case-insensitive offset safety, and mixed closed/unclosed
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: minor review items — consistent ascii_lowercase, closing_tag_for tests
- Switch has_native_thinking() from to_lowercase() to to_ascii_lowercase()
for consistency with truncate_at_tool_tags() approach
- Add unit tests for closing_tag_for(): standard tags, space-suffixed
patterns, pipe-delimited tags, and exhaustive coverage of all
TOOL_TAG_PATTERNS entries
- Add test for mixed closed+unclosed tags of different types
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---------
Co-authored-by: Claude Opus 4.6 <[email protected]>
* Feat/docker shell edition (#804)
* 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]>
---------
Co-authored-by: Henry Park <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
* fix(mcp): strip top-level null params before forwarding to MCP servers (#795)
* feat(llm): per-provider unsupported parameter filtering (#749, #728) (#809)
Add declarative `unsupported_params` field to provider definitions in
providers.json. Parameters listed are stripped from requests before
sending, preventing 400 errors from providers that reject them (e.g.
gpt-5 family and kimi-k2.5 rejecting custom temperature values).
- Add `unsupported_params` to ProviderDefinition and RegistryProviderConfig
- Propagate from registry through config resolution
- Generic strip helpers handle temperature, max_tokens, stop_sequences
- Apply filtering in RigAdapter and AnthropicOAuthProvider
- Mark openai and tinfoil providers as unsupporting temperature
- Update openai default model to gpt-5-mini
Co-authored-by: Claude Opus 4.6 <[email protected]>
* fix(mcp): strip top-level null params before forwarding to MCP servers
LLMs frequently emit `"field": null` for optional parameters in tool
calls. Many MCP servers reject explicit nulls for fields that should
simply be absent — e.g. Notion returns 400 for `"sort": null` in a
search call, expecting the field to be omitted entirely.
Strip top-level null keys from the params object before calling
`call_tool()`. Only top-level keys are stripped; nested nulls are
preserved since they may be semantically meaningful.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---------
Co-authored-by: Illia Polosukhin <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
* Add event-triggered routines and workflow skill templates (#756)
* Add event-triggered routines and workflow skill templates
* 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: address PR review feedback for event_emit security and quality
Security fixes:
- Require approval (UnlessAutoApproved) for event_emit, matching routine_fire
- Enable sanitization on event_emit payload (external JSON reaches LLM)
- Remove user_id parameter from event_emit to prevent IDOR — always use ctx.user_id
Correctness fixes:
- Rename source → event_source in event_emit for consistency with routine_create
- Use json_value_as_filter_string for filter parsing (handles numbers/booleans)
- Case-insensitive matching for event source and event_type
- Add debug logging for missing filter keys in payload
- Fix skill_install_routine_webhook_sim test missing .with_skills()
- Fix schema_validator test for event_emit payload properties
Code quality:
- Move EventEmitTool struct/impl after RoutineHistoryTool (fix split layout)
- Deduplicate routine_to_info into RoutineInfo::from_routine in types.rs
- Add test section headers in e2e_routine_heartbeat.rs
- Clarify event_emit description to specify system_event routines only
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]>
* fix: make routine_system_event_emit test create routine before emitting
- Add routine_create step to trace fixture so event_emit has a matching
routine to fire
- Assert fired_routines > 0, not just key presence (Copilot review)
- Add .with_auto_approve_tools(true) since event_emit now requires approval
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: renumber test headers after system_event test insertion
Test 4 was duplicated (routine_cooldown and heartbeat_findings).
Renumber heartbeat_findings to Test 5 and heartbeat_empty_skip to Test 6.
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: merge staging and add missing RoutineEngine args in test
RoutineEngine::new on staging requires `tools` and `safety` params.
Update system_event_trigger_matches_and_filters test to pass them.
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: address new Copilot review comments
- Add .with_auto_approve_tools(true) to skill_install_routine_webhook_sim
test so event_emit doesn't block on approval
- Fix module-level doc comment for event_emit to specify system_event trigger
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: deduplicate json_value_as_string helper
Remove private `json_value_as_string` from routine_engine.rs and use
the identical public `json_value_as_filter_string` from routine.rs,
eliminating divergence risk. (Copilot review)
[skip-regression-check]
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]>
* fix: enable WASM credential injection in No-DB environments (#845)
* fix(wasm): enable credential injection in no-DB environments via env var fallback
When a secrets store is unavailable (e.g. no-DB mode), WASM channel
credentials were silently not injected, causing channels to start without
credentials. Fix by:
- Changing `inject_channel_credentials_from_secrets` to accept
`Option<&dyn SecretsStore>` — secrets store is tried first when present
- Adding env var fallback (`inject_env_credentials`) for credentials not
covered by the secrets store
- Enforcing a channel-name prefix security check on env var names to
prevent WASM channels from reading unrelated host credentials
(e.g. `AWS_SECRET_ACCESS_KEY`)
- Extracting pure `resolve_env_credentials` helper for testability
- Adding case-insensitive prefix matching for secrets store lookup
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* fix(wasm): inject credentials at startup when no secrets store (setup.rs path)
The startup path (setup_wasm_channels -> register_channel) was guarded by
`if let Some(secrets) = secrets_store`, so in No-DB mode credentials were
never injected and the channel started without them.
Fix by:
- Changing inject_channel_credentials to accept Option<&dyn SecretsStore>
- Always calling it (removing the if-let guard) — env var fallback runs
even when secrets_store is None
- Adding channel-name prefix security check to the env var fallback path
(e.g. TELEGRAM_ for channel "telegram"), consistent with manager.rs
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* fix(test): correct misleading comment on ICTEST1_UNRELATED_OTHER placeholder
* fix(wasm): guard against empty channel name in credential injection
An empty channel_name would produce prefix "_", allowing any env var
starting with "_" to pass the security check and be injected. Add an
early-return guard in resolve_env_credentials, inject_env_credentials,
and inject_channel_credentials. Add a test to cover this path.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
---------
Co-authored-by: lizican123 <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
* fix: promote to main (#878)
* fix: replace unsafe env::set_var with thread-safe inject_single_var in SIGHUP handler
Fixes race condition where SIGHUP handler modifies global environment variables
while other threads may be reading them via Config::from_env().
Changes:
- Replace unsafe { std::env::set_var() } with ironclaw::config::inject_single_var()
- Uses INJECTED_VARS mutex instead of unsafe global state modification
- All reads via optional_env() check the thread-safe overlay first
- Prevents data races between SIGHUP reload and concurrent config reads
Verification:
- All 2,787 tests pass
- Zero clippy warnings
- Code compiles successfully
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* fix: spawn webhook restart as background task to avoid blocking I/O across lock
Prevents holding Mutex lock during async I/O operations (TcpListener::bind,
task shutdown). The SIGHUP handler no longer blocks webhook processing during
listener restart.
Changes:
- Read old_addr and drop lock immediately
- Spawn restart_with_addr() as background task via tokio::spawn
- Lock is only held during the actual restart operation, not the signal handler
Benefits:
- SIGHUP handler returns immediately without blocking
- Webhook requests not delayed by listener restart I/O
- Lock contention significantly reduced
Verification:
- All 2,787 tests pass
- Zero clippy warnings
- Code compiles successfully
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* fix: add graceful shutdown mechanism for SIGHUP handler background task
Prevents unbounded loop without cancellation token. The SIGHUP handler now
listens for a shutdown signal and exits cleanly during graceful termination.
Changes:
- Create broadcast channel for shutdown signaling
- SIGHUP handler uses tokio::select! to wait for shutdown or SIGHUP
- Send shutdown signal to all background tasks after agent.run() completes
- Ensures clean task lifecycle and no orphaned background tasks
Benefits:
- Proper task cancellation during graceful shutdown
- Follows Tokio best practices for background task management
- No background tasks orphaned when runtime shuts down
Verification:
- All 2,787 tests pass
- Zero clippy warnings
- Code compiles successfully
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* refactor: replace stringly-typed parameter filtering with typed enum and single helper
Fixes DRY violation where unsupported parameter filtering was duplicated across
rig_adapter.rs and anthropic_oauth.rs using string contains checks.
Changes:
- Add UnsupportedParam typed enum in provider.rs (Temperature, MaxTokens, StopSequences)
- Create strip_unsupported_completion_params() helper function
- Create strip_unsupported_tool_params() helper function
- Update rig_adapter.rs to use shared helpers
- Update anthropic_oauth.rs to use shared helpers
- Replace 60+ lines of duplicate stringly-typed logic
Benefits:
- Type safety: parameter names checked at compile time
- Single source of truth: adding a new param updates one place
- Reduced maintenance burden: no duplicate logic to keep in sync
- Better code clarity: named enum variant is self-documenting
Verification:
- All 2,787 tests pass
- Zero clippy warnings
- Code compiles successfully
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* docs: clarify intentional parameter asymmetry between completion and tool requests
Add documentation explaining why strip_unsupported_tool_params does not handle
StopSequences: the field doesn't exist in ToolCompletionRequest.
Changes:
- Add clarifying comments to strip_unsupported_tool_params()
- Explain why StopSequences is only in CompletionRequest
- Note that ToolCompletionRequest only supports Temperature and MaxTokens
- Inline comment confirms no action needed for StopSequences
This addresses the appearance of incomplete implementation without changing logic,
as the asymmetry is intentional and correct (ToolCompletionRequest lacks the field).
Verification:
- All 2,787 tests pass
- Zero clippy warnings
- Code compiles successfully
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* perf: isolate webhook_secret to reduce lock contention on hot path
Move webhook_secret from shared HttpChannelState RwLock into its own Arc<RwLock<>>.
This eliminates contention between secret validation and other state operations.
Changes:
- Change webhook_secret field type from RwLock<Option<SecretString>> to Arc<RwLock<Option<SecretString>>>
- Update initialization in HttpChannel::new()
- Update comments to explain isolation rationale
Benefits:
- Reduce lock contention on webhook request hot path (secret validation)
- Rarely-changing field (SIGHUP only) isolated from frequent state accesses
- Other state operations (tx, pending_responses) no longer wait behind secret reads
- Minimal code change: only field declaration and initialization
The Arc wrapper allows cloning the RwLock handle to separate concerns. With this
change, every webhook request acquires its own isolated lock for secret validation,
not the shared HttpChannelState lock. This scales better under high request volume.
Verification:
- All 2,787 tests pass
- Zero clippy warnings
- Code compiles successfully
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* fix: prevent partial state corruption on SIGHUP restart failure
Ensure atomicity of configuration reload: if webhook listener restart fails,
secret update is skipped to prevent inconsistent state.
Changes:
- Wait for restart_with_addr() to complete (don't spawn background task)
- Track restart result with restart_failed flag
- Only update secret if restart succeeded or wasn't needed
- Ensure listener and secret stay synchronized
Problem addressed:
- Before: restart spawned as background task, secret updated immediately
- If restart failed, secret was changed but listener still on old address
- This left system in inconsistent state (partial corruption)
Solution:
- Make restart blocking (SIGHUP handler can wait, it's not on request hot path)
- Atomically update secret only after successful restart
- Flag prevents race between restart and secret update
Benefits:
- Configuration changes are atomic (both succeed or both fail together)
- No partial state corruption on restart failure
- Failed restarts don't silently leave inconsistent state
- Secret and listener address stay in sync
Verification:
- All 2,787 tests pass
- Zero clippy warnings
- Code compiles successfully
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* refactor: generalize hot-secret-swapping with ChannelSecretUpdater trait
Decouple SIGHUP handler from HTTP channel internals by introducing a trait
for channels that support zero-downtime secret updates.
Changes:
- Add ChannelSecretUpdater trait in channels/channel.rs
- Implement ChannelSecretUpdater for HttpChannelState
- Export trait from channels module
- Update SIGHUP handler to use trait-based secret updater collection
- Replace explicit HTTP channel knowledge with generic updater loop
Benefits:
- SIGHUP handler no longer depends on HttpChannelState details
- Tight coupling removed: main.rs doesn't need HTTP channel imports
- Extensible: new channels can opt-in by implementing the trait
- Scalable: multiple channels supported without main.rs changes
- Maintainable: adding channels requires only trait implementation, not SIGHUP handler edits
Pattern:
- ChannelSecretUpdater trait defines the interface for all updaters
- Channels that support hot-secret-swapping implement the trait
- SIGHUP handler loops through all registered updaters generically
Verification:
- All 2,787 tests pass
- Zero clippy warnings
- Code compiles successfully
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* feat: validate parameter names at deserialization time, not just tests
Add custom serde deserializer for unsupported_params that validates parameter
names at runtime when loading providers.json (or user overrides).
Changes:
- Add unsupported_params_de module with custom deserializer
- Only allows: "temperature", "max_tokens", "stop_sequences"
- Invalid parameter names cause immediate deserialization error
- Update ProviderDefinition to use custom deserializer
- Enhanced test with explicit parameter name validation
- Add new test that verifies invalid parameters are rejected
Problem solved:
- Before: Invalid param names (e.g., "temperrature") silently ignored
- Now: Rejected at deserialization time with clear error message
- Prevents runtime failures caused by typos in configuration
Example error:
unsupported parameter name 'temperrature': must be one of: temperature, max_tokens, stop_sequences
Benefits:
- Fail-fast: errors caught when loading config, not at runtime
- Clear feedback: error message lists valid parameter names
- Type safety: validators run during deserialization
- Configuration errors detected immediately, not silently ignored
Verification:
- All 2,788 tests pass (including new validation test)
- Zero clippy warnings
- Code compiles successfully
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
---------
Co-authored-by: Claude Haiku 4.5 <[email protected]>
* 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]>
* Revert "Feat/docker shell edition" + fix fmt/clippy (#886)
* Revert "Feat/docker shell edition (#804)"
This reverts commit
|
||
|
|
76375f2eaa |
refactor: centralize test credential constants into testing::credentials (#829)
* refactor: centralize test credential constants into testing::credentials Scattered test credential strings (API keys, OAuth tokens, crypto keys, Telegram tokens, session tokens) across ~25 files made security auditing harder and created unnecessary duplication. Centralize all test-only fake credentials into a new `src/testing/credentials.rs` module with named constants and a shared `test_secrets_store()` helper. - Convert `src/testing.rs` to directory module (`src/testing/mod.rs`) - Add `src/testing/credentials.rs` with ~30 named constants - Replace hardcoded literals in 24 source files - Deduplicate `test_store()` helper (was copy-pasted in 3 files) - Leave leak_detector/shell/signature tests as-is (inline values aid readability for pattern detection tests) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: replace real Telegram bot token with obviously fake test stub Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * Update src/testing/credentials.rs Co-authored-by: Copilot <[email protected]> * Update src/testing/credentials.rs Co-authored-by: Copilot <[email protected]> * refactor: address PR review feedback on test credentials - Fix TEST_CRYPTO_KEY doc comment ("32-byte hex" → "32-character key string") - Rename confusing "real"/"fake" Anthropic constant names and values - Change TEST_STRIPE_KEY from "sk-live" to "sk_test_fake123" to avoid scanners - Use test_secrets_store() helper in orchestrator and http tool tests - Clarify config_round_trip.rs doc comment about integration test visibility Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Copilot <[email protected]> |
||
|
|
3a2989d009 |
feat(setup): quick onboarding, preserve .env vars, clean up boot logging (#821)
* feat(setup): quick onboarding, preserve .env vars, clean up boot logging (#751, #674) - Add upsert_bootstrap_vars() to preserve user-added .env vars on re-onboarding - Add --quick mode: auto-defaults DB + security, asks only LLM provider (2 steps) - Auto-triggered onboarding uses quick mode for near-instant first run - Fix NEAR AI model fetch to use cloud-api.near.ai when API key is set - Handle missing WASM tools/channels directories gracefully - Downgrade all boot/shutdown tracing::info! to debug (boot screen shows user output) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(setup): gate env_backend variable behind postgres feature flag [skip-regression-check] Clippy lint fix — not a behavioral change, just moving a variable declaration inside the cfg(feature = "postgres") block where it's used. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(review): address PR review comments - WASM loaders: use tokio::fs::metadata, only treat NotFound as empty, propagate other IO errors, handle TOCTOU in read_dir - bootstrap: only ignore NotFound in read_to_string, propagate other errors - wizard: restore print_info/print_success for migrations in interactive mode (gated by !config.quick), keep tracing::debug for diagnostics - tests: use shared crate::config::helpers::ENV_MUTEX instead of separate NEARAI_ENV_MUTEX to prevent cross-test env var races - README: fix quick mode description to mention model selection, clarify auto_setup_database may prompt when DATABASE_URL is set Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(setup): skip prompts for DATABASE_URL in quick mode [skip-regression-check] auto_setup_database() now uses DATABASE_URL directly without calling step_database_postgres() (which prompts for confirmation). Quick mode should be fully non-interactive when env vars are already set. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(cli): update --quick help text to mention model selection [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
c3ce26278a |
refactor: simplify config resolution and consolidate main.rs init (#287)
* refactor: simplify config resolution and consolidate main.rs init into AppBuilder - Add parse_bool_env() and parse_string_env() helpers to eliminate repetitive 5-line optional_env/parse/map_err/unwrap_or boilerplate across 12 config files - Add EmbeddingsConfig::create_provider() to centralize embeddings construction (fixes hardcoded 1536 dimensions and missing Ollama provider in app.rs) - Extract init_cli_tracing(), setup_wasm_channels(), start_tunnel(), run_memory_command(), run_worker(), run_claude_bridge() from main.rs - Replace ~600 lines of inline init in main.rs with AppBuilder::build_all() - Expose catalog_entries from AppComponents for gateway registry entries - Net reduction: ~738 lines across 15 files Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: propagate dev_loaded_tool_names from AppBuilder and add parse_option_env helper Address PR review feedback: - Capture dev_loaded_tool_names from WASM loading in init_extensions() and expose via AppComponents so bootstrap_hooks receives the actual dev tool names instead of an empty slice (fixes silent hook skip) - Add parse_option_env<T>() helper for Option<T> config fields, simplifying max_cost_per_day_cents and max_actions_per_hour in agent.rs Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: fetch real NEAR AI pricing and unify cost calculation path CostGuard was independently looking up pricing via costs::model_cost(), falling back to GPT-4o default rates when NEAR AI model names didn't match the static table — causing ~3x cost overestimates in logs. - Add pricing map to NearAiChatProvider that fetches real rates from /v1/model/list at startup (background, non-blocking) - Update cost_per_token() to check fetched pricing first, then static table, then default - Add cost_per_token parameter to CostGuard::record_llm_call() so the dispatcher passes provider-sourced rates directly Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: update default NEAR AI model to GLM-latest Replace fireworks llama4-maverick-instruct-basic with zai-org/GLM-latest as the default model in config and setup wizard. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: align wizard default model name with config Change "zai/GLM-latest" to "zai-org/GLM-latest" in wizard.rs to match the default in config/llm.rs. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
3829d81269 |
fix: consolidate per-module ENV_MUTEX into crate-wide test lock (#246)
Each config test module (llm.rs, embeddings.rs) defined its own ENV_MUTEX, which doesn't prevent cross-module env races since cargo test runs in parallel. Move to a single shared mutex in config/helpers.rs so all unsafe set_var/remove_var calls are serialized crate-wide. Closes #245 Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
097a26ace6 |
fix: harden openai-compatible provider, approval replay, and embeddings defaults (#237)
* fix: harden openai-compatible tool flow and local defaults * fix: close approval replay gaps and harden openai-compatible flow * fix: address review feedback and code improvements (takeover #112) - Make ChatCompletionResponse.id Optional<String> to handle providers that omit or null the field - Propagate HTTP client builder errors instead of silently dropping timeout configuration (openai_compatible_chat, nearai_chat) - Add EMBEDDING_DIMENSION env var with smart per-model defaults instead of hardcoding 768/1536 everywhere - Remove duplicated dimension inference logic from main.rs Co-Authored-By: panosAthDBX <[email protected]> Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: harden src/llm/ module from crate audit findings - Replace 9x .expect() on RwLock with graceful poison recovery (nearai.rs: 7, nearai_chat.rs: 2) — eliminates production panics - Propagate HTTP client builder errors in nearai.rs instead of silently dropping timeout config (NearAiProvider::new now returns Result) - Make nearai_chat ChatCompletionResponse.id Optional<String> (mirrors openai_compatible_chat.rs fix for providers that omit id) - Make nearai_chat usage fields optional with defensive parse_usage() helper (was required u32 fields that crash on null/missing) - Truncate error responses to 512 chars in nearai_chat.rs error messages to prevent log bloat and potential data leakage - Delegate 4 missing LlmProvider methods in FailoverProvider (model_metadata, seed_response_chain, get_response_chain_id, calculate_cost) to last-used provider instead of trait defaults Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor(llm): add RetryProvider, remove openai_compatible_chat, harden decorators - Add composable RetryProvider decorator wrapping any LlmProvider with exponential backoff + jitter, respecting RateLimited retry_after hints - Remove openai_compatible_chat.rs — replaced by rig adapter + RetryProvider - Remove internal retry loop from nearai.rs (was causing double-retry with external RetryProvider, up to 16 attempts instead of 4) - Remove internal retry loop from nearai_chat.rs (same issue) - Wire RetryProvider into main.rs composition chain: each provider gets its own retry wrapper before failover - Move normalize_tool_name to rig_adapter.rs for all rig-based providers - Reconcile is_retryable() vs is_transient() error classification: ModelNotAvailable no longer retryable, Json no longer transient - Fix unchecked Duration subtraction panic in circuit_breaker.rs - Make failover.rs use shared is_retryable() from retry.rs - Remove stale #[allow(dead_code)] on NearAiResponse::id (field is used) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review feedback — error handling, dimension validation, libSQL warning - Replace response.text().await.unwrap_or_default() with proper error propagation in nearai.rs and nearai_chat.rs (4 call sites). Failures now return LlmError::RequestFailed with context instead of silently proceeding with an empty string. - Add embedding dimension validation in OllamaEmbeddings::embed_batch(): returns EmbeddingError if Ollama returns embeddings with a dimension that doesn't match the configured value. - Add runtime warning when libSQL backend is used with non-1536 embedding dimension, since the libSQL schema uses F32_BLOB(1536) and cannot store different-dimension vectors. Co-Authored-By: Claude Opus 4.6 <[email protected]> * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: panosAthDbx <[email protected]> Co-authored-by: panosAthDBX <[email protected]> Co-authored-by: panosAthDBX <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Copilot <[email protected]> |
||
|
|
ffb1cc9be8 |
refactor: architecture improvements for contributor velocity (#198)
* refactor: split large files and consolidate test stubs for contributor velocity - Extract 7 Database sub-traits (ConversationStore, JobStore, SandboxStore, RoutineStore, ToolFailureStore, SettingsStore, WorkspaceStore) with Database as a supertrait combining them all - Split libsql_backend.rs (2769 lines) into src/db/libsql/ directory with one file per sub-trait implementation - Split config.rs (1753 lines) into src/config/ directory with 16 domain files - Consolidate 3 duplicate test LLM stubs into shared StubLlm in src/testing.rs - Split server.rs handlers into src/channels/web/handlers/ directory - Extract main.rs init phases into AppBuilder (src/app.rs) - Add developer setup script (scripts/dev-setup.sh) Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: move heartbeat test from examples/ to tests/ Convert standalone example binary into a proper #[ignore] integration test, matching the convention of the other integration tests. Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix rustfmt formatting for CI Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review comments from Copilot - tunnel.rs: replace .ok().flatten() with ? to propagate env var errors - secrets.rs: remove misleading "process-wide cache" comment - database.rs: use uppercase "DATABASE_URL" in error key - testing.rs: gate harness tests with #[cfg(feature = "libsql")] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Illia Polosukhin <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> |