mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
a580c1d75fee342eb8ac54981036528b20cfec75
16
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
8a26cfae73 |
fix(mcp): open MCP OAuth in same browser as gateway (#951)
* fix(mcp): use gateway callback for MCP OAuth so auth opens in same browser When MCP OAuth is triggered from the web gateway, the auth URL was being opened via `open::that()` which launches the OS default browser instead of the browser already running the gateway UI. This changes the MCP OAuth flow to use the same gateway callback pattern as WASM extensions: in gateway mode, the auth URL is returned to the frontend via SSE and opened with `window.open()`, keeping the user in the same browser. Also adds RFC 8707 `resource` parameter support to the gateway token exchange path, scoping issued tokens to the correct MCP server. Closes #299 Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: cargo fmt Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(mcp): persist DCR client_id in gateway OAuth callback for token refresh The gateway callback handler stored access and refresh tokens but not the DCR client_id. When the token expired, refresh failed with "No client ID found" because get_client_id() could not find it in secrets. Adds client_id_secret_name to PendingOAuthFlow so the gateway callback handler persists the client_id alongside the tokens, matching the behavior of the CLI flow in authorize_mcp_server(). Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(mcp): return AuthRequired on 401 so activate triggers OAuth flow activate_mcp() returned ActivationFailed for all errors including 401 auth responses, so the activate handler never triggered the OAuth flow. Now 401/auth errors return AuthRequired, which the handler detects and redirects to the OAuth flow — matching the WASM extension pattern. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(mcp): fix gateway OAuth flow, approval cards, and auto-activation - Add explicit gateway_mode flag on ExtensionManager (set at startup by web gateway) so MCP OAuth returns auth URLs to the frontend instead of calling open::that() on the server machine. - Auto-activate extensions after successful OAuth callback so the UI transitions from "Activate" to "Active" without a second click. - Send ApprovalNeeded status (not generic "Awaiting approval") from thread_ops.rs for all three NeedApproval paths so the web UI shows approval cards for deferred tool calls. - Remove duplicate ApprovalNeeded send from agent_loop.rs (thread_ops.rs is now the canonical sender). - Skip approval for tool_auth in gateway mode since it only returns a URL. - Revert fragile active-server detection heuristic from system prompt. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review findings - Use Release/Acquire ordering for gateway_mode AtomicBool instead of Relaxed to ensure visibility across threads. - Report activation failure as error in OAuth callback SSE event instead of silently falling back to the success message. - Fix EnvGuard::drop to remove env var when original was unset. - Replace hardcoded /tmp/ path with std::env::temp_dir() in test helper. Co-Authored-By: Claude Opus 4.6 <[email protected]> * test(mcp): add E2E trace test for MCP extension lifecycle with mock server Add a full MCP extension lifecycle E2E test that exercises: - Turn 1: tool_search → tool_install → text (extension discovery and install) - Token injection + activate (simulating OAuth completion) - Turn 2: MCP tool calls (notion-search → notion-fetch → text) Includes a mock MCP server (tests/support/mock_mcp_server.rs) with OAuth discovery, DCR, token exchange, and JSON-RPC endpoints. The mock server validates Bearer auth and serves pre-configured tool responses. Also adds inject_registry_entry() to ExtensionManager for test use and exposes extension_manager from TestRig. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review findings (round 2) - Only fall back to manual token entry on AuthNotSupported, propagate real errors from auth_mcp_build_url() instead of masking them - Use mcp:-prefixed provider string in PendingOAuthFlow for consistency with CLI MCP auth token storage - Only persist client_id_secret_name for DCR flows (not pre-configured OAuth) - Fix gateway_callback_redirect_uri to use /oauth/callback path - Bypass exchange proxy when flow has RFC 8707 resource parameter - Remove client_id double-prefix in oauth callback handler - Remove weak tests that didn't exercise production logic - Add clarifying comments for exchange_oauth_code delegation Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: keep OAuth success independent of activation, fix wait_for_responses scoping - OAuth success is now reported accurately even when auto-activation fails (tokens are already stored, so auth succeeded) - E2E test waits for turn1_count + 1 responses to ensure turn-2 behavior is actually observed Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
f05896fe6a |
Migrate GitHub webhook normalization into github tool (#758)
* Add event-triggered routines and workflow skill templates * Add generic host-verified webhook ingress for tools * Migrate GitHub webhook normalization into github tool * Bump github tool registry version * Stabilize trace E2E test rig and approval behavior * Add reusable gateway workflow harness with mock LLM server (#762) * Add reusable gateway workflow test harness with mock LLM server * Fix clippy issues in workflow harness * Stabilize trace E2E test rig and approval behavior * Address PR review feedback on gateway workflow harness - Extract shared TestChannelHandle into test_channel.rs with name override support, eliminating ~55 lines of duplication between test_rig.rs and gateway_workflow_harness.rs - Remove redundant RoutineEngine creation that was immediately overwritten by Agent::run() - Replace flaky sleep(500ms) with polling loop for routine run count check - Use components.context_manager instead of creating a fresh ContextManager for job tools, ensuring agent and tools share the same instance Co-Authored-By: Claude Opus 4.6 <[email protected]> * Fix import ordering in gateway_workflow_harness Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> * Address PR #758 review feedback - Fix header_value to use fully case-insensitive lookup (iterate with to_ascii_lowercase) instead of checking only exact/lower/upper variants - Change comment_id from u32 to u64 to handle GitHub's billion-range IDs - Remove handle_webhook from LLM-facing JSON schema to prevent direct invocation bypassing HMAC verification - Rename enrichment keys from repository/sender to repository_name/ sender_login to preserve original JSON objects in webhook payloads - Remove put_string_normalized helper (no longer needed) - Replace no-op tests (test_validate_event_in_create_pr_review, test_validate_merge_method) with test_header_value_case_insensitive - Add README docs for 6 undocumented actions (list_issue_comments, create_issue_comment, list_pull_request_comments, reply_pull_request_comment, get_pull_request_reviews, get_combined_status) - Add comment explaining max_tool_calls <= 8 bound in e2e test - Fix gateway workflow harness: add webhook_capability with secret auth to MockGithubWebhookTool, matching staging's hardened webhook security - Fix merge artifacts: remove duplicate test function, orphaned code fragment in e2e_routine_heartbeat [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * Fix formatting in gateway workflow harness Co-Authored-By: Claude Opus 4.6 <[email protected]> * Address Copilot review: filter keys, pr_number fallback, feature gate, version alignment - Update SKILL.md and workflow-routines.md templates to use `repository_name` and `sender_login` (matching enriched payload field names) - Mark webhook HMAC secret as required in SKILL.md prerequisites - Fall back to `/issue/number` for `pr_number` on issue_comment PR webhooks - Gate `gateway_workflow_harness` module behind `#[cfg(feature = "libsql")]` - Align tool version to 0.2.1 in Cargo.toml and capabilities.json Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
369741fc60 |
Add generic host-verified /webhook/tools/{tool} ingress (#757)
* Add generic host-verified webhook ingress for tools * Stabilize trace E2E test rig and approval behavior * Fix webhook security issues from review feedback - Reject tools without webhook_capability() (was unauthenticated RCE) - Remove secret-in-query-string fallback (leak via logs/referrers) - Require approval for event_emit tool (escalation via routine triggers) - Simplify header_value() (HeaderMap already case-insensitive) - Redact internal errors from webhook HTTP responses - Remove unused hmac_timestamp_tolerance_secs field - Add regression test for tool without webhook capability [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * Harden webhook ingress: require auth mechanism, body limit layer, health check - Reject webhook capabilities that declare no auth mechanism (empty WebhookCapability would previously allow unauthenticated access) - Add DefaultBodyLimit layer to reject oversized payloads before buffering - Health check (GET) now verifies tool has webhook_capability(), not just existence - Add regression tests for all three fixes [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * Fix auto_approve_tools inconsistency between dispatcher and thread_ops dispatcher.rs skips all approval checks (including Always) when auto_approve_tools is true, but thread_ops.rs still required approval for Always tools. This caused deferred tool calls to unexpectedly halt in test rigs and auto-approve configurations. Match dispatcher behavior: short-circuit all approval when auto_approve_tools is enabled. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
6e1ed939cc |
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]> |
||
|
|
0e04123188 |
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]> |
||
|
|
a868b14221 |
Fix/lightweight action tool (#785)
* feat: add tool execution support to lightweight routines
Lightweight routines now execute tools instead of outputting raw tool-call XML.
**Problem:** Lightweight routines had no tool execution loop, causing the LLM to generate
tool-call XML as text output (visible to users as garbage on Telegram). All 4 scheduled
routines were disabled and Emil saw the same issue in health-ping routine.
**Solution:** Implement a simplified agentic loop for lightweight routines that:
- Supports up to 3-5 tool iterations (configurable, capped at 5)
- Executes tools sequentially (not parallel, keeps overhead low)
- Auto-approves non-Always tools (lightweight routines are autonomous)
- Sanitizes and wraps tool outputs via SafetyLayer (same as dispatcher)
- Forces text-only response at iteration limit (guarantees termination)
- Maintains backward compatibility (disabled by default, toggled by config)
**Changes:**
1. **src/config/routines.rs:**
- Added lightweight_tools_enabled (default: true)
- Added lightweight_max_iterations (default: 3, capped at 5)
- Added env var support: ROUTINES_LIGHTWEIGHT_TOOLS, ROUTINES_LIGHTWEIGHT_MAX_ITERATIONS
2. **src/agent/routine_engine.rs:**
- Extended EngineContext with tools and safety fields
- Split execute_lightweight into three functions:
- execute_lightweight: router that dispatches to tool or no-tool version
- execute_lightweight_no_tools: original single-call behavior
- execute_lightweight_with_tools: new agentic loop with tool support
- Added execute_routine_tool: isolated tool execution with validation and timeout
- Uses ToolCompletionRequest/ToolCompletionResponse for tool-aware LLM calls
- Integrates SafetyLayer for tool output sanitization
3. **src/agent/agent_loop.rs:**
- Updated RoutineEngine::new call to pass tools and safety
**Tool Execution Loop:**
1. Build initial messages (system + user prompt)
2. Get tool definitions (empty at iteration limit)
3. Call LLM with ToolCompletionRequest
4. If text response: check for ROUTINE_OK sentinel, return result
5. If tool calls: execute sequentially, sanitize, wrap, add to context, loop
6. Safety ceiling at 5 iterations prevents runaway execution
**Approval Handling:** Auto-approves UnlessAutoApproved and Never tools;
blocks Always tools with error message (routines are autonomous by design).
**Testing:** All 2756 tests pass. Zero clippy warnings.
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* test: add comprehensive unit tests for lightweight routine tool execution
Added 9 new unit tests covering:
- Configuration defaults (lightweight_tools_enabled, lightweight_max_iterations)
- Max iterations capped at 5 (safety ceiling)
- Routine name sanitization (special chars, alphanumeric preservation)
- Sentinel detection for ROUTINE_OK (exact match, contains, whitespace handling)
- Iteration limit safety ceiling enforcement
- Approval requirement pattern matching (Never, UnlessAutoApproved, Always)
- Empty response handling (finish_reason detection)
All 2765 tests pass (11 routine_engine tests, +9 new).
The tests cover the core logic paths of:
- Configuration validation
- Response parsing and sentinel detection
- Name sanitization for workspace paths
- Approval requirement logic
- Iteration limits and safety ceilings
Note: These are unit tests for core logic. Full integration tests with mock LLM
and tool registry would require more complex test infrastructure and are a future enhancement.
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* style: format routine_engine.rs per cargo fmt
Apply consistent formatting to match Rust style guidelines:
- Break long import lines
- Reformat method chains for readability
- Format multi-line return tuples
No functional changes.
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* fix: address security and code quality issues in lightweight routine tool execution
**Security Fixes:**
1. Sanitize tool error messages (medium severity)
- Tool error messages were sent directly to LLM without sanitization
- Now wrapped through SafetyLayer like successful outputs
- Prevents leakage of API keys, internal paths, or PII from errors
2. Use unique job_id for each routine run (medium severity)
- Previously reused routine.id across all executions
- Caused state collisions and race conditions
- Now generates unique run_id (Uuid::new_v4()) for each execution
- Matches behavior of full_job routines
**Code Quality Fixes:**
3. Remove unreachable code
- Deleted dead if iteration > 5 check
- max_iterations is capped at 5 via .min(5), so check was impossible
- Improves code clarity
4. Extract duplicated response handling logic
- Created handle_text_response() helper function
- Eliminated 20+ lines of duplicated ROUTINE_OK sentinel detection
- Reduces maintenance burden and risk of inconsistencies
5. Fix test duplication
- Tests now call actual super::sanitize_routine_name()
- Removes duplicate implementation in tests
- Ensures tests detect changes to original function
**Testing:**
- All 2765 tests pass (no regressions)
- Zero clippy warnings
- Test coverage maintained
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* fix: address security issue and improve code quality in lightweight routine tool execution
**SECURITY FIX (High Severity):**
1. Block UnlessAutoApproved tools in lightweight routines
- Previously auto-approved UnlessAutoApproved tools, creating prompt injection vulnerability
- Lightweight routines can be triggered by external events (channel messages, webhooks)
- If susceptible to prompt injection, attacker could trick LLM into calling sensitive tools
- Now blocks both UnlessAutoApproved and Always tools (only Never tools allowed)
- Only safe approach without requiring tool_permissions allowlist in routine data model
- Prevents unauthorized file access, network requests, and other sensitive operations
**Code Quality Improvements:**
2. Use ToolError::Timeout for consistent error handling (medium)
- Changed from std::io::Error to proper ToolError::Timeout variant
- More idiomatic and consistent with tool execution error handling
- Makes errors easier to debug and handle uniformly
3. Fix misleading test names and remove tautological tests (medium)
- Renamed test_routine_config_lightweight_max_iterations_capped_at_five to
test_routine_config_can_hold_uncapped_max_iterations
- Clarified comments to explain where capping actually occurs
- Removed test_iteration_limit_safety_ceiling (tautological: asserts x.min(5) <= 5)
- Improves test clarity and prevents false sense of coverage
**Testing:**
- 2764 tests passing (1 test removed, no regressions)
- Zero clippy warnings
- Security vulnerability eliminated
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* style: format routine_engine.rs per cargo fmt
Apply consistent formatting:
- Fix method chain indentation for LLM completion calls
- Reformat error handling closures for readability
- Break long method calls (wrap_for_llm) across multiple lines
No functional changes.
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* style: apply cargo fmt formatting fixes to routine_engine.rs
Align formatting with project standards:
- Break long method chains across multiple lines for readability
- Reformat error return statements for consistency
- Split long assert/assert_eq statements across multiple lines
No logic changes; purely cosmetic formatting.
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
* test: update routine engine tests for tool/safety layer parameters
Update test code to pass newly required ToolRegistry and SafetyLayer
parameters to RoutineEngine::new(). Also add missing lightweight_tools_enabled
and lightweight_max_iterations fields to RoutineConfig initializers in tests.
Tests affected:
- tests/support/test_rig.rs: Added tools and safety layer to RoutineEngine::new()
- tests/e2e_routine_heartbeat.rs: Added three instances of tools and safety layer construction
All tests pass (2764 tests).
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
---------
Co-authored-by: Claude Haiku 4.5 <[email protected]>
Co-authored-by: Henry Park <[email protected]>
|
||
|
|
732b3ecfeb |
test(agent): wire TestRig job tools through the scheduler (#716)
Align TestRig with the production agent wiring so create_job exercises the real scheduler path instead of silently falling back to an unscheduled context-only job. Tighten the e2e assertion to lock in the in-progress scheduler behavior for future refactors. Made-with: Cursor Co-authored-by: Zaki Manian <[email protected]> |
||
|
|
461d7712e8 |
fix(config): init_secrets no longer overwrites entire config (#726)
* fix(config): init_secrets no longer overwrites entire config init_secrets() was calling Config::from_db_with_toml() to re-resolve config after injecting credentials. This rebuilt the entire config from env/DB/defaults, nuking all other config fields (agent, safety, tools, etc.) even though only LlmConfig depends on injected credentials. This caused 5 CI test failures: the test rig's carefully chosen config values (max_tool_iterations, allow_local_tools, etc.) were silently overwritten with production defaults after secret injection. Fix: add Config::re_resolve_llm() that re-resolves only the LLM config after credential injection, leaving all other config fields untouched. Also fix TraceLlm::complete() to skip ToolCalls steps when called in force_text mode (iteration limit). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(test): update test to match TraceLlm::complete() skip-tool-calls behavior [skip-regression-check] TraceLlm::complete() now skips ToolCalls steps (force_text mode) instead of erroring. Update the test to verify it skips past a ToolCalls step and returns the subsequent Text step. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Zaki <[email protected]> |
||
|
|
d144484b06 |
feat: WASM channel attachments with LLM pipeline integration (#596)
* feat: add inbound attachment support to WASM channel system Add attachment record to WIT interface and implement inbound media parsing across all four channel implementations (Telegram, Slack, WhatsApp, Discord). Attachments flow from WASM channels through EmittedMessage to IncomingMessage with validation (size limits, MIME allowlist, count caps) at the host boundary. - Add `attachment` record to `emitted-message` in wit/channel.wit - Add `IncomingAttachment` struct to channel.rs and re-export - Add host-side validation (20MB total, 10 max, MIME allowlist) - Telegram: parse photo, document, audio, video, voice, sticker - Slack: parse file attachments with url_private - WhatsApp: parse image, audio, video, document with captions - Discord: backward-compatible empty attachments - Update FEATURE_PARITY.md section 7 - Add fixture-based tests per channel and host integration tests [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: integrate outbound attachment support and reconcile WIT types (#409) Reconcile PR #409's outbound attachment work with our inbound attachment support into a unified design: WIT type split: - `inbound-attachment` in channel-host: metadata-only (id, mime_type, filename, size_bytes, source_url, storage_key, extracted_text) - `attachment` in channel: raw bytes (filename, mime_type, data) on agent-response for outbound sending Outbound features (from PR #409): - `on-broadcast` WIT export for proactive messages without prior inbound - Telegram: multipart sendPhoto/sendDocument with auto photo→document fallback for files >10MB - wrapper.rs: `call_on_broadcast`, `read_attachments` from disk, attachment params threaded through `call_on_respond` - HTTP tool: `save_to` param for binary downloads to /tmp/ (50MB limit, path traversal protection, SSRF-safe redirect following) - Message tool: allow /tmp/ paths for attachments alongside base_dir - Credential env var fallback in inject_channel_credentials Channel updates: - All 4 channels implement on_broadcast (Telegram full, others stub) - Telegram: polling_enabled config, adjusted poll timeout - Inbound attachment types renamed to InboundAttachment in all channels Tests: 1965 passing (9 new), 0 clippy warnings [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add audio transcription pipeline and extensible WIT attachment design Add host-side transcription middleware (OpenAI Whisper) that detects audio attachments with inline data on incoming messages and transcribes them automatically. Refactor WIT inbound-attachment to use extras-json and a store-attachment-data host function instead of typed fields, so future attachment properties (dimensions, codec, etc.) don't require WIT changes that invalidate all channel plugins. - Add src/transcription/ module: TranscriptionProvider trait, TranscriptionMiddleware, AudioFormat enum, OpenAI Whisper provider - Add src/config/transcription.rs: TRANSCRIPTION_ENABLED/MODEL/BASE_URL - Wire middleware into agent message loop via AgentDeps - WIT: replace data + duration-secs with extras-json + store-attachment-data - Host: parse extras-json for well-known keys, merge stored binary data - Telegram: download voice files via store-attachment-data, add duration to extras-json, add /file/bot to HTTP allowlist, voice-only placeholder - Add reqwest multipart feature for Whisper API uploads - 5 regression tests for transcription middleware Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: wire attachment processing into LLM pipeline with multimodal image support Attachments on incoming messages are now augmented into user text via XML tags before entering the turn system, and images with data are passed as multimodal content parts (base64 data URIs) to LLM providers. This enables audio transcripts, document text, and image content to reach the LLM without changes to ChatMessage serialization or provider interfaces. - Add src/agent/attachments.rs with augment_with_attachments() and 9 unit tests - Add ContentPart/ImageUrl types to llm::provider with OpenAI-compatible serde - Carry image_content_parts transiently on Turn (skipped in serialization) - Update nearai_chat and rig_adapter to serialize multimodal content - Add 3 e2e tests verifying attachments flow through the full agent loop Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: CI failures — formatting, version bumps, and Telegram voice test - Fix cargo fmt formatting in attachments.rs, nearai_chat.rs, rig_adapter.rs, e2e_attachments.rs - Bump channel registry versions 0.1.0 → 0.2.0 (discord, slack, telegram, whatsapp) to satisfy version-bump CI check - Fix Telegram test_extract_attachments_voice: add missing required `duration` field to voice fixture JSON Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: bump WIT channel version to 0.3.0, fix Telegram voice test, add pre-commit hook - Bump wit/channel.wit package version 0.2.0 → 0.3.0 (interface changed with store-attachment-data) - Update WIT_CHANNEL_VERSION constant and registry wit_version fields to match - Fix Telegram test_extract_attachments_voice: gate voice download behind #[cfg(target_arch = "wasm32")] so host functions aren't called in native tests, update assertions for generated filename and extras_json duration - Add @0.3.0 linker stubs in wit_compat.rs - Add .githooks/pre-commit hook that runs scripts/check-version-bumps.sh when WIT or extension sources are staged - Symlink commit-msg regression hook into .githooks/ [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: extract voice download from extract_attachments into handle_message Move download_voice_file + store_attachment_data calls out of extract_attachments into a separate download_and_store_voice function called from handle_message. This keeps extract_attachments as a pure data-mapping function with no host calls, making it fully testable in native unit tests without #[cfg(target_arch)] gates. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review comments — security, correctness, and code quality Security fixes: - Add path validation to read_attachments (restrict to /tmp/) preventing arbitrary file reads from compromised tools - Escape XML special characters in attachment filenames, MIME types, and extracted text to prevent prompt injection via tag spoofing - Percent-encode file_id in Telegram getFile URL to prevent query injection - Clone SecretString directly instead of expose_secret().to_string() Correctness fixes: - Fix store_attachment_data overwrite accounting: subtract old entry size before adding new to prevent inflated totals and false rejections - Use max(reported, stored_size) for attachment size accounting to prevent WASM channels from under-reporting size_bytes to bypass limits - Add application/octet-stream to MIME allowlist (channels default unknown types to this) Code quality: - Extract send_response helper in Telegram, deduplicating on_respond and on_broadcast - Rename misleading Discord test to test_parse_slash_command_interaction - Fix .githooks/commit-msg to use relative symlink (portable across machines) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add tool_upgrade command + fix TOCTOU in save_to path validation Add `tool_upgrade` — a new extension management tool that automatically detects and reinstalls WASM extensions with outdated WIT versions. Preserves authentication secrets during upgrade. Supports upgrading a single extension by name or all installed WASM tools/channels at once. Fix TOCTOU in `validate_save_to_path`: validate the path *before* creating parent directories, so traversal paths like `/tmp/../../etc/` cannot cause filesystem mutations outside /tmp before being rejected. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: unify WIT package version to 0.3.0 across tool.wit and all capabilities tool.wit and channel.wit share the `near:agent` package namespace, so they must declare the same version. Bumps tool.wit from 0.2.0 to 0.3.0 and updates all capabilities files and registry entries to match. Fixes `cargo component build` failure: "package identifier near:[email protected] does not match previous package name of near:[email protected]" [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: move WIT file comments after package declaration WIT treats `//` comments before `package` as doc comments. When both tool.wit and channel.wit had header comments, the parser rejected them as "doc comments on multiple 'package' items". Move comments after the package declaration in both files. Also bumps tool registry versions to 0.2.0 to match the WIT 0.3.0 bump. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: display extension versions in gateway Extensions tab Add version field to InstalledExtension and RegistryEntry types, pipe through the web API (ExtensionInfo, RegistryEntryInfo), and render as a badge in the gateway UI for both installed and available extensions. For installed WASM extensions, version is read from the capabilities file with a fallback to the registry entry when the local file has no version (old installations). Bump all extension Cargo.toml and registry JSON versions from 0.1.0 to 0.2.0 to keep them in sync. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add document text extraction middleware for PDF, Office, and text files Extract text from document attachments (PDF, DOCX, PPTX, XLSX, RTF, plain text, code files) so the LLM can reason about uploaded documents. Uses pdf-extract for PDFs, zip+XML parsing for Office XML formats, and UTF-8 decode for text files. Wired into the agent loop after transcription middleware. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: download document files in Telegram channel for text extraction The DocumentExtractionMiddleware needs file bytes in the attachment `data` field, but only voice files were being downloaded. Document attachments (PDFs, DOCX, etc.) had empty `data` and a source_url with a credential placeholder that only works inside the WASM host's http_request. Add `download_and_store_documents()` that downloads non-voice, non-image, non-audio attachments via the existing two-step getFile→download flow and stores bytes via `store_attachment_data` for host-side extraction. Also rename `download_voice_file` → `download_telegram_file` since it's generic for any file_id. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: allow Office MIME types and increase file download limit for Telegram Two issues preventing document extraction from Telegram: 1. PPTX/DOCX/XLSX MIME types (application/vnd.*) were dropped by the WASM host attachment allowlist — add application/vnd., application/msword, and application/rtf prefixes. 2. Telegram file downloads over 10 MB failed with "Response body too large" — set max_response_bytes to 20 MB in Telegram capabilities. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: report document extraction errors back to user instead of silently skipping - Bump max_response_bytes to 50 MB for Telegram file downloads - When document extraction fails (too large, download error, parse error), set extracted_text to a user-friendly error message instead of leaving it None. This ensures the LLM tells the user what went wrong. - On Telegram download failure, set extracted_text with the error so the user sees feedback even when the file never reaches the extraction middleware. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: store extracted document text in workspace memory for search/recall After document extraction succeeds, write the extracted text to workspace memory at `documents/{date}/{filename}`. This enables: - Full-text and semantic search over past uploaded documents - Cross-conversation recall ("what did that PDF say?") - Automatic chunking and embedding via the workspace pipeline Documents are stored with metadata header (uploader, channel, date, MIME type). Error messages (extraction failures) are not stored — only successful extractions. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: CI failures — formatting, unused assignment warning - Run cargo fmt on document_extraction and agent_loop modules - Suppress unused_assignments warning on trace_llm_ref (used only behind #[cfg(feature = "libsql")]) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review comments — security, correctness, and code quality Security fixes: - Remove SSRF-prone download() from DocumentExtractionMiddleware (#13) - Sanitize filenames in workspace path to prevent directory traversal (#11) - Pre-check file size before reading in WASM wrapper to prevent OOM (#2) - Percent-encode file_id in Telegram source URLs (#7) Correctness fixes: - Clear image_content_parts on turn end to prevent memory leak (#1) - Find first *successful* transcription instead of first overall (#3) - Enforce data.len() size limit in document extraction (#10) - Use UTF-8 safe truncation with char_indices() (#12) Robustness & code quality: - Add 120s timeout to OpenAI Whisper HTTP client (#5) - Trim trailing slash from Whisper base_url (#6) - Allow ~/.ironclaw/ paths in WASM wrapper (#8) - Return error from on_broadcast in Slack/Discord/WhatsApp (#9) - Fix doc comment in HTTP tool (#4) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: formatting — cargo fmt Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address latest PR review — doc comments, error messages, version bumps - Fix DocumentExtractionMiddleware doc comment (no longer downloads from source_url) - Fix error message: "no inline data" instead of "no download URL" - Log error + fallback instead of silent unwrap_or_default on Whisper HTTP client - Bump all capabilities.json versions from 0.1.0 to 0.2.0 to match Cargo.toml Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: remove unsupported profile: minimal from CI workflows [skip-regression-check] dtolnay/rust-toolchain@stable does not accept the 'profile' input (it was a parameter for the deprecated actions-rs/toolchain action). Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: merge with latest main — resolve compilation errors and PR review nits - Add version: None to RegistryEntry/InstalledExtension test constructors - Fix MessageContent type mismatches in nearai_chat tests (String → MessageContent::Text) - Fix .contains() calls on MessageContent — use .as_text().unwrap() - Remove redundant trace_llm_ref = None assignment in test_rig - Check data size before clone in document extraction to avoid unnecessary allocation [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
30790439ee |
perf: build system prompt once per turn, skip tools on force-text (#583)
* perf: build system prompt once per turn, skip tools on force-text, fix nudge role (#565) Three fixes to agentic loop prompt handling: 1. Build system prompt once per turn instead of every tool iteration. `build_system_prompt_with_tools` is now pub; callers pass the result via `ReasoningContext::system_prompt` to avoid rebuilding ~1,500 tokens per iteration. 2. Skip `## Available Tools` section when `force_text = true`. The dispatcher passes a no-tools prompt variant on the final iteration, saving ~460 tokens and removing misleading instructions. 3. Change nudge message from `Role::System` to `Role::User`. A second system message mid-conversation is unsupported by most providers. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: revert nudge role change to keep ChatMessage::system Copilot review correctly identified that using Role::User for the nudge breaks compact_messages_for_retry, which uses rposition for Role::User to find the last real user message. Role::Assistant would cause back-to-back assistant messages. Since no production issues were reported with the original system role, revert to ChatMessage::system. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address PR review — omit tool guidance when tools empty, rename shadowed var - Conditionalize "Call tools…" guidelines and "## Tool Call Style" section in the system prompt so they are only included when tools are non-empty. Previously the force-text (no-tools) prompt still contained misleading tool-calling instructions. (Copilot review comment) - Rename `system_prompt` → `cached_prompt` in dispatcher to avoid shadowing the earlier workspace identity `system_prompt` variable. (Copilot review) - Add regression tests: `test_system_prompt_with_tools_contains_tool_guidance` and extended assertions in `test_system_prompt_without_tools_omits_tools_section`. 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]> |
||
|
|
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]> |
||
|
|
cf96a3253c |
fix(tests): replace hardcoded /tmp paths with tempdir + add 300 unit tests (#659)
* test: add unit tests across 20 modules for coverage push Add 300+ unit tests covering config, context, evaluation, extensions, LLM, secrets, tools/builder, and tools/mcp modules. All tests are pure unit tests (no mocks) exercising serde roundtrips, edge cases, error paths, and business logic. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(tests): replace hardcoded /tmp paths with tempfile::tempdir The e2e_metrics_test::test_metrics_collected_from_tool_trace test was failing because setup_test_dir() created /tmp/ironclaw_metrics_test but the fixture referenced /tmp/ironclaw_e2e_test/hello.txt (path mismatch). Added LlmTrace::replace_paths() to substitute fixture paths at runtime, then converted all 12 test files from hardcoded /tmp/ironclaw_* paths to tempfile::tempdir(). Tests are now isolated, parallel-safe, and leave no debris on disk. Regression test: test_metrics_collected_from_tool_trace now passes consistently regardless of prior /tmp state. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
ae89a52ac2 |
feat(routines): approval context for autonomous job execution (#577)
* feat(routines): add approval context for autonomous job execution Routines and background jobs were unable to use any tools that required approval (file ops, shell, message, http), making them effectively useless. This adds an ApprovalContext system that lets autonomous jobs pre-authorize tools at dispatch time. - Add ApprovalContext enum with Autonomous variant that auto-approves UnlessAutoApproved tools and optionally pre-authorizes Always tools - Add tool_permissions field to RoutineAction::FullJob for pre-authorizing Always-gated tools (e.g. destructive shell, cross-channel messaging) - Add Scheduler::dispatch_job_with_context() to thread approval context through to workers - Set message tool default channel/target from routine NotifyConfig so routines can send results without cross-channel approval - Fix Completed→Completed state transition error in worker (plan marks job completed, then direct loop or outer run() tries again) Co-Authored-By: Claude Opus 4.6 <[email protected]> * test(routines): add E2E trace for routine news digest workflow Add a 3-turn trace fixture and test that exercises: - Turn 1: routine_create with full_job mode and tool_permissions - Turn 2: Simulated digest workflow with echo + memory_write - Turn 3: Verification via memory_search [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(test): wire RoutineEngine into test rig for routine_create E2E - Add `with_routines()` to TestRigBuilder that passes a RoutineConfig to Agent::new, enabling routine tool registration during agent startup - Add Turn 2 (routine_list) to the trace to verify routine persistence in the database after routine_create - Fix formatting issues flagged by CI (cargo fmt) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor(scheduler): deduplicate dispatch_job and dispatch_job_with_context Extract shared logic into private `dispatch_job_inner` to prevent divergence when dispatch behavior changes in the future. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(routines): add routine_fire tool and real E2E routine execution test - Add `routine_fire` tool that calls `RoutineEngine::fire_manual` to trigger a routine on demand. Registered alongside the other 5 routine tools (now 6 total). - Rewrite the routine_news_digest E2E trace to exercise the full execution stack end-to-end: 1. routine_create (manual trigger, full_job, tool_permissions: [message]) 2. routine_fire → RoutineEngine → Scheduler::dispatch_job_with_context → autonomous Worker consuming TraceLlm steps 3. Worker calls echo → memory_write → message (broadcast to test channel) 4. Test verifies the message broadcast arrived, proving ApprovalContext correctly allowed the Always-approval message tool - Register message tools in TestRig so routines can send messages to the test channel via channel_manager.broadcast(). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(routines): wire HttpInterceptor through scheduler for routine worker http calls Propagate http_interceptor from AgentDeps → Scheduler → WorkerDeps → JobContext so that routine workers (and any scheduler-dispatched workers) can use the ReplayingHttpInterceptor for mock HTTP responses during tests. Changes: - Add http_interceptor field to Scheduler and WorkerDeps - Set job_ctx.http_interceptor in Worker before tool execution - Add with_http_exchanges() builder method to TestRigBuilder - Replace echo tool with http tool in routine_news_digest trace - Test now exercises real http tool with mock response → memory_write → message Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review comments from Copilot on PR #577 - Extract `ApprovalContext::is_blocked_or_default()` helper to deduplicate approval check logic in worker.rs and scheduler.rs - Extract `parse_tool_permissions()` helper to deduplicate JSON array parsing in routine.rs and builtin/routine.rs - Fix test name: `test_mark_completed_twice_does_not_error` → `test_mark_completed_twice_returns_error` (matches actual behavior) - Fix ApprovalContext doc comment to clarify it only models autonomous mode - Fix flaky index-based assertion in routine_news_digest test — now uses content-based search instead of fixed position - Fix stale comment: echo → http in routine test header - Add TODO for subtask approval context propagation (latent, not in active code paths) - Add TODO for global message tool context race in routine_engine Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix formatting in is_blocked_or_default test Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor(test_rig): destructure self in build() to avoid partial-move fragility Destructure TestRigBuilder at the top of build() instead of accessing self.* fields after moving self.http_exchanges. While the prior code compiled (remaining fields are Copy), it was fragile and would break if any non-Copy field were added. Co-Authored-By: Claude Opus 4.6 <[email protected]> * docs: clarify that routine_fire bypasses cooldown Manual fires are explicitly user-initiated and intentionally bypass cooldown checks (which only apply to automated cron/event triggers). Updated tool description and fire_manual docstring to make this clear. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(routines): fix message tool approval in routine context Two fixes for message tool failures in autonomous routine jobs: 1. MessageTool::requires_approval() now returns UnlessAutoApproved when the explicit channel param matches the default channel (was Always, causing "requires authentication" errors for routine workers). 2. routine_create tool now accepts notify_channel and notify_user params, wired into NotifyConfig. Without these, routines had channel: None, so set_message_tool_context was never called, causing "No channel specified" errors. Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor(message): remove approval requirement from message tool The message tool only sends to user-owned channels via ChannelManager::broadcast (TUI, Telegram, Slack, web gateway, etc.). It cannot reach arbitrary external services, so approval adds friction with no security benefit. This also eliminates the routine context errors entirely since approval is never checked. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review comments — routine_fire approval + test rename - routine_fire now returns UnlessAutoApproved since firing a routine can dispatch a full_job with pre-authorized Always-gated tools - Rename test_approval_context_never_always_passes to test_approval_context_never_is_not_blocked for clarity Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review nits — update stale docs and comments - Remove 'message' from tool_permissions example (no longer Always) - Reword message tool approval comment for accuracy - Clarify with_routines() docstring re: tool registration vs engine wiring Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
37bba72397 |
test: add 29 E2E trace tests for issues #571-575 (#593)
* test: add 29 E2E trace tests for worker, threading, tools, workspace, and routines (#571-575) Add comprehensive E2E test coverage across five test files: - e2e_worker_coverage (7 tests): parallel tool calls, error feedback, unknown tools, invalid params, rate limiting, iteration limits, planning mode - e2e_thread_scheduling (3 tests + 2 deferred): multi-turn state, undo/redo, concurrent dispatch - e2e_builtin_tool_coverage (8 tests): time parse/diff/invalid, routine CRUD/history, job create/status/list/cancel, HTTP replay - e2e_workspace_coverage (6 tests): chunked search, multi-doc search, hybrid search, directory tree, document lifecycle, identity in system prompt - e2e_routine_heartbeat (5 tests): cron triggers, event matching, cooldown enforcement, heartbeat findings, empty checklist skip Infrastructure: extend TestRig with database/workspace/trace_llm accessors, register job and routine tools by default, add with_extra_tools() for custom stub tools. Includes 24 JSON trace fixtures across worker/, threading/, tools/, and workspace/. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use 6-field cron format in routine_create_list fixture The cron 0.13 crate accepts both 6 and 7 fields, but the routine_create tool documents 6-field format. Align the fixture to match. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: eliminate vacuous passes and silently-skipped assertions in E2E tests - job_create_status: replace job_status (needs dynamic UUID) with list_jobs, assert both succeed via completed() not just started() - job_list_cancel: keep cancel_job but explicitly assert it fails with invalid canned job_id "latest", verify create_job + list_jobs succeed - unknown_tool_name: add !is_empty() guard before .all() to prevent vacuous pass on empty iterator - workspace tests: change `if let Some(ws)` to `.expect()` so assertions are never silently skipped when workspace/trace_llm is available [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add template substitution to TraceLlm for dynamic tool result forwarding Add {{call_id.json_path}} template syntax to trace fixtures, enabling tool results from one step to flow into subsequent steps' arguments. TraceLlm extracts variables from Role::Tool messages (stripping the safety layer's <tool_output> XML wrapper and unescaping entities) and substitutes them in canned tool_call arguments before returning. This fixes job_create_status and job_list_cancel tests to properly test job_status and cancel_job with real dynamic UUIDs from create_job, instead of using invalid canned IDs that silently failed. Also adds tool result content assertions to job_create_status to verify the actual tool output contains expected data (job_id, title). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review feedback on E2E tests - undo_redo_cycle: assert exactly 3 turns instead of >= 2 - tool_error_feedback: use tempfile::tempdir() instead of hardcoded /tmp path, patch fixture path at runtime for CI portability - worker_timeout → iteration_limit: rename to accurately describe what's tested - post_plan_work_remaining → simple_echo_flow: rename, test doesn't exercise planning - identity_in_system_prompt: seed IDENTITY.md before test, assert system prompt contains the seeded content instead of just checking Role::System exists [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: strengthen workspace E2E test assertions per PR review - write_chunk_search: assert memory_search was called and returned payment/architecture-related results - multi_document_search: assert memory_search was called for cross-document search - hybrid_search_with_embeddings: assert both memory_write and memory_search were called to confirm write-then-search pipeline - directory_tree: assert tree output contains expected alpha/beta project paths [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
470de5bd2d |
feat: merge http/web_fetch tools, add tool output stash for large responses (#578)
* feat: merge http/web_fetch tools, add tool output stash for large responses Merge `web_fetch` into `http` tool with smart approval: plain GETs (no headers, no body) run without approval and follow redirects with SSRF re-validation per hop; all other requests require approval as before. Add `tool_output_stash` on JobContext so full tool outputs are preserved before safety-layer truncation. The `json` tool gains a `source_tool_call_id` parameter to reference stashed outputs, enabling reliable parsing of large API responses that exceed the 100KB context limit. Other improvements: - Descriptive User-Agent header using CARGO_PKG_VERSION - Truncation now keeps partial data + hint about source_tool_call_id - System prompt reinforces tool_calls over narration - json tool query/stringify handle pre-parsed (non-string) data [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: delete dead web_fetch.rs (merged into http tool) Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix rustfmt formatting Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: rename shadowed data binding for clarity in json tool Address PR review: rename owned `data` to `data_value` before re-binding as `let data = &data_value` to make ownership explicit. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(ci): mark network-dependent trace tests as #[ignore] The weather_sf and baseball_stats tests hit live external APIs (wttr.in, ESPN) which are unreliable in CI. Mark them #[ignore] so they don't block the pipeline. Run locally with `--ignored` to include them. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: replay recorded HTTP exchanges in trace tests instead of hitting live APIs Wire ReplayingHttpInterceptor into TestRig when the trace fixture contains http_exchanges. This replays recorded responses instead of making live network calls, making tests deterministic and CI-stable. Add captured HTTP responses to weather_sf.json (wttr.in) and baseball_stats.json (ESPN API) fixtures. Revert #[ignore] on both tests — they now run offline. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: recover inline bracket-format tool calls from LLM text responses When flatten_tool_messages converts tool calls to text like `[Called tool `http` with arguments: {...}]` for NEAR AI compatibility, the LLM sometimes echoes this format back in its text responses instead of using proper tool_calls. Add recovery for this bracket format in recover_tool_calls_from_content and strip it in clean_response so users don't see raw tool call syntax. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
b4b19738a8 |
Trajectory benchmarks and e2e trace test rig (#553)
* refactor: extract shared assertion helpers to support/assertions.rs Move 5 assertion helpers from e2e_spot_checks.rs to a shared module. Add assert_all_tools_succeeded and assert_tool_succeeded for eliminating false positives in E2E tests. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add tool output capture via tool_results() accessor Extract (name, preview) from ToolResult status events in TestChannel and TestRig, enabling content assertions on tool outputs. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: correct tool parameters in 3 broken trace fixtures - tool_time.json: add missing "operation": "now" for time tool - robust_correct_tool.json: same fix - memory_full_cycle.json: change "path" to "target" for memory_write Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: add tool success and output assertions to eliminate false positives Every E2E test that exercises tools now calls assert_all_tools_succeeded. Added tool output content assertions where tool results are predictable (time year, read_file content, memory_read content). Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: capture per-tool timing from ToolStarted/ToolCompleted events Record Instant on ToolStarted and compute elapsed duration on ToolCompleted, wiring real timing data into collect_metrics() instead of hardcoded zeros. Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: add RAII CleanupGuard for temp file/dir cleanup in tests Replace manual cleanup_test_dir() calls and inline remove_file() with Drop-based CleanupGuard that ensures cleanup even if a test panics. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: add Drop impl and graceful shutdown for TestRig Wrap agent_handle in Option so Drop can abort leaked tasks. Signal the channel shutdown before aborting for future cooperative shutdown. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: replace agent startup sleep with oneshot ready signal Use a oneshot channel fired in Channel::start() instead of a fixed 100ms sleep, eliminating the race condition on slow systems. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: replace fragile string-matching iteration limit with count-based detection Use tool completion count vs max_tool_iterations instead of scanning status messages for "iteration"/"limit" substrings. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use assert_all_tools_succeeded for memory_full_cycle test Remove incorrect comment about memory_tree failing with empty path (it actually succeeds). Omit empty path from fixture and use the standard assert_all_tools_succeeded instead of per-tool assertions. Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: promote benchmark metrics types to library code Move TraceMetrics, ScenarioResult, RunResult, MetricDelta, and compare_runs() from tests/support/metrics.rs to src/benchmark/metrics.rs. Existing tests use re-export for backward compatibility. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add Scenario and Criterion types for agent benchmarking Scenario defines a task with input, success criteria, and resource limits. Criterion is an enum of programmatic checks (tool_used, response_contains, etc.) evaluated without LLM judgment. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add initial benchmark scenario suite (12 scenarios across 5 categories) Scenarios cover tool_selection, tool_chaining, error_recovery, efficiency, and memory_operations. All loaded from JSON with deserialization validation test. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add benchmark runner with BenchChannel and InstrumentedLlm BenchChannel is a minimal Channel implementation for benchmarks. InstrumentedLlm wraps any LlmProvider to capture per-call metrics. Runner creates a fresh agent per scenario, evaluates success criteria, and produces RunResult with timing, token, and cost metrics. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add baseline management, reports, and benchmark entry point - baseline.rs: load/save/promote benchmark results - report.rs: format comparison reports with regression detection - benchmark_runner.rs: integration test with real LLM (feature-gated) - Add benchmark feature flag to Cargo.toml Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: apply cargo fmt to benchmark module Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): add multi-turn scenario types with setup, judge, ResponseNotContains Add BenchScenario, Turn, TurnAssertions, JudgeConfig, ScenarioSetup, WorkspaceSetup, SeedDocument types for multi-turn benchmark scenarios. Add ResponseNotContains criterion variant. Add TurnAssertions::to_criteria() converter for backward compat with existing evaluation engine. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): add JSON scenario loader with recursive discovery and tag filter Add load_bench_scenarios() for the new BenchScenario format with recursive directory traversal and tag-based filtering. Create 4 initial trajectory scenarios across tool-selection, multi-turn, and efficiency categories. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): multi-turn runner with workspace seeding and per-turn metrics Add run_bench_scenario() that loops over BenchScenario turns, seeds workspace documents, collects per-turn metrics (tokens, tool calls, wall time), and evaluates per-turn assertions. Add TurnMetrics to metrics.rs and clear_for_next_turn() to BenchChannel. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): add LLM-as-judge scoring with prompt formatting and score parsing Create judge.rs with format_judge_prompt, parse_judge_score, and judge_turn. Wire into run_bench_scenario for turns with judge config -- scores below min_score fail the turn. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): add CLI subcommand (ironclaw benchmark) Add BenchmarkCommand with --tags, --scenario, --no-judge, --timeout, --update-baseline flags. Wire into Command enum and main.rs dispatch. Feature-gated behind benchmark flag. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): per-scenario JSON output with full trajectory Add save_scenario_results() that writes per-scenario JSON files alongside the run summary. Each scenario gets its own file with turn_metrics trajectory. Update CLI to use new output format. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): add ToolRegistry::retain_only and wire tool filtering in scenarios Add a retain_only() method to ToolRegistry that filters tools down to a given allowlist. Wire this into run_bench_scenario() so that when a scenario specifies a tools list in its setup, only those tools are available during the benchmark run. Includes two tests for the new method: one verifying filtering works and one verifying empty input is a no-op. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): wire identity overrides into workspace before agent start Add seed_identity() helper that writes identity files (IDENTITY.md, USER.md, etc.) into the workspace before the agent starts, so that workspace.system_prompt() picks them up. Wire it into run_bench_scenario() after workspace seeding. Include a test that verifies identity files are written and readable. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): add --parallel and --max-cost CLI flags Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(benchmark): use feature-conditional snapshot names for CLI help tests Prevents snapshot conflicts between default (no benchmark) and all-features (with benchmark) builds by using separate snapshot names per feature set. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): parallel execution with JoinSet and budget cap enforcement Replace sequential loop in run_all_bench() with parallel execution using JoinSet + semaphore when config.parallel > 1. Add budget cap enforcement that skips remaining scenarios when max_total_cost_usd is exceeded. Track skipped count in RunResult.skipped_scenarios and display it in format_report(). Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): add tool restriction and identity override test scenarios Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: fix formatting for Phase 3 Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): add SkillRegistry::retain_only and wire skill filtering in scenarios Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(benchmark): add --json flag for machine-readable output Co-Authored-By: Claude Opus 4.6 <[email protected]> * ci: add GitHub Actions benchmark workflow (manual trigger) Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor(benchmark): remove in-tree benchmark harness, keep retain_only utilities Move benchmark-specific code out of ironclaw in preparation for the nearai/benchmarks trajectory adapter. This removes: - src/benchmark/ (runner, scenarios, metrics, judge, report, etc.) - src/cli/benchmark.rs and the Benchmark CLI subcommand - benchmarks/ data directory (scenarios + trajectories) - .github/workflows/benchmark.yml - The "benchmark" Cargo feature flag What remains: - ToolRegistry::retain_only() and SkillRegistry::retain_only() - Test support types (TraceMetrics, InstrumentedLlm) inlined into tests/support/ instead of re-exporting from the deleted module Co-Authored-By: Claude Opus 4.6 <[email protected]> * docs: add README for LLM trace fixture format Documents the trajectory JSON format, response types, request hints, directory structure, and how to write new traces. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(test): unify trace format around turns, add multi-turn support Introduce TraceTurn type that groups user_input with LLM response steps, making traces self-contained conversation trajectories. Add run_trace() to TestRig for automatic multi-turn replay. Backward-compatible: flat "steps" JSON is deserialized as a single turn transparently. Includes all trace fixtures (spot, coverage, advanced), plan docs, and new e2e tests for steering, error recovery, long chains, memory, and prompt injection resilience. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(test): fix CI failures after merging main - Fix tool_json fixture: use "data" parameter (not "input") to match JsonTool schema - Fix status_events test: remove assertion for "time" tool that isn't in the fixture (only "echo" calls are used) - Allow dead_code in test support metrics/instrumented_llm modules (utilities for future benchmark tests) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * Working on recording traces and testing them * feat(test): add declarative expects to trace fixtures, split infra tests Add TraceExpects struct with 9 optional assertion fields (response_contains, tools_used, all_tools_succeeded, etc.) that can be declared in fixture JSON instead of hand-written Rust. Add verify_expects() and run_recorded_trace() so recorded trace tests become one-liners. Split trace infra tests (deserialization, backward compat) into tests/trace_format.rs which doesn't require the libsql feature gate. Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor(test): add expects to all trace fixtures, simplify e2e tests Add declarative expects blocks to all 19 trace fixture JSONs across spot/, coverage/, advanced/, and root directories. Update all 8 e2e test files to use verify_trace_expects() / run_and_verify_trace(), replacing ~270 lines of hand-written assertions with fixture-driven verification. Tests that check things beyond expects (file content on disk, metrics, event ordering) keep those extra assertions alongside the declarative ones. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(test): adapt tests to AppBuilder refactor, fix formatting Update test files to work with refactored TestRigBuilder that uses AppBuilder::build_all() (removing with_tools/with_workspace methods). Update telegram_check fixture to use tool_list instead of echo. Fix cargo fmt issues in src/llm/mod.rs and src/llm/recording.rs. Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor(test): deduplicate support unit tests into single binary Support modules (assertions, cleanup, test_channel, test_rig, trace_llm) had #[cfg(test)] mod tests blocks that were compiled and run 12 times — once per e2e test binary that declares `mod support;`. Extracted all 29 support unit tests into a dedicated `tests/support_unit_tests.rs` so they run exactly once. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix trailing newlines in support files Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor(test): unify trace types and fix recorded multi-turn replay Import shared types (TraceStep, TraceResponse, TraceToolCall, RequestHint, ExpectedToolResult, MemorySnapshotEntry, HttpExchange*) from ironclaw::llm::recording instead of redefining them in trace_llm.rs. Fix the flat-steps deserializer to split at UserInput boundaries into multiple turns, instead of filtering them out and wrapping everything into a single turn. This enables recorded multi-turn traces to be replayed as proper multi-turn conversations via run_trace(). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(test): fix CI failures - unused imports and missing struct fields - Add #[allow(unused_imports)] on pub use re-exports in trace_llm.rs (types are re-exported for downstream test files, not used locally) - Add `..` to ToolCompleted pattern in test_channel.rs to match new `error` and `parameters` fields Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(test): fix CI failures after merging main - Add missing `error` and `parameters` fields to ToolCompleted constructors in support_unit_tests.rs - Add `..` to ToolCompleted pattern match in support_unit_tests.rs - Add #[allow(dead_code)] to CleanupGuard, LlmTrace impl, and TraceLlm impl (only used behind #[cfg(feature = "libsql")]) Co-Authored-By: Claude Opus 4.6 <[email protected]> * Adding coverage running script * fix(test): address review feedback on E2E test infrastructure - Increase wait_for_responses polling to exponential backoff (50ms-500ms) and raise default timeout from 15s to 30s to reduce CI flakiness (#1) - Strengthen prompt_injection_resilience test with positive safety layer assertion via has_safety_warnings(), enable injection_check (#2) - Add assert_tool_order() helper and tools_order field in TraceExpects for verifying tool execution ordering in multi-step traces (#3) - Document TraceLlm sequential-call assumption for concurrency (#6) - Clean up CleanupGuard with PathKind enum instead of shotgun remove_file + remove_dir_all on every path (#8) - Fix coverage.sh: default to --lib only, fix multi-filter syntax, add COV_ALL_TARGETS option - Add coverage/ to .gitignore - Remove planning docs from PR [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review - use HashSet in retain_only, improve skill test - Use HashSet for O(N+M) lookup in SkillRegistry::retain_only and ToolRegistry::retain_only instead of linear scan - Strengthen test_retain_only_empty_is_noop in SkillRegistry to pre-populate with a skill before asserting the no-op behavior [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(test): revert incorrect safety layer assertion in injection test The safety layer sanitizes tool output, not user input. The injection test sends a malicious user message with no tools called, so the safety layer never fires. Reverted to the original test which correctly validates the LLM refuses via trace expects. Also fixed case-sensitive request hint ("ignore" -> "Ignore") to suppress noisy warning. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: clean stale profdata before coverage run Adds `cargo llvm-cov clean` before each run to prevent "mismatched data" warnings from stale instrumentation profiles. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix formatting in retain_only test [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Illia Polosukhin <[email protected]> |