Commit Graph
6 Commits
Author SHA1 Message Date
716629809c fix: eliminate panic paths in production code (#1184)
* fix: eliminate panic paths in production code and document infallible operations

PolicyRule::new() now returns Result instead of panicking on invalid
caller-supplied regex. CreateJobTool returns ToolError when job_manager
is unconfigured instead of panicking. Remaining infallible unwrap/expect
calls (hardcoded regexes, compile-time constants, guarded accesses)
are annotated with SAFETY comments. Where possible, unwraps are replaced
with safer patterns: split_last(), if-let, match-destructure, and
reusing peek() values.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix: use inline lowercase safety comments to match CI pattern

The no-panics CI check greps for '// safety:' (lowercase, inline)
to suppress false positives. Switch from block SAFETY comments to
inline safety comments on the .unwrap() lines.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* test: add regression tests for panic-path fixes

- PolicyRule::new returns Err on invalid regex (not panic)
- CreateJobTool::execute_sandbox returns ToolError when job_manager is None

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix: add inline // safety: comments on all infallible unwrap/expect lines

The CI no-panics check requires '// safety:' on the same line as
unwrap()/expect() to suppress false positives. Move safety annotations
from block comments to inline comments on every infallible production
unwrap/expect across all touched files.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* chore: trigger CI with skip-regression-check label

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* refactor: remove redundant block-level SAFETY comments

Each unwrap/expect now carries its own inline // safety: annotation,
making the standalone block comments above them redundant.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-15 03:17:03 +00:00
1f5b582c5f fix: agent logging (#888)
* fix: optimize agent logging to reduce DataDog bill

* fix: log permanent repair failures as ERROR not WARN

RepairResult::Failed is permanent failure requiring attention (ERROR level)
not a temporary/retryable condition (WARN level).

[skip-regression-check]

Co-Authored-By: Claude Haiku 4.5 <[email protected]>

* security: remove user message content from trace logs

Never log user message content at any log level (includes TRACE).
Log only safe metadata: content length, message ID, image count.

This prevents accidental exposure of sensitive user data in logs
even at the most verbose logging level.

[skip-regression-check]

Co-Authored-By: Claude Haiku 4.5 <[email protected]>

* security: move LLM response body logging to TRACE level

Response bodies can contain user-generated content, tool outputs, and
leaked secrets. Moving to TRACE (not enabled in production) prevents
exposure in DEBUG logs. Status log remains at DEBUG.

[skip-regression-check]

Co-Authored-By: Claude Haiku 4.5 <[email protected]>

* refactor: simplify URL sanitization using url::Url API

Use set_query, set_fragment, set_username, set_password methods
instead of manual string reconstruction. Cleaner, handles edge cases,
eliminates port branching complexity.

[skip-regression-check]

Co-Authored-By: Claude Haiku 4.5 <[email protected]>

* test: add comprehensive unit tests for sanitize_url_for_logging

Add 9 test cases covering:
- URL with query parameters
- URL with credentials (user:pass@host)
- URL with fragment
- URL with port
- URL with all components combined
- Malformed URL fallback behavior
- Short strings (pass-through)
- Non-URL-like strings
- Path preservation

Tests verify that sanitization correctly removes sensitive components
while preserving safe components like host, port, and path.

Co-Authored-By: Claude Haiku 4.5 <[email protected]>

* fix: libsql per-migration logs should be DEBUG, not TRACE

Individual migration logs are now visible with standard debug logging
(RUST_LOG=ironclaw=debug), improving debuggability when troubleshooting
migration issues. Summary log remains at INFO level.

Fixes behavioral change that made it harder to identify which specific
migration ran or failed without enabling full TRACE logging.

[skip-regression-check]

---------

Co-authored-by: Claude Haiku 4.5 <[email protected]>
2026-03-10 13:55:06 -07:00
14aadd3063 refactor: make src/llm/ self-contained for crate extraction (#767)
* refactor: make src/llm/ self-contained for crate extraction

Move LlmError, LLM config types, and OAuth callback helpers into
src/llm/ so the module has zero `use crate::` imports outside of
crate::llm. This prepares the module for extraction into a standalone
workspace crate.

- Move LlmError enum from src/error.rs to src/llm/error.rs
- Move LlmConfig, NearAiConfig, RegistryProviderConfig, BedrockConfig,
  CacheRetention, OAUTH_PLACEHOLDER from src/config/llm.rs to
  src/llm/config.rs
- Move OAuth callback utilities (callback_url, bind_callback_listener,
  wait_for_callback, landing_html, etc.) from src/cli/oauth_defaults.rs
  to src/llm/oauth_helpers.rs
- Remove session.rs dependency on crate::bootstrap (inline default path)
- Add cache_retention field to RegistryProviderConfig, resolve from env
  in config/llm.rs instead of reading env var in llm/mod.rs
- Add Check 6 to scripts/check-boundaries.sh enforcing LLM isolation
- All original locations re-export for backward compatibility

[skip-regression-check]

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

* style: fix formatting

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

* fix: address PR #767 review — session path bug and boundary check

1. Fix SessionConfig::default() usage in setup wizard: the fallback at
   wizard.rs:995 now constructs SessionConfig with the real
   default_session_path() instead of a relative "session.json", which
   would write auth tokens to the CWD instead of ~/.ironclaw/.

2. Widen check-boundaries.sh Check 6 to catch all `crate::` references
   (not just `use crate::` imports). Pre-existing inline references
   (16 occurrences) are reported as warnings; only new `use crate::`
   imports are hard violations.

[skip-regression-check]

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

* fix: address PR #767 review and audit findings in src/llm/

PR review fixes:
- Reject wildcard addresses (0.0.0.0, ::) in OAuth callback listener
  to prevent session token exposure on all interfaces
- Fix boundary check comment-stripping that could hide real violations
  (use sed to strip inline comments before matching)

Audit fixes:
- Fix UTF-8 byte-index slicing panic in recording.rs hint extraction
- Add effective_model_name() delegation to RetryProvider and
  SmartRoutingProvider for consistency with other wrappers
- Add calculate_cost() delegation to CachedProvider and RecordingLlm
- Deduplicate retry loop logic in RetryProvider via generic helper
- Replace hardcoded /tmp path in recording tests with tempfile

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

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-09 22:31:17 +00:00
424a0366a9 feat: enable Anthropic prompt caching via automatic cache_control injection (#660)
* feat(llm): add Anthropic prompt caching and cache token tracking

- Inject cache_control via additional_params for Claude models in rig_adapter
- Add cache_read_input_tokens and cache_creation_input_tokens to
  CompletionResponse and ToolCompletionResponse
- Extract cached_input_tokens from rig-core unified Usage
- Add is_anthropic_model() detection helper with provider prefix support
- Log prompt cache hits at debug level (consistent with response_cache)
- Add 7 unit tests for cache injection and model detection
- Update all mock providers and test fixtures with new fields

* feat(cost): apply 90% cache discount to prompt-cached tokens in CostGuard

- Add cache_read_input_tokens to TokenUsage so cache counts flow from
  CompletionResponse through the reasoning layer to the dispatcher
- Update CostGuard::record_llm_call() to accept cache_read_input_tokens:
  cached tokens are billed at 10% of the normal input rate
- Thread cache_read_input_tokens from dispatcher into CostGuard
- Add test_cache_discount_reduces_cost verifying exact savings match
  90% of input cost for fully-cached requests
- Update all existing test callers with zero-cache parameter

* refactor(cache): scope cache_control to Anthropic backend and validate model support

- Replace model-name-based is_anthropic_model() with explicit
  enable_prompt_cache flag on RigAdapter, set only for the direct
  Anthropic backend via with_prompt_cache(true)
- Add supports_prompt_cache() to validate model names per Anthropic
  docs: only Claude 3+ models support caching; claude-2 and
  claude-instant are excluded to prevent 400 errors
- Warn when caching is enabled but model does not support it
- Replace is_anthropic_model tests with flag-based and model
  validation tests

* fix(cache): validate model at construction and propagate cache metrics through proxy

- Move supports_prompt_cache() check into with_prompt_cache() so
  unsupported models are detected once at construction, not per request
- Add cache_read_input_tokens and cache_creation_input_tokens to
  ProxyCompletionResponse and ProxyToolCompletionResponse with
  serde(default) for backward compatibility
- Pass cache metrics through orchestrator proxy instead of zeroing
- Use claude-opus-4-6 in cache discount test to match Anthropic
  semantics

* feat(llm): add configurable cache retention with write surcharge

- Add CacheRetention enum (none/short/long) to AnthropicDirectConfig
- Parse ANTHROPIC_CACHE_RETENTION env var (default: short)
- Inject TTL-aware cache_control (short=5m ephemeral, long=1h)
- Extract cache_creation_input_tokens from raw Anthropic response
- Add cache_write_multiplier() to LlmProvider trait (1.25x short, 2.0x long)
- Pipe dynamic write multiplier through dispatcher to CostGuard
- Add TokenUsage.cache_creation_input_tokens field
- Add tests for Long TTL injection, 5m and 1h write surcharges
- Document ANTHROPIC_CACHE_RETENTION in .env.example

* docs: fix stale cache_retention field comment

* fix: resolve CI failures after upstream merge

- Add missing cost_per_token arg to cache test callsites
- Apply cargo fmt to long lines in tests and tracing macros

* fix: address Copilot review feedback

- Use saturating_add for cache token sum to prevent u32 overflow
- Tighten supports_prompt_cache to explicitly match claude-3+/claude-4+
  and named families (claude-sonnet/claude-opus/claude-haiku)

* fix: adapt prompt caching to registry architecture and add missing cache fields

- Resolve merge conflicts: adapt CacheRetention and cache injection to
  the declarative provider registry (RegistryProviderConfig replaces
  AnthropicDirectConfig)
- Parse ANTHROPIC_CACHE_RETENTION env var in create_anthropic_from_registry()
- Use Anthropic automatic caching via top-level cache_control in
  additional_params (rig-core #[serde(flatten)] places it at request root)
- Add cache_read/creation_input_tokens fields to all mock LlmProviders
  added on main after PR #291 branched (response_cache, dispatcher,
  provider_chaos, trace_llm)
- Suppress clippy::too_many_arguments on record_llm_call and
  build_rig_request
- Add regression tests for cache injection (short/long/none) and
  cache_write_multiplier values

Co-Authored-By: Canvinus <[email protected]>

* fix: delegate cache_write_multiplier through provider wrappers and make cache_read_discount configurable

The 6 decorator providers (Retry, CircuitBreaker, Failover, SmartRouting,
CachedProvider, RecordingLlm) did not delegate cache_write_multiplier()
to their inner provider, causing it to always return 1.0 instead of the
actual 1.25x/2.0x from RigAdapter. This fix adds delegation for both
cache_write_multiplier() and the new cache_read_discount() method.

Also makes the cache read discount per-provider instead of hardcoding
Anthropic's 90% discount (÷10). OpenAI uses 50% (÷2), so the discount
is now returned by each provider via the LlmProvider trait.

Addresses review feedback on PR #660.

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

* style: cargo fmt

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

* test: add CacheRetention FromStr/Display unit tests

Tests cover primary values, aliases (off/disabled/5m/ephemeral/1h),
case-insensitivity, invalid input error, and Display round-trip.

Addresses Copilot review feedback on PR #660.

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

---------

Co-authored-by: Andrey <[email protected]>
Co-authored-by: Andrey Gruzdev <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-07 09:10:05 +00:00
69cddb10fd feat: integrate 13-dimension complexity scorer into smart routing (#529)
* feat(llm): add smart model routing based on request complexity

Automatically selects optimal model tier (flash/standard/pro/frontier) for each
request based on 13-dimension complexity scoring:

- Reasoning words, multi-step signals, code indicators
- Domain-specific terms, creativity, precision
- Safety sensitivity, tool likelihood, question complexity
- Token estimate, context dependency, sentence complexity

Features:
- Pattern overrides for fast-path routing (greetings → flash, security audits → frontier)
- Configurable tier-to-model mappings (defaults to -latest aliases)
- Thinking mode per tier (pro: low, frontier: medium)
- User-configurable pattern overrides
- Zero-config for default benefits, full control for power users

Expected cost savings: 50-70% vs always-using-frontier baseline.

Refs: smart-routing-spec.md

* fix(routing): address Gemini Code Assist review feedback

- Add tracing warnings for invalid tier/regex in user overrides (router.rs)
- Use unreachable!() for tier hint match since regex enforces valid tiers (scorer.rs)
- Refactor weighted total to array iteration for maintainability (scorer.rs)
- Add TODO for making domain keywords configurable (scorer.rs)

Refs: PR #208

* feat(routing): make domain keywords configurable

- Add ScorerConfig with optional domain_keywords field
- Add DEFAULT_DOMAIN_KEYWORDS constant (exported for reference)
- Add domain_keywords to RouterConfig for top-level configuration
- Build domain regex at runtime from config, fallback to defaults
- Add score_complexity_with_config() function
- Add test for custom domain keywords

Users can now provide project-specific keywords:

  RouterConfig {
      domain_keywords: Some(vec!["mycompany".into(), "myproduct".into()]),
      ..Default::default()
  }

Addresses Gemini Code Assist review feedback on PR #208.

Tests: 20/20 passing

* docs: add domain_keywords to routing config example

* feat: integrate 13-dimension complexity scorer into smart routing (takeover #208)

Folds the 13-dimension complexity scorer and pattern overrides from PR #208
into the existing SmartRoutingProvider, replacing the simpler keyword-based
classifier. Adds 4-tier system (Flash/Standard/Pro/Frontier), configurable
scorer weights, domain keywords, regex pattern overrides, tier hints, and
multi-dimensional boost. Removes separate routing/ directory and lazy_static
dependency in favor of std::sync::LazyLock. Includes 44 tests covering all
scoring dimensions, tier boundaries, pattern overrides, and provider routing.

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

* fix: address review feedback on smart routing PR (#529)

- Cache compiled domain regex in SmartRoutingProvider (built once at
  construction, not per-request) and add score_complexity_with_regex() API
- Check explicit tier hints before pattern overrides so user intent wins
  (e.g. "[tier:flash] security audit" routes as Flash, not Frontier)
- Trim input before matching/scoring so trailing whitespace doesn't break
  anchored override regexes or skew token-length scoring
- Fix token estimate comment (>=520 chars = 100, not >500)
- Update spec: check implementation plan boxes, fix file paths, add note
  that llm.routing YAML schema is target design (current config uses env vars)
- Add regression tests for tier hint precedence and trimmed greeting matching

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

* fix: restore Cargo.lock from main to fix html_to_markdown test

The lockfile was fully regenerated during the PR #208 merge conflict
resolution, which bumped html-to-markdown-rs from 2.25.1 to 2.27.2.
The new version produces different output that breaks the golden-file
snapshot test. Restore the original lockfile from main — lazy_static
was never in main's lockfile, so no further changes needed.

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

* fix: address second round of review feedback (#529)

- Tighten quick-lookup override regex with end anchor to prevent matching
  complex questions like "What time complexity is merge sort?"
- Handle empty domain keywords list by falling back to defaults instead of
  producing a broken regex that matches empty strings everywhere
- Clarify spec architecture diagram: current impl uses 2-provider split
  (cheap/primary), per-tier model mapping is target design
- Add regression tests for both fixes

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

---------

Co-authored-by: Microwave <[email protected]>
Co-authored-by: Joe <[email protected]>
Co-authored-by: onlyamicrowave <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-05 09:14:07 +00:00
c038c7705b feat: add smart routing provider for cost-optimized model selection (#281)
* feat: add smart routing provider for cost-optimized model selection

Route simple tasks (greetings, status checks, short questions) to a cheap
model (e.g. Haiku) and complex tasks (code generation, analysis) to the
primary model, reducing agent costs without sacrificing quality.

Activates automatically when NEARAI_CHEAP_MODEL is set. Cascade mode
retries uncertain cheap-model responses with the primary model.

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

* style: apply cargo fmt formatting

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

* refactor: extract provider chain into shared build_provider_chain()

Consolidate the duplicated LLM provider chain construction from main.rs
and app.rs into a single build_provider_chain() function in llm/mod.rs.

This fixes the inconsistency where app.rs was missing retry wrapping
that main.rs had, and ensures both paths apply identical decorators:
retry → smart routing → failover → circuit breaker → cache.

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

* fix: address PR review — uncertainty detection and clippy lint

- Remove false-positive short response (<20 chars) uncertainty check
  that would escalate "Yes.", "42" etc. Now only empty responses and
  explicit uncertainty phrases trigger cascade escalation.
- Add #[allow(clippy::type_complexity)] to build_provider_chain() to
  fix CI clippy -D warnings failure.

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

---------

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