mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
7008e9a881
* feat(db): add per-user CachedSettingsStore decorator SettingsStore methods hit the database on every call. The v2 engine path (effect_adapter) and the dispatcher's per-turn tool permission loading both called get_all_settings() without caching, adding unnecessary DB round-trips on every agentic loop iteration. Add a write-through CachedSettingsStore decorator that caches get_all_settings() results per user_id. Write operations (set_setting, delete_setting, set_all_settings) delegate to the inner store then invalidate that user's cache entry. The write lock is held across DB loads to prevent stale-data races from concurrent invalidations. Wire the cache into TenantScope via a new settings_store field on AgentDeps, so all settings reads in the agent loop go through the cache. Remove the per-turn cached_tool_permissions Mutex hack from ChatDelegate that was working around the missing cache layer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR review feedback - Store Arc<HashMap> in cache instead of bare HashMap to avoid cloning the full settings map on every cache hit. get_setting/has_settings now only clone the single requested value or check emptiness through the Arc. - Route get_setting_with_admin_fallback() through self.settings() instead of self.inner so both the per-user and admin lookups go through the cache. - Update settings section comment to accurately describe which methods delegate through settings(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(gate): persist "always approve" decisions to DB in v2 engine path The v2 engine's resolve_gate() only stored "always approve" decisions in-memory via EffectBridgeAdapter::auto_approve_tool(), losing them on process restart. The v1 path (thread_ops.rs) already persisted to DB. Add persist_always_allow() and revert_always_allow() helpers that write tool_permissions.{name} = AlwaysAllow to the SettingsStore, preferring the CachedSettingsStore for write-through cache invalidation. Includes defense-in-depth: tools declaring ApprovalRequirement::Always are never persisted regardless of what the client sends. Reverts the DB write if the resumed tool execution fails. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: remove no-op test flagged in PR review Remove test_single_approval_does_not_persist — it only asserted an empty store was empty without exercising any production code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(gate): address PR review — security and correctness fixes - Use pending.parameters (not empty json) for defense-in-depth check so param-dependent tools like shell correctly detect Always requirement - Validate action_name with is_valid_admin_tool_name() before persisting to prevent settings key injection via dots or special characters - Save pre-existing permission value before overwriting; restore it on revert instead of blindly deleting (preserves long-standing prefs) - Replace serde_json::to_value().unwrap_or() with json!("always_allow") - Add tests: prior-value restoration, invalid tool name rejection, settings_store=None fallback to state.db Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(gate): downgrade warn! to debug! in persist/revert paths Internal diagnostics in persist_always_allow and revert_always_allow used tracing::warn!, which corrupts the REPL/TUI per CLAUDE.md logging rules. Downgraded all 5 call sites to debug!. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(gate): address ilblackdragon + serrrfirat review feedback - Upgrade persist/revert failure logs from debug! to warn! — DB persistence failures are security-relevant (user believes preference is permanent but it silently vanishes on restart). Matches v1 pattern at thread_ops.rs:1256. Safe in v2 (web gateway, not TUI). - Fix serialization drift: use serde_json::to_value(PermissionState:: AlwaysAllow) instead of hardcoded json!("always_allow"), coupling to the enum's serde rename attribute. - Add dispatch-exempt comments on direct set_setting/delete_setting calls per .claude/rules/tools.md. - Add #[cfg(feature = "libsql")] gate to test_persist_falls_back_to_ state_db — test_db() requires the libsql feature. - Update ApprovalGate docstring: v2 persistence is now wired, not aspirational. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(e2e): add Playwright persistence happy-path test (#2485) Add 3 e2e tests for always-approve persistence: - test_always_approve_persists_to_db: verifies DB row after "always" - test_revoke_always_approve_updates_db: verifies PUT revocation - test_always_approve_survives_restart: restartable server, verifies auto-approve persists across process restart Fix: remove raw Database fallback from persist/revert_always_allow. The state.db fallback bypassed CachedSettingsStore cache invalidation, causing GET /api/settings/tools to serve stale data until the 5-min TTL expired. In production agent.deps.settings_store is always available when the DB is; the fallback was dead code that broke cache coherence. Also: unit test for Settings::from_db_map tool_permissions parsing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(gate): downgrade always when allow_always is false A crafted client could send always:true on a gate where the pending ResumeKind had allow_always:false (e.g. ApprovalRequirement::Always tools). The in-memory auto_approve_tool would be set, silently bypassing future approval prompts. persist_always_allow already guarded against this via the ApprovalRequirement::Always check, but the in-memory path did not. Now resolve_gate downgrades always to false when the pending gate's resume_kind doesn't permit it, before touching either the in-memory set or the DB. Also: fix ApprovalGate docstring to distinguish persistence from hydration per Copilot review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>