Commit Graph

973 Commits

Author SHA1 Message Date
Henry Park
63a48e4e40 fix(ci): target wasm32-wasip2 in WASM build script (#2175)
* fix(ci): target wasm32-wasip2 in WASM build script

cargo-component defaults to wasm32-wasip1 in CI, placing the binary at
the wrong path. All slack_auth_integration tests panic because they
look for the module at the wasm32-wasip2 target directory.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: add regression test for wasm32-wasip2 build target

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Zaki Manian <zaki@iqlusion.io>
2026-04-09 11:52:10 +03:00
Henry Park
1eaea59465 fix(test): use canonical extension name in setup submit test (#2158)
The test used a hyphenated channel name ("test-failing-channel") but
canonicalize_extension_name() converts hyphens to underscores. This
caused configure() to look for "test_failing_channel.capabilities.json"
which didn't exist, returning an early Err before reaching the
activation code path the test was designed to exercise.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 14:34:26 -07:00
firat.sertgoz
bb2c3e1dd1 fix (skills) installs for invalid catalog names (#2040)
* Fix skill installs for invalid catalog names

* Fix clippy test module ordering

* fix: address PR review feedback

* fix: use PairingStore::new_noop() in SSRF test after merge with staging

The staging branch introduced a new test (test_http_request_rejects_private_ip_targets)
that calls PairingStore::new(), but this branch changed the signature to require
db and cache arguments. Use new_noop() since this is a test context.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address PR #2040 review — remove expect() and dead strip_prefix

- Restructure download_key flow in skills_install_handler to use the
  value directly instead of round-tripping through Option + expect(),
  satisfying the no-expect-in-production-code rule.
- Remove dead strip_prefix("---\n") in render_skill_md — serde_yml does
  not emit a leading document marker for structs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(skills): preserve unknown frontmatter and tighten install matching

- rewrite install-recovery to mutate the `name` field via raw YAML
  Value rather than re-serializing the typed SkillManifest, so unknown
  frontmatter keys (vendor extensions, future fields) survive the
  install rewrite
- catalog_entry_is_installed: case-insensitive comparison for the
  display-name and normalized-slug branches, matching the slug branch
- normalize_skill_identifier: document non-ASCII handling
- normalizing-invalid-name log: warn -> debug (REPL/TUI rule)
- add round-trip test asserting unknown top-level keys, nested
  mappings, and sequences survive install recovery

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
2026-04-08 21:29:42 +03:00
firat.sertgoz
d75e2b2c22 feat(workspace): admin system prompt shared with all users (#2109)
* feat(workspace): admin system prompt shared with all users (#2088)

Introduce SYSTEM.md in a well-known __admin__ scope so admins can set a
system prompt that all tenants receive. Gated behind multi-tenant mode
(WorkspacePool sets admin_prompt_enabled on each workspace; owner
workspace in app.rs also gets the flag when has_any_users() is true).

New endpoints:
- GET  /api/admin/system-prompt — read admin system prompt
- PUT  /api/admin/system-prompt — set admin system prompt (64 KB limit)

Safety:
- SYSTEM.md added to injection scan list
- is_reserved_scope() guard on user creation (defense-in-depth)
- Multi-tenancy gate on both API and prompt assembly layers

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: remove review audit file from tracked files

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: add 64 KB size limit to admin system prompt PUT handler

Addresses PR review feedback:
- Enforce 64 KB limit on system prompt content to prevent token budget
  exhaustion (the content is injected into every user's system prompt)
- Add regression tests for the size limit (413 for oversized, not-413
  for at-limit)
- Document that is_multi_tenant is evaluated once at startup and the
  owner workspace requires a restart after the first user is created

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address remaining review feedback on admin system prompt

- Restore rustdoc comments stripped from document.rs (DocumentMetadata,
  HygieneMetadata, DocumentVersion, VersionSummary, PatchResult, etc.)
  to keep the diff focused on feature additions only
- Replace silent error swallowing (if let Ok) with discriminated match
  in admin prompt read — only DocumentNotFound is silent, other errors
  logged at debug! level
- Cache admin system prompt on WorkspacePool to avoid an extra DB read
  on every turn; invalidated on PUT via invalidate_admin_prompt()
- Add cache invalidation integration test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(workspace): tighten reserved-scope check and admin-prompt body limit

- is_reserved_scope: case-insensitive, whitespace-tolerant, and reserves
  the entire `__*__` namespace so future system scopes (alongside
  `__admin__`) cannot be impersonated by hand-crafted user IDs
- admin system-prompt route: layer-level DefaultBodyLimit of 128 KB
  rejects oversized payloads before JSON parse, complementing the
  in-handler 64 KB content cap
- system_prompt put_handler: clarify that the in-handler size check is
  a clearer-error fallback for the layer cap
- users_create_handler: drop the dead is_reserved_scope check on a
  freshly-minted UUID; the guard belongs at a code path that actually
  accepts user-supplied IDs
- expand is_reserved_scope tests for case, whitespace, and the wider
  `__*__` namespace

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
2026-04-08 21:28:39 +03:00
Illia Polosukhin
315c4cf859 [codex] allow private local llm endpoints (#1955)
* allow private local llm endpoints

* Fix private endpoint review issues

* Fix link-local clippy warning

* Tighten base URL validation follow-ups

* fix(config): non-blocking DNS validation and admin-only LLM key filtering

Address PR #1955 review feedback:

- Wrap to_socket_addrs() in tokio::task::block_in_place when called from
  a multi-threaded async runtime so the LLM utility handlers
  (/api/llm/test_connection, /api/llm/list_models) can no longer stall a
  worker thread on slow DNS. Synchronous callers (env config, CLI) are
  unaffected.

- Add ADMIN_ONLY_LLM_SETTING_KEYS + strip_admin_only_llm_keys helper as
  defense-in-depth: Config::from_db_with_toml and re_resolve_llm_with_secrets
  now take an is_operator flag and strip admin-only base-URL-bearing keys
  (llm_builtin_overrides, llm_custom_providers, ollama_base_url,
  openai_compatible_base_url) from the DB merge for non-operator users.
  This guards future per-user resolve paths and any pre-existing legacy
  rows from reactivating a private/loopback endpoint via the operator
  validation policy. Existing call sites pass true (owner_id is the
  operator scope).

Adds regression tests covering:
  * strip_admin_only_llm_keys removes all four keys, leaves others
  * validate_base_url is callable from a multi-thread tokio runtime
    without panicking on the strict short-circuit path
  * validate_operator_base_url remains callable from async handlers
  * re_resolve_llm filters admin-only keys when is_operator=false and
    keeps them when is_operator=true

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(settings): de-dup admin-only LLM key list (#1955 review)

`handlers/settings.rs::is_admin_only_setting_key` now delegates to
`crate::config::helpers::ADMIN_ONLY_LLM_SETTING_KEYS` so the write-side
gate cannot drift from the read-side `strip_admin_only_llm_keys` filter.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 03:01:18 +09:00
firat.sertgoz
8aa094125b feat(engine): restage skill repair learning loop on staging (#1962)
* feat(engine): add skill repair learning loop

* fix(engine): guard skill repair mission updates

* fix(engine): persist skill repair provenance

* fix(engine): address skill-repair PR review feedback

- Fix hex formatting: iterate GenericArray bytes individually instead of
  relying on Display impl which produces debug-like output
- Always recompute content hash from actual doc.content when archiving a
  revision to prevent drift from out-of-band writes
- Prune repair history on rollback to remove records for versions newer
  than the one being restored
- Combine collect_error_messages + collect_observed_actions into a single
  pass (collect_errors_and_actions) to avoid redundant event iteration
- Document bounded revision eviction policy (cap at 10)
- Add comment clarifying concurrent skill-repair / error-diagnosis triggers

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): constrain skill repair updates

* fix(engine): keep insights on completed threads

* style(engine): satisfy fmt and clippy on mission.rs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
2026-04-09 01:01:15 +09:00
firat.sertgoz
482ee57c5f feat(tui): port full-featured Ratatui terminal UI onto staging (#1973)
* feat: port ratatui tui onto staging

* Add TUI model picker for /model

* Fix TUI CI lint failures

* Format /tools output as vertical list

* Restore TUI approval modal on thread switch

* Re-emit pending approval events on follow-up messages

* Improve TUI thread handling and activity UI

* Sort TUI resume conversations by activity

* fix(tui): address PR review feedback

* Add TUI thread detail modal for activity sidebar

* feat(tui): improve conversation scrolling UX

- Mouse wheel: 1-line increments (was 3-line jumps)
- PageUp/PageDown: full-page scroll based on viewport height (was 5 lines)
- Add scrollbar widget on conversation right edge (track │, thumb ┃)
- Add "↓ N more ↓ End to return" indicator when scrolled up
- Add auto-follow (pinned_to_bottom) that disengages on scroll-up
  and re-engages when reaching bottom or pressing End
- Clamp scroll offset to valid range (can't scroll past content)
- Add End key binding to jump to bottom

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tui): use engine context pressure data for status bar

The context bar was using cumulative session tokens (total_input +
total_output) which grow unboundedly across turns, making the bar
always show 100% after a few exchanges. Now uses the actual context
window usage from ContextPressure events when available, falling back
to cumulative tokens only before the first engine update arrives.

Also syncs context_window from the engine's max_tokens so the limit
reflects the real model capability instead of name-based heuristics.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tui): render markdown in thread detail modal

The thread detail modal was displaying raw markdown text (plain
line splitting). Now uses render_markdown() for proper formatting
of headers, lists, bold, code blocks, etc.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(tui): hydrate sidebar with engine threads and routines at startup

The TUI sidebar was empty until the first user message because
EngineThreadList and RoutineUpdate events were only sent after
processing a message. Now sends initial data right before the
message loop so the activity panel shows existing threads and
routines immediately on startup.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tui): use owner_id for engine thread hydration at startup

list_engine_threads filters by user_id, so passing "" matched no
threads. Now uses self.owner_id() which matches the TUI channel's
user_id, so threads are visible in the sidebar immediately.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tui): fix CI — type errors and formatting in TUI tests

Wrap `started_at` and `updated_at` in `Some(...)` to match
`Option<DateTime<Utc>>` after upstream struct change, and run
`cargo fmt` on files with formatting drift.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): resolve clippy warnings — collapsible ifs and needless borrow

Collapse three nested `if` blocks into `if && let` chains and remove
a needless `&` on the `process_list_threads` call, all in agent_loop.rs.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): add live_harness.rs with updated StatusUpdate patterns

The live_harness.rs file was added to staging after this branch diverged.
When CI merges the PR into staging, the file uses old StatusUpdate patterns
that don't account for the new `detail` and `call_id` fields added by this
branch. Add the file with `..` rest patterns to fix the merge-time compile
errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 23:23:39 +09:00
Illia Polosukhin
3df1bf3830 chore(ci): add Dependabot and pin GitHub Actions by SHA (#2043)
* chore(ci): add Dependabot and pin GitHub Actions by SHA

Add automated dependency vulnerability scanning via Dependabot for both
Cargo crates (weekly) and GitHub Actions (weekly). Pin all 101 external
action references across 14 workflow files to full commit SHAs to prevent
supply-chain attacks via compromised tags.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): harden workflows — persist-credentials, permissions, template injection

Address zizmor security audit findings:
- Add persist-credentials: false to all checkout steps (artipacked)
- Add explicit minimal permissions to all workflows (excessive-permissions)
- Move workflow-level write permissions to job level where possible
- Fix template injection in regression-test-check.yml by using env vars

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): address PR review comments

- Group Dependabot updates by ecosystem to reduce PR noise (gemini)
- Add persist-credentials: false to docker.yml checkout (Copilot)
- Move inputs.tag and other expansions to env vars in docker.yml to
  eliminate template injection from workflow_dispatch user input

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): restore git push auth and harden git fetch

Address PR #2043 review comments:
- staging-ci create-promotion-pr: generate App token before checkout
  and pass it to checkout so 'git push origin "$BRANCH"' works
- staging-ci update-tag: re-enable credential persistence so the
  'staging-tested' tag force-push succeeds (job is internal-only)
- release update-registry-checksums: re-enable credential persistence
  so the checksum-update branch push succeeds
- regression-test-check: add '--' to git fetch to prevent refs that
  start with '-' from being interpreted as options

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): address serrrfirat PR review comments

- release.yml: move github.ref_name, needs.plan.outputs.tag, and
  needs.plan.outputs.tag-flag to env vars across plan, build-local-artifacts,
  build-global-artifacts, and host jobs. The tag pattern
  '[0-9]+.[0-9]+.[0-9]+*' has a trailing glob, so a tag like
  '1.2.3\$(curl evil)' could match and be shell-expanded.
- dependabot.yml: split Cargo groups into tokio-ecosystem, serialization,
  wasm, and everything-else to make regression bisection easier when
  CI fails on a Dependabot PR.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): add missing job-level permissions for gh CLI calls

- resolve-promotion-base: add pull-requests: read for 'gh pr list'
- gate: add checks: read for 'gh api .../commits/{sha}/check-runs'

Both were dropped when workflow-level permissions moved to job level.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Zaki Manian <zaki@iqlusion.io>
2026-04-08 19:38:44 +09:00
firat.sertgoz
10d970d456 Fix routine Telegram notification summaries (#2033) 2026-04-08 17:03:51 +09:00
firat.sertgoz
f2b5813a32 test(channels): add Slack E2E tests, integration tests, and smoke runner (#2042)
* test: add Slack E2E tests, Rust integration tests, and smoke runner

Replicate the Telegram test infrastructure for the Slack WASM channel:
- Add Slack URL rewriting in wrapper.rs for test API redirection
- Create fake_slack_api.py mock server for E2E tests
- Add 12 Python E2E tests covering setup, DM, mentions, auth, threads, files
- Add 12 Rust integration tests for WASM channel behavior
- Add conftest.py fixtures for isolated Slack test instances
- Add local smoke test runner for pre-release validation with real Slack

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: wrap env::set_var/remove_var in unsafe blocks for Rust 1.83+

CI uses Rust 1.94 which requires unsafe blocks for std::env::set_var
and std::env::remove_var. Wrap the test-only calls in unsafe blocks
with safety comments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address PR review feedback

- Replace fragile time.time()-1 fallback with explicit SmokeError in
  run_smoke.py attachment case (reviewer finding #1)
- Add OnceLock<Mutex> guard around env var mutation in wrapper.rs unit
  test to prevent parallel test races (reviewer finding #2)
- Extract duplicated git-worktree discovery into find_project_file()
  helper in slack_auth_integration.rs (reviewer finding #3)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test(channels): generalize WASM HTTP test rewrites

* fix(channels): gate Slack test URL rewrites from release builds

* fix(ci): update wrapper test pairing store ctor

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-08 17:03:09 +09:00
Illia Polosukhin
fdb093fb42 chore(engine): rename ENGINE_V2_TRACE to IRONCLAW_RECORD_TRACE (#2114)
* chore(engine): rename ENGINE_V2_TRACE to IRONCLAW_RECORD_TRACE

Aligns the trace-recording env var with the project-wide IRONCLAW_*
naming convention so it's discoverable alongside other ironclaw flags.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* unify engine v2 trace recording with v1 RecordingLlm

Address PR review feedback. Instead of having two separate trace
systems both named IRONCLAW_RECORD_TRACE (the v1 RecordingLlm in
src/llm/recording.rs and the engine v2 executor/trace.rs JSON dumper),
collapse them to one.

Engine v2's LlmBackend is wired to the host's full LLM provider chain,
which already includes RecordingLlm when IRONCLAW_RECORD_TRACE=1.
That means engine v2 LLM interactions are already captured by the
unified trace_*.json fixture file -- no engine-side env var, no second
JSON output, no risk of one flag enabling two divergent recorders.

Removed:
- is_trace_enabled() and write_trace() from executor/trace.rs
- the engine_trace_*.json write site in runtime/manager.rs

Kept (still useful, runs unconditionally for the self-improvement
mission):
- build_trace() / analyze_trace() / log_trace_summary()

Docs updated to point to RecordingLlm as the single trace mechanism.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 12:31:39 +09:00
Henry Park
a56fec7ebc perf: fix multi-tenant inference latency (per-conversation locking + workspace indexing) (#2127) 2026-04-07 20:05:45 -07:00
Henry Park
e82ee33704 fix: universal engine-version tool visibility filtering (#2132) 2026-04-07 20:05:16 -07:00
Henry Park
288fe49aad fix(ownership): remove silent cross-tenant credential fallback (#2099)
* fix(ownership): remove silent cross-tenant credential fallback in WASM wrappers (#2069, #2070)

WASM tool credential resolution silently fell back to looking up secrets
under the hardcoded "default" scope when the calling user had no credential
configured, leaking the instance owner's API keys to other users without
error or audit trail.

- Remove "default" fallback in resolve_host_credentials(); return
  Err(ToolError::NotAuthorized) with actionable message instead of
  silently skipping missing credentials
- Fix resolve_websocket_identify_message() to accept owner_scope_id
  parameter instead of hardcoding "default"
- Document legacy broadcast metadata fallback with removal tracking
- Document setup.rs boot-time owner_id lookups as intentional
  instance-level resource ownership
- Add regression tests proving cross-tenant credentials do not leak

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(review): differentiate expired vs missing credential errors, exclude UrlPath from store check

Address PR review feedback:
- Filter out UrlPath credentials in the no-store check so tools with
  only UrlPath mappings don't incorrectly get NotAuthorized
- Match SecretError::Expired separately to produce "has expired" message
  instead of misleading "not found"
- Add tests for both: UrlPath-only no-store (Ok), expired credential
  (specific error message)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(review): map backend SecretErrors to ExecutionFailed, not NotAuthorized

Address @serrrfirat review: Database, DecryptionFailed, KeychainError,
and other backend errors were incorrectly mapped to "not found". Now:
- NotFound → ToolError::NotAuthorized ("not found, configure via secrets set")
- Expired → ToolError::NotAuthorized ("has expired, refresh or re-set")
- All others → ToolError::ExecutionFailed (preserves real cause)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(review): AccessDenied → NotAuthorized, fix issue refs, reduce visibility

- Map SecretError::AccessDenied to ToolError::NotAuthorized (not
  ExecutionFailed) since it's an authorization failure
- Update legacy fallback comments to reference #2100 (the tracking
  issue) instead of #2069
- Revert resolve_websocket_identify_message to private — test uses
  super:: import instead of pub(crate) path

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(review): redact secrets in Debug impl, document all-or-nothing invariant

- Replace #[derive(Debug)] on ResolvedHostCredential with custom impl
  that redacts secret_value and auth headers to prevent latent leakage
- Add comment documenting that all declared non-UrlPath credentials are
  required — tool execution fails on first missing credential rather
  than running with partial auth

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
2026-04-07 16:44:09 -07:00
Henry Park
265fe0e524 fix(engine): repair mission ACL regression and 4 stale engine tests (#2130)
* fix(engine): repair mission ACL regression and 4 stale engine tests

The mission access control in pause_mission/resume_mission had a
logic error that allowed any user to manage shared/system missions.
The `&& !mission.owner_id().is_shared()` condition short-circuited
the entire check for shared missions. Replaced with proper two-branch
logic using is_shared_owner().

Also fixed 4 test assertions that checked thread.messages instead of
thread.internal_messages (orchestrator stores working messages in the
internal transcript), and made the trace test resilient to event
ordering by filtering by EventKind.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(engine): extend ACL fix to update_mission, error on missing missions

Address review feedback:
- Fix same shared-mission ACL bug in update_mission (line 130)
- pause_mission/resume_mission now error on missing missions instead
  of silently proceeding, matching update_mission's pattern
- Update doc comments to reflect that the engine enforces shared
  ownership checks directly

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 16:41:35 -07:00
Henry Park
ecd8826bec fix(e2e): canonicalize extension names + fix remaining test failures (#2129)
* fix(e2e): canonicalize extension names in configure/setup + update tests

1. Bug fix: `configure()` and `get_setup_schema()` in ExtensionManager
   called `validate_extension_name()` which discards the canonical form.
   Hyphenated names from URL paths (e.g., `web-search`) were used raw
   for capabilities file lookups, causing "Capabilities file not found"
   errors. Now both methods canonicalize with `canonicalize_extension_name()`
   so `web-search` → `web_search` before any file I/O.

2. Bug fix: `extensions_setup_handler` in server.rs compared raw URL
   path param against canonical stored names in the kind lookup.

3. Test fix: `test_wasm_lifecycle.py` — update all `web-search` refs
   to `web_search` (canonical form returned by registry/API).

4. Test fix: `test_mcp_auth_flow.py` — update `mock-mcp` / `mock-mcp-400`
   to `mock_mcp` / `mock_mcp_400`.

5. Test fix: `test_chat.py` — `test_send_message_and_receive_response`
   now counts assistant messages before sending to avoid picking up the
   pre-existing onboarding greeting as the LLM response.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use centralized SEL selector in wait_for_function JS call

Address review feedback: pass the selector from SEL dict into the
JS function instead of hardcoding '#chat-messages .message.assistant'.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use send_chat_and_wait_for_terminal_message helper for robustness

Replace manual count+wait_for_function with the existing helper that
correctly handles streaming chunks and waits for the terminal message.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 16:41:26 -07:00
Henry Park
755116aee1 fix(ownership): unify ownership checks via Owned trait and fix mission visibility bug (#2126)
Missions created via the agent were invisible in the gateway UI for
non-owner users because list_engine_missions/list_engine_threads fell
back to the engine owner's default_project_id instead of resolving
the authenticated user's per-user project.

Broader change: replace three inconsistent ownership patterns
(can_act_on, engine is_owned_by, raw string comparisons) with a
single Owned trait providing uniform is_owned_by(user_id) checks
across jobs, routines, and all internal code paths.

- Fix project_id resolution in list_engine_missions/list_engine_threads
- Add Owned trait to src/ownership with impls on AgentJobRecord,
  SandboxJobRecord, Routine, and JobContext
- Migrate all can_act_on calls in web handlers (jobs.rs, routines.rs)
- Migrate raw user_id comparisons in tenant.rs, commands.rs,
  routine_engine.rs, context/manager.rs, tools/builtin/job.rs
- Add missing ownership check in routines_trigger_handler
- Migrate active routines_runs_handler and verify_project_ownership
  in server.rs
- Remove dead can_act_on function and ownership_identity helper
- Add regression tests for concrete Owned impls on real types

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 16:41:03 -07:00
Henry Park
5c8618402a fix(web): intercept approval text input in chat (#2124)
* fix(web): intercept approval text input ("yes"/"no"/"always") in chat

When a tool requires approval in the web UI, typing "yes", "no", or
"always" in the chat input now resolves the approval card directly
instead of sending a regular message. This prevents duplicate approval
prompts and "No pending approval" errors that occurred when text went
through the backend message pipeline.

The frontend intercepts approval keywords in sendMessage() and routes
them through sendApprovalAction() — the same code path as clicking
the Approve/Deny/Always buttons on the card.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: find most recent unresolved approval card for text interception

Address review feedback: instead of checking the last card and then
separately checking if it's resolved, find the most recent unresolved
card directly. Handles the edge case where the last card is resolved
(during 1.5s removal animation) but an earlier one isn't.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: add E2E test for skip-resolved-card behavior

Addresses review feedback: adds a test where two approval cards are
visible, the newer one is resolved via button click, then typing "yes"
correctly targets the older unresolved card instead of falling through.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 16:37:28 -07:00
Henry Park
79c1b0fd7e Improve channel onboarding and Telegram pairing flow (#2103)
* Improve channel onboarding and Telegram pairing flow

* fix: remove dead restart_required code, fix review findings, and stabilize polling E2E test

- Remove restart_required/needs_restart dead code from 6 files (no real
  extension uses it; all channels hot-activate at runtime)
- Remove dead extensions.configuredRestart i18n key from all 3 locales
- Fix pairing test asserting wrong upsert semantics (test expected
  idempotent behavior but impl always rotates codes)
- Fix pairing test using expired code for approval (req.code -> req_again.code)
- Fix missing i18n fallback for auth.extensionTokenPlaceholder
- Validate setup_url scheme (https?://) before assigning to <a>.href
- Replace hardcoded English "Approve"/"Pairing code is required" with i18n keys
- Demote misleading "bot is open to all users" Telegram log from Warn to Debug
- Move polling E2E test to run first (polling loop dies during refresh_active_channel)
- Add poll_interval_ms config field to Telegram WASM channel
- Fix conversations.rs compilation (missing ? operator)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address remaining review comments (i18n regressions)

- Remove hardcoded pairing_instructions() function from server.rs;
  use onboarding metadata from ExtensionManager instead (fixes i18n
  regression where pairing instructions were always English)
- Restore i18n calls for stepper labels in renderWasmChannelStepper
  (was using hardcoded English strings instead of missions.step* keys)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: restart polling loop on channel refresh, address Copilot review

- Add WasmChannel::ensure_polling() that stops any stale polling task
  and starts a fresh one from the on_start config
- Call ensure_polling() in refresh_active_channel after re-running
  on_start, fixing the root cause of the dead polling loop in E2E tests
- Move polling test back to its original position (no longer order-dependent)
- Fix requires_pairing in channel_onboarding_for_state to use
  channel_requires_pairing() instead of legacy owner_id-only check
- Add rel='noopener noreferrer' to all setup_url target=_blank links

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: collapse nested if-let to satisfy clippy collapsible_if

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: return promise from approvePairing, stop polling unconditionally in ensure_polling

- Add missing `return` before apiFetch in approvePairing() so callers
  can await/chain the result
- Move poll_shutdown_tx.take() before the enabled check in
  ensure_polling() so switching from polling to webhook stops the old
  polling task

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove trailing commas in JSON test fixtures after restart_required removal

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 15:26:26 -07:00
Henry Park
a1ef85f8a8 fix(staging): repair 4 categories of CI test failures (#2091)
* fix(staging): repair 4 categories of CI test failures

1. Telegram token test flaky race: add env mutex guard so concurrent
   tests that override IRONCLAW_TEST_TELEGRAM_API_BASE_URL don't
   pollute the unguarded read in the colon-preservation test.

2. SSE/connection E2E tests: #sse-status element was removed from HTML
   and replaced with #sse-dot colored indicator. Update tests to check
   the dot's CSS class instead of text content. Add SSE-ready wait to
   the page fixture so chat tests don't race against connection setup.

3. Tool approval E2E tests: API unified legacy pending_approval and
   engine v2 gates into a single pending_gate response field. Update
   all E2E test helpers to use the correct field name.

4. WASM tar.gz extraction bug: canonicalized extension names use
   underscores (web_search) but release archives use hyphens
   (web-search.wasm). Accept both filename forms when extracting.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review feedback — stronger SSE signal, consistent naming

- SSE wait: use sseHasConnectedBefore JS flag (set in onopen) instead
  of checking #sse-dot CSS class, which defaults to connected state
  before SSE actually connects
- Rename _wait_for_pending_approval → _wait_for_pending_gate and
  _wait_for_no_pending_approval → _wait_for_no_pending_gate
- Update all docstrings/error messages to say pending_gate
- Deduplicate name.replace('_', '-') in tar.gz extraction and include
  both accepted filenames in the error message

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address second round of review feedback

- Add comment documenting invariant: canonical names use underscores,
  archives may use hyphens, reverse is not supported
- Fix quote wrapping in single-name error case
- Remove dead SEL["sse_status"] selector from helpers.py
- Simplify SSE wait: use window.sseHasConnectedBefore === true
  (fails fast on rename instead of silent 10s timeout)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use global binding for sseHasConnectedBefore, not window property

sseHasConnectedBefore is declared with let at global scope, which
does not create a window property. window.sseHasConnectedBefore
would always be undefined. Use typeof guard + direct reference instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(security): cap tar.gz entry pre-allocation to MAX_ENTRY_SIZE

The tar header's declared size is attacker-controlled. Without capping,
Vec::with_capacity could attempt a huge allocation and OOM before the
read_to_end take() limit kicks in.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: make test_sse_status_shows_connected non-redundant, document flag reset

- test_sse_status_shows_connected now checks #sse-dot CSS class (visual
  indicator) instead of re-checking sseHasConnectedBefore which the
  page fixture already guarantees
- Add comment to test_sse_reconnect_after_disconnect explaining why
  sseHasConnectedBefore is reset and that the history-reload path is
  covered by test_sse_reconnect_preserves_chat_history

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 15:19:01 -07:00
firat.sertgoz
86c1590350 feat(slack): implement on_broadcast and fix message tool hints (#2113)
* feat(slack): implement on_broadcast and fix message tool channel hints

Implement the on_broadcast callback for the Slack WASM channel, enabling
proactive message delivery to Slack channels/users via the message tool.
Previously this was a stub returning "not implemented".

The implementation:
- Uses the user_id parameter as the broadcast target (channel ID or user ID)
- Strips leading # from targets for convenience
- Warns when target doesn't look like a Slack ID (C/U/D/G prefix)
- Posts via chat.postMessage with host-injected Bearer token
- Tracks active threads for broadcast replies (consistent with on_respond)

Also fixes the message tool's channel parameter description to list 'slack'
alongside 'slack-relay' and clarifies that Slack targets must be IDs.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(slack): extract shared post helper, harden broadcast validation

Address review findings on the slack broadcast implementation:

- Extract `post_slack_message()` shared by `on_respond` and `on_broadcast`,
  eliminating ~40 lines of duplicated HTTP-call-then-parse logic.
- Log `track_active_thread` errors at Warn level instead of silently
  swallowing them with `let _ =` (restores observability lost in original).
- Make non-ID broadcast targets a hard error instead of a soft warning —
  consistent with the message tool schema that says "must be an ID, not a
  name".
- Fix empty-target error message to not assume a name was provided.
- `resolve_broadcast_target` now returns `&str` (avoids allocation).
- Add 2 tests covering the resolve+validate pipeline end-to-end.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(slack): track broadcast message ts so replies are recognized

Address Gemini review: broadcast messages now track the Slack-returned
timestamp as an active thread (falling back from response.thread_id to
the posted message ts). This ensures that if a user replies to a
broadcast, the agent recognizes the reply as an active thread.

Also fix stale doc comment on looks_like_slack_id (was missing W prefix).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 21:23:18 +03:00
firat.sertgoz
00fd2e881f fix(web): emit Done after response — SSE ordering fix (#2079) (#2104)
* fix(web): emit Done status after response to fix SSE ordering (#2079)

Move the terminal "Done" status out of thread_ops and emit it only
after the gateway successfully responds via a new respond_then_done()
helper in agent_loop. This guarantees the browser receives the
assistant message before the turn-closing event, preventing the web UI
from appearing stuck.

Adds a regression test asserting the response event is captured before
the Done status in the ordered event log.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(web): add frontend safety net for lost SSE response events (#2079)

Track whether a `response` SSE event was received for the current turn.
When "Done" arrives without a preceding response, schedule a
loadHistory() call after 1500ms so the user sees the answer even if
the response event was lost to broadcast lag or a brief disconnect.

This is the second prong of the fix described in #2079 — the backend
ordering fix alone prevents the race, but this fallback handles
residual edge cases (proxy buffering, SSE reconnection gaps).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review findings — Done on all paths, fix frontend timer leaks

Backend:
- Send Done status when BeforeOutbound hook blocks the response, so the
  client still knows the turn is complete.
- Send Done status for empty/suppressed responses (e.g. approval handled
  via send_status) to match pre-refactor behavior.

Frontend:
- Set _turnResponseReceived on stream_chunk events so streaming
  responses don't trigger a spurious loadHistory() when Done arrives.
- Clear _doneWithoutResponseTimer on sendMessage() to prevent stale
  timers from a previous turn firing during the new one.
- Clear turn-tracking state on switchThread() to prevent cross-thread
  contamination of the timer and flag.
- Clear turn-tracking state on SSE reconnect (eventSource.onopen) to
  prevent stale timers from before the disconnect.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address PR review — always emit Done, extract helper, add test

- respond_then_done now emits Done regardless of respond outcome so the
  client always knows the turn ended, even on delivery failure
- Extract send_done() helper to deduplicate the inline Done+warn blocks
  in the hook-blocked and empty-response paths
- Add done_emitted_for_empty_response test covering the empty-response
  branch ordering invariant
- Lift 1500ms magic number to DONE_WITHOUT_RESPONSE_TIMEOUT_MS constant
- Add comment explaining _turnResponseReceived single-thread tracking

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(agent): suppress Done while awaiting approval; introduce HandleOutcome

Distinguish "no response, turn complete" from "no response, turn paused"
in handle_message's return type so the run loop can decide whether to
emit the terminal Done status. The previous code lumped both into
Ok(Some("")), causing v1 NeedApproval to incorrectly emit Done after
ApprovalNeeded — which then tripped the new web UI safety net and
triggered a spurious loadHistory() under the live approval prompt.

- New HandleOutcome enum with Shutdown / Respond / NoResponse / Pending
- SubmissionResult::NeedApproval now maps to HandleOutcome::Pending
- Bridge handlers wrapped via HandleOutcome::from_legacy (their approval
  flows return non-empty descriptive text, so they never need Pending)
- Regression test no_done_emitted_while_awaiting_approval drives a
  v1 Always-approval probe and asserts no Done is captured
- Repaired pre-existing done_emitted_for_empty_response test, which
  asserted the wrong invariant: the dispatcher substitutes empty LLM
  responses with a fallback message, so a truly empty response never
  reaches the run loop. Renamed and updated to assert the ordering.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: ilblackdragon@gmail.com <ilblackdragon@gmail.com>
2026-04-07 19:11:30 +03:00
rajulbhatnagar
f765958fc3 fix(tools): gate claude_code and acp modes behind enabled flags (#2003)
* fix(tools): gate claude_code and acp modes behind enabled flags (#1987)

CreateJobTool always exposed claude_code and acp as valid modes in its
schema and silently accepted them at runtime, even when
CLAUDE_CODE_ENABLED=false / ACP_ENABLED=false. This caused the LLM to
sometimes select disabled modes, spawning containers that fail.

- Add claude_code_enabled and acp_enabled flags to ContainerJobConfig
  (following the existing mcp_per_job_enabled pattern)
- Expose via ContainerJobManager accessors, query from CreateJobTool
  through the already-injected job_manager
- Dynamically build the mode enum in parameters_schema() — only show
  enabled modes; omit mode field entirely when only worker is available
- Conditionally include agent_name field only when ACP is enabled
- Add defense-in-depth guards in execute() rejecting disabled modes
  with ToolError::InvalidParameters
- Add 10 regression tests covering schema gating and runtime rejection

* fix(tools,web): address review feedback on mode gating (#2003)

- Remove hardcoded "Set mode to claude_code" from CreateJobTool description;
  mode guidance is already provided dynamically via parameters_schema()
- Add check_mode_enabled() guard in jobs_restart_handler to reject disabled
  modes on job restart via REST API, closing the bypass path
- Add ContainerJobManager::is_mode_enabled(mode) to centralize mode validation
- Clean up fully-qualified paths in jobs.rs with proper use imports
- 5 regression tests (description, restart rejection, is_mode_enabled)

* fix: harden mode gating with defense-in-depth and synchronous persistence

- Add ModeDisabled variant to OrchestratorError and guard inside
  ContainerJobManager::create_job() so disabled modes are rejected
  even if callers forget to validate
- Make job mode persistence synchronous instead of fire-and-forget
  to prevent silent mode loss on transient DB errors (restarts would
  silently downgrade to worker mode)
- Refactor parameters_schema() to build serde_json::Map directly,
  removing an unreachable if-let guard and an .expect() call

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Rajul Bhatnagar <brajul@amazon.com>
Co-authored-by: serrrfirat <f@nuff.tech>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 13:31:06 +03:00
rajulbhatnagar
13774cc022 fix(acp): propagate follow-up prompt failures as job errors (#1981)
* fix(acp): propagate follow-up prompt failures as job errors (#1915)

The follow-up loop silently swallowed ACP prompt failures — logging and
posting a status event but continuing the loop, so the job always
reported success: true. Extract the loop into a testable
`run_follow_up_loop` function with trait-based dependency injection and
return Err on failure so run() reports success: false.

Also: downgrade follow-up loop info! logs to debug! (background task),
add PartialEq to JobEventPayload, narrow AcpPromptSender to private.

* fix(acp): kill child process on protocol failure, race poll against exit

Address two review findings from #1981:

1. (gemini-code-assist) poll_prompt() blocked the follow-up loop — child
   exit wasn't detected during active HTTP polling. Wrap poll_prompt() in
   tokio::select\! alongside child_exit_rx.

2. (serrrfirat, high severity) Protocol failure could leave the child
   alive, causing stderr_handle.await to block forever and the job to
   hang. Add a kill channel: spawn_child_monitor() returns (child_exit_rx,
   kill_tx); on error, run_acp_session sends the kill signal before
   awaiting stderr so the pipe closes and cleanup completes.

Extract spawn_child_monitor() as a standalone function so both production
code and the regression test exercise the same code path.

Tests:
- follow_up_loop_exits_during_long_poll_when_child_dies (ForeverPromptSource
  + timeout guard — hangs without the select\! fix)
- child_monitor_kills_process_so_stderr_reader_completes (real sleep
  subprocess + kill signal — hangs without the kill channel)

* fix(acp): use non-terminal turn events, add poll retry limits

Per-turn ACP results were emitting event_type "result" which maps to
AppEvent::JobResult (the terminal signal). This caused job monitors to
exit after the first prompt, making all subsequent follow-up outcomes
invisible. Changed per-turn events to "turn_result" and emit exactly
one terminal "result" from run() after the session ends.

Also added retry discrimination for poll_prompt errors: permanent
errors (OrchestratorRejected, LlmProxyFailed) fail immediately,
transient errors (ConnectionFailed) retry with a cap of 5.

---------

Co-authored-by: Rajul Bhatnagar <brajul@amazon.com>
2026-04-07 12:39:46 +03:00
Coffee
6fa2d0ec41 fix: color for tools use (#2096)
* Use info colors for tool call summaries

* Revert tool summary background and chevron color changes

* fix: color
2026-04-07 10:54:05 +03:00
Illia Polosukhin
0ab1a47479 fix(registry): use canonical underscore names in manifests to fix WASM install (#2029)
* fix(registry): use canonical underscore names in manifests to fix WASM install

Manifest `name` fields used hyphens (e.g. "google-calendar") but the internal
canonical form uses underscores ("google_calendar"). The release workflow
packages .wasm files named after the manifest `name`, so archives contained
"google-calendar.wasm". The extension manager canonicalized the name to
"google_calendar" before extraction, looked for "google_calendar.wasm", and
failed with "tar.gz archive does not contain 'google_calendar.wasm'".

Two-part fix:
- Update all 9 hyphenated manifest `name` fields and `_bundles.json` refs to
  use the canonical underscore form. Future releases will package archives
  with matching filenames.
- Add hyphenated-name fallback in both tar.gz extractors so existing v0.22.0
  release artifacts (which contain hyphenated filenames) remain installable.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address PR review — extract shared helper, rename manifest files, improve errors

- Extract `ArchiveFilenames` helper to `naming.rs` to deduplicate alias
  matching logic between `manager.rs` and `installer.rs`
- Rename all 9 manifest JSON files to match their canonical underscore
  `name` fields (e.g. `google-calendar.json` → `google_calendar.json`)
- Improve "not found" error messages to list both canonical and alias
  filenames that were tried
- Update `test_extract_correct_wasm_from_tool_bundle` to use canonical
  `slack_tool` name matching current production path
- Update artifact naming test script for renamed manifests

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 13:43:57 +09:00
Joseph Bloggs
e7fa16730f fix(safety): add credential patterns and sensitive path blocklist (#1675)
* fix(safety): add credential patterns and sensitive path blocklist

Addresses critical credential leakage found by security testing
(~/.ironclaw/tests/SECURITY_REPORT.md, test ce-02).

Leak detector (crates/ironclaw_safety/src/leak_detector.rs):
- Add OpenRouter API key pattern (sk-or-v1-<hex>)
- Add Anthropic OAuth token pattern (sk-ant-oat<NN>-<base64url>)
- Add Telegram bot token pattern (word-bounded, 8-12 digit bot ID)
- Add Groq API key pattern (gsk_<alphanumeric>)
- 12 new tests with synthetic keys (positive, false-positive, integration)

File tools (src/tools/builtin/file.rs):
- Add sensitive path blocklist to ReadFileTool, WriteFileTool, and
  ApplyPatchTool (defense-in-depth for all file access vectors)
- Blocks: .env (and .env.local/.env.production/etc.), .ssh/, .aws/,
  .netrc, .pgpass, .npmrc, .pypirc, .docker/config.json, .kube/config,
  .git-credentials, .gcloud/, .config/gcloud/, .gnupg/, .vault-token,
  .ironclaw/secrets/
- Allows .env.example, .env.template, .env.sample (safe suffixes)
- Case-insensitive; resolves symlinks via canonicalize() before check
- 8 new tests covering blocking, safe suffixes, .env variants, case

Known gap: shell tool can still `cat ~/.env` — different security
domain (denylist-gated in autonomous mode, user-initiated in
interactive mode). Tracked for follow-up.

Note: new patterns use .unwrap() on Regex::new() matching the
established convention of the 16 existing patterns in this file
(all use // safety: hardcoded literal). Follow-up to address the
existing .unwrap() debt across all patterns.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update src/tools/builtin/file.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update crates/ironclaw_safety/src/leak_detector.rs

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* fix(safety): address review feedback on credential patterns and path blocklist

- Move is_sensitive_path to ironclaw_safety crate for shared use
- Guard ListDirTool with sensitive path checks (including recursive traversal)
- Add missing sensitive paths: ~/.config/gh/hosts.yml, /etc/shadow,
  ~/.terraform.d/credentials.tfrc.json, ~/.azure/
- Add path traversal regression test and ListDirTool blocking test

Addresses review feedback from zmanian, gemini-code-assist, and copilot.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): annotate sensitive dirs as blocked in recursive listing

When ListDirTool's recursive traversal encounters a sensitive directory,
annotate it with [sensitive - access blocked] so users understand why
its contents are suppressed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): integrate is_sensitive_path into shell tool file-access checking

Add defense-in-depth check for shell commands that read sensitive
credential files (cat, head, tail, less, cp, etc.). Extracts file
path arguments from known file-reading commands and checks them
against the shared is_sensitive_path function from ironclaw_safety.

This is best-effort — shell-level bypass via aliases, variable
expansion, or encoding is still possible. Full mitigation requires
filesystem-level sandboxing (seccomp/landlock).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): add output redirection checks, fix segment splitting

Address zmanian's re-review findings:
- Check > and >> redirection targets against is_sensitive_path
  (write-path equivalent of read-path protection)
- Replace single-char & splitting with proper &&/|| aware parser
  to avoid fragmenting double operators
- Document subshell/command-substitution gap (partially covered by
  detect_command_injection upstream)
- Extract helpers: split_shell_segments, check_segment_file_commands,
  check_redirect_target, expand_tilde
- Add tests for output redirection, chained commands, segment splitting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): address PR #1675 review feedback and absorb #1713 patterns

Absorb #1713's sensitive path patterns into ironclaw_safety crate:
- Add shell history files, SSH key types, /etc/gshadow
- Add sensitive file extensions (.pem, .key, .p12, .pfx, .jks, .keystore)
- Add .dist safe suffix; smart .env matching (excludes .envrc, .environment)
- Directory-level blocking for .aws/, .docker/, .kube/ (not just specific files)
- Trailing-slash matching so bare directory paths trigger detection

Shell tool hardening:
- Strip surrounding quotes from tokens before sensitive path check
- Add grep, awk, sed to FILE_READ_COMMANDS
- Scan ALL redirect operators in a segment, not just the first

Other fixes:
- Add trailing \b to Telegram bot token regex to prevent over-matching
- Update error messages to reference secret_list/secret_create
- Strengthen ListDirTool test with tempfile-based .ssh directory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(safety): close 4 adversarial bypass vectors in leak detector

- Fix .env suffix check: use exact remainder matching instead of
  ends_with, so .env.production.dist is no longer allowed through
- Detect process substitution <(...) in redirect checks and scan
  inner tokens for sensitive paths
- Add missing /id_rsa to SENSITIVE_PATH_PATTERNS (other SSH key
  types were already present)
- Check --flag=value tokens for sensitive paths instead of skipping
  all tokens starting with -

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix 3 items from zmanian re-review on leak detector

1. Document blocking canonicalize() in async context: added comment
   explaining the trade-off (sub-ms on local FS, could block on NFS)
   with guidance to make async if needed.

2. Fix overly broad standalone key patterns: moved id_rsa, id_ed25519,
   id_ecdsa, id_dsa, authorized_keys, known_hosts from substring-based
   SENSITIVE_PATH_PATTERNS to exact filename matching via SENSITIVE_FILENAMES.
   This prevents false positives on paths like /project/grid_rsa_data while
   still blocking /project/test_fixtures/id_rsa.

3. Remove duplicate is_sensitive_path unit tests from file.rs: these
   belong in sensitive_paths.rs which already has comprehensive coverage.
   Kept integration-level tests that exercise tool execute() methods.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: j-bloggs <j-bloggs@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2026-04-07 13:01:37 +09:00
firat.sertgoz
5d6a247d45 fix(channels): allow telegram wasm channel name (#2051)
* fix: allow telegram wasm channel name

* test: use relative import in wasm setup tests

* style: fix wasm setup test import order

* test: use noop pairing store in wasm http test

* fix(channels): reject reserved names during hot activation
2026-04-06 16:18:00 -07:00
Illia Polosukhin
8b6298513d feat(i18n): add Korean translation, fix zh-CN drift, and prevent future drift via pre-commit hook (#2065)
* feat(i18n): add Korean translation, fix zh-CN drift, cover hardcoded strings

Adds Korean (ko) as the third web UI language, brings zh-CN back into
parity with en, converts ~80 hardcoded English strings in app.js into
i18n keys, and installs a pre-commit hook that prevents future drift.

## Korean web UI

- New `src/channels/web/static/i18n/ko.js` — full translation of all
  663 keys, mirroring the structure of `en.js`/`zh-CN.js`
- New `src/channels/web/server.rs` route `/i18n/ko.js` + handler
- New language menu button in `index.html`
- Browser auto-detect now special-cases `ko-*` (in addition to `zh-*`)
  so Korean visitors land on Korean by default
- Toast label map in `i18n-app.js` becomes a small lookup table so the
  next language is a single-line addition

## zh-CN drift fix

`zh-CN.js` was missing 9 keys that had been added to `en.js` after the
Chinese pack was last touched (`config.telegramOpenBot`,
`settings.tools`, and 7 keys under the `tools.*` namespace for the new
Tool Permissions tab). Backfilled with Chinese translations so users on
the Tools settings panel see proper labels instead of raw key strings.

## Hardcoded strings in app.js

`app.js` had ~80 user-facing English string literals that bypassed
`I18n.t()` entirely — toasts, confirms, alerts, button labels, meta-item
labels for jobs/routines/missions detail panels, the theme dynamic
label, dynamic auth states ("Connecting...", "Authenticated"), etc.
These were invisible to the language switcher and would always render
in English regardless of the user's choice.

Replaced every literal with `I18n.t('key', { ...placeholders })` and
added the corresponding ~95 new keys to `en.js`, `zh-CN.js`, AND `ko.js`
in lockstep so all three packs stay at 663 keys with identical key sets
and matching `{name}`-style placeholder tokens.

Existing keys were reused where possible (`message.copy`,
`approval.approved`, `connection.reconnected`, etc.).

## Pre-commit parity hook

New `scripts/check-i18n-parity.sh` (pure POSIX bash, no Node) verifies:

1. No duplicate keys within any single language file
2. Every language has the same key set as `en.js` (the source of truth)
3. Placeholder tokens like `{name}`, `{count}` match across all
   languages — catches the silent bug where a translator drops an
   interpolation token

Wired into both pre-commit hook install paths:
- `scripts/pre-commit-safety.sh` (installed by `dev-setup.sh` as a
  symlink at `.git/hooks/pre-commit`; symlink is followed via
  `readlink` so the script location resolves correctly)
- `.githooks/pre-commit` (used when devs set
  `git config core.hooksPath .githooks`)

Both block the commit on failure with a clear error message and the
`git commit --no-verify` escape hatch. Tested by deliberately removing
a key from `ko.js` (caught) and stripping a `{path}` placeholder
(caught).

## Korean README

New `README.ko.md` — full Korean translation of `README.md`. Follows
the layout of `README.ja.md` (6-item single-word ToC to keep anchors
clean for non-Latin headings). All code blocks, image paths, and badge
URLs preserved verbatim.

`한국어` link added to the language switcher in all 5 READMEs
(`README.md`, `.zh-CN.md`, `.ru.md`, `.ja.md`, and the new `.ko.md`).

## Verification

- `./scripts/check-i18n-parity.sh` — `OK (663 keys × 3 languages)`
- `node --check` clean on every modified JS file
- Three-way parity: identical sorted key sets across en/zh-CN/ko, zero
  placeholder mismatches
- Hook tested by removing/mutating keys and confirming the commit is
  blocked

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(i18n): address PR review feedback [skip-regression-check]

Addresses 6 review comments on #2065. All changes are in
src/channels/web/static/ (per .claude/rules/review-discipline.md
exemption) plus a bash helper script — no Rust code is touched.

## scripts/check-i18n-parity.sh

- **Portable mktemp** (Copilot): bare `mktemp` works on GNU but BSD/macOS
  `mktemp` requires an explicit template with at least 6 trailing X's.
  Wrap in a small `mktemp_file()` helper that always passes a template
  (`${TMPDIR:-/tmp}/check-i18n-parity.XXXXXX`) so the script runs on
  every platform.

- **Symlink-attack-prone /tmp path** (gemini-code-assist): the
  placeholder-mismatch buffer was using `/tmp/i18n-ph-mismatch.$$`,
  which is predictable and vulnerable to symlink races in shared
  /tmp. Replace with `mktemp_file()` for consistency with the rest
  of the script.

## src/channels/web/static/app.js

- **Hardcoded `'Mode'` label** (gemini): jobs detail meta-grid had
  `metaItem('Mode', job.job_mode)` — convert to
  `I18n.t('jobs.mode')` and add the new key to all 3 language packs.

- **Hardcoded `'Yes'`/`'No'`** (Copilot): routine detail showed
  `routine.enabled ? 'Yes' : 'No'` even though the surrounding labels
  were translated. Reuse the existing `settings.on`/`settings.off`
  keys ("On"/"Off") which already render in all languages.

- **Hardcoded `'N/A'`** (Copilot): mission detail showed
  `m.next_fire_at ? formatDate(...) : 'N/A'`. Reuse the existing
  `common.noData` key. Also fixed the same pattern in the TEE popover
  (`renderTeePopover`) where `'N/A'` was used as a fallback for
  three different attestation fields, since fixing the pattern
  across the file is the principled response per the repo's
  review-discipline rule.

## src/channels/web/static/i18n-app.js

- **Hardcoded `LANG_LABELS` map** (gemini): the language-switch toast
  was reading from a per-call `{ 'en': 'English', 'zh-CN': '简体中文',
  'ko': '한국어' }` literal that would grow with every new language
  and drift from the actual supported set. Move each language's own
  native name into its own pack under a new `language.name` key:

      en.js    → 'language.name': 'English'
      zh-CN.js → 'language.name': '简体中文'
      ko.js    → 'language.name': '한국어'

  Then the toast becomes `I18n.t('language.switch') + ': ' +
  I18n.t('language.name')` — both halves are read from the language
  pack that was just switched in, so the entire toast appears in the
  newly selected language. Adding a future language is now a single
  key addition with NO changes to i18n-app.js.

## Verification

  $ ./scripts/check-i18n-parity.sh
  i18n parity: OK (665 keys × 3 languages)

  $ cargo test --lib
  test result: ok. 4241 passed; 0 failed; 3 ignored

Three-way parity preserved with the 2 new keys (`jobs.mode` and
`language.name`) added to all three language packs in lockstep.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 09:22:19 -07:00
Henry Park
508b526db1 test(e2e): expand SSE resilience coverage (#1897)
* test(e2e): expand SSE resilience coverage

* fix: address PR review follow-ups

* refactor(web): consolidate duplicate chat_events_handler, improve docs

- Remove duplicate chat_events_handler from server.rs; wire route to
  handlers::chat::chat_events_handler instead
- Update from_sender doc comment to document boot_id/event-ID reset
- Document that WebSocket (subscribe_raw) does not expose event IDs

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: fix formatting from merge safety annotations

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address PR #1897 review comments and CI formatting

- auth.rs: use safe_truncate() for signature_prefix log (user-supplied
  string may contain multibyte UTF-8 that panics on byte-index slicing)
- scripting.rs: add truncate_for_assert() helper, use it in both
  resource-limit test assertions (Python stdout may contain multibyte)
- app.js: route plan_update through addTrackedEventListener so
  _lastSseEventId advances on plan_update frames (reconnect dedup)
- store_adapter.rs: rewrite slug truncation with explicit ASCII-only
  byte search and fix formatting (rustfmt moved trailing safety
  comment onto a new line)

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: ilblackdragon@gmail.com <ilblackdragon@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 09:20:20 -07:00
Illia Polosukhin
9cf37364af fix(staging): repair broken test build and macOS-incompatible SSRF tests (#2064)
* fix(staging): repair broken test build and macOS-incompatible SSRF tests

Staging tip (f9ed8152) was failing `cargo test` for several unrelated
reasons. This commit gets the test suite back to green on both Linux CI
and macOS.

1. **wrapper.rs:5595** — `PairingStore::new()` was called with zero args
   in `test_http_request_rejects_private_ip_targets`. The signature
   changed to `(db, cache)` in #1898 and the other test in the same file
   (line 5573) was updated to use `PairingStore::new_noop()`, but this
   one was missed. Switch to `new_noop()` to match.

2. **CLI snapshot** — clap's `render_long_help` for `--auto-approve`
   now emits an indented blank line between the short and long
   description (10 spaces, not empty). Update the snapshot to match the
   new output and refresh `assertion_line` (438 -> 461).

3. **validate_base_url IPv6 bracket bug** — `Url::host_str()` returns
   IPv6 literals WITH the surrounding brackets (e.g. `[::1]`), but
   `IpAddr::parse` does not accept brackets — it wants bare `::1`. As a
   result the IPv6 SSRF defense was effectively dead code: every `[…]`
   host failed to parse and fell through to the DNS-resolution path.
   This passed on Linux CI by accident (because `to_socket_addrs` on
   Linux also fails on bracketed strings), but broke on any host whose
   resolver returns a public IP for unresolvable lookups (ISP captive
   portals, ad-injecting DNS providers). Strip the brackets before
   parsing so the IPv6 detection actually works as intended.

4. **DNS-hijack-tolerant test guards** — two tests
   (`validate_base_url_rejects_dns_failure`,
   `test_validate_public_https_url_fails_closed_on_dns_error`) rely on
   RFC 6761's promise that `.invalid` lookups fail. On networks with
   DNS hijacking that promise doesn't hold and the lookups succeed
   (typically resolving to a public ad-server IP). Probe with
   `ironclaw-dns-hijack-probe.invalid` and skip the test with an
   eprintln on hijacked-DNS networks. Coverage on CI is unchanged.

5. **ExtensionManager test isolation** — the
   `extension_manager_with_process_manager_constructs` integration test
   passes `store: None`, which makes `list()` fall back to file-based
   `load_mcp_servers()` reading `~/.ironclaw/mcp-servers.json`. Any
   locally installed MCP server (e.g. notion) leaked into the test and
   broke the empty assertion. Set `IRONCLAW_BASE_DIR` to a fresh
   tempdir at the top of the test (before the LazyLock is initialized)
   to fully isolate.

After this commit, `./scripts/dev-setup.sh` runs end-to-end and
`cargo test` passes 4709/0 locally on macOS as well as CI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(staging): address PR review feedback

- helpers.rs: simplify IPv6 bracket stripping to `host.trim_matches(...)`
  per gemini-code-assist suggestion. Functionally equivalent to the
  chained strip_prefix/strip_suffix/unwrap_or but cleaner; for any host
  string from `Url::host_str()` (which only ever returns matched
  brackets), the result is identical.

- setup/channels.rs: replace blocking `std::net::ToSocketAddrs` probe
  with `tokio::time::timeout(2s, tokio::net::lookup_host(...))` per
  Copilot review. The previous synchronous lookup could block a tokio
  worker thread inside `#[tokio::test]` and stall the suite on slow or
  offline DNS. The async resolver with a hard 2-second cap eliminates
  both risks.

- module_init_integration.rs: drop the brittle `is_empty()` assertion
  and the env-var override entirely, addressing both Copilot's review
  comment about parallel test ordering and the reviewer's deeper
  concern about touching process env from an integration test that
  cannot access the crate-private ENV_MUTEX. The test's actual purpose
  is to verify that ExtensionManager constructs and `list()` returns
  Ok — that's exactly what `is_ok()` checks. The empty assertion was
  always brittle (the test creates empty TOOL/CHANNEL dirs but does not
  isolate ~/.ironclaw, so any locally installed MCP server leaks in)
  and trying to "fix" it by mutating IRONCLAW_BASE_DIR from inside an
  integration test introduces order-dependent behaviour that the user
  flagged: parallel tests in the same binary can race the LazyLock,
  and there is no integration-test-visible mutex to serialise env
  mutations. Removing the assertion is the principled fix.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 08:34:27 -07:00
Coffee
d0096dfc38 feat: NEAR AI MCP server (#2009)
Co-authored-by: Robert Yan <46699230+think-in-universe@users.noreply.github.com>
2026-04-06 21:51:30 +08:00
firat.sertgoz
f9ed81522f test: add Telegram E2E tests and Rust integration tests (#2037)
* Add Telegram local regression test harness

* Add local Telegram smoke test runner

* test: add high-priority Telegram regression tests

Cover 6 previously untested API-level flows using fake axum Telegram
servers: photo attachment download, voice attachment download, long
message splitting (>4096 chars), Markdown parse error fallback to
plain text, sendChatAction typing indicator, and polling mode
(getUpdates with offset tracking).

Test count: 13 → 19. All use real WASM channel execution with
env-var URL override.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test: add full-process Telegram E2E tests

Add 4 end-to-end tests that boot IronClaw, activate the Telegram WASM
channel via the setup API, POST webhook updates, and verify the
sendMessage round-trip through the mock LLM to a fake Telegram API.

Tests cover:
- DM round-trip (setup → webhook → LLM → sendMessage)
- Edited message handling
- Unauthorized user rejection (dm_policy = pairing)
- Invalid webhook secret rejection (401)

New files:
- fake_telegram_api.py: aiohttp server faking the Telegram Bot API
- test_telegram_e2e.py: the 4 test scenarios

conftest.py changes:
- Add fake_telegram_server and telegram_e2e_server fixtures
- Extend _wasm_build_symlinks to also cover channels-src/

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test: expand Telegram E2E coverage with 8 new regression tests

Add 8 new tests covering core functionality gaps and high-priority
error resilience scenarios for the Telegram WASM channel:

Round 1 (functionality):
- Group mention filtering (ignore without @bot, reply with @bot)
- Long message chunking (>4096 chars split correctly)
- Polling mode roundtrip (getUpdates picks up queued messages)
- Markdown fallback (400 parse error triggers plain-text retry)

Round 2 (resilience):
- Missing webhook secret header (401 rejection)
- 429 rate limit resilience (system survives, recovers)
- Document download failure (getFile 500, text still processed)
- Malformed payload resilience (invalid JSON handled, bot continues)

Also extends fake_telegram_api.py with reject_markdown, rate_limit,
and fail_downloads simulation flags plus control endpoints, and adds
a "long response" canned pattern to mock_llm.py.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: resolve CI failures in Telegram test suite

- Add #[cfg(feature = "integration")] gate to
  test_bot_mention_detection_case_insensitive and
  build_telegram_update_value (fixes compilation on default/libsql)
- Run cargo fmt on telegram_auth_integration.rs
- Fix race condition in fake_telegram_api.py get_updates
- Increase rate_limit_count from 5 to 20 for retry resilience
- Move helper functions to proper section in test_telegram_e2e.py

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-06 16:21:01 +09:00
firat.sertgoz
f073eb68af fix: honor auto-approve tools in engine v2 (#2013) 2026-04-06 15:40:31 +09:00
firat.sertgoz
792357b7b6 (fix) WASM channel HTTP SSRF protections (#1976)
* fix: harden WASM channel HTTP SSRF protections

* fix: clean up wasm http security helpers for clippy
2026-04-06 15:39:34 +09:00
firat.sertgoz
98f44711c4 fix(bridge): sanitize orphaned tool results in v2 adapter (#1975)
* fix(bridge): sanitize orphaned tool results in v2 adapter

* test(bridge): cover no-tool sanitizer path
2026-04-06 15:38:20 +09:00
firat.sertgoz
13852ff5e4 fix(docker): ensure ironclaw runtime home exists (#1918) 2026-04-06 08:22:10 +02:00
Joseph Bloggs
5083aed462 fix(agent): prevent self-repair notification spam for stuck jobs (#1867)
* fix(agent): prevent self-repair notification spam for stuck jobs

When a stuck job exceeds max repair attempts, self-repair returns
ManualRequired but never transitions the job to a terminal state.
detect_stuck_jobs() re-finds it every cycle (~60s), sending a
Telegram notification each time — infinite spam.

Two-layer fix:
1. repair_stuck_job: transition to Failed before returning
   ManualRequired, so detect_stuck_jobs stops finding the job
2. Agent loop: HashSet dedup prevents duplicate ManualRequired
   notifications per job (defense-in-depth if transition fails)

Also adds Pending → Failed to the state machine — stuck Pending
jobs (dispatched but never started) could not be terminated.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(agent): handle transition failure in ManualRequired and adjust message

Log error if Failed transition fails, and adjust the ManualRequired
message to accurately reflect whether the job was marked failed or not.

Addresses gemini-code-assist feedback on PR #1867.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(agent): flatten nested Result, dedup Failed notifications, fix test state path

Adversarial review findings:
1. CRITICAL: update_context returns Result<Result<()>>. Using .is_ok()
   on the outer only checks job existence, not transition success.
   Fixed: matches!(result, Ok(Ok(()))).
2. IMPORTANT: RepairResult::Failed arm had same spam potential as
   ManualRequired. Applied same dedup pattern.
3. IMPORTANT: Test exercised Pending→Failed (bypassing production
   path). Now transitions through InProgress→Stuck→Failed.
4. Dropped unrelated Cargo.lock version bump.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: update state machine diagram to include Pending -> Failed

Adversarial review finding: CLAUDE.md state diagram was the canonical
reference but didn't show the new Pending -> Failed transition.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: j-bloggs <j-bloggs@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-05 22:10:20 +09:00
Illia Polosukhin
1c2d2f2694 feat(test): dual-mode live/replay test harness with LLM judge (#2039)
* feat(test): add dual-mode live/replay test harness with LLM judge

Add a general-purpose test infrastructure for running E2E tests in two modes:
- Live mode (IRONCLAW_LIVE_TEST=1): real LLM calls with real tools, records
  traces to disk for future replay
- Replay mode (default): loads saved trace fixtures, deterministic, no API keys

The harness uses Config::from_env() in live mode so the test agent mirrors
the real binary's behavior (engine_v2, allow_local_tools, approval gates).
Includes an LLM judge for semantic verification of non-deterministic output,
and saves human-readable session logs alongside trace fixtures for inspection
and diffing between live and replay runs.

First test case: zizmor security scanner against ironclaw's own workflows.

New files:
- tests/support/live_harness.rs — LiveTestHarness, builder, LLM judge
- tests/e2e_live.rs — zizmor_scan test
- tests/fixtures/llm_traces/live/ — recorded trace + session log

TestRigBuilder additions:
- with_http_interceptor() for injecting RecordingHttpInterceptor
- with_config() for real-binary config parity (respects allow_local_tools,
  engine_v2 from env instead of forcing test defaults)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(live): add engine v2 zizmor scan test, add with_engine_v2 to harness

Add zizmor_scan_v2 test that exercises the same scenario through engine v2.
Documents the current v2 limitation: auto_approve_tools config flag is not
honored by EffectBridgeAdapter — it only checks the per-session "always"
set, so shell calls pause at the approval gate.

Also:
- Add with_engine_v2() to LiveTestHarnessBuilder for config override
- Refactor v1 test to use shared run_zizmor_scan() helper
- V2 test has relaxed assertions matching current v2 behavior

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(test): address PR review feedback

- Fix UTF-8 unsafe string truncation in session log (use char_indices
  to find safe boundary instead of byte-index slicing)
- Remove forced auto_approve_tools(true) from LiveTestHarness build_live;
  let Config::from_env() drive it, with per-test override via new
  with_auto_approve_tools() builder method
- Apply engine_v2 builder override in TestRig's config-override branch
  so with_engine_v2() is not silently ignored when with_config() is used
- Remove unused timeout field and with_timeout() from LiveTestHarnessBuilder
- Tighten judge_response parsing to require strict PASS:/FAIL: prefix;
  anything else is treated as a failure with diagnostic message

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-05 22:03:32 +09:00
firat.sertgoz
733678dd16 Ignore default model override and empty WASM polls (#1914) 2026-04-05 08:35:18 +02:00
Joseph Bloggs
42cbe5f153 fix(self-repair): skip built-in tools in broken tool detection and repair (#1991)
* fix(self-repair): skip built-in tools in broken tool detection and repair

Built-in tools (http, shell, json, etc.) are part of the ironclaw binary.
Errors on them are caller-side issues (bad LLM parameters), not tool
defects. The self-repair system was attempting to rebuild these via
SoftwareBuilder, wasting LLM tokens and spamming users with notifications.

- Add PROTECTED_TOOL_NAMES list covering all 47+ built-in tool names
- Export is_protected_tool_name() for use outside the registry module
- Filter built-in tools in detect_broken_tools() (primary guard)
- Add defense-in-depth guard in repair_broken_tool() (rejects builtins
  even if detection failed to filter them)
- Document SelfRepair trait contract about built-in tool exclusion
- Add regression tests: detect_broken_tools_filters_out_builtins (with
  real libSQL store), repair_broken_tool_skips_builtin (with mock builder),
  is_protected_tool_name_covers_common_builtins (spot-check)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove duplicate tool_info from PROTECTED_TOOL_NAMES

Addressed Gemini review: tool_info was listed twice (extension management
and incorrectly under image tools after merge conflict resolution).
Moved tool_permission_set under its own "Permission tools" category.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: add plan_update to PROTECTED_TOOL_NAMES

The plan_update builtin was missing from the protected list, allowing
the self-repair system to incorrectly attempt rebuilding it on errors.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use SystemScope in test after ownership model rebase

AdminScope::new now takes (Identity, Database) and returns Option.
The test needs SystemScope which has the simpler single-arg constructor.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: j-bloggs <j-bloggs@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-05 09:25:56 +03:00
Reid
e169591432 test(workspace): add direct regression tests for scoped_to_user rebinding (#1652) (#1875)
- Verify primary user_id switches correctly and original workspace is unchanged
  - Verify private memory layers rescope to new user while shared layers stay intact
  - Verify secondary read scopes are preserved exactly and old primary is removed with no duplicates
  - Verify identity reads via both read_primary() and read() pin to new primary, not old or shared scopes
  - Verify non-identity reads still span preserved shared scopes after old primary removal
  - Verify bootstrap flags (pending + completed) preserve on same-user rebind and reset on different-user rebind
  - Establish precondition probes to ensure bootstrap reset tests validate true→false, not default false
2026-04-05 07:55:12 +02:00
lycheepuppy
f3036388af fix(security): safety layer bypass via output truncation [HIGH] (#1851)
* fix(security): run safety checks on truncated tool output

Previously, `sanitize_tool_output()` returned immediately after
truncating oversized output, skipping leak detection, policy
enforcement, and injection scanning entirely. This allowed an attacker
to embed malicious payloads in the first N bytes of oversized tool
output and have them delivered unsanitized to the LLM.

Restructure the truncation path so it feeds into the same safety
pipeline as non-truncated content: leak detection, policy checks,
and Aho-Corasick injection scanning all run on the (possibly
truncated) content before it is returned.

Adds regression tests to verify truncated output is still scanned.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* style: apply rustfmt to fix CI formatting check

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Wui <wui@Wui-Work-2.local>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
2026-04-04 22:08:22 -07:00
Nige
833dd32a8b feat: add AWS Bedrock embeddings provider (#1568)
* feat(embeddings): add bedrock provider

* refactor(embeddings): address review feedback

* fix: CI failures and review feedback for Bedrock embeddings

- Replace ENV_MUTEX.lock() with lock_env() to match staging's test pattern
- Use db_first_or_default() for non-bedrock model resolution (staging API)
- Validate returned embedding dimension matches configured dimension

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-05 12:37:49 +09:00
Henry Park
5451977683 fix: unblock bootstrap ownership on dynamic_tools (#2005)
* Fix bootstrap ownership migration for dynamic tools

* Add bootstrap ownership regression coverage
2026-04-03 18:36:55 -07:00
Henry Park
0588dd1bc3 fix(llm): invert reasoning default — unknown models skip think/final tags (#1952)
* fix(llm): invert reasoning default — unknown models skip <think>/<final> injection

When NEAR AI model="auto" resolves server-side to Qwen 3.5, the system
prompt injected <think>/<final> tags because "auto" didn't match any
known native-thinking pattern. This caused empty responses:

1. Qwen 3.5's native thinking puts reasoning in a `reasoning` field
   (not `reasoning_content`) — silently dropped due to field name mismatch
2. Content contained only <think> tags or <tool_call> XML, which
   clean_response() stripped to empty → "I'm not sure how to respond"

Three fixes:
- Invert the default: new requires_think_final_tags() with empty allowlist
  means unknown/alias models get the safe direct-answer prompt
- Add #[serde(alias = "reasoning")] so vLLM's field name is accepted
- Update active_model from API response.model so capability checks
  use the resolved model name after the first call

Confirmed via direct API testing against NEAR AI staging with
Qwen/Qwen3.5-122B-A10B.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* remove model alias resolution from nearai_chat

auto should stay as the active model name — no reason to overwrite it
with the resolved model since requires_think_final_tags() returns false
for both "auto" and the resolved name.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix wording: remove native-thinking assumption from direct-answer prompt

The direct-answer prompt is now the default for all models, not just
native-thinking ones. Remove misleading "handled natively" language.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 17:51:54 -07:00
Henry Park
e7c89016fb Fix turn cost footer and per-turn usage accounting (#1951)
* Fix turn cost footer and per-turn usage accounting

* Avoid panicking on poisoned turn usage mutex

* Tighten SSE usage regression coverage

* Report usage for interrupted turns
2026-04-03 17:51:40 -07:00
Henry Park
3004583b2a feat(ownership): centralized ownership model with typed identities, DB-backed pairing, and OwnershipCache (#1898)
* feat(ownership): add OwnerId, Identity, UserRole, can_act_on types

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(ownership): private OwnerId field, ResourceScope serde derives, fix doc comment

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* refactor(tenant): replace SystemScope::db() escape hatch with typed workspace_for_user(), fix stale variable names

- Add SystemScope::workspace_for_user() that wraps Workspace::new_with_db
- Remove SystemScope::db() which exposed the raw Arc<dyn Database>
- Update 3 callers (routine_engine.rs x2, heartbeat.rs x1) to use the new method
- Fix stale comment: "admin context" -> "system context" in SystemScope
- Rename `admin` bindings to `system` in agent_loop.rs for clarity

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(tenant): rename stale admin binding to system_store in heartbeat.rs

* refactor(tenant): TenantScope/TenantCtx carry Identity, add with_identity() constructor and bridge new()

- TenantScope: replace `user_id: String` field with `identity: Identity`; add `with_identity()` preferred constructor; keep `new(user_id, db)` as Member-role bridge; add `identity()` accessor; all internal method bodies use `identity.owner_id.as_str()` in place of `&self.user_id`
- TenantCtx: replace `user_id: String` field with `identity: Identity`; update constructor signature; add `identity()` accessor; `user_id()` delegates to `identity.owner_id.as_str()`; cost/rate methods updated accordingly
- agent_loop: split `tenant_ctx(&str)` into bridge + new `tenant_ctx_with_identity(Identity)` which holds the full body; bridge delegates to avoid duplication

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* feat(db): add V16 tool scope, V17 channel_identities, V18 pairing_requests migrations

- PostgreSQL: V16__tool_scope.sql adds scope column to wasm_tools/dynamic_tools
- PostgreSQL: V17__channel_identities.sql creates channel identity resolution table
- PostgreSQL: V18__pairing_requests.sql creates pairing request table replacing file-based store
- libSQL SCHEMA: adds scope column to wasm_tools/dynamic_tools, channel_identities, pairing_requests tables
- libSQL INCREMENTAL_MIGRATIONS: versions 17-19 for existing databases
- IDEMPOTENT_ADD_COLUMN_MIGRATIONS: handles fresh-install/upgrade dual path for scope columns
- Runner updated to check ALL idempotent columns per version before skipping SQL
- Test: test_ownership_model_tables_created verifies all new tables/columns exist after migrations

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(db): use correct RFC3339 timestamp default in libSQL, document version sequence offset

Replace datetime('now') with strftime('%Y-%m-%dT%H:%M:%fZ', 'now') in the
channel_identities and pairing_requests table definitions (both in SCHEMA and
INCREMENTAL_MIGRATIONS) to match the project-standard RFC 3339 timestamp format
with millisecond precision. Also add a comment clarifying that libSQL incremental
migration version numbers are independent from PostgreSQL VN migration numbers.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* feat(ownership): bootstrap_ownership(), migrate_default_owner, V19 FK migration, replace hardcoded 'default' user IDs

- Add V19__ownership_fk.sql (programmatic-only, not in auto-migration sweep)
- Add `migrate_default_owner` to Database trait + both PgBackend and LibSqlBackend
- Add `get_or_create_user` default method to UserStore trait
- Add `bootstrap_ownership()` to app.rs, called in init_database() after connect_with_handles
- Replace hardcoded "default" owner_id in cli/config.rs, cli/mcp.rs, cli/mod.rs, orchestrator/mod.rs
- Add TODO(ownership) comments in llm/session.rs and tools/mcp/client.rs for deferred constructors

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(ownership): atomic get_or_create_user, transactional migrate_default_owner, V19 FK inline constant, fix remaining 'default' user IDs

- Delete migrations/V19__ownership_fk.sql so refinery no longer auto-applies FK constraints before bootstrap_ownership runs; add OWNERSHIP_FK_SQL constant with TODO for future programmatic application
- Remove racy SELECT+INSERT default in UserStore::get_or_create_user; both PostgreSQL (ON CONFLICT DO NOTHING) and libSQL (INSERT OR IGNORE) now use atomic upserts
- Wrap migrate_default_owner in explicit transactions on both backends for atomicity
- Make bootstrap_ownership failure fatal (propagate error instead of warn-and-continue)
- Fix mcp auth/test --user: change from default_value="default" to Option<String> resolved from configured owner_id
- Replace hardcoded "default" user IDs in channels/wasm/setup.rs with config.owner_id
- Replace "default" sentinel in OrchestratorState test helper with "<unset>" to make the test-only nature explicit

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(ownership): remove default user_id from create_job(), change sentinel strings to <unset>

- Gate ContextManager::create_job() behind #[cfg(test)]; production code must
  use create_job_for_user() with an explicit user_id to prevent DB rows with
  user_id = 'default' being silently created on the production write path.
- Change the placeholder user_id in McpClient::new(), new_with_name(), and
  new_with_config() from "default" to "<unset>" so accidental secrets/settings
  lookups surface immediately rather than silently touching the wrong DB partition.
- Same sentinel change for SessionManager::new() and new_async() in session.rs;
  these are overwritten by attach_store() at startup with the real owner_id.
- Update tests that asserted the old "default" sentinel to expect "<unset>", and
  switch test_list_jobs_tool / test_job_status_tool to create_job_for_user("default")
  to keep ownership alignment with JobContext::default().

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* feat(db): add ChannelPairingStore sub-trait with resolve_channel_identity, upsert/approve pairing, PostgreSQL + libSQL implementations

Adds PairingRequestRecord, ChannelPairingStore trait (5 methods), and
generate_pairing_code() to src/db/mod.rs; implements for PgBackend in
postgres.rs and LibSqlBackend in libsql/pairing.rs; wires ChannelPairingStore
into the Database supertrait bound; all 6 libSQL unit tests pass.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(db): atomic libSQL approve_pairing with BEGIN IMMEDIATE, add case-insensitive/expired/double-approve tests

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* feat(ownership): add OwnershipCache for zero-DB-read identity resolution on warm path

Converts src/ownership.rs to src/ownership/ module directory and adds
src/ownership/cache.rs with a write-through in-process cache mapping
(channel, external_id) -> Identity. Wired as Arc<OwnershipCache> on
AppComponents for Task 8 pairing integration. All 7 cache unit tests pass.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* test(e2e): add ownership model E2E tests and extend pairing tests for DB-backed store

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(e2e): remove unused asyncio import, add fallback assertion in test_pairing_response_structure

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* test(tenant): unit tests for TenantScope::with_identity and AdminScope construction

Adds 5 focused unit tests verifying TenantScope::with_identity stores the
full Identity (owner_id + role), TenantScope::new creates a Member-role
identity, and AdminScope::new returns Some for Admin and None for Member.
Uses LibSqlBackend::new_memory() as the test DB stub.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(ownership): recover from RwLock poison instead of expect() in OwnershipCache

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* test(ownership): integration tests for bootstrap, tenant isolation, and ChannelPairingStore

Adds tests/ownership_integration.rs covering migrate_default_owner idempotency,
TenantScope per-user setting isolation (including Admin role bypass check),
and the full ChannelPairingStore lifecycle (upsert, approve, remove, multi-channel isolation).

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(test): remove duplicate pairing tests and flaky random-code assertion from integration suite

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* feat(pairing): rewrite PairingStore to DB-backed async with OwnershipCache

Replaces the file-based pairing store (~/.ironclaw/*-pairing.json,
*-allowFrom.json) with a DB-backed async implementation that delegates
to ChannelPairingStore and writes through to OwnershipCache on reads.

- PairingStore::new(db, cache) uses the DB; new_noop() for test/no-DB
- resolve_identity() cache-first lookup via OwnershipCache
- approve(code, owner_id) removes channel arg (DB looks up by code)
- All WASM host functions updated: pairing_upsert_request uses block_in_place,
  pairing-is-allowed renamed to pairing-resolve-identity returning Option<String>,
  pairing-read-allow-from deprecated (returns empty list)
- Signal channel receives PairingStore via new(config, db) constructor
- Web gateway pairing handlers read from state.store (DB) directly
- extensions.rs derive_activation_status drops PairingStore dependency;
  derives status from extension.active and owner_binding flag instead
- All test call sites updated to use new_noop()

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(pairing): add missing pairing_store field to all GatewayState initializers, fix disk-full post-edit compile

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* feat(channels): remove owner_id from IncomingMessage, user_id is the canonical resolved OwnerId

`owner_id` on `IncomingMessage` was always a duplicate of `user_id` —
both fields held the same value at every call site. Remove the field and
`with_owner_id()` builder, update the four WASM-wrapper and HTTP test
assertions to use `user_id`, and drop the redundant struct literal field
in the routine_engine test helper.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(channels): remove stale owner_id param from make_message test helper

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* test(e2e): add browser/Playwright tests for ownership model — auth screen, chat UI, owner login

Adds five Playwright-based browser tests to the ownership model E2E suite
verifying the web UI experience: authenticated owner sees chat input, unauthenticated
browser sees auth screen, owner can send a message and receive a response, settings
tab renders without errors, and basic page structure is correct after login.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* feat(settings): migrate channel credentials from plaintext settings to encrypted secrets store

Moves nearai.session_token from the plaintext DB settings table to the
AES-256-GCM encrypted secrets store (key: nearai_session_token).

- SessionManager gains an `attach_secrets()` method that wires in the
  secrets store; `save_session` writes to it when available and
  `load_session_from_secrets` is called preferentially over settings
- `migrate_session_credential()` runs idempotently on each startup in
  `init_secrets()`, reading the JSON session from settings, writing it
  to secrets, then deleting the plaintext copy
- Wizard's `persist_session_to_db` now writes to secrets first, falling
  back to plaintext settings only when secrets store is unavailable
- Plaintext settings path is preserved as fallback for installs without
  a secrets store (no master key configured)

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(settings): settings fallback only when no secrets store, verify decryption before deleting plaintext

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(ownership): ROLLBACK in libSQL migrate_default_owner, shared OwnershipCache across channels, add dynamic_tools to migration, fix doc comment

- libSQL migrate_default_owner: wrap UPDATE loop in async closure + match to emit ROLLBACK on any mid-transaction failure (mirroring approve_pairing pattern)
- Both backends: add dynamic_tools to the migrate_default_owner table list so agent-built tools are migrated on first pairing
- setup_wasm_channels: accept Arc<OwnershipCache> parameter instead of allocating a fresh cache, share the AppComponents cache
- SignalChannel:🆕 accept Arc<OwnershipCache> parameter and pass it to PairingStore instead of allocating a new cache
- PairingStore: fix module-level and struct-level doc comments to accurately describe lazy cache population after approve()

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(web): use can_act_on for authorization in job/routine handlers instead of raw string comparisons

Replace 12 raw `user_id != user.user_id` / `user_id == user.user_id` string comparisons
in jobs.rs and 4 in routines.rs with calls through the canonical `can_act_on` function
from `crate::ownership`, which is the spec-mandated authorization mechanism.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* chore: include remaining modified files in ownership model branch

* fix: add pairing_store field to test GatewayState initializers, update PairingStore API calls in integration tests

Add missing `pairing_store: None` to all GatewayState struct initializers
in test files. Migrate old file-based PairingStore API calls
(PairingStore::new(), PairingStore::with_base_dir()) to the new DB-backed
API (PairingStore::new_noop()). Rewrite pairing_integration.rs to use
LibSqlBackend with the new async DB-backed PairingStore API.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* chore: cargo fmt

* fix(pairing): truly no-op PairingStore noop mode, ensure owner user in CLI, fix signal safety comments

- PairingStore::upsert_request now returns a dummy record in noop mode instead of
  erroring, and approve silently succeeds (matching the doc promise of "writes
  are silently discarded").
- PairingStore::approve now accepts a channel parameter, matching the updated
  DB trait signature and propagated to all call sites (CLI, web server, tests).
- CLI run_pairing_command ensures the owner user row exists before approval to
  satisfy the FK constraint on channel_identities.owner_id.
- Signal channel block_in_place safety comments corrected from "WASM channel
  callbacks" to "Signal channel message processing".

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(pairing): thread channel through approve_pairing, add created flag, retry on code collision, remove redundant indexes

Addresses PR review comments:
- approve_pairing validates code belongs to the given channel
- PairingRequestRecord.created replaces timing heuristic
- upsert retries on UNIQUE violation (up to 3 attempts)
- redundant indexes removed (UNIQUE creates implicit index)

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(ownership): migrate api_tokens, serialize PG approvals, propagate resolved owner_id

Addresses PR review P1/P2 regressions:

- api_tokens included in migrate_default_owner (both backends)
- PostgreSQL approve_pairing uses FOR UPDATE to prevent concurrent approvals
- Signal resolve_sender_identity returns owner_id, set as IncomingMessage.user_id
  with raw phone number preserved as sender_id for reply routing
- Feishu uses resolved owner_id from pairing_resolve_identity in emitted message
- PairingStore noop mode logs warning when pairing admission is impossible

[skip-regression-check]

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* fix(pr-review): sanitize DB errors in pairing handlers, fix doc comments, add TODO for derive_activation_status

- Pairing list/approve handlers no longer leak DB error details to clients
- NotFound errors return user-friendly 'Invalid or expired pairing code' message
- Module doc in pairing/store.rs corrected (remove -> evict, no insert method)
- wit_compat.rs stub comment corrected to match actual Val shape
- TODO added for derive_activation_status has_paired approximation

* fix(pr-review): propagate libSQL query errors in approve_pairing, round-trip validate session credential migration, fix test doc comment

- libSQL approve_pairing: .ok().flatten() replaced with .map_err() to propagate DB errors
- migrate_session_credential: round-trip compares decrypted secret against plaintext before deleting
- ownership_integration.rs: doc comment corrected to match actual test coverage

* fix(pairing): store meta, wrap upserts in transactions, case-insensitive role/channel, log Signal DB errors, use auth role in handlers

- Store meta JSONB/TEXT column in pairing_requests (PG migration V18, libSQL schema + incremental migration 19)
- Wrap upsert_pairing_request in transactions (PG: client.transaction(), libSQL: BEGIN IMMEDIATE/COMMIT/ROLLBACK)
- Case-insensitive role parsing: eq_ignore_ascii_case("admin") in both backends
- Case-insensitive channel matching in approve_pairing: LOWER(channel) = LOWER($2)
- Log DB errors in Signal resolve_sender_identity instead of silently discarding
- Use auth role from UserIdentity in web handlers (jobs.rs, routines.rs) via identity_from_auth helper
- Fix variable shadowing: rename `let channel` to `let req_channel` in libsql approve_pairing

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(security): add auth to pairing list, cache eviction on deactivate, runtime assert in Signal, remove default fallback, warn on noop pairing codes

Addresses zmanian's review:
- #1: pairing_list_handler requires AuthenticatedUser
- #2: OwnershipCache.evict_user() evicts all entries for a user on suspension
- #3: debug_assert! for multi-thread runtime in Signal block_in_place
- #9: Noop PairingStore warns when generating unredeemable codes
- #10: cli/mcp.rs default fallback replaced with <unset>

* fix(pairing): consistent LOWER() channel matching in resolve_channel_identity, fix wizard doc comment, fix E2E test assertion for ActionResponse convention

* fix(pairing): apply LOWER() consistently across all ChannelPairingStore queries (upsert, list_pending, remove)

All channel matching now uses LOWER() in both PostgreSQL and libSQL backends:
- upsert_pairing_request: WHERE LOWER(channel) = LOWER($1)
- list_pending_pairings: WHERE LOWER(channel) = LOWER($1)
- remove_channel_identity: WHERE LOWER(channel) = LOWER($1)

Previously only resolve_channel_identity and approve_pairing used LOWER(),
causing inconsistent matching when channel names differed by case.

* fix(pairing): unify code challenge flow and harden web pairing

* test: harden pairing review follow-ups

* fix: guard wasm pairing callbacks by runtime flavor

* fix(pairing): normalize channel keys and serialize pg upserts

* chore(web): clean up ownership review follow-ups

* Preserve WASM pairing allowlist compatibility

---------

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 17:51:09 -07:00
Evrard-Nil
994f96079d Publish ironclaw-worker image from Dockerfile.worker (#1979)
* feat(docker): publish ironclaw-worker image alongside ironclaw

Build and push nearaidev/ironclaw-worker from Dockerfile.worker in the
same workflow. Both images share the same version/sha/tag scheme.

This lets ironclaw-dind pull the pre-built worker image for sandbox
baking instead of cloning the repo and building from source.

[skip-regression-check]

* feat(docker): daily scheduled build of :staging from staging branch

* perf(docker): worker image copies binary from ironclaw image instead of rebuilding

* revert Dockerfile.worker changes, keep it building from source
2026-04-03 11:27:53 -07:00
Henry Park
b91bdbdb27 feat(tools): persistent per-user tool permission system (#1911)
Add a three-state permission model (AlwaysAllow / AskEachTime / Disabled)
backed by per-user DB settings so that safe built-in tools run without
approval prompts by default, while destructive tools still require
explicit user confirmation.

Key changes:
- PermissionState enum + TOOL_RISK_DEFAULTS tier map (permissions.rs)
- Security: tools returning UnlessAutoApproved (http, create_job,
  event_emit, routine_create, routine_update) default to AskEachTime
  to prevent SSRF, resource abuse, and privilege escalation
- Dispatcher filters Disabled tools and pre-approves AlwaysAllow tools;
  clears and re-populates session auto-approvals each iteration so
  permission downgrades take effect immediately; caches permissions at
  iteration 0 to avoid repeated DB round-trips
- Graceful DB failure handling: keeps existing session approvals on
  transient DB errors instead of clearing them
- "Always Approve" persists to DB across sessions (thread_ops.rs);
  defense-in-depth skips persist for ApprovalRequirement::Always tools
- tool_permission_set LLM tool (always requires approval to change);
  returns error when no settings store configured
- tool_list extended with builtin tools + permission state fields;
  filters by builtin_tool_names() to avoid duplicating WASM/MCP tools
- Web UI: Tools settings tab with 3-way toggle + lock icons;
  JSON error body for locked tool rejection
- Dot-separated tool name validation at registration time
- Playwright E2E: 6 test scenarios for permissions lifecycle
- seed_tool_permissions writes tier defaults to DB at startup with
  idempotency test

ApprovalRequirement::Always is an unbypassable hard floor — even
AlwaysAllow cannot bypass it. All DB operations scoped to user_id via
TenantScope for multi-tenant correctness.

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 09:51:09 -07:00