mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-09-01 00:59:33 +00:00
52551f0ef4f894f0ae09466fc95fbf9a593022d8
111
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
dd0a0e10ab |
fix(routines): recover delete name after failed update fallback (#1108)
Co-authored-by: [email protected] <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
Henry ParkandGitHub
|
ab0ad948f3 |
Normalize cron schedules on routine create (#1648)
* Fix REPL single-message hang and cap CI test duration * Fix Clippy nested-if lint in REPL startup * Fix single-message approval flow * Handle empty single-message REPL exits * Wait for one-shot event routines before exit * Fix MCP lifecycle trace user scope * Normalize cron schedules on routine create |
||
|
|
706c3a1b47 |
refactor: extract AppEvent to crates/ironclaw_common (#1615)
* refactor: extract AppEvent to crates/ironclaw_common SseEvent was defined in src/channels/web/types.rs but imported by 12+ modules across agent, orchestrator, worker, tools, and extensions — it had become the application-wide event protocol, not a web transport concern. Create crates/ironclaw_common as a shared workspace crate and move the enum there as AppEvent. Also move the truncate_preview utility which was similarly leaked from the web gateway into agent modules. - New crate: crates/ironclaw_common (AppEvent, truncate_preview) - Rename SseEvent → AppEvent, from_sse_event → from_app_event - web/types.rs re-exports AppEvent for internal gateway use - web/util.rs re-exports truncate_preview - Wire format unchanged (serde renames are on variants, not the enum) Aligned with the event bus direction on refactor/architectural-hardening where DomainEvent (≡ AppEvent) is wrapped in a SystemEvent envelope. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: add AppEvent::event_type() helper, deduplicate match blocks Address Gemini review: extract the variant→string match into a single method on AppEvent, replacing the duplicated 22-arm matches in sse.rs and types.rs. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: rename leftover sse vars/tests to match AppEvent rename Address Copilot review: rename sse_event vars to app_event in orchestrator/api.rs and ws.rs, rename test functions from test_ws_server_from_sse_* to test_ws_server_from_app_event_*, and update stale SSE comments. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: add Deserialize to AppEvent, round-trip test, fix stale comments Address zmanian review: - Add Deserialize derive to AppEvent so downstream consumers can deserialize incoming events - Add event_type_matches_serde_type_field test that round-trips every variant through serde and asserts event_type() matches the serialized "type" field — catches drift between serde renames and the manual match - Add round_trip_deserialize test for basic Serialize/Deserialize parity - Update remaining "SSE" references in comments across server.rs, manager.rs, ws_gateway_integration.rs, and worker/job.rs Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
|
b441ebec02 |
feat: multi-tenant auth with per-user workspace isolation (#1118)
* feat: multi-tenant auth with per-user scoping Multi-user authentication and authorization for IronClaw gateway: - Token-based auth mapping tokens to user IDs via GATEWAY_USER_TOKENS - Per-user SSE broadcast scoping - Per-user rate limiting with poisoned lock recovery - Handler auth and ownership checks for jobs, settings, routines - Extension secrets scoped per-user - Chat handlers use authenticated identity - Reverse proxy deployment documentation - Comprehensive integration tests for auth, SSE, rate limiting, and job isolation * fix: scope memory tools per-user in multi-tenant mode Memory tools (search, write, read, tree) held a single workspace created at startup with GATEWAY_USER_ID. In multi-tenant mode, all users' tool calls searched the default user's scope. Add WorkspaceResolver trait that resolves workspaces per-request using JobContext.user_id. In single-user mode, returns the startup workspace. In multi-tenant mode (GATEWAY_USER_TOKENS configured), creates and caches per-user workspaces on demand. Includes regression tests for workspace resolution and user isolation. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: comprehensive multi-tenant isolation audit Address all review findings from @serrrfirat plus 7 additional gaps found via full security audit: Reviewer findings (5): - WorkspacePool now applies search config, memory layers, embedding cache, identity read scopes, and global config scopes (was bare) - jobs_summary_handler uses per-user queries instead of global counters - jobs_prompt_handler restructured to not 404 agent jobs + ownership check - jobs_restart_handler agent branch now verifies user ownership - agent_job_summary_for_user added to Database trait + both backends Audit findings (7): - Delete dead handlers/memory.rs (stale copies with no auth) - Add AuthenticatedUser to logs_events, logs_level_get, logs_level_set - Add AuthenticatedUser to extensions_tools_handler, gateway_status_handler - Add auth + ownership checks to all 6 routines handlers - Add auth to all 4 skills handlers with audit logging on mutations - Scope extension setup SSE broadcast to user (broadcast_for_user) - Fix pre-existing test compilation errors in extensions/manager.rs 17 new multi-tenant isolation tests covering: - WorkspacePool config propagation and scope merging - Jobs handler per-user isolation (summary, restart, prompt, cancel) - Routines handler auth enforcement and cross-user rejection - Auth middleware enforcement on logs, skills, status endpoints Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: second-pass multi-tenant audit — scope SSE broadcasts, DB queries, dead handlers Second audit pass applying learned patterns across the codebase: - OAuth callback SSE broadcasts now use broadcast_for_user (lines 773, 912) - jobs_list_handler uses list_agent_jobs_for_user instead of fetching all users' jobs and filtering in Rust - list_agent_jobs_for_user added to Database trait + postgres + libsql - Dead handler files (extensions.rs, static_files.rs) hardened with AuthenticatedUser to prevent auth regression if migrated Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address review findings — token hashing, broadcast scoping, error handling Security fixes: - Hash tokens with SHA-256 at construction time so authentication compares fixed-size 32-byte digests, eliminating length-oracle timing leaks - Scope auth SSE broadcasts per-user in chat_auth_token_handler — AuthRequired/AuthCompleted events were leaking across tenants - Propagate DB errors in restart handlers instead of silently swallowing via `if let Ok(Some(...))` pattern Code quality: - Log SSE serialization failures instead of silently producing empty strings via unwrap_or_default() - Remove dead `pub type AuthState = MultiAuthState` alias - Replace `.unwrap()` with `Arc::clone(db)` in app.rs multi-tenant workspace setup (db is guaranteed Some in context, but unwrap violates project convention) - Fix telegram setup test to inject UserIdentity into request extensions (handler now requires AuthenticatedUser) - Add safety comments on test-only expect/unwrap calls for CI - Apply cargo fmt to fix pre-existing formatting Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address review findings — unify workspace pool, fix SSE regression, cache job owners - Unify WorkspacePool and PerUserWorkspaceResolver: WorkspacePool now implements WorkspaceResolver, eliminating duplicate per-user workspace construction logic. app.rs uses WorkspacePool directly. - Fix sse_tx: None scheduler regression: change scheduler/worker SSE broadcasting from broadcast::Sender<SseEvent> to Arc<SseManager>, restoring SSE event delivery for scheduled agent jobs. - Cache job owner in orchestrator: add job_owner_cache to OrchestratorState so job_event_handler avoids a DB round-trip on every event after the first per job. - Deduplicate ext_user_id computation in main.rs. - Remove unused _gateway_state variable. - Fix pre-existing test: first_token() returns None in multi-user mode by design; align test assertion. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * style: fix formatting in app.rs Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: extract memory handlers back into handlers/memory.rs Move memory API handlers out of server.rs into their own module, consistent with how jobs, routines, and skills handlers are organized. The resolve_workspace() helper moves with them since it is only used by memory handlers. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: [email protected] <[email protected]> |
||
|
|
fa51b9f52d |
fix: post-merge review sweep — 8 fixes across security, perf, and correctness (#1550)
* fix: post-merge review sweep — 8 fixes across security, perf, and correctness 1. Fix code fence detection in extract_suggestions() (issue #1180) - rfind("```") couldn't handle odd fence counts (unclosed blocks) - Now counts all fence positions and checks parity 2. Cache routine parameters_schema() with OnceLock (issue #1361) - routine_create_parameters_schema() and event_emit_parameters_schema() were regenerating JSON on every LLM call 3. Replace O(n) LRU eviction with lru crate (issue #1430) - Embedding cache now uses lru::LruCache for O(1) eviction - Removes manual HashMap + last_accessed tracking 4. Fix WASM router secret_validated semantics (issue #1281) - Now reflects whether any auth (secret/Ed25519/HMAC) was performed - Previously only checked if a secret was configured 5. Sanitize channel/user in routine prompt interpolation (issue #1364) - Defense-in-depth: strip newlines, replace backticks, truncate to 128 chars before injecting into LLM prompt 6. Remove duplicate 401 retry in github_copilot.rs (PR #1512 review) - Internal retry conflicted with outer RetryProvider causing nested retries; now invalidates token and lets RetryProvider handle retry 7. Fix token error classification in github_copilot.rs (PR #1512 review) - AccessDenied/Expired errors now map to AuthFailed (non-retryable) - Transient errors remain RequestFailed (retryable) 8. Fix parse_extra_headers() hardcoded env var name (PR #1512 review) - Error messages now report the actual env var being parsed instead of always saying LLM_EXTRA_HEADERS Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address PR review comments and fix formatting - sanitize_prompt_field: single-pass with map() instead of collect+replace - embed(): re-check cache under lock before cloning (thundering herd) - embed_batch(): limit caching to cache capacity, skip overflow entries - router: thread did_authenticate bool instead of re-calling async methods - github_copilot 401: use generic error message, avoid leaking response body - cargo fmt: fix two formatting violations caught by CI Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * chore: trigger CI re-run with updated refs [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
Henry ParkandGitHub
|
dea789cca9 |
Default new lightweight routines to tools-enabled (#1573)
* Default new lightweight routines to tools-enabled * Fix fmt and clippy on lightweight routine PR * Use grouped execution field in routine no-tools fixture * Align CLI routine defaults with tools-enabled lightweight mode |
||
|
|
d9358b0fa9 |
feat(workspace): multi-scope workspace reads (#1117)
* feat(workspace): multi-scope workspace reads Adds the ability for a workspace to read from multiple user scopes while keeping writes isolated to the primary scope. Configuration via WORKSPACE_READ_SCOPES env var (comma-separated user IDs). Includes identity file isolation (read_primary), multi-scope search, list, and read operations, WorkspaceConfig refactor, and comprehensive integration tests. * fix: address review feedback for multi-scope workspace reads - fix(memory): deduplicate timezone parsing for daily_log target parse_timezone was called twice when target was "daily_log" without a layer — once in path resolution, again in the fallback. Now computed once and reused. - fix(config): add character validation for WORKSPACE_READ_SCOPES and layer scopes — both enforce [a-zA-Z0-9_-] to prevent path traversal or injection via scope strings used as user_id in SQL queries. - fix(config): use chars().take(32) instead of byte-index slicing for scope length error messages (UTF-8 safety). - fix(error): remove unused WorkspaceError::NotFound variant Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * style: downgrade search log to debug, add comments on list iteration - Downgrade hybrid_search_multi tracing::info! to debug! — fires on every multi-scope search with the default backend, too noisy for info - Add comments explaining why list/list_all iterate per-scope instead of using _multi trait methods (identity path filtering needs scope attribution that merged results lose) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: [email protected] <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
Henry ParkandGitHub
|
4d7501a968 |
Fix owner-scoped message routing fallbacks (#1574)
* Fix owner-scoped message routing fallbacks * Address PR feedback on routing regressions * Address review notes on routing fallbacks |
||
|
|
b58b421535 |
feat(shell): add Low/Medium/High risk levels for graduated command approval (closes #172) (#368)
* feat(shell): add Low/Medium/High risk levels for graduated approval (#172) - Add `RiskLevel` enum (Low/Medium/High, Ord-comparable) to `tool.rs` and re-export from `tools/mod.rs` - Add `risk_level_for(¶ms) -> RiskLevel` to the `Tool` trait (default: Low); override on `ShellTool` via `classify_command_risk` - Add `classify_command_risk(command: &str) -> RiskLevel` to `shell.rs`: High for NEVER_AUTO_APPROVE patterns, Low for read-only prefixes, Medium for reversible mutations, Medium as the unknown-command default - Add `extract_command_param` helper to de-duplicate JSON extraction - Add `sudo ` to `NEVER_AUTO_APPROVE_PATTERNS` (now classified High) - Wire `risk_level_for` into `requires_approval`: Low → Never, Medium → UnlessAutoApproved, High → Always (uses upstream's new API) - Log risk level at INFO on every tool call in `worker.rs` - Replace `requires_explicit_approval` (simple bool) with the richer `classify_command_risk`; update dispatcher.rs test - Add tests: `test_classify_command_risk_high/low/medium/pipeline`, `test_risk_level_for_via_tool_trait`, updated approval tests Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * style: apply cargo fmt to shell.rs and dispatcher.rs Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(shell): fix pipeline risk aggregation and word-boundary matching Address reviewer feedback: - `classify_command_risk` now iterates ALL pipeline segments and takes the maximum risk, so `echo hello | cargo build` → Medium instead of the previous (wrong) Low - Replace `starts_with` with `matches_command_pattern`: single-word patterns use exact first-token comparison so `lsblk` no longer matches `ls`, `makeself` no longer matches `make`, etc.; multi-word patterns (e.g. `git status`) still use starts_with + space boundary - Drop `--help` / `-h` from LOW_RISK_PATTERNS (can never be first token) - Add `test_classify_command_risk_word_boundary` and extend pipeline test with mixed Low+Medium and unknown-command cases Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(shell): move sed/awk/find from Low to Medium risk `sed -i`, `awk -i inplace`, and `find -delete`/`find -exec rm` can all modify or delete files. Classifying these as Low (auto-approve) was unsafe. Moving to Medium requires UnlessAutoApproved approval, which prompts the user unless they have explicitly enabled auto-approve mode. Fixes review feedback from zmanian on PR #368. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(shell): update test to use classify_command_risk after requires_explicit_approval removal The rebase brought in upstream commits that removed requires_explicit_approval. Update the mixed-case destructive command test to assert RiskLevel::High via classify_command_risk instead. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(shell): use word-boundary matching for High-risk patterns to prevent false positives The NEVER_AUTO_APPROVE_PATTERNS check used `contains()` on the full command string, causing false positives: `makeshutdownscript` matched `shutdown`, `nftables-config` matched `nft`, and `passwdqc-check` matched `passwd`. Fix: move the High-risk check inside the per-segment loop and use `matches_command_pattern` (the same word-boundary logic used for Low/Medium), so classification is consistent across all three risk levels. Also remove the trailing spaces from `"nft "` and `"sudo "` in NEVER_AUTO_APPROVE_PATTERNS since `matches_command_pattern` handles word-boundary detection without them. Adds three regression tests for the false-positive cases. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(shell): address zmanian review — redirect safety + explicit git push pattern Two issues from zmanian's CHANGES_REQUESTED review on PR #368: 1. **Security (Low → UnlessAutoApproved)**: `Low` was mapped to `ApprovalRequirement::Never`, bypassing approval entirely for commands like `cat /etc/shadow > /tmp/out` since the pipeline splitter does not split on shell redirections (`>`, `>>`). Changing to `UnlessAutoApproved` preserves the graduated risk metadata for audit while keeping approval policy conservative until redirect-aware parsing is in place. 2. **Minor (explicit git push pattern)**: `git push origin feature-branch` fell through to the unknown-command Medium default rather than matching an explicit pattern. Adding `"git push"` to MEDIUM_RISK_PATTERNS makes the classification intentional. Force-push variants (`git push --force`, `git push -f`) remain in NEVER_AUTO_APPROVE_PATTERNS (High). Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * test(shell): add regression tests for redirect bypass and git push pattern fixes Two regression tests for the fixes in the previous commit: 1. `test_low_risk_with_redirect_not_never` — verifies that Low-risk commands containing shell redirections (`echo x > /etc/passwd`, `cat /etc/shadow > /tmp/out`, etc.) return `UnlessAutoApproved`, not `Never`. Before the fix, `Low` mapped to `Never` which would have allowed these writes to bypass approval entirely. 2. `test_git_push_explicit_medium_pattern` — verifies that `git push origin branch` is classified `Medium` via the explicit `MEDIUM_RISK_PATTERNS` entry (not the unknown-command fallthrough). Force variants (`--force`, `-f`) remain `High`. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * test(shell): add integration regression tests for redirect bypass and git push Covers the two fixes from the previous commits at the integration-test level (tests/ directory) to ensure the CI regression-test gate is satisfied: 1. `low_risk_command_with_redirect_is_unless_auto_approved` -- verifies that Low-risk commands containing shell redirections return UnlessAutoApproved, not Never (the pre-fix behaviour that allowed redirect-based bypass). 2. `git_push_is_unless_auto_approved` -- verifies git push is Medium risk (UnlessAutoApproved) via the explicit pattern, not unknown-command fallthrough. 3. `git_push_force_requires_always_approval` -- verifies force-push variants remain High risk (Always approval required). Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * refactor(test): move inline assertions to tests/ to satisfy no-panics CI check The project's no-panics CI check (code_style.yml) scans src/**/*.rs for assert_eq!/assert_ne!/.unwrap() in added lines. Moving classify_command_risk tests to tests/shell_risk_regression.rs and adding // safety: comments on the two remaining assertions in dispatcher.rs eliminates all false positives. - Remove test_classify_command_risk_* and related functions from shell.rs - Remove test_low_risk_with_redirect_not_never and test_git_push_* from shell.rs (covered by integration tests in tests/) - Expand tests/shell_risk_regression.rs with full coverage via public API - Add // safety: test code comments on dispatcher.rs assert lines Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(shell): address review findings — force-with-lease, test runners, Display - Add `git push --force-with-lease` to NEVER_AUTO_APPROVE_PATTERNS — the word-boundary matching in matches_command_pattern would not match it against the existing `git push --force` pattern (next char is `-`, not space), causing it to fall through to Medium instead of High. - Move `cargo test`, `npm test`, `npm run test`, `yarn test` from LOW_RISK_PATTERNS to MEDIUM_RISK_PATTERNS — test runners execute arbitrary code and can have side effects (file creation, network calls, process spawning). - Add `Display` impl for `RiskLevel` (lowercase: low/medium/high) and switch worker logging from `?risk` (Debug) to `%risk` (Display) for cleaner audit logs. - Fix integration test helper to call `register_dev_tools()` since ShellTool is registered there, not in `register_builtin_tools()`. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Sonnet 4.6 <[email protected]> Co-authored-by: [email protected] <[email protected]> |
||
|
|
8ad7d78a70 |
fix: parameter coercion and validation for oneOf/anyOf/allOf schemas (#1397)
* fix: parameter coercion and validation for oneOf/anyOf/allOf schemas WASM extension tools with multi-action schemas (e.g. github extension) fail when the LLM passes numeric parameters as strings because the coercion layer skips JSON Schema combinators. This causes serde deserialization errors like `invalid type: string "100", expected u32`. Add discriminated-union resolution to the coercion layer: for oneOf/anyOf, match the active variant by const or single-element enum discriminators; for allOf, merge all variants' properties. Also propagate combinator awareness to schema validators, WASM wrapper helpers, and tool discovery so they no longer reject or ignore valid combinator-based schemas. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * test: add e2e tests for oneOf discriminated union parameter coercion Add three end-to-end tests using a fixture tool that mirrors the github WASM tool's oneOf schema with #[serde(tag = "action")] deserialization. Each test sends string-typed numeric/boolean params through the full agent loop, verifying that coercion resolves them before serde runs: - list_issues: limit "100" → 100 (integer in oneOf variant) - get_issue: issue_number "42" → 42 (integer in different variant) - create_pull_request: draft "true" → true (boolean in variant) Without the coercion fix these fail with: invalid type: string "100", expected u32 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * test: add real WASM github tool e2e tests with HTTP interception Load the actual compiled github WASM binary, send params with string-typed numbers through the coercion layer, and verify the WASM tool constructs correct HTTP API calls via a new HTTP interceptor in the WASM wrapper. Changes: - Add `http_interceptor` field to `StoreData` and `WasmToolWrapper` so WASM tool HTTP requests can be captured/mocked in tests - Make `prepare_tool_params` and `coercion` module public for integration tests - Add 3 e2e tests loading the real github WASM binary: - list_issues: `limit: "50"` → URL contains `per_page=50` - get_issue: `issue_number: "42"` → URL contains `/issues/42` - list_pull_requests: `limit: "25"` → URL contains `per_page=25` Tests gracefully skip if the WASM binary isn't compiled. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: simplify WASM e2e tests to use TestRig with with_wasm_tool() Replace the manual WasmToolWrapper construction with TestRig integration: - Add `with_wasm_tool(name, wasm_path, capabilities_path)` to TestRigBuilder that loads real WASM binaries and wires the shared HTTP interceptor - Build the HTTP interceptor before tool registration so it can be shared between AgentDeps and WASM tool wrappers - Rewrite github WASM e2e tests to use the standard trace pattern: TraceLlm sends tool calls with string params, http_exchanges specify expected outgoing requests and canned responses The test code is now identical to other trace-based e2e tests — no custom interceptors or manual WASM construction needed. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address review comments on combinator schema support - Validate `has_combinators` checks array type (`.as_array().is_some()`) instead of bare `.is_some()` to reject malformed `{ "oneOf": {} }` - Validate top-level `required` keys against merged combinator variant properties when no top-level `properties` exists (both validators) - Deduplicate oneOf/anyOf handling into single loop in coercion.rs - Revert `pub mod coercion` to private; only re-export `prepare_tool_params` - Call `after_response` on interceptor after real HTTP when `before_request` returns None (recording mode correctness) - Fix formatting (CI failure) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address second round of review comments - Fix headers deserialization bug: deserialize resp.headers_json as HashMap<String, String> then convert to Vec, not directly as Vec - Sort interceptor headers for deterministic trace fixtures - Update after_response comment: RecordingHttpInterceptor does exercise this path (returns None from before_request) - Mark WASM tests #[ignore] instead of silent skip — avoids false-green CI while keeping them runnable with --ignored - Fix with_wasm_tool signature: Option<PathBuf> instead of Option<impl Into<PathBuf>> which doesn't compile in nested position - Fix with_wasm_tool doc comment to match actual behavior - Revert prepare_tool_params to pub(crate) — no longer needed publicly Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: coerce empty strings to null for optional tool parameters LLMs often send "" instead of null/omitting optional parameters, causing parse errors in tools that expect typed values (e.g., timezone, schedule). PR #1127 fixed this per-field in the time tool. This commit adds dispatcher-level coercion so all tools benefit: - Non-required properties with value "" are coerced to null at the object level (based on the schema's `required` array) - Explicitly nullable schemas (`type: ["string", "null"]`) coerce "" to null in the per-value coercion path - Required string-only fields keep "" unchanged Closes #755 Co-Authored-By: spiritj <[email protected]> Co-Authored-By: Xing Ji <[email protected]> Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: complete coercion coverage for $ref, nested combinators, and additionalProperties Close remaining coercion gaps so 3rd-party tools (MCP servers, complex WASM tools) work correctly: - $ref resolution: inline all #/definitions/<name> and #/$defs/<name> references in a pre-pass before coercion, with depth limit (16) for circular ref safety - Nested combinators: resolve_effective_properties now recurses into variants that themselves contain allOf/oneOf/anyOf (depth limit 4) - additionalProperties inheritance: check allOf variants and matched oneOf/anyOf variant for additionalProperties schemas New tests: - resolves_ref_and_coerces_referenced_properties - resolves_nested_refs_in_oneof_variants - coerces_nested_combinators_allof_containing_oneof - coerces_array_items_with_oneof_discriminator - circular_ref_does_not_infinite_loop Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address third round of review comments - Validators: tighten has_combinators to require at least one object-typed variant (has type:"object" or properties), rejecting non-object combinator schemas like { "oneOf": [{"type":"integer"}] } - Empty-string coercion: only coerce "" → null when schema allows null or doesn't allow string; pure type:"string" fields keep "" as meaningful - Fix comment: "coerce to null" → "return unchanged" for empty strings with no type match (code returns None, not null) - Redact credentials before passing to after_response interceptor to prevent secret leakage into recorded trace files - Switch to tokio::fs::read for async WASM binary loading in test rig - Add doc comment explaining soft URL check in WASM e2e tests Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * ci: retrigger after staging merge [skip-regression-check] * fix: merge staging, report non-array combinator values as errors Merge latest staging to fix CI (missing fallback_deliverable field). Add explicit error reporting when oneOf/anyOf/allOf values are not arrays in both strict and lenient validators. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: recurse into combinator variants that have properties but no explicit type Both validators only recursed into variants with `type: "object"`, missing variants that define `properties` without an explicit type (common in allOf patterns). Now recurse when variant has either. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: spiritj <[email protected]> Co-authored-by: Xing Ji <[email protected]> |
||
|
|
212d661e20 |
feat(workspace): layered memory with sensitivity-based privacy redirect (#1112)
* feat(workspace): layered memory with sensitivity-based privacy redirect Introduce MemoryLayer type for named memory layers with sensitivity levels and write permissions. Layers map to synthetic user_id values in workspace tables, enabling shared/private memory isolation. - Add MemoryLayer, LayerSensitivity types with default_for_user() - Add layer-aware write methods (write_to_layer, append_to_layer) - Add PatternPrivacyClassifier to guard shared layer writes - Add optional 'layer' parameter to memory_write tool and HTTP API - Add 'redirected' and 'actual_layer' fields to write response - Add MEMORY_LAYERS env var (JSON) for layer configuration - Workspace user_id now derived from GATEWAY_USER_ID (was hardcoded "default") - 10 integration tests for layered memory operations Addresses prerequisite for Issue #59 (multi-tenancy). Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: add explicit default to memory_write layer schema Add "default": "private" to the layer parameter's JSON schema so LLM tool consumers can see the default without reading code. Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: extract resolve_layer_target to deduplicate layer writes Consolidate shared layer-lookup, writable check, and privacy classification logic from write_to_layer and append_to_layer into a single resolve_layer_target helper. Flagged on #349 review — the duplication originates in this PR. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review feedback on layered memory PR - Fix email regex pipe bug in TLD character class (privacy.rs) - Add append support to web memory_write handler via `append` field - Validate MemoryLayer name/scope: reject empty, check duplicates - Remove hardcoded 'private' default from tool schema; omit layer fields from output when no layer specified - Document scope isolation risk for multi-tenant (Issue #59) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address adversarial review findings - CRITICAL: fix identity file protection bypass via trailing slash (normalize target path before protection checks) - HIGH: check private layer is writable before privacy redirect - HIGH: map LayerNotFound/ReadOnly to proper 4xx HTTP status codes - HIGH: honor `append` field in non-layer HTTP write path - MEDIUM: remove redundant DB fetch in append_to_layer (narrower TOCTOU window) - MEDIUM: remove dead memory_write_handler from handlers/memory.rs Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: opt-in privacy classifier, force override, confidence scoring Address review feedback from @zmanian: - Privacy classifier is now opt-in via with_privacy_classifier() instead of always-on. Default hardcoded patterns (doctor, therapy, email, phone) had unacceptable false positive rates in household contexts. LLM chooses the correct layer via system prompt; regex can't improve on that. - Add ConfigurablePrivacyClassifier for operator-supplied patterns. - PatternPrivacyClassifier defaults narrowed to hard PII only (SSN, credit card, credentials). - Add force param to write_to_layer/append_to_layer to skip classifier. - PrivacyClassifier trait returns SensitivityResult { is_sensitive, confidence } instead of bool, ready for probabilistic classifiers. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: remove redundant heartbeat match arm in memory_write The heartbeat arm was identical to the catch-all — resolved_path already points to paths::HEARTBEAT when target is "heartbeat". Addresses review feedback from gemini-code-assist on #1112. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: return Result from PatternPrivacyClassifier::new() Replace .expect() with proper error propagation per project no-panics policy. Remove Default impl (unused in production). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: move memory_layers from GatewayConfig to WorkspaceConfig Resolve merge conflicts between HEAD (transcription, search, env helpers) and the workspace config branch. GatewayConfig no longer owns memory_layers; WorkspaceConfig::resolve() handles parsing, validation (name length >64, character set, empty scope, duplicates), and fallback defaults. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * test: strengthen privacy classifier and layer isolation coverage Add 8 privacy classifier edge case tests (format variants, keywords, longer documents, empty/partial inputs) and 5 layer write isolation integration tests (cross-scope invisibility, overwrite, empty path, sensitive-to-private no-redirect). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: tautological test assertion and add WorkspaceConfig validation tests Replace always-true `is_ok() || is_err()` in write_empty_path_to_layer with actual behavior assertion (write succeeds with normalized empty path). Add 8 unit tests for WorkspaceConfig::resolve() covering valid JSON parsing, invalid JSON, empty/long/invalid-char layer names, empty scopes, duplicates, and default fallback behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * style: cargo fmt after staging merge Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: [email protected] <[email protected]> |
||
|
|
6d847c6009 |
feat(webhooks): add public webhook trigger endpoint for routines (#736)
* feat(webhooks): add public webhook trigger endpoint for routines
Add POST /api/webhooks/{path} endpoint that matches incoming webhooks
against routines with Trigger::Webhook, validates secrets using
constant-time comparison (subtle crate), and fires the matched routine
through the message pipeline.
Closes #651
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix(webhooks): address PR review feedback - access control, targeted query, rate limiting
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix(ci): add missing webhook_rate_limiter field and fix formatting
Add the webhook_rate_limiter field to the GatewayState initializer in
gateway_workflow_harness.rs and fix rustfmt formatting for the webhook
tuple in types.rs.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix(security): require webhook secret, add rate limiting, improve tests
Extract validate_webhook_secret() from the handler so the security-critical
secret validation logic (mandatory secret, constant-time comparison) is
directly testable without mocking the database layer. Improves the error
message for misconfigured routines to guide users toward the fix.
Replaces the previous unit tests (which only tested Rust pattern matching
and status code constants) with tests that exercise the actual validation
function against all rejection paths: missing secret (403), non-webhook
trigger (403), wrong secret (401), empty secret (401), and different-length
secret (401).
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
* Route webhook triggers through RoutineEngine instead of chat pipeline
Adds fire_webhook() to RoutineEngine and updates the webhook handler
to use it. This ensures webhook-triggered routines get proper run
tracking, guardrail enforcement (cooldown + max_concurrent),
notifications, and FullJob dispatch support.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* style: fix formatting in webhook handler
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---------
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: [email protected] <[email protected]>
|
||
|
Henry ParkandGitHub
|
ee6f5cd62a |
Use live owner tool scope for autonomous routines and jobs (#1453)
* Use live owner tool scope for autonomous runs * Address autonomous tool scope review feedback * Normalize routine context paths again |
||
|
|
e82f4bd2e5 |
fix: register sandbox jobs in ContextManager for query tool visibility (#1426)
* fix: register sandbox jobs in ContextManager for query tool visibility
Sandbox jobs created via execute_sandbox() were persisted to the database
but never registered in the in-memory ContextManager. Since all query tools
(list_jobs, job_status, job_events, cancel_job) only search the
ContextManager, sandbox jobs were invisible to the agent despite running
successfully in Docker containers.
Changes:
- Add register_sandbox_job() to ContextManager (pre-determined UUID,
starts InProgress, respects max_jobs)
- Extract insert_context() helper to deduplicate create_job_for_user
and register_sandbox_job
- Add update_context_state / update_context_state_async to sync
ContextManager state on sandbox job completion/failure
- Extend job_monitor with spawn_job_monitor_with_context() and
spawn_completion_watcher() so fire-and-forget jobs transition out
of InProgress when the container finishes
- Make CancelJobTool sandbox-aware (stops container + updates DB)
- Wire sandbox deps into CancelJobTool in register_job_tools()
- 8 regression tests across context manager and job monitor
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
* fix: add missing allow_always field in PendingApproval test literal
Upstream commit
|
||
|
Henry ParkandGitHub
|
b952d229f9 |
fix: prefer execution-local message routing metadata (#1449)
* fix: prefer execution-local message routing metadata * test: cover message routing fallback metadata * refactor: simplify message target resolution * fix: ignore stale channel defaults for notify user metadata |
||
|
|
806d402876 |
feat: chat onboarding and routine advisor (#927)
* feat: port NPA psychographic profiling system into IronClaw
Port the complete psychographic profiling system from NPA into IronClaw,
including enriched profile schema, conversational onboarding, profile
evolution, and three-tier prompt augmentation.
Personal onboarding moved from wizard Step 9 to first assistant
interaction per maintainer feedback — the First Contact system prompt
block now instructs the LLM to conduct a natural onboarding conversation
that builds the psychographic profile via memory_write.
Changes:
- Enrich profile.rs with 5 new structs, 9-dimension analysis framework,
custom deserializers for backward compatibility, and rendering methods
- Add conversational onboarding engine with one-step-removed questioning
technique, personality framework, and confidence-scored profile generation
- Add profile evolution with confidence gating, analysis metadata tracking,
and weekly update routine
- Replace thin interaction style injection with three-tier system gated on
confidence > 0.6 and profile recency
- Replace wizard Step 9 with First Contact system prompt block that drives
conversational onboarding during the user's first interaction
- Add autonomy progression to SOUL.md seed and personality framework to
AGENTS.md seed
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* feat: replace chat-based onboarding with bootstrap greeting and workspace seeds
Remove the interactive onboarding_chat.rs engine in favor of a simpler
bootstrap flow: fresh workspaces get a proactive LLM greeting that
naturally profiles the user. Identity files are now seeded from
src/workspace/seeds/ instead of being hardcoded. Also removes the
identity-file write protection (seeds are now managed), adds routine
advisor integration, and includes an e2e trace for bootstrap greeting.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* feat(safety): sanitize identity file writes via Sanitizer to prevent prompt injection
Identity files (SOUL.md, AGENTS.md, USER.md, IDENTITY.md) are injected into
every system prompt. Rather than hard-blocking writes (which broke onboarding),
scan content through the existing Sanitizer and reject writes with High/Critical
severity injection patterns. Medium/Low warnings are logged but allowed.
Also clarifies AGENTS.md identity file roles (USER.md = user info, IDENTITY.md =
agent identity) and adds IDENTITY.md setup as an explicit bootstrap step.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* docs: update profile_onboarding_completed comment to reflect current wiring
The field is now actively used by the agent loop to suppress BOOTSTRAP.md
injection — remove the stale "not yet wired" TODO.
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix(setup): use env_or_override for NEARAI_API_KEY in model fetch config
When the user authenticates via NEAR AI Cloud API key (option 4),
api_key_login() stores the key via set_runtime_env(). But
build_nearai_model_fetch_config() was using std::env::var() which
doesn't check the runtime overlay — so model listing fell back to
session-token auth and re-triggered the interactive NEAR AI
authentication menu.
Switch to env_or_override() which checks both real env vars and the
runtime overlay.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix(agent): correct channel/user_id in bootstrap greeting persist call
persist_assistant_response was called with channel="default",
user_id="system" but the assistant thread was created via
get_or_create_assistant_conversation("default", "gateway") which owns
the conversation as user_id="default", channel="gateway". The mismatch
caused ensure_writable_conversation to reject the write with:
WARN Rejected write for unavailable thread id user=system channel=default
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix(web): remove all inline event handlers for CSP compliance
The Content-Security-Policy header (added in
|
||
|
|
cac6f4013c |
Add owner-scoped permissions for full-job routines (#1440)
* docs: add comments explaining CLI_ENABLED=false in service templates (#990) Clarify that CLI_ENABLED=false is needed in daemon mode (launchd/systemd) to prevent blocking on stdin when running as a background service. Closes #990 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * Add owner-scoped full-job routine permissions * Address PR review feedback * Fix owner gate test timing --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
|
65062f3cc0 |
feat: structured fallback deliverables for failed/stuck jobs (#236)
* feat: structured fallback deliverables for failed/stuck jobs (#221) When a job fails or gets stuck, build a FallbackDeliverable that captures partial results, action statistics, cost, timing, and repair attempts. This replaces opaque error strings with structured data users can act on. - Add FallbackDeliverable, LastAction, ActionStats types in context/fallback.rs - Store fallback in JobContext.metadata["fallback_deliverable"] on failure - Surface fallback in job_status tool output and SSE job_result events - Update mark_failed() and mark_stuck() in worker to build fallback - 8 unit tests covering zero/mixed actions, truncation, timing, serialization Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review comments on fallback deliverables - Fix doc comment: "200 chars" -> "200 bytes (UTF-8 safe)" since truncate_str operates on byte length, not character count. - Add code comment documenting that SSE fallback_deliverable is currently always None (forward-compatible infrastructure). Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: take Option<&FallbackDeliverable> instead of &Option<…> Addresses Gemini review feedback: idiomatic Rust prefers Option<&T> over &Option<T> for borrowed optional values. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: guard against non-object metadata and add fallback test - store_fallback_in_metadata now resets metadata to {} when it's any non-object type (string, array, number), not just null. Prevents panic on index assignment. - Add test_job_status_includes_fallback_deliverable to verify the fallback field is surfaced in job_status tool output. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use sanitized output in fallback preview + add integration tests Security fix: FallbackDeliverable::build() now uses output_sanitized instead of output_raw, preventing secrets/PII from leaking through the job_status tool and SSE job_result events. Also adds: - test_fallback_uses_sanitized_output: proves raw secrets don't leak - test_store_fallback_in_metadata_roundtrip: full serialize/deserialize - test_store_fallback_handles_non_object_metadata: edge case coverage - test_store_fallback_none_is_noop: None input is safe Addresses serrrfirat review feedback on PR #236. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: harden fallback deliverables against review findings - Truncate failure_reason to 1000 bytes to prevent metadata bloat - Add tracing::warn on fallback serialization failure (was silently discarded) - Fix module/struct docs to cover stuck jobs, remove stale SSE claim - Fix job.rs test to use real FallbackDeliverable field names - Add tests for failure_reason truncation and completed_at=None elapsed time - Fix pre-existing clippy warning in settings.rs (field_reassign_with_default) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address Copilot review findings on fallback deliverables - Fix output_raw/output_sanitized field swap in ActionRecord::succeed() so sanitized data actually goes into the sanitized field (security) - Return None instead of empty Memory when get_memory fails in build_fallback, with tracing::warn for observability - Replace manual elapsed calculation with ctx.elapsed() which already clamps negative durations Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: resolve rebase conflicts and update tests for parameter swap - Add fallback field to SseEvent::JobResult in job_monitor - Fix type annotation in fallback deliverable test - Update test_action_record_succeed_sets_fields for new parameter order - Use create_job_for_user in test (API changed on main) Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: trigger CI re-check after rebase * fix: fall back to error message for failed action output_preview When the last action is a failed tool call, output_sanitized is None, leaving output_preview empty. Now falls back to the action's error message so users see what went wrong. [skip-regression-check] * ci: add safety comments to test code for no-panics check The CI no-panics grep check cannot distinguish test code inside src/ files from production code. Add // safety: test annotations to .unwrap(), .expect(), and assert!() calls in #[cfg(test)] modules. * fix: clarify succeed() doc and avoid clone in output_preview - Fix doc comment: output_raw is stored as pretty-printed JSON string, not a raw JSON value - Borrow string slice directly in fallback preview to avoid cloning potentially large sanitized outputs before truncation * refactor: reuse floor_char_boundary in truncate_str Replace hand-rolled UTF-8 boundary logic with existing crate::util::floor_char_boundary to reduce duplication. * fix: rename SSE fallback field to fallback_deliverable for consistency The SSE JobResult field was named `fallback` while everywhere else (metadata key, job_status tool) uses `fallback_deliverable`. Align the SSE wire format to avoid forcing clients to handle two names. --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
09e1c97a27 |
fix(approval): make "always" auto-approve work for credentialed HTTP requests (#1257)
The HTTP tool returned `ApprovalRequirement::Always` for requests with credentials, but `Always` is hardcoded to ignore the session auto-approve set. This meant users who clicked "always" were re-prompted on every subsequent HTTP call — the UI offered "always" but the backend ignored it. Two fixes: 1. HTTP credentialed requests now return `UnlessAutoApproved` instead of `Always`, so the session auto-approve set is respected. 2. `StatusUpdate::ApprovalNeeded` now carries `allow_always: bool`. All channel UIs (Telegram, Slack, Signal, REPL, Web) conditionally hide the "always" option when a tool truly requires per-invocation approval (`ApprovalRequirement::Always`, e.g. destructive shell commands). Also boxes `PendingApproval` in `AgenticLoopResult::NeedApproval` to fix a pre-existing clippy `large_enum_variant` warning. Regression tests included (test_credentialed_requests_respect_auto_approve, test_allow_always_matches_approval_requirement) but CI heuristic cannot detect them in cross-fork PR diffs. [skip-regression-check] Co-authored-by: Tyler <[email protected]> |
||
|
|
ec04354c6b |
fix: address valid review comments from PR #1359 (#1380)
- Cache discovery_schema() with OnceLock for routine tools (fixes #1361, #1371) - Early-return on empty event cache before allocating Vec (fixes #1369) - Extract batch concurrent count query helper to reduce duplication - Fix ROUTINE_OK sentinel substring matching - Migrate crate::safety import to ironclaw_safety per project convention Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
Henry ParkandGitHub
|
428303af11 |
Redesign routine create requests for LLMs (#1147)
* Redesign routine create requests for LLMs * Fix panic-check false positives in routine tests * Tighten routine schema requirements * Tighten routine schema tests * Mark test assertions safe for CI scan * Align test assertions with panic scan * Polish routine schema metadata * Simplify routine test assertions * Improve tool discovery guidance * Clarify lightweight routine delivery prompts * Fix routine delivery target defaults |
||
|
Henry ParkandGitHub
|
4675e9618c |
Fix Telegram auto-verify flow and routing (#1273)
* Fix Telegram auto-verify flow and routing * Fix CI formatting and clippy follow-ups * Simplify Telegram waiting state update * Fix notification fallback scopes * Fix message metadata routing and zh-CN copy |
||
|
Henry ParkandGitHub
|
878a67cdb6 |
Refactor owner scope across channels and fix default routing fallback (#1151)
* refactor: add explicit owner scope across channels * fix: tighten routine owner target routing * fix: address owner scope review feedback * Fix owner-scope onboarding and event trigger isolation * Tighten routing fallback and wizard owner validation * fix: address owner-scope follow-up review * fix: tighten owner-scope follow-up details * fix: import Channel trait in telegram test * fix: normalize http webhook sender ids * fix: address remaining owner-scope review issues * fix: reconcile config rebase fallout * fix: reconcile extension manager rebase drift * fix: address current copilot review regressions * fix: restore clippy matrix after rebase |
||
|
|
bde0b77a86 |
fix(security): prevent metadata spoofing of internal job monitor flag (#1195)
The `__internal_job_monitor` metadata key that bypassed the entire agent pipeline (hooks, safety checks, LLM processing) was spoofable by external channels — WASM channel plugins could inject arbitrary metadata including this key, causing attacker-controlled content to be forwarded directly as assistant responses. Replace the metadata-based check with a dedicated `is_internal` field on `IncomingMessage` that can only be set via `into_internal()` by trusted in-process code. Both the field and setter are `pub(crate)` to prevent external crates from spoofing the flag. Also remove `notify_metadata` forwarding (the monitor only needs channel/user/thread routing) and the unused `__job_monitor_job_id` metadata key. Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
Xing JiandGitHub
|
3f6d2ab6c2 |
fix(skill): treat empty url param as absent when installing skills (#1128)
LLMs sometimes pass "" for optional parameters instead of omitting them. Previously, passing url: "" to skill_install would match the explicit-URL branch and attempt to fetch from an empty string, producing an invalid URL error instead of falling back to the catalog lookup. Fix by adding .filter(|s| !s.is_empty()) so an empty url is treated the same as a missing field. A unit test verifies the parameter filtering behaviour directly; the full execute path (catalog lookup + install) requires a real catalog and database and cannot be covered at the unit level. |
||
|
|
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]> |
||
|
Henry ParkandGitHub
|
7d745d5479 | tools: improve routine schema guidance (#1089) | ||
|
Xing JiandGitHub
|
275bcfb658 |
fix(time): treat empty timezone string as absent (#1127)
LLMs sometimes pass "" for optional parameters instead of omitting
them. Previously, passing timezone: "" or from_timezone: "" to the
time tool would trigger a parse error ("Unknown timezone ''") rather
than falling back to the context timezone or UTC.
Fix by adding .filter(|s| !s.is_empty()) after .as_str() in
resolve_timezone_for_output and optional_timezone, so empty strings
are treated the same as a missing field.
The same pattern exists in routine.rs (cron trigger timezone and
schedule fields), where "" produces "invalid IANA timezone: ''" or a
cron parse error. That will be addressed separately once routine.rs
has a test harness in place.
Regression tests added for the now and convert operations with
empty timezone strings.
Closes #1127
|
||
|
|
e805ec61aa |
fix: 5 critical/high-priority bugs (auth bypass, relay failures, unbounded recursion, context growth) (#1083)
* fix: address 5 critical and high-priority bugs from issue tracker - #1033: reject webhook requests when secret is cleared at runtime via update_secret(None), preventing auth bypass through SIGHUP hot-swap - #908: reset consecutive_failures counter on successful SSE stream reconnection in relay channel, so circuit breaker counts truly consecutive failures - #975: add depth limit (16) to validate_tool_schema() to prevent stack overflow on deeply nested schemas - #974: add depth limit (8) to resolve_nested() to prevent stack overflow on deeply nested capabilities wrappers - #826: truncate oversized tool outputs (>8KB) in routine lightweight loop to prevent unbounded context growth across iterations Each fix includes a regression test. Closes #1033, #908, #975, #974, #826 Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: 5 more high-priority bugs (routine cache, job signals, input limits) - #1077: recompute next_fire_at when re-enabling cron routines via web toggle, mirroring CLI behavior so cron ticker picks them up - #1076: refresh event trigger cache after web toggle/delete operations so event/system_event routines reflect changes immediately - #892: remove Stuck from check_signals() stop-states in JobDelegate since Stuck is recoverable (Stuck -> InProgress via self-repair) - #976: truncate oversized description strings in CapabilitiesFile to 4KB to prevent memory abuse from malicious capabilities files - #977: drop oversized parameters schema JSON (>64KB) in CapabilitiesFile to prevent unbounded memory growth Each fix includes regression tests where applicable. Closes #1077, #1076, #892, #976, #977 Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: prevent ReDoS in event trigger regex patterns - #825: use RegexBuilder with 64KB size limit when compiling user-supplied event trigger patterns, both at creation time (routine tool) and at cache refresh (routine engine) Note: Rust's regex crate already guarantees O(n) matching, so the size limit prevents excessive memory use during compilation rather than catastrophic backtracking at match time. Closes #825 Co-Authored-By: Claude Opus 4.6 <[email protected]> * Harden HTTP SSRF IP filtering * Apply rustfmt after staging merge --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
Henry ParkandGitHub
|
8a60fa2d37 |
fix: add tool_info schema discovery for WASM tools (#1086)
* fix: add tool_info schema discovery for WASM tools * refactor: simplify WASM schema and hint state * refactor: store tool_info registry reference as Weak |
||
|
NigeandGitHub
|
c54f739354 |
fix: resolve bug_bash UX/logging issues (#1054 #1055 #1058) (#1072)
* fix(web,db): improve auth UX + reduce naive timestamp log noise * fix(clippy): keep memory test modules at end of file |
||
|
|
6f00490900 |
fix: relax approval requirements for low-risk tools (#922)
* fix: relax approval requirements for low-risk tools Remove unnecessary UnlessAutoApproved friction from list_dir, image_gen, image_analyze, image_edit, tool_install, tool_auth, tool_upgrade, and build_tool — these operate on trusted inputs or are low-risk operations so they now use the trait default (Never). For the http tool, GET requests without credentials now return Never instead of UnlessAutoApproved, while credential-bearing requests and non-GET methods retain their existing approval levels. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: apply cargo fmt Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review feedback on approval changes Rename test_requires_approval_returns_unless_auto_approved to test_requires_approval_returns_never to match the asserted behavior. In http requires_approval(), treat missing method as unknown (falls through to UnlessAutoApproved) instead of defaulting to GET, since the schema requires method. Updated comment to reflect this. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: make http method optional, default to GET Make method optional in schema (only url is required) and default to GET in both execute() and requires_approval(). This aligns approval logic with execution and reduces friction for simple GET requests. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: restore UnlessAutoApproved for build_tool, tool_install, tool_upgrade Address review feedback: these tools modify the system's trust boundary (shell execution, WASM installation, version mutation) and should retain approval gating. tool_auth kept as Never per owner decision. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
NigeandGitHub
|
7a9cbb3b50 |
fix(routines): run cron checks immediately on ticker startup (#1066)
* fix(routines): run cron check immediately at ticker startup * test/ci: add routine_engine test and fix style lint drift |
||
|
|
e1691a8d42 |
feat: configurable hybrid search fusion strategy (#234)
* feat: configurable hybrid search fusion strategy (#169) Add WeightedScore fusion as an alternative to the default RRF algorithm. Users can now tune search behavior via env vars (SEARCH_FUSION_STRATEGY, SEARCH_FTS_WEIGHT, SEARCH_VECTOR_WEIGHT, SEARCH_RRF_K) or by passing SearchConfig with the new fields. Default behavior (RRF, k=60) is unchanged. - Add FusionStrategy enum (Rrf/WeightedScore) to workspace::search - Add weighted_score_fusion() and fuse_results() dispatcher - Add config/search.rs with WorkspaceSearchConfig from env vars - Wire search defaults through Workspace struct - Update both postgres and libsql backends to use fuse_results() - Add 7 new tests (4 fusion + 3 config) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: swap default search weights to match issue #169 spec (0.7 vector / 0.3 FTS) The issue spec says "0.7/0.3 (vector/keyword) for weighted mode" but our defaults had fts_weight=0.7, vector_weight=0.3 (inverted). Also fixes the misleading docstring on weighted_score_fusion that claimed 1/rank normalizes to [0,1]. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: validate weight inputs and update stale doc comments - Reject NaN, infinite, and negative values for SEARCH_FTS_WEIGHT and SEARCH_VECTOR_WEIGHT with a clear ConfigError - Fix module-level docs that incorrectly claimed WeightedScore "normalizes per-method scores to [0,1]" - Update SearchResult.score doc from "Combined RRF score" to strategy-agnostic "Combined fusion score" Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: validate weight setters against NaN/inf/negative values with_fts_weight() and with_vector_weight() now silently ignore non-finite (NaN, ±inf) and negative values, matching the env var validation already in place for SEARCH_FTS_WEIGHT / SEARCH_VECTOR_WEIGHT. Values > 1.0 remain valid since weights are normalized internally. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use crate-wide ENV_MUTEX in search config tests Replace the module-local `ENV_MUTEX` in `search.rs` with a shared `crate::config::helpers::ENV_MUTEX` to prevent cross-module env races when `cargo test` runs tests in parallel. Addresses copilot review comment. Tracked in #245. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: per-strategy weight defaults to match issue #169 spec RRF mode now defaults to 0.5/0.5 (fts/vector) and WeightedScore defaults to 0.3/0.7, matching the acceptance criteria in #169. Previously both modes used 0.3/0.7 uniformly. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: reject both weights=0 in weighted fusion mode When both SEARCH_FTS_WEIGHT and SEARCH_VECTOR_WEIGHT are 0.0 under WeightedScore strategy, all scores would be 0.0, producing arbitrary ordering. RRF mode is unaffected since it ignores weights entirely. Addresses Copilot review comment. The other comment (rrf_k=0 division by zero) is a false positive — ranks are 1-based, so k=0 just gives inverse-rank scoring with no infinity. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: clarify weight doc comments and error key - SearchConfig field docs: clarify that Default always uses 0.5, per-strategy defaults only apply via WorkspaceSearchConfig::resolve() - WorkspaceSearchConfig field docs: same clarification - Error key for both-weights-zero now references both env vars Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: remove broken intra-doc links to pub(crate) resolve() WorkspaceSearchConfig::resolve is pub(crate), so linking to it from public field docs triggers rustdoc private_intra_doc_links warnings. Switch to plain-text references. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: add document_path to weighted_score_fusion results The weighted_score_fusion function was missing the document_path field added in a recent main branch commit, causing a compile error after rebase. Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: trigger CI re-check after rebase * fix: resolve pre-existing staging fmt and clippy issues - Fix import ordering in cli/mod.rs (cargo fmt) - Fix line wrapping in tools/mcp/auth.rs (cargo fmt) - Move path_routing_tests before MemoryTreeTool to fix clippy::items_after_test_module [skip-regression-check] * fix: remove duplicate path_routing_tests module after rebase [skip-regression-check] --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
8ac24e775b |
style: fix formatting in cli/mod.rs and mcp/auth.rs (#1071)
* style: fix formatting in cli/mod.rs and mcp/auth.rs Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(cli): add missing use_tools and max_tool_rounds fields to routines create The routines CLI create command was missing the new Lightweight fields added after the cron->routines rename merged. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(clippy): move path_routing_tests after production code in memory.rs Fixes items_after_test_module lint by moving the test module to the end of the file, after all production structs and impls. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
shibenandGitHub
|
8df51c04ae |
feat: enhance HTTP tool parameter parsing (#911)
* feat: enhance HTTP tool parameter parsing - Add support for stringified JSON arrays in headers parameter. - Introduce timeout_secs parameter parsing to accept both numbers and string representations. - Implement save_to parameter parsing to handle empty strings as None. - Update HTTP request handling to incorporate timeout and save_to parameters. - Add unit tests for new parsing functions to ensure correct behavior. * feat(http): enhance HTTP tool with timeout and header parsing improvements - Introduced default and maximum request timeout constants to manage resource usage. - Refactored header parsing logic to separate functions for better readability and maintainability. - Updated timeout handling to ensure it respects the maximum allowed value. - Added unit tests to validate new header parsing functionality. * refactor(http): replace hardcoded timeout with effective_timeout variable in HTTP tool error handling |
||
|
ReidandGitHub
|
6bbf87ba3a |
feat(routines): enable tool access in lightweight routine execution (#257) (#730)
* Rebase onto staging
* fix(routines): prevent autonomy-escalation in lightweight routines
- Add ROUTINE_TOOL_DENYLIST to block routine_create/update/delete/fire
and restart from being callable by lightweight routines
- Deduplicate sentinel logic by reusing handle_text_response() in the
no-tools path
- Filter tool definitions sent to LLM to only include callable tools,
avoiding wasted tokens on tools that would be rejected
|
||
|
NigeandGitHub
|
d420abfa6a |
fix(memory): reject absolute filesystem paths with corrective routing (#934)
* ci(staging): use default branch instead of hardcoded main * fix(memory): route absolute paths to filesystem tools |
||
|
|
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]> |
||
|
|
bb06565770 |
fix(security): resolve DNS once and reuse for SSRF validation to prevent rebinding (#518)
* fix(security): resolve DNS once and reuse for SSRF validation to prevent rebinding The previous SSRF protection resolved DNS in validate_url() to check IPs against a blocklist, but then reqwest independently re-resolved DNS when making the actual HTTP connection. Between validation and connection, a DNS rebinding attack could flip the record from a public IP (passes validation) to a private IP like 169.254.169.254 (AWS metadata endpoint). Fix: split URL validation into two phases: - validate_url(): synchronous URL structure checks (scheme, localhost, IP literals) -- no DNS resolution - validate_and_resolve_url(): async DNS resolution via tokio::net::lookup_host, validates all resolved IPs, returns SocketAddrs - build_pinned_client(): constructs a per-request reqwest Client with resolve() pinning so reqwest connects to the pre-validated IPs without a second DNS lookup Applied to both HttpTool and WebFetchTool. WebFetchTool builds a fresh pinned client per redirect hop, ensuring DNS rebinding cannot occur at any point in a redirect chain. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * style: run cargo fmt Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --------- Co-authored-by: Claude Sonnet 4.6 <[email protected]> |
||
|
|
19d9562b4f |
feat(extensions): unify auth and configure into single entrypoint (#677)
* feat(extensions): unify auth and configure into single entrypoint Refactors the extension lifecycle to eliminate the divergence between chat and gateway paths that caused Telegram setup via chat to fail (missing webhook secret auto-generation, no token validation). Key changes: - Rename save_setup_secrets() → configure(): single entrypoint for providing secrets to any extension (WasmChannel, WasmTool, MCP). Validates, stores, auto-generates, and activates. - Add configure_token(): convenience wrapper for single-token callers (chat auth card, WebSocket, agent auth mode). - Refactor auth() to pure status check: remove token parameter, delete token-storing branches from auth_mcp/auth_wasm_tool, rename auth_wasm_channel → auth_wasm_channel_status. - Add ConfigureResult/MissingSecret types for structured responses. - Replace hardcoded Telegram token validation with generic validation_endpoint from capabilities.json. - Update all callers (9 files) to use the new interface. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: use ValidationFailed error variant instead of string matching Replace brittle msg.contains("Invalid token") checks with a proper ExtensionError::ValidationFailed variant. configure() now returns this variant for token validation failures, and callers match on it directly instead of parsing error message strings. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address review — SSRF protection, error typing, missing-secret selection, WS auth 1. SSRF: call validate_fetch_url() before validation_endpoint HTTP request 2. Transport errors map to ExtensionError::Other (not ValidationFailed) 3. configure_token() picks first *missing* secret, not first non-optional 4. WebSocket error path re-emits AuthRequired on ValidationFailed Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * test: add regression tests for extension lifecycle refactoring - test_configure_token_picks_first_missing_secret: verifies multi-secret channels can be configured one secret at a time (commit ce106f4) - test_auth_is_read_only_for_wasm_channel: verifies auth() has no side effects and doesn't store secrets (commit 47f8eb6) - test_validation_failed_is_distinct_error_variant: verifies the typed error variant can be pattern-matched (commit a318161) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review comments — activation dispatch, dead code, caps consolidation - Fix configure() fallthrough bug: dispatch activation by ExtensionKind instead of unconditionally calling activate_wasm_channel() for all non-WasmTool types (MCP servers and channel relays now use their correct activation methods) - Remove dead MissingSecret struct and missing_secrets field (never populated, flagged by reviewer) - Consolidate capabilities file parsing in configure(): parse once and reuse for allowed names, validation_endpoint, and auto-generation - Fix auth() doc comment: note MCP OAuth side effects - Fix stale save_setup_secrets reference in server.rs comment - Add regression test for activation dispatch bug Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> |
||
|
|
76375f2eaa |
refactor: centralize test credential constants into testing::credentials (#829)
* refactor: centralize test credential constants into testing::credentials Scattered test credential strings (API keys, OAuth tokens, crypto keys, Telegram tokens, session tokens) across ~25 files made security auditing harder and created unnecessary duplication. Centralize all test-only fake credentials into a new `src/testing/credentials.rs` module with named constants and a shared `test_secrets_store()` helper. - Convert `src/testing.rs` to directory module (`src/testing/mod.rs`) - Add `src/testing/credentials.rs` with ~30 named constants - Replace hardcoded literals in 24 source files - Deduplicate `test_store()` helper (was copy-pasted in 3 files) - Leave leak_detector/shell/signature tests as-is (inline values aid readability for pattern detection tests) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: replace real Telegram bot token with obviously fake test stub Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * Update src/testing/credentials.rs Co-authored-by: Copilot <[email protected]> * Update src/testing/credentials.rs Co-authored-by: Copilot <[email protected]> * refactor: address PR review feedback on test credentials - Fix TEST_CRYPTO_KEY doc comment ("32-byte hex" → "32-character key string") - Rename confusing "real"/"fake" Anthropic constant names and values - Change TEST_STRIPE_KEY from "sk-live" to "sk_test_fake123" to avoid scanners - Use test_secrets_store() helper in orchestrator and http tool tests - Clarify config_round_trip.rs doc comment about integration test visibility Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Copilot <[email protected]> |
||
|
|
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]> |
||
|
|
94d101924e |
refactor: encapsulate leaked abstractions into owning modules (#778)
* refactor: encapsulate leaked abstractions from main.rs and app.rs into owning modules Move module-specific initialization logic out of main.rs (1222→665 lines, -46%) and app.rs (944→780 lines, -17%) into their respective owning modules as public factory functions. This enforces separation of concerns so that adding a new DB backend, MCP transport, or channel doesn't require editing main.rs/app.rs. Key changes: - Tracing init functions → src/tracing_fmt.rs - DB connection factory (connect_with_handles + DatabaseHandles) → src/db/mod.rs - Secrets store factory (create_secrets_store) → src/secrets/mod.rs - MCP transport dispatch factory (create_client_from_config) → src/tools/mcp/factory.rs - Orchestrator setup (setup_orchestrator + OrchestratorSetup) → src/orchestrator/mod.rs - WASM channel setup (setup_wasm_channels) → src/channels/wasm/setup.rs - Worker entry points (run_worker, run_claude_bridge) → src/worker/mod.rs - Shared CLI secrets init (init_secrets_store) → src/cli/mod.rs - Tunnel startup (start_managed_tunnel) → src/tunnel/mod.rs - Onboard check (check_onboard_needed) → src/setup/mod.rs - ExtensionManager unified MCP: uses create_client_from_config via McpProcessManager, enabling stdio/Unix transports for hot-activated MCP servers - Deduplicated ~130 lines of secrets store init across cli/mcp.rs and cli/tool.rs - CLAUDE.md updated with module-owned initialization guideline [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: address review feedback — deduplicate db factory, extract channel helper - connect_from_config() now delegates to connect_with_handles() to eliminate duplicated backend-matching logic (Copilot review feedback) - Extract register_channel() helper from setup_wasm_channels() loop body to improve readability (Gemini review feedback) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix rustfmt line wrapping in setup_wasm_channels Co-Authored-By: Claude Opus 4.6 <[email protected]> * test: add integration test for module-owned initialization factories Exercises the full factory chain end-to-end to verify nothing was lost when initialization logic was moved from main.rs/app.rs into owning modules: - connect_with_handles returns Database + populated backend handles - connect_from_config delegates correctly (produces working Database) - secrets::create_secrets_store builds working store from DatabaseHandles - db::create_secrets_store standalone factory round-trips secrets - Both secrets factories produce compatible stores (cross-read works) - ExtensionManager constructs with McpProcessManager and is functional - DatabaseHandles default is empty All tests run without external services using libsql in-memory/tempfile. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: wire cli/mcp.rs and cli/tool.rs to shared init_secrets_store() Both files had inline implementations identical to cli::init_secrets_store(). Replace with delegation to complete the claimed deduplication. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix rustfmt line wrapping in integration test Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(review): remove unused Config import and deduplicate Error Handling section - Remove `#[allow(unused_imports)]` and unused `use crate::config::Config` from cli/tool.rs (no longer needed after delegating to shared `cli::init_secrets_store()`) - Remove duplicate Error Handling subsection from CLAUDE.md Key Patterns (all four bullets already exist in Code Style section and review-discipline.md) Addresses Copilot review comments. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(review): address remaining Copilot review comments - secrets/mod.rs: clarify docstring that None is a normal no-db condition - app.rs: add comment explaining the empty_handles fallback path - orchestrator/mod.rs: combine duplicated sandbox condition into single block - setup/mod.rs: document env var reads and thread-safety caveat [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Henry Park <[email protected]> |
||
|
Nick PismenkovandGitHub
|
7d8576a464 |
fix: destructive actions from ambiguous user prompts (#782)
* fix: destructive actions from ambiguous user prompts * review fixes * review fixes |
||
|
Protocol ZeroandGitHub
|
98e9a40762 |
test(job): cover job tool validation and state transitions (#681)
Add focused coverage for create/list/status/cancel job tools so validation errors, summary formatting, and cancellation behavior stay stable. This locks in the current user-facing responses for running and completed jobs without changing production code. Made-with: Cursor |
||
|
|
553c306c52 |
feat: full image support across all channels (#725)
* feat: full image support across all channels End-to-end image handling: upload, generation, analysis, editing, and rendering across web gateway, HTTP webhook, WASM (Telegram/Slack), and REPL channels. Builds on the attachment infrastructure from #596 and draws inspiration from PR #641's image pipeline approach — credit to that PR's author for the sentinel JSON pattern and base64-in-JSON upload design. Key changes: - Image upload in web UI (file picker, paste, preview strip) - Image generation tool (FLUX/DALL-E via /v1/images/generations) - Image edit tool (multipart /v1/images/edits with fallback) - Image analysis tool (vision model for workspace images) - Model detection utilities (image_models.rs, vision_models.rs) - Sentinel JSON detection in dispatcher for generated image rendering - StatusUpdate::ImageGenerated → SSE/WS/REPL/WASM broadcast - HTTP webhook attachment support (base64, 5MB/file, 10MB total) - WASM channel image download (Telegram via file API, Slack via host HTTP) - Tool registration wiring in app.rs [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR #725 review comments (16 issues) - SecretString for API keys in all image tools (image_gen, image_edit, image_analyze) - Binary image read via tokio::fs::read instead of DB-backed workspace.read() - Replace Arc<Workspace> with Option<PathBuf> base_dir (workspace has no filesystem API) - ApprovalRequirement::UnlessAutoApproved for cost-sensitive image tools - Scope sentinel detection to image_generate/image_edit tool names only - Skip ToolResult preview broadcast for image sentinels (avoids multi-MB base64 in SSE) - Extract shared media_type_from_path() to builtin/mod.rs - Rename fallback_chat_edit → fallback_generate with tracing::warn - Increase gateway body limit from 1MB to 10MB for image uploads - Increase webhook body limit to 15MB (base64 overhead) - Log warning on invalid base64 in images_to_attachments - Client-side image size limits (5MB/file, 5 images max) in app.js - aria-label on attach button for accessibility - Update body_too_large test for new 10MB limit [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: add Slack file size check before download (PR review item #15) Skip downloading files larger than 20 MB in the Slack WASM channel to avoid excessive memory use and slow downloads in the WASM runtime. Logs a warning when a file is skipped. Also bumps channel versions for Slack and Telegram (prior branch changes). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: cargo fmt Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(security): add path validation and approval requirement to image tools Add sandbox path validation via validate_path() to both ImageAnalyzeTool and ImageEditTool to prevent path traversal attacks that could exfiltrate arbitrary files through external vision/edit APIs. Also fix ImageAnalyzeTool::requires_approval to return UnlessAutoApproved, consistent with ImageEditTool and ImageGenerateTool. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: post-download size guards and empty data_url sentinel check - Slack: add post-download size check on actual bytes when metadata size_bytes is absent, preventing bypass of the 20MB limit - Telegram: add 20MB download size limit (matching Slack) enforced in download_telegram_file() after receiving response bytes - Dispatcher: skip broadcasting ImageGenerated SSE event when data_url is empty from unwrap_or_default(), log warning instead Closes correctness issues #3, #4, #5 from PR #725 review. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use mime_guess for media type detection, add alt attrs and media_type validation - Replace hardcoded media type mapping with mime_guess crate (already in deps) - Add alt attributes to img elements in web UI for accessibility - Validate media_type starts with "image/" in images_to_attachments() - Update bmp test assertion to match mime_guess behavior Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Zaki <[email protected]> |
||
|
FrankandGitHub
|
9401ab0d58 | fix: add timezone conversion support to time tool (#687) | ||
|
|
4d61d3eedf |
fix(routines): resolve message tool channel/target from per-job metadata (#708)
* fix(routines): resolve message tool channel/target from per-job metadata When a routine's notify.channel is None, the message tool had no way to resolve channel/target for full-job workers, causing "No target specified" errors. The previous approach mutated shared global state via set_message_tool_context(), which also raced with concurrent jobs. Now the routine's notify config (channel + user) is carried in the job's metadata JSON, and MessageTool::execute falls back to ctx.metadata when neither explicit params nor conversation defaults are available. This eliminates both the None-channel bug and the concurrent-job race. Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: apply cargo fmt Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(message): broadcast to all channels when notify.channel is None Address review feedback: - Fix stale "see above" comment → "populated below" - When notify.channel is None, use broadcast_all instead of erroring with "No channel specified". This matches NotifyConfig semantics where channel=None means "broadcast to all channels" - Channel resolution is now Option<String>: param → default → metadata → None - When None, MessageTool uses ChannelManager::broadcast_all(target, response) and reports which channels succeeded/failed - Add regression test for broadcast-all behavior Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use failed channels in error message, remove redundant comment Address review feedback: - Use `failed` vec in error message instead of re-querying channel_names - Remove redundant orphaned comment block in routine_engine.rs Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
|
df3635d6be |
feat(timezone): add timezone-aware session context (#671)
* feat(timezone): add timezone-aware session context (#661) All timestamps were UTC-only, causing daily logs to split at UTC midnight, cron schedules to fire in UTC, and no quiet hours for heartbeat. This adds timezone as a per-session property flowing from the client. Key changes: - New `src/timezone.rs` module with resolution chain, parsing, and detection - `IncomingMessage` carries optional timezone from client - `JobContext.user_timezone` flows timezone to tools - `next_cron_fire()` accepts timezone for schedule evaluation - `Trigger::Cron` stores optional timezone (backward-compatible) - Workspace gains `_tz` variants for daily logs and system prompt - Heartbeat supports quiet hours (`HEARTBEAT_QUIET_START/END`) - Web frontend sends `Intl.DateTimeFormat().resolvedOptions().timeZone` - REPL auto-detects system timezone - `DEFAULT_TIMEZONE` env var / settings for server-wide default Storage stays UTC. Conversion happens at display boundaries. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(timezone): address review feedback on timezone-aware sessions - Validate quiet hours values (0-23) in HeartbeatConfig::resolve() - Fall back to settings values when env vars are unset for quiet hours - Validate IANA timezone strings in routine_create/update with parse_timezone - Add timezone field to routine_create tool schema - Allow standalone timezone update on cron routines without changing schedule - Return path from append_daily_log_tz to avoid TOCTOU race at midnight - Delegate append_daily_log to append_daily_log_tz(entry, UTC) to avoid drift - Preserve timezone through approval flow via PendingApproval.user_timezone - Improve test_today_in_tz to not depend on hardcoded year - Add 3 regression tests for quiet hours config validation Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix formatting in routine.rs Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(timezone): address second round of review feedback - Remove .claude/scheduled_tasks.lock from repo and add to .gitignore - Store resolved timezone (not raw message.timezone) in PendingApproval - Carry forward user_timezone through chained approvals in thread_ops - Wire quiet_hours_start/end from config to HeartbeatRunner - Support X-Timezone header as fallback in chat_send_handler [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(timezone): include user's local time in time tool response The time tool's "now" operation now returns local_iso and timezone fields based on ctx.user_timezone, so the LLM can report time in the user's timezone instead of always UTC. Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix formatting in time.rs Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(timezone): address Copilot review round 3 — validation, deterministic tests, schema fixes - Validate DEFAULT_TIMEZONE and HEARTBEAT_TIMEZONE at config load time - Add timezone field to HeartbeatSettings and config::HeartbeatConfig - Wire heartbeat timezone from config through agent_loop to HeartbeatRunner - Add timezone to routine_update tool schema (was accepted but not advertised) - Error on schedule/timezone update for non-cron routines - Validate timezone in Trigger::from_db (coerce invalid to None with warning) - Validate timezone in approval path (thread_ops.rs) before overwriting - Time tool always includes timezone/local_iso fields (fallback to UTC) - Make quiet hours tests deterministic using current UTC hour - Add regression tests for config validation [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> |