mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
3c7925c100abecd6ab6e225c727bbd73a43f048b
35 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Illia Polosukhin
|
3c7925c100 |
refactor(gateway): delete server.rs shim + relocate tests to slices — ironclaw#2599 stage 6 (#2706)
Finishes the feature-slice migration started in stage 4a. After this: - `src/channels/web/server.rs` no longer exists. - Every caller of `crate::channels::web::server::*` now points at `platform::router::start_server` or `platform::state::*` directly. - All ~60 caller-level tests that used to live in `server.rs::tests` now live inside the feature slice they actually exercise, next to the handler they test. ## What moved where Classification driven by the handler each test drives: | Slice | Tests | |---|---| | `features/chat/mod.rs::tests` | 3 × history, 4 × auth-token/cancel + gate-resolve, 1 × approval, 3 × pending-gate-extension-name, 1 × test_auth_manager helper | | `features/pairing/mod.rs::tests` | 1 × list, 5 × approve (claim / no-followup / with-thread / external-callback / blank-code), `make_pairing_test_state` helper | | `features/extensions/mod.rs::tests` | 2 × activation classifier, 2 × path-traversal guards, 1 × setup-submit-not-activated, 2 × list-inactive-wasm-channel, 1 × phase-precedence, 1 × readiness handler, 2 × apply_extension_readiness | | `features/oauth/mod.rs::tests` | 13 × oauth callback (missing params / unknown state / expired × 2 / no-ext-mgr / strip-prefix / versioned × 2 / happy × 3 / exchange-fail), 5 × relay oauth callback, + `TestOauthProxy`, `EnvVarGuard`, `set_env_var`, `fresh_pending_oauth_flow`, `expired_flow_created_at`, `test_oauth_router`, `test_relay_oauth_router` helpers | | `platform/static_files.rs::tests` | 3 × CSP header / base / nonce, 2 × css etag, 1 × css handler, 2 × css multi-tenant, 4 × stamp nonce + build frontend HTML, 1 × test_build_frontend_html_returns_none_in_multi_tenant_mode | | `platform/state.rs::tests` | 1 × workspace_pool_resolve_seeds_new_user_workspace | | `handlers/llm.rs::tests` | 3 × llm admin-role guards | | `handlers/users.rs::tests` | 1 × delete_user_evicts_auth_and_pairing_caches | ## Cross-slice test fixtures Four helpers that multiple slices share (`insert_test_user`, `test_secrets_store`, `test_ext_mgr`, `test_ext_mgr_with_db`) moved into `src/channels/web/test_helpers.rs` as `#[cfg(test)] pub(crate)` free functions, following the pattern from stage 6a (#2704) for `test_gateway_state*`. All four keep the exact signatures they had in `server.rs::tests`, so the move was mechanical. Rust expect suppressions on the five `.expect(...)` lines inside these fixtures carry `// safety: cfg(test) fixture` comments — the pre-commit safety check is diff-line based and doesn't look up whether the containing function is already `cfg(test)`-gated. ## Mechanical renames (25 files) `channels::web::server::<item>` call sites now import from: - `platform::router::start_server` - `platform::state::{GatewayState, RateLimiter, PerUserRateLimiter, WorkspacePool, FrontendCacheKey, FrontendHtmlCache, ActiveConfigSnapshot, PromptQueue, RoutineEngineSlot, rate_limit_key_from_headers}` Covers `src/main.rs`, `src/app.rs`, `src/tools/builtin/{job,memory}.rs`, all 13 handlers in `handlers/*.rs`, the four integration tests (`ws_gateway_integration`, `openai_compat_integration`, `multi_tenant_integration`, `oauth_greeting_integration`), plus `tests/support/gateway_workflow_harness.rs` and `src/channels/web/tests/multi_tenant.rs`. No behavior change. ## Boundary checker retained `scripts/check_gateway_boundaries.py` still rejects any `crate::channels::web::server::` path as a defense-in-depth guard against accidental re-introduction (literal new `server.rs`, stray imports, etc.). The explanatory comment and the regression test's docstring now reflect "shim is gone; this guard prevents re-creation" instead of "shim exists; don't route through it." ## Documentation updates - `src/channels/web/CLAUDE.md`: deleted the `server.rs` File Map row, updated the `test_helpers.rs` row to list all seven `pub(crate)` fixtures (stages 6a + 6 together), fixed all prose references that pointed at `server.rs`, and updated the "Adding a New API Endpoint" recipe to point at `features/<slice>/` and `platform/router.rs`. - `src/channels/web/platform/state.rs`: module docstring now says "shim was removed" instead of "shim exists pending migration." - `src/bridge/CLAUDE.md`: `pending_gate_extension_name` reference now points at `features/chat/mod.rs`. ## Quality gate - [x] `cargo fmt --all` - [x] `cargo clippy --all --benches --tests --examples --all-features` — zero warnings - [x] `cargo check -p ironclaw --no-default-features --features libsql --tests` — clean - [x] `cargo test -p ironclaw --lib channels::web` — 434 passed (up from 431 — three tests that were incorrectly filtered under `channels::web::server::tests` now surface under their proper slice's module path) - [x] `cargo test -p ironclaw --test multi_tenant_integration` — 40 passed - [x] `cargo test -p ironclaw --test openai_compat_integration` — 16 passed - [x] `cargo test -p ironclaw --test ws_gateway_integration` — 11 passed - [x] `python3 scripts/check_gateway_boundaries.py` — clean - [x] `python3 scripts/check_gateway_boundaries.py test` — 16/16 - [x] `bash scripts/pre-commit-safety.sh` — clean ## Regression coverage Pure relocation + mechanical rename; no behavior change. The existing ~60 tests from `server.rs::tests` continue to pass unmodified, which is the regression evidence. A "test that would have caught this" would necessarily duplicate the existing tests — no new test adds coverage. [skip-regression-check] Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
a35f9d9ab5 |
refactor(gateway): split monolithic style.css and app.js into per-surface modules (#2683)
The gateway's static frontend had grown into two merge-conflict hotspots: a
6 887-line `style.css` and an 11 189-line `app.js`, each a catch-all for every
surface of the SPA. Any two PRs touching different tabs were likely to collide.
This change splits both files by surface/concern while preserving bytes and
behavior. `STYLE_CSS` and `APP_JS` in `crates/ironclaw_gateway/src/assets.rs`
now `concat!(include_str!(...))` the split pieces at compile time, so the
served `/style.css` and `/app.js` URLs are unchanged and the existing
workspace-overlay (`custom.css`) path still works. Cuts land on function /
block boundaries; `node --check` validates the concat. Admin assets move
under `static/admin/` for symmetry. Per-commit safety + CI workflow validate
the split files per-file instead of the old monolith.
- Styles split into 20 files under `static/styles/{base,layout}.css +
styles/{components,primitives,surfaces}/*.css`
- JS split into 24 files under `static/js/{core,surfaces}/*.js`
- No URL, CSP, or behavioural change — pure file-layout refactor
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Illia Polosukhin
|
a524bf8aee |
refactor(gateway): stage 4b slices + empty allowlist — ironclaw#2599 (#2665)
* refactor(gateway): stage 4b slices + empty allowlist — ironclaw#2599
Collapses the last pre-existing back-edges tracked by the boundary
checker and moves the first three small feature slices out of
server.rs.
Stage 4b slices:
- features/logs/ — /api/logs/events, /api/logs/level (GET/PUT)
- features/pairing/ — /api/pairing/{channel} (GET),
/api/pairing/{channel}/approve (POST)
- features/status/ — /api/gateway/status (+ GatewayStatusResponse,
ModelUsageEntry, each now owned by the slice)
Platform extensions (co-located with existing platform modules so
every caller — handlers/, features/, and the still-shrinking
server.rs — can reach them without a back-edge):
- platform/legacy_auth.rs:
- handle_legacy_auth_token_submission
- handle_legacy_auth_cancel
- clear_auth_mode, clear_auth_mode_for_thread
Consumers: server.rs chat HTTP shims + platform/ws.rs.
- platform/engine_dispatch.rs:
- dispatch_engine_submission
- dispatch_engine_external_callback
- dispatch_onboarding_ready_followup (now takes &ExtensionName)
Consumers: server.rs chat + extensions_setup_submit + features/pairing.
- platform/static_files.rs gains the workspace-backed layout/widget
readers (read_layout_config, load_resolved_widgets,
read_widget_manifest, LAYOUT_PATH, WIDGETS_DIR, MAX_WIDGET_* caps).
handlers/frontend.rs imports them back from platform.
ExtensionName adoption:
- Deletes sanitize_extension_name and its 5 unit tests from
server.rs. The defensive "never fails, returns 'unknown'" helper is
replaced with ironclaw_common::ExtensionName validation at the one
untrusted boundary we still expose (pairing_approve_handler's URL
path). Invalid names now return 400 at the handler — the old
behavior sanitized injection-shaped input into a safe-but-nonsense
string that would never have matched a real extension anyway, so
this is strictly better telemetry with no loss of reachable
behavior. Registry-sourced names (derive_onboarding in
handlers/extensions.rs) drop the sanitize call entirely; the
comment notes a follow-up to type Extension.name as ExtensionName
directly.
- Other shared helpers that needed to leave server.rs as collateral:
images_to_attachments moves to web/util.rs alongside the other
pure message-building helpers.
ws.rs cleanup:
- Switches GatewayState / PerUserRateLimiter / RateLimiter /
ActiveConfigSnapshot imports from the server.rs re-export path to
crate::channels::web::platform::state directly, removing the last
state-type allowlist entries.
Allowlist:
- scripts/check_gateway_boundaries.py: ALLOWLIST is now empty. All
eight pre-existing entries (widget helpers, seven ws.rs shim
symbols) are gone — every relocation landed in platform/. The
allowlist mechanism stays in place for future narrowly-scoped
exceptions.
Diff is roughly −700 lines net from server.rs (now ~5,700 down
from ~6,300), spread across three new feature-slice files and two
new platform modules. No behavior change — this is a pure
relocation + one type-boundary upgrade.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(gateway): lock in pairing boundary tests + tighten layout helper visibility — PR #2665
Two valid findings from the PR #2665 review:
- `read_layout_config` was carried over from `handlers/frontend.rs` as
`pub`, but every caller lives inside `src/channels/web/`. Tightened
to `pub(crate)` to match the rest of the workspace/widget helpers in
the same module (Copilot).
- Added regression coverage for the new 400 boundary in
`features/pairing/` — `parse_channel` now has 8 unit tests pinning
that it accepts the lowercase / snake_case shapes pairing uses and
lowercases mixed-case URL paths, and that it rejects empty, path
traversal, invalid chars, consecutive underscores, edge
underscores, and oversized input with `StatusCode::BAD_REQUEST`.
This locks in the stricter contract the PR introduced so a future
edit can't accidentally regress to silent canonicalization (Copilot).
The third review note (Gemini: `engine_v2` + `engine_v2_enabled`
redundancy in `GatewayStatusResponse`) is a pre-existing wire-contract
shape — `crates/ironclaw_gateway/static/app.js:8120` reads
`engine_v2` and `app.js:8130` reads `engine_v2_enabled`, so dropping
either field without a coordinated frontend change would regress the
browser UI. Out of scope for this PR's pure relocation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(gateway): parse_channel preserves hyphens for slack-relay — PR #2665 review
Copilot's round-2 review caught a real regression I introduced in this
PR: `parse_channel` returned `ExtensionName::new(...)` directly, and
`ExtensionName`'s canonical form folds `-` into `_`. The pairing store
(via `crate::pairing::normalize_channel_name` in `src/pairing/mod.rs`)
only lowercases — it does *not* fold hyphens — so the live WASM channel
`slack-relay` (see `src/channels/wasm/setup.rs` and
`crate::channels::relay::DEFAULT_RELAY_NAME`) stores hyphenated rows
that a folded `slack_relay` query would silently miss. Empty pairing
lists and failed approvals for every `slack-relay` code.
Fix: keep `ExtensionName::new` at the boundary for its rejection
semantics (path traversal, invalid chars, oversize, edge/consecutive
underscores) but discard the typed value. `parse_channel` now returns
the pre-fold lowercased `String`, which flows directly into
`pairing_store.list_pending` / `approve` and
`ext_mgr.complete_pairing_approval`. The two AppEvent / dispatch call
sites that need a typed `ExtensionName` wrap via
`ExtensionName::from_trusted` — same escape hatch staging's
`pairing_approve_handler` was using before this PR moved it. The
module docstring spells out why the discard-and-keep-the-raw-string
dance exists.
Regression test added (`parse_channel_preserves_hyphens_for_slack_relay`)
pinning both `slack-relay` and `SLACK-RELAY` round-trip through
`parse_channel` as `slack-relay`. Existing tests updated for the new
`Result<String, _>` return type. 9 tests pass.
Also fixed a stale comment in `platform/router.rs` (Copilot): the
"feature handlers still inline in server.rs pending migration" note
predated the logs/oauth/pairing/status slices being extracted. Rewrote
to describe the current split. And dropped the forward-looking
`derive_onboarding` comment about typing `Extension.name` as
`ExtensionName` — this PR just demonstrated that such a naive swap
would break `slack-relay` and related hyphenated channels, so the
follow-up is larger than "type the field".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Illia Polosukhin
|
77c3821f33 |
feat(common): apply ExtensionName newtype to fan-out sites (PR 2/2) (#2617)
* feat(common): add CredentialName and ExtensionName newtypes Introduce typed identifiers for the backend-secret vs user-facing extension identity split that the Extension/Auth Invariants section of CLAUDE.md describes. Four recent PRs (#2561, #2473, #2512, #2574) have been identity- confusion bugs with the same shape: a stringly-typed value passed through multiple layers with each layer meaning a different thing. Newtypes make each of those a compile error. This is PR 1 of 2. PR 1 lands the newtypes and migrates the core auth seam (ResumeKind::Authentication, MissingCredential, ToolReadiness::NeedsAuth, LatentActionExecution::NeedsAuth, extensions/naming.rs). PR 2 will migrate AppEvent.extension_name, OAuth/pending-flow stores, TUI events, and the remaining extension_name: String fields. Wire format is unchanged — both newtypes use #[serde(transparent)] so on- wire and on-disk representations stay plain strings and legacy persisted rows keep deserializing. Validation runs at explicit construction (::new / ::try_from / ::from_str), not at deserialize time. Also adds .claude/rules/types.md codifying the "no stringly-typed internals" rule. Regression coverage: 17 new unit tests in identity.rs; existing auth_manager, router, and gate tests (130+ cases) all pass unchanged. * fix(common): address PR #2611 review feedback Four fixes from Copilot, Gemini, and Claude reviews: - **identity.rs docs**: drop reference to a non-existent `validate()` re-validation API. Document that instances represent "passed validation at some point in history" rather than "guaranteed valid right now" — by design. - **effect_adapter.rs**: the `awaiting_authorization` / `awaiting_token` gate path was using `CredentialName::from_trusted` to wrap a value read straight out of a tool's JSON output. Tool output is external/untrusted; use `CredentialName::new` (validating) with a cascade: external → tool name → `from_trusted(tool_name)` as final fallback. Closes a credential-name shape-injection vector. - **canonicalize()**: reorder checks cheapest-first against the trimmed slice so invalid inputs reject without allocating a canonicalized `String`. `replace('-', "_")` is deferred until after the structural checks pass; since `-`/`_` are both one byte, the earlier length check stays valid. - **Remove `Deref<Target = str>`** from identity newtypes, keep `AsRef<str>`. Auto-deref let `&cred_name` silently coerce to `&str`, which is exactly the implicit-conversion pattern these newtypes exist to prevent. Callers that had a `&CredentialName` where `&str` was expected now write `.as_str()` explicitly. Added a regression test for the accessor contract and updated the rule template in `.claude/rules/types.md` to document the decision. Declined one review item (Claude): the remaining `to_string()` calls inside `IdentityError` variants are on the exception path; the common invalid-input case no longer allocates twice after the canonicalize reorder, and errors must carry owned strings so they can escape the function. Regression coverage: 5035 lib tests + 18 identity tests (one new — `explicit_accessors_work`) pass. Zero clippy warnings. * feat(common): apply ExtensionName newtype to fan-out sites (PR 2/2) Follow-up to #2611. Migrates the remaining stringly-typed extension_name and credential_name fields to use the ExtensionName and CredentialName newtypes introduced in ironclaw_common::identity. Fields now typed: - AppEvent::{OnboardingState, GateRequired, ExtensionStatus}.extension_name (serde transparent — wire format unchanged) - StatusUpdate::{AuthRequired, AuthCompleted}.extension_name - TuiEvent::{AuthRequired, AuthCompleted}.extension_name (adds ironclaw_common dep to ironclaw_tui) - PendingOAuthLaunchParams.extension_name - PendingOAuthFlow.extension_name - PendingAuth.extension_name, PendingAuthPrompt.extension_name - ParsedAuthData.extension_name, selected_auth_prompt tuple - emit_auth_required_status() and Session::enter_auth_mode() parameters - event_from_configure_result() parameter - resolve_extension_for_action() and resolve_auth_gate_display_name() return types - normalize_extension_name() return type PendingAuthPrompt::new is now infallible (accepts ExtensionName directly) since the identity validator carries the non-empty invariant the constructor used to re-check. The "blank extension name" rejection test moved out — that logic lives in ironclaw_common::identity tests. Test updates use `ExtensionName::new("...").unwrap()` at construction sites and `from_trusted(...)` where a trusted upstream string is being adapted. Every site is a compile-time audit of where the type was crossing a boundary untyped. Regression coverage: existing 5034 lib tests + 26 engine_v2_gate integration tests + 40 ironclaw_common tests all pass. Zero clippy warnings across all features. * fix(web): return ExtensionName from pending_gate_extension_name Addresses Claude's review comment on #2611: the function was doing `Some(credential_name.as_str().to_string())` in the fallback branch, defeating the newtype's purpose by re-stringifying the identity. Return `Option<ExtensionName>` instead. Plumbs through `PendingGateInfo. extension_name` (wire format unchanged — `#[serde(transparent)]`). The fallback path's cross-identity conversion (credential name → extension name) is now an explicit `ExtensionName::from_trusted` call, making the boundary crossing visible at the call site. Also fixes the `Deref<Target = str>` removal fallout that followed the rebase onto the updated PR 1: call sites that relied on auto-deref (`ext.contains(...)`, `auth_manager.submit_auth_token(&cred_name, ...)`) now explicitly call `.as_str()`. * fix(router,web): address PR #2617 review feedback Four Gemini review comments, all on the boundary between credential/ extension identifiers and user input. 1. [HIGH, security] extensions_setup_submit_handler was wrapping the URL path segment in ExtensionName::from_trusted, which skips the newtype's path-traversal / invalid-character validation. That path is user-controlled (`/api/extensions/{name}/setup`). Validate with ExtensionName::new at the handler entry and return 400 on failure; downstream uses switch to .as_str() or .clone() of the validated value, and the three in-handler from_trusted sites disappear. 2. Rename resolve_auth_gate_display_name -> resolve_auth_gate_extension_name. The function returns an identifier/slug, not a human-readable display name — the old name was a leftover from when the value was a String. 3. Return Option<ExtensionName> from the renamed function. Previously the non-Authentication gate branch fabricated an ExtensionName::from_trusted(pending.action_name), which was semantically wrong (an action name is not an extension identifier) and silently defeated the type's invariants. Now it returns None for Approval/External gates, and callers thread an Option through. send_pending_gate_status accepts Option<&ExtensionName> and only uses it on the Authentication arm, with a warn! log if upstream plumbing ever reaches the arm with None. The GateRequired SSE event's extension_name is now a clean .clone() of the Option. 4. Rename auth_display_name -> extension_name on send_pending_gate_status so the parameter name matches both its type and the StatusUpdate::AuthRequired.extension_name field it feeds. Regression: new test_extensions_setup_submit_rejects_path_traversal_name at the handler tier (per .claude/rules/testing.md "Test Through the Caller, Not Just the Helper") drives the handler with malformed path segments and asserts 400 before the value reaches extension lookup or any from_trusted wrap. 5035 lib tests pass, zero clippy warnings. * docs(identity): codify web-boundary rules + add static check Three rule additions + one enforcement hook covering the identity boundary that PR #2617 review uncovered: - src/channels/web/CLAUDE.md — extend "Unified Extension Onboarding" with explicit rules: * Setup/configure/activate routes MUST validate `{name}` via `ExtensionName::new` at handler entry (return 400 on failure). * Web DTOs and handlers MUST NOT reference `CredentialName` — credential identity is backend-only; the dispatcher/auth_manager resolves it from the ExtensionName server-side. * Auth-flow extension resolution happens in *one* place (`AuthManager::resolve_extension_name_for_auth_flow`). Wrappers are thin and delegate; they must not duplicate the precedence logic or re-derive from credential prefixes. The four recent identity bugs (#2561, #2473, #2512, #2574) were duplicate- resolution drift. - src/bridge/CLAUDE.md — new module spec documenting auth_manager.rs as the single authority for auth-flow extension resolution, with the resolver's four-step precedence order and the approved wrapper call sites. - scripts/pre-commit-safety.sh — new check #8 (CREDNAME): flags `CredentialName` references in newly-added production lines under `src/channels/web/**`. Test-mod code is excluded via the existing `strip_test_mod_lines` filter. Suppression via `// web-identity-exempt: <reason>` for the rare legitimate case of reading an already-typed value off a backend struct. Smoke-tested: * baseline (current branch) — no warnings * injected violation — fires with CREDNAME warning * injected violation + `// web-identity-exempt:` — suppressed The rules and the check live at the same level — humans read the rule, CI enforces it. * fix(auth): validate user-influenced names at the resolver boundary Addresses four Copilot review comments on PR #2617 that all pointed at the same seam: the canonical `AuthManager::resolve_extension_name_for_auth_flow` returned a raw `String` whose first branch (the LLM-supplied `name` parameter on `tool_install` / `tool_activate` / `tool_auth` actions) passed through without `ExtensionName` validation. Both call sites then wrapped the result in `ExtensionName::from_trusted`, promoting an unvalidated user-influenced value to a typed identity. - **Resolver now returns `ExtensionName`.** Branch 1 validates the user-controlled name via `ExtensionName::new` and falls through on failure; branches 2–4 use `from_trusted` because their sources (tool registry hint, canonicalizer, typed credential fallback) are already trusted upstream. This consolidates validation in the single "resolve once" site documented in `src/bridge/CLAUDE.md`. - **router.rs and server.rs drop their wraps.** `resolve_extension_for_action` (router) and `pending_gate_extension_name` (server) return the resolver's typed output directly. The tool-registry fallback in router.rs (no-auth-manager path) keeps its `from_trusted` wrap since it operates on the same trusted sources as branch 2. - **`restore_selected_auth_prompt` re-validates rehydrated prompts.** `PendingAuthPrompt` is `#[serde(transparent)]`, so deserialize does not re-check the inner `ExtensionName` string. A legacy-persisted invalid name would previously have been dropped by the old `PendingAuthPrompt::new(String, ...)` empty-string rejection; now `restore_selected_auth_prompt` re-runs `ExtensionName::new` and drops + warns on failure, upgrading the old non-empty-only check to the full identity invariant. New test `test_restore_selected_auth_prompt_rejects_invalid_legacy_row` forges three invalid rows (empty / uppercase / path-traversal) straight through serde and asserts each is dropped. - **Docstring on `PendingAuthPrompt` refreshed.** The old comment claimed `::new` "trims and validates extension_name is non-empty", which is no longer true — `::new` is infallible and the invariant lives in `ExtensionName` itself. The new comment documents the split: validation runs at `ExtensionName::new` construction and at restore-from-persistence, not inside `PendingAuthPrompt`. Regression: 5063 lib tests pass (+1 new). Clippy zero warnings. * fix(ci): adapt post-merge-from-staging sites to ExtensionName Staging shipped #2640 (repl unlock) and gateway refactor commits after my last merge. The CI build picked them up via auto-merge and hit three type mismatches my branch hadn't seen: - src/channels/repl.rs:908 — new test constructs `StatusUpdate::AuthRequired { extension_name: "google_oauth_token" .to_string(), ... }`. Typed field; now `ExtensionName::new(...).unwrap()`. - src/channels/web/server.rs:1405-1424 — staging added a no-auth-manager fallback chain to `pending_gate_extension_name` that returned raw `Some(String)` on three branches. Aligned with `AuthManager::resolve_extension_name_for_auth_flow`: branch 1 (user-influenced `tool_install`/`tool_activate`/`tool_auth` `name` param) validates via `ExtensionName::new` and falls through on failure; branches 2-3 (provider-extension hint, credential-name fallback) use `from_trusted` because they're sourced from typed upstream state. Mirrors the fix applied to the canonical resolver in |
||
|
Illia Polosukhin
|
7fb41555a9 |
ci(gateway): enforce platform/feature boundaries — ironclaw#2599 stage 5 (#2647)
* refactor(gateway): relocate auth / sse / ws into platform/ — ironclaw#2599 stage 3 Third increment of the ironclaw#2599 platform/feature split (follow-up to #2628 and #2643). Moves the three transport / framing modules into the platform/ subtree so the platform layer now contains the full set of cross-cutting infrastructure (state, router, static_files, auth, sse, ws). Changes: - src/channels/web/auth.rs -> src/channels/web/platform/auth.rs - src/channels/web/sse.rs -> src/channels/web/platform/sse.rs - src/channels/web/ws.rs -> src/channels/web/platform/ws.rs - platform/mod.rs declares the three new submodules. - channels/web/mod.rs adds backward-compat re-exports (`pub use platform::{auth, sse, ws};`) so every existing `crate::channels::web::{auth,sse,ws}::...` call site - roughly 40 files across handlers, tests, integration tests, and sibling modules - continues to resolve without edits. Follow-up PRs will migrate call sites to the canonical `platform::` path incrementally. - platform/mod.rs doc comment now describes the platform layer as having auth / SSE / WS (no longer "in later stages of #2599"). - CLAUDE.md file map points at the new paths and notes the re-exports. Pure move + re-export. No behavior change. Module contents are byte-identical to pre-move. Verified: cargo fmt --all; cargo clippy --all --benches --tests --examples --all-features clean; python3 scripts/check_no_panics.py clean; cargo check --all-features --all-targets clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(gateway): extract OAuth / relay callbacks into features/oauth/ — ironclaw#2599 stage 4a Fourth increment of the ironclaw#2599 platform/feature split. Opens the `features/` subtree with the OAuth feature slice — the first vertical slice to move out of server.rs into its own module under the ironclaw#2599 target layout. Slice contents: - `features/oauth/mod.rs` owns the three public gateway routes that receive OAuth-style callbacks: * `oauth_callback_handler` — generic OAuth callback for installable extensions (CSRF lookup, token exchange, storage, optional auto-activation). * `relay_events_handler` — HMAC-signed webhook from channel-relay. * `slack_relay_oauth_callback_handler` — Slack-specific relay completion flow. - Slice-private helpers `oauth_error_page` and `redact_oauth_state_for_logs` move with the slice (they have no other callers). Wiring: - `platform/router.rs` imports the three handlers from `features::oauth` instead of `server`; no route-table change. - `channels/web/mod.rs` registers `pub(crate) mod features;`. - `server.rs` loses the three handlers and their helpers, plus the imports they owned (`Sha256`, `Digest`, `HeaderMap`, `DEFAULT_RELAY_NAME`, `extension_name_candidates`, `SecretConsumeResult`). The test module re-imports the ones it still uses for the integration-level OAuth callback tests. Pure move. No behavior change. Each handler body is byte-identical to its pre-move counterpart. Every test in `server.rs` that exercises the OAuth callbacks (`test_oauth_callback_missing_params`, etc.) continues to pass against the re-imported handlers. Stats: server.rs 6973 → 6248 lines (−725); new `features/oauth/mod.rs` is 775 lines; new `features/mod.rs` 14 lines. The +30 delta is comment headers documenting the slice boundary. Verified: `cargo fmt --all`; `cargo clippy --all --benches --tests --examples --all-features` clean; `python3 scripts/check_no_panics.py` clean; `cargo test --lib` 5069 passed (one more than stage 3 — the new `css_handler_returns_base_in_multi_tenant_mode` test from staging lands green), same 2 pre-existing failures carried over (fixture and test-infra issues unrelated to gateway layout). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci(gateway): enforce platform/feature boundaries — ironclaw#2599 stage 5 Adds `scripts/check_gateway_boundaries.py` and wires it into the `code_style` CI workflow as a required check. The script enforces the ironclaw#2599 layering rule: every file under `src/channels/web/platform/` except `router.rs` must not import from `handlers/` or `features/`. How it works: - Walks `src/channels/web/platform/*.rs`, skipping `router.rs` (the intentional composition point) and test modules. - Strips line comments, block comments, and string / raw-string / char literals so references inside docstrings and explanatory text don't trigger false positives. - Matches six forbidden import shapes: `crate::channels::web::{handlers,features}::`, `super::{handlers,features}::`, `super::super::{handlers,features}::`. - Prints diagnostics with file:line and the matched pattern for every violation; exits non-zero on any. - Carries unit tests behind a `test` subcommand (`python3 scripts/check_gateway_boundaries.py test`) that the CI job runs alongside the check itself. Simultaneous fix: one pre-existing back-edge that the check surfaced was the OIDC `check_email_domain()` helper living in `handlers/auth.rs` but called from `platform/auth.rs`. The helper is platform-level (it gates JWT validation before any handler runs), so it moves into `platform::auth` along with its five unit tests; the handler call site in `handlers::auth::handle_callback` now imports from the new home. No behavior change. The second pre-existing back-edge is the frontend bundle assembly path: `platform/static_files::build_frontend_html` calls `read_layout_config` and `load_resolved_widgets`, both still in `handlers/frontend.rs`. Migrating them requires also moving `read_widget_manifest` and the widget-size constants, which touches `load_widget_manifests` (used by `/api/frontend/widgets` and the engine-v2 widget endpoint). That's a separate focused PR — tracked via a narrow allowlist entry in the script with a follow-up comment. The allowlist is explicitly documented as "must not grow without reviewer sign-off". CLAUDE.md's "Platform vs. feature layering" section now names the script as the enforcement point. Verified: `python3 scripts/check_gateway_boundaries.py test` — 9 tests pass; `python3 scripts/check_gateway_boundaries.py` — clean; `cargo fmt --all`; `cargo clippy --all --benches --tests --examples --all-features` clean; `python3 scripts/check_no_panics.py` clean; `cargo test --lib channels::web` — 425 passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci(gateway): close boundary-checker bypasses — PR #2647 review Four issues raised on PR #2647's review are addressed: - Grouped `use crate::channels::web::{ handlers::... }` imports escape the per-line scan because the forbidden segment lands on a continuation line. Adds a multiline GROUPED_FORBIDDEN_PATTERN that matches across newlines and reports the line where `handlers::`, `features::`, or `server::` actually appears. - `use crate::channels::web::server::...` routes through the `server.rs` compatibility shim and still creates a platform → feature back-edge. Adds `server::` (and its `super::` variants) to FORBIDDEN_PATTERNS. Existing pre-existing shim usage in `platform/ws.rs` is captured as a tracked allowlist entry — the allowlist shrinks as individual types migrate out of `server.rs`. - `#[cfg(test)] mod ...` and `mod tests { ... }` bodies are now actually blanked before pattern matching, matching the docstring's stated exemption. Caller-level regression tests in platform files can import handler/feature modules without tripping the check. - `gateway-boundaries` is no longer gated solely on `has_code`. A new `has_boundary_check` output on the `changes` job fires when the checker script or this workflow itself changes, so PRs that only edit `scripts/check_gateway_boundaries.py` or `.github/workflows/code_style.yml` still run the guardrail. Also picks up a small perf nit: `text.splitlines()` is now computed once outside the loop instead of per-violation. Regression tests cover each case (grouped crate-web import, grouped super import, server-shim back-edge, cfg(test)/mod tests skip, and a sanity check that the test-module skip doesn't blanket-ignore the rest of the file). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci(gateway): brace-aware grouped scan + narrower ws.rs allowlist — PR #2647 Copilot review Two issues raised by Copilot on the round-1 fixes: - `GROUPED_FORBIDDEN_PATTERN` used `[^{}]*?` and so could not match grouped imports that contain *nested* braces — e.g. `use crate::channels::web::{ platform::{state::GatewayState}, handlers::auth::login_handler };` produced zero violations even though the forbidden segment is plainly inside the web::{...} group. Replaced the regex with a depth-tracking walk: find each `crate::channels::web::{` / `super::{` / `super::super::{` header, find the matching `}` by counting braces (`{` / `}` only; string and comment contents are already blanked), then scan the body for `(handlers|features|server)::`. Report line numbers off absolute offsets so the reported line is where the forbidden segment lives, not where the header's `{` is. - `ws.rs`'s allowlist entry whitelisted the whole `crate::channels::web::server::` prefix, which would let any *new* accidental server-shim import in ws.rs silently pass. Narrowed to seven per-symbol entries covering the current pre-existing uses (GatewayState, PerUserRateLimiter, RateLimiter, ActiveConfigSnapshot, images_to_attachments, and the two handle_legacy_auth_* helpers). Future accidental shim imports fail the check and require explicit reviewer sign-off to add. Added `test_detects_nested_brace_grouped_import` as the regression test for the brace-aware scanner. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
ff119531d4 |
test(replay): promote engine traces to insta-backed snapshot gate (#2621)
* test(replay): promote engine replay traces to insta-backed snapshot gate Adds a ReplayOutcome snapshot type, a replay-gate CI workflow, and a developer script wrapper for cargo-insta. Replaces unreviewable 3,000-line JSON diffs on engine changes with a YAML snapshot of the observable run shape (tool sequence, final state, retrospective analyzer issues). Why: engine v2 live-fixture traces had grown past reviewability. A single prompt-wording change could move the whole fixture, and reviewers had no way to see which behaviour actually changed. Splitting the fixture into a "replay driver" (JSON stays in tests/fixtures/) and a "regression snapshot" (YAML in tests/snapshots/) gives reviewers a narrow, stable diff to approve, while keeping the full recorded context for deterministic replay. Changes: - `tests/support/replay_outcome.rs` — ReplayOutcome + assert_replay_snapshot! macro; snapshots include retrospective analyzer output (TraceIssue severity/category) via a new `ironclaw::bridge::engine_retrospectives_for_test()` helper that runs `build_trace()` over engine threads - `tests/e2e_engine_v2.rs` — three POC snapshot tests (single_tool_echo, tool_error_recovery, zizmor_scan_v2) - `tests/e2e_bug_bash_snapshots.rs` + `tests/fixtures/llm_traces/bug_bash/` — bug-regression fixture template, mapped to open issues in the README - `.github/workflows/replay-gate.yml` — cargo insta test --check on engine/agent/LLM/tools/bridge path changes; rejects committed .snap.new - `scripts/replay-snap.sh` — review/accept/test/record wrappers around cargo-insta and IRONCLAW_RECORD_TRACE - `scripts/trace-coverage.sh` — reports EventKind variants with snapshot coverage; `--strict` mode for future CI promotion - `tests/e2e_live.rs` — `#[ignore]` swapped for `cfg_attr(not(feature="replay"), ignore)` so the replay CI job can run the scenarios without `-- --ignored` - `Cargo.toml` — new `replay = ["libsql"]` feature; insta gains the `yaml` feature - `tests/fixtures/llm_traces/README.md` — documents the two-role driver/snapshot split Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(replay): address PR #2621 review + swap cargo-insta installer Review fixes: - Replay gate was missing the bug-bash snapshot suite. Adds `tests/e2e_bug_bash_snapshots.rs` to the workflow paths trigger and the `cargo insta test --check` invocation so bug-regression snapshots are actually gated. (copilot-pull-request-reviewer) - `cargo install cargo-insta --locked` added ~40s of cold-cache compile to the gate. Swapped for `taiki-e/install-action@v2`, which downloads a precompiled binary in a few seconds. Also updated `scripts/replay-snap.sh` to *fail closed* when cargo-insta is missing instead of silently auto-installing it. (gemini-code-assist) - `engine_retrospectives_for_test` was `pub` and re-exported under the default-enabled `libsql` feature, contradicting its "not part of any public API" doc. Split the re-export, kept `reset_engine_state` as a plain `pub use`, and hid `engine_retrospectives_for_test` behind `#[doc(hidden)]` — it still needs to cross the crate boundary for integration tests (which live in a separate crate, so `#[cfg(test)]` doesn't reach them), but no longer appears in published docs. (copilot-pull-request-reviewer) - Added an explicit "caller must serialize" note on `engine_retrospectives_for_test` explaining the `ENGINE_STATE` singleton and pointing new callers at `engine_v2_test_lock()` / `reset_engine_state()`. Matches what the existing snapshot tests already do. (gemini-code-assist) Doc corrections: - `snapshot_zizmor_scan_v2` doc claimed the snapshot pinned `ApprovalNeeded` events and response wording — it doesn't. Rewrote to describe what the snapshot actually asserts (tool order, step count, retrospective issues, final state). (copilot-pull-request-reviewer) - `llm_call_count` was documented as "bucketed" but passed through verbatim. Updated the field doc to reflect the raw value. Bucketing wasn't needed because fixtures are deterministic. (copilot-pull-request-reviewer) - `src/bridge/router.rs` doc referenced a non-existent `ReplayOutcome.trace_issues` field — the struct uses `engine_threads`. Fixed the reference. (copilot-pull-request-reviewer) - `scripts/trace-coverage.sh` header claimed CI runs it with `--strict`; the workflow runs it in advisory mode. Rewrote the header to match, with a pointer for when to promote to strict. (copilot-pull-request-reviewer) No-change replies (rationale commented in the code): - `event_kind_name` uses an exhaustive `match` on `EventKind` rather than `Debug` or a `strum` derive. The compile-time exhaustiveness check is the point — adding a new engine event should force a conscious decision about how the snapshot represents it, not a silent fallthrough. Added a comment making that intent explicit. - `trace-coverage.sh` awk parser of `event.rs` is fragile — agreed, but the script is advisory and its failure mode is false negatives (uncovered variants simply aren't gated). Documented the tradeoff and the rewrite-in-Rust escape hatch in the script header. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci(replay-gate): prime cache on staging, restrict PR runs to read-only The second run on PR #2621 missed the cache ("No cache found" in the rust-cache restore step) even though the workflow is wired correctly. Root cause: the repo sits close to GitHub's 10 GB per-repo cache quota (~59 entries, many >500 MB), and the LRU policy evicts PR-scoped caches before they get reused. Fix: - Add `push: [staging, main]` so the gate runs (and saves a ~1.2 GB cache under the `replay-gate` key) on every merge to the branches PRs actually target. Subsequent PRs restore from that base-branch cache — GitHub Actions permits cross-ref restore when the restoring ref's base matches the saved ref. - Set `save-if: ${{ github.event_name == 'push' }}` so PR runs only *read* the cache. Without this gate, each PR push would save its own copy and crowd out the primed base-branch cache, putting us right back in the eviction loop. Expected effect: cold-cache 9m → warm ~2-3m once staging has a run with the new workflow. Base-branch prime run still pays 9m (no regression). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(replay): drop bug-bash fixture scaffolding Replay fixtures can't reproduce the Phase 3 target bugs because the fixture *is* the LLM's output — handwriting a trace where the LLM emits a tool call doesn't test whether the real LLM would have emitted that call, only that the harness dispatches a scripted one. What `summarization_uses_tools.json` actually pinned was the happy path, not the #2541 bug. Of the 7 open bug-bash issues, only #2544 ("plans and delegates but never executes") is catchable by replay, and only via a live-recorded fixture. The other six are LLM-behavior or infra-timing bugs outside replay's reach. Rather than ship regression theater, tear out the scaffolding. Removed: - tests/e2e_bug_bash_snapshots.rs - tests/fixtures/llm_traces/bug_bash/ - tests/snapshots/replay__bug_bash_summarization_uses_tools.snap Unwired: - Replay-gate workflow paths + test list no longer mention bug_bash - scripts/replay-snap.sh test command drops the extra --test flag Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: switch to cargo-nextest with per-test timeouts Nextest runs each integration test in its own process and runs test binaries in parallel, which is a big unlock for this repo: - Engine v2 tests share a process-global `ENGINE_STATE` singleton (OnceLock), which the current test lock serialises inside a single test binary. Nextest's process-per-test model gives each test a clean state automatically, so the 16 engine_v2 tests stop running one-by-one. - Cross-binary parallelism: `cargo test --test A --test B` runs binaries in sequence; nextest runs them concurrently. Measured locally: the replay-gate test set (3 binaries, 21 tests) went from ~30s sequential to **2.7s parallel**. Adds `.config/nextest.toml` with: - `slow-timeout = 60s / terminate-after 3` in the default profile so a hung test fails fast instead of blocking the workflow-level 25- minute cap. - A `ci` profile with `fail-fast = false` (one flake shouldn't mask other failures), `failure-output = immediate-final`, `success-output = never` for readable Actions logs. - Per-test 300s override for the handful of genuinely slow scenarios (zizmor scan, e2e_thread_scheduling). Workflows updated: - `replay-gate.yml`: installs cargo-nextest via taiki-e/install-action alongside cargo-insta (one step), runs `cargo insta test --test-runner nextest` with `NEXTEST_PROFILE=ci`. - `test.yml`: all five `cargo test` invocations swapped for `cargo nextest run --profile ci`. Nextest doesn't execute doctests, so every nextest step is paired with a `cargo test --doc` follow-up to preserve coverage. Local dev is unchanged — `cargo test` still works; nextest is only required in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: re-trigger replay-gate workflow after nextest migration Previous push only modified workflow files and `.config/nextest.toml`; GitHub skipped the `pull_request` workflow events for that sync, so the nextest migration didn't actually get exercised in CI. Empty commit forces re-evaluation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(replay): note nextest wiring in the fixtures README Also forces a CI re-run: the previous empty commit had no matching paths, so the `pull_request.paths` filters skipped every workflow including replay-gate. Touching a file under `tests/fixtures/llm_traces/**` re-matches the filter and runs the nextest-based gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci(test): defer test.yml nextest migration Staging restructured test.yml significantly while this PR was open (matrix-config dynamic matrix, `changes` code-detection job, composite install-cargo-component action, save-if restricted to base-branch pushes). The merge into staging had heavy conflicts for every nextest-swap hunk. Rather than force a re-layering of the new staging structure on top of the nextest migration in this PR, revert test.yml to staging's current version. This PR now scopes the nextest change to just the replay-gate workflow (where it cleanly demonstrates the value) plus the shared `.config/nextest.toml` profile. Migrating the rest of test.yml to nextest is a follow-up that can rebase on the new structure without the heavy conflict surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Henry Park <henrypark133@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
7f5b02d7f0 |
feat(docs): animated architecture overview video for contributors (#2365)
* feat(docs): animated architecture overview video for contributors Adds a Remotion-based animated video (82s, 12 scenes at 30fps) that visualizes the IronClaw architecture for new contributors. Covers engine v2 primitives, CodeAct execution, thread state machine, skills pipeline, tool dispatch, channel routing, trait implementations, and LLM decorator chain. - docs/architecture-video/ — Remotion project with 12 animated scenes - scripts/render-architecture-video.sh — render script - .claude/skills/architecture-video/ — Claude Code skill to update the video when architecture changes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(docs): address PR review feedback and fix cargo-deny CI - Resolve relative output paths in render script before cd - Use npm ci when package-lock.json exists for reproducible builds - Fix file paths in TraitsScene, ChannelImplsScene, CodeActScene - Label Channel trait code as simplified in ChannelsRoutingScene - Fix TypeScript version (5.9.3 → 5.7.3) and update lockfile - Add dom/esnext to tsconfig lib for React 19 compatibility - Fix license to MIT OR Apache-2.0 to match repo - Fix TOTAL_DURATION to count only scenes with transitions - Fix cargo-deny: add publish = false, ignore RUSTSEC-2026-0097 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(docs): address remaining PR review feedback - Remove `publish = false` from Cargo.toml (unrelated build policy change, should be a separate PR if desired) - Add `--` before output path in render script to prevent argument injection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(docs): address second round of PR review feedback - Add npm command check to render script (was only checking node/npx) - Memoize highlight() tokenization in CodeBlock — Remotion re-renders every frame and code is static per instance, so useMemo avoids repeated work - Rewrite README to describe the IronClaw architecture video project instead of the default Remotion template Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
5fa60f66b1 |
feat: discover tool source in working directory during install (#2396)
* feat: discover tool source code in working directory during install When `tool_install` can't find a tool in the registry, it now searches common directories relative to the current working directory before returning "not found": - tools-src/<name>/ - tool-src/<name>/ - <name>/ (direct subdirectory) Matches both hyphenated and underscored name variants, and strips/adds `_tool`/`-tool` suffixes. A directory is only considered a match if it contains a Cargo.toml. This lets users say "install portfolio tool" when tool source is at tool-src/portfolio/ without needing the explicit path. * style: apply cargo fmt formatting * fix(extensions): address PR #2396 review feedback - Restrict local tool source discovery to WASM kinds only; skip non-WASM kind hints (McpServer, ChannelRelay, AcpAgent) that can't be built from a local Cargo source. - Refactor candidate name generation to use HashSet, avoiding weird combos like `my_portfolio-tool` and `*_tool-tool` from the old suffix logic. - Update NotFound error message to mention all 3 search patterns (tools-src/, tool-src/, direct subdir). - Include source path in InstallResult.message so the user/LLM can verify provenance when a tool is installed from a local directory instead of the verified registry (confused-deputy mitigation). - Change local-discovery log from info! to debug! per CLAUDE.md REPL/TUI logging rule. - Extract install_from_local_source() helper and add caller-level tests per testing.md ("Test Through the Caller, Not Just the Helper") to cover kind defaulting, target_dir routing, and message annotation. * fix(extensions): resolve wasm artifact via Cargo.toml crate name Address follow-up review feedback on PR #2396: 1. Suffix-stripping name mismatch (HIGH): when `find_local_tool_source` matched a directory via suffix add/strip (e.g. input `portfolio_tool` -> dir `portfolio/`), `install_from_local_source` passed `None` for `crate_name`, so artifact lookup searched for `<name>.wasm` instead of the real `<crate>.wasm` and every suffix-matched install failed. Parse `Cargo.toml` from the discovered source and pass `[package].name` as `crate_name`. 2. Non-deterministic candidate ordering (MEDIUM): the `HashSet` of name variants gave non-deterministic iteration, so directory matches within one search dir could vary across runs. Replace with a priority-ordered `Vec` + `retain` dedup: canonical underscore form first, hyphen next, suffix-adjusted variants last. Adds a caller-level regression test for the name-mismatch bug and a determinism test covering the underscore-vs-hyphen ordering. * style: apply cargo fmt * fix(extensions): tighten local tool source discovery (PR #2396 review) - find_local_tool_source_in: require Cargo.toml to be a regular file (is_file) rather than merely existing, so a directory named Cargo.toml cannot falsely qualify a candidate source directory. - install_from_local_source: reject non-UTF-8 source paths with a clear InstallFailed error instead of silently lossy-converting them into a build-dir path that will not resolve. Addresses Copilot review comments on nearai/ironclaw#2396. * fix(extensions): drop dead -tool strip branch in local source discovery `underscore_name` is built via `name.replace('-', "_")`, so the `underscore_name.strip_suffix("-tool")` fallback can never match — it was unreachable code. The single `_tool` strip already covers both `name_tool` and `name-tool` inputs because hyphens are normalized first. Added `find_local_tool_source_strips_hyphen_tool_suffix` to lock in that the hyphenated suffix input still resolves to the unsuffixed dir. Addresses Copilot review comment on nearai/ironclaw#2396. |
||
|
Henry Park
|
1c7a991060 |
fix(gateway): restore web login bootstrap (#2592)
* fix(gateway): restore web login bootstrap * fix(ci): address gateway syntax review feedback |
||
|
firat.sertgoz
|
532fc61d25 |
feat: admin management panel — web UI for users and usage monitoring (#1963)
* feat(web): add admin management panel * fix(web): address admin panel review findings * fix(web): address remaining admin review feedback * refactor(web): type admin api responses * fix(db): aggregate admin usage summary in sql * Add audit logging for admin privileged state-changes Add structured tracing (warn-level) to suspend, activate, delete, and update handlers so that privileged admin actions are recorded with the acting admin's user_id, the action performed, and the target user. Addresses security assessment item #1 from PR review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix PairingStore::new() call in test after staging merge Use PairingStore::new_noop() since the test doesn't need a real DB. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(admin): address PR #1963 review feedback - Fix total_jobs semantics: query agent_jobs directly instead of counting via LEFT JOIN on llm_calls (which missed jobs without LLM calls). Fixed in both libSQL and PostgreSQL backends. - Fix showConfirmModal XSS: escape message parameter internally instead of relying on callers to sanitize. - Add explicit ::numeric cast to PG COALESCE(SUM(cost), 0) to prevent integer type inference. - Use info! instead of warn! for successful admin audit events (update, suspend, activate, delete) — warn implies anomaly. - Add missing index on llm_calls.created_at for both PG (V21 migration) and libSQL (incremental migration 21). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address remaining admin panel review follow-ups * fix: address review comments — query consolidation, CSP, docs, security notes - Collapse 4 redundant llm_calls subqueries into single subquery (libsql + pg) - Add WARNING to V21 migration about table lock risk with CONCURRENTLY note - Add performance doc comments on admin_usage_summary full-table scan - Add CSP and noindex meta tags to admin.html - Add JSDoc for showConfirmModal documenting auto-escaping - Add sessionStorage threat model security comment - Add serde(flatten) collision risk doc on AdminUserDetailResponse - Add TODO(#1968) for inline styles migration to CSS custom properties - Add PG parity test stub for admin_usage_summary Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: renumber migration from V21/23 to V24 to avoid conflicts with staging Staging added V21 (backfill_conversation_source_channel), V22 (sandbox_restart_params), and V23 (list_workspace_files_escape_like). Renumber our llm_calls_created_at_index migration to V24 in both PG and libSQL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review feedback — CSP, logging, validation, dispatch-exempt, tests - Remove 'unsafe-inline' from script-src CSP; move CSP to HTTP response header - Change audit tracing::info! to tracing::debug! (TUI corruption) - Add dispatch-exempt annotation on usage_summary_handler - Add server-side input validation on users_create_handler (name length, email, role) - Rename detailRowHtml to detailRowRawHtml with XSS safety comment - Add real PG integration test for admin_usage_summary with non-zero data Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(ci): skip test-only directories in no-panics check Files under `src/**/tests/*.rs` are Rust test sub-modules included behind `#[cfg(test)]` — they are never compiled in production builds. The no-panics checker was flagging `.unwrap()` and `assert!()` in helper functions at module level in these files as production code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(admin): scope cost aggregates to 30d, drop external fonts, flatten detail response Addresses review feedback on #1963: - Scope all llm_calls aggregates to the 30d `since` window so the admin dashboard query is served by `idx_llm_calls_created_at` rather than a full table scan. Drops the unused all-time `total_cost` subquery from both libsql and postgres backends. - Self-contain the admin SPA — remove `fonts.googleapis.com` / `fonts.gstatic.com` link tags from admin.html and tighten the admin CSP to fully same-origin. Typography degrades to the system-font fallback already listed in `font-family`. - Fold `metadata` into `AdminUserInfo` (optional, skip-if-none) and remove the `#[serde(flatten)]` wrapper, eliminating the documented collision risk. - Add regression test asserting `since` actually bounds the LLM aggregates (future `since` should yield zero LLM counts without affecting non-windowed counts). --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: ilblackdragon@gmail.com <ilblackdragon@gmail.com> |
||
|
Henry Park
|
a53eac5c2d |
fix(ci): bump 5 channel versions + fix lifetime desync in panics check (#2300)
Version bumps for channels with source changes:
- discord 0.2.2 -> 0.2.3 (pairing message UX)
- feishu 0.1.4 -> 0.2.0 (pairing flow refactor + multi-tenancy)
- slack 0.2.2 -> 0.3.0 (broadcast feature implementation)
- telegram 0.2.6 -> 0.2.8 (webhook dedup + configurable polling)
- whatsapp 0.2.0 -> 0.2.2 (pairing message UX)
Fix check_no_panics.py: Rust lifetime annotations ('static, 'a) were
parsed as char literal openings, blanking the rest of the line including
any opening brace. This caused the brace-depth tracker to desync in
large test modules, producing false positives (e.g. server.rs:6378).
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
||
|
Illia Polosukhin
|
2cc5546017 |
feat(tools): production-grade coding tools, file history, and skills (#2025)
* feat(tools): add production-grade coding tools, file history, and coding skills Add dedicated coding tools inspired by Claude Code's architecture to make IronClaw a more effective coding assistant: New tools: - GlobTool: fast file pattern matching via `glob` crate, sorted by mtime, with default exclusions (.git, node_modules, target, etc.) - GrepTool: content search wrapping ripgrep with 3 output modes (content, files_with_matches, count), pagination, and context lines - FileUndoTool: restore files to pre-modification state using in-memory file history snapshots Enhanced tools: - ReadFileTool: 10MB limit, 2000-line default, binary detection, device path blocking (/dev/zero, /proc/*/fd/*) - ApplyPatchTool: uniqueness validation (error on ambiguous matches), workspace path rejection, 10MB size limit, file history integration - WriteFileTool: file history integration for undo support Updated tool descriptions to guide LLM behavior (prefer apply_patch over write_file, always read before editing, use glob/grep instead of shell). New skills: - coding: best practices for code editing, search, and file operations - commit: git commit message generation workflow - review: code review workflow with structured checklist Shared infrastructure: - DEFAULT_EXCLUDED_DIRS constant in path_utils.rs - FileHistory module with SharedFileHistory for cross-tool snapshots 66 new tests covering all tools, edge cases, and regression scenarios. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: apply cargo fmt formatting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(tools): address PR review — security, correctness, and robustness fixes - Move device path blocking after validate_path() to prevent traversal bypass - Add /proc/kcore, /proc/kmem to blocked paths - Reject absolute patterns and '..' in glob tool, add strip_prefix defense - Wrap glob sync I/O in spawn_blocking to avoid blocking tokio executor - Sort files_with_matches globally before pagination in grep tool - Add default exclusions for node_modules/target in grep tool - Inject ctx.extra_env into rg environment matching ShellTool policy - Use per-line strip_prefix for content mode path relativization - Change FileSnapshot.content_before to Vec<u8> for binary file support - Log snapshot errors with tracing::debug instead of silently discarding - Fix skill name mismatch: code-review → review to match directory Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(skills): rename review skill directory to code-review Aligns the directory name with the manifest name (code-review) to prevent incorrect override/dedup behavior in the bundled-skill loader. The name stays "code-review" since other domains may also need review-type skills. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(tools): add file edit guards — staleness detection, fuzzy matching, encoding preservation Add file_edit_guard module with production-grade safeguards for file editing: - ReadFileState tracks file reads with mtime for staleness detection - 4-level fuzzy matching fallback (exact → whitespace-normalized → quote-normalized → both) - UTF-16LE BOM detection and line ending style preservation (LF/CRLF/CR) - Read-before-edit enforcement for ApplyPatch and WriteFile tools - No-op edit rejection (old_string == new_string) - Shared state injection via Arc<RwLock<>> across ReadFile, WriteFile, ApplyPatch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(tools): address all PR review comments — session scoping, parallelism, security - Session-scoped state: ReadFileState and FileHistory now keyed by job_id so concurrent sessions sharing the same registry don't leak state (#2025) - Parallel metadata: grep files_with_matches uses JoinSet (max 64 concurrency) instead of sequential await per file for mtime sorting - Shared env allowlist: grep_tool imports SAFE_ENV_VARS from shell.rs (made pub(crate)) instead of maintaining a divergent copy - Glob traversal: uses Component::ParentDir check instead of substring ".." match, so patterns like "foo..bar" are no longer falsely rejected - UTF-16LE in read_file: binary detection skips null-byte check for files with UTF-16LE BOM; read_file uses encoding-aware read path - Partial flag: default 2000-line truncation now marks read as partial, preventing edits against unseen content - write_file guard softened: staleness check logs warning instead of hard error (full-file replacement has lower risk than apply_patch) - Updated e2e trace to include read_file before apply_patch - Updated expected tool list in schema validation tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(tools): use async metadata instead of blocking path.exists() in write_file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(ci): fix false-positive panic detection for lifetimes in char lexer The check_no_panics.py lexer misinterpreted Rust lifetimes ('static) as char literal starts, causing in_char state to persist across lines and hide all subsequent brace-delimited blocks — including #[cfg(test)] mod tests. Reset in_char at line boundaries since Rust char literals cannot span lines. https://claude.ai/code/session_012bJjER6L5zSAFd9BqYMUmC * test: verify MCP push works * test * chore: remove test file * style: apply cargo fmt to file.rs Collapse multi-line method chain to single line per rustfmt. https://claude.ai/code/session_012bJjER6L5zSAFd9BqYMUmC * style: apply cargo fmt to file.rs Collapse multi-line method chain to single line per rustfmt. https://claude.ai/code/session_012bJjER6L5zSAFd9BqYMUmC * fix(file-tools): harden fuzzy patch matching and undo * fix(ci): formatting + wasmtime 43 cache config compatibility After merging latest staging, cargo fmt had diffs in file tools and the wasmtime cache TOML format changed (v43 dropped the `enabled` field under `[cache]`). Also removes accidental .fmt-test artifact. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(file-tools): simplify strip_trailing_whitespace Remove redundant double-pass through .lines() — the first collect+join was a no-op since .lines() already handles line endings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(tools): address PR review comments — security, correctness, tests - Add is_sensitive_path checks to GlobTool and GrepTool, matching the defense-in-depth posture of ReadFileTool/WriteFileTool/ListDirTool - Fix UTF-8 panicking byte-index slice in apply_patch error preview (old_string[..200] → chars().take(200)) - Add 10MB size guard on file_history snapshots to prevent memory exhaustion from snapshotting large files - Replace dead turn_number field with auto-incrementing sequence_number in FileHistory — callers no longer pass a hardcoded 0 - Fix glob mtime test flakiness by increasing sleep to 1100ms (above 1s filesystem granularity) - Fix emoji test to actually include emoji/non-ASCII content 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> |
||
|
Illia Polosukhin
|
4147c6d587 |
feat(gateway): extract gateway frontend into ironclaw_gateway crate with widget system (#1725)
* feat(frontend): extract frontend into ironclaw_frontend crate with widget extension system
Moves all frontend static assets (app.js, style.css, index.html, i18n/*,
theme-init.js, favicon.ico) from src/channels/web/static/ into a dedicated
ironclaw_frontend crate. The crate also adds:
- Layout configuration types (branding, tab order, chat features, per-widget config)
- Widget manifest types with named slot system (tab, chat_header, sidebar, etc.)
- CSS scoping utility (auto-prefixes selectors with [data-widget="id"])
- Bundle assembly (injects layout config, widgets, and custom CSS into HTML)
- Frontend API endpoints (GET/PUT layout, list widgets, serve widget files)
- Browser-side IronClaw.registerWidget() API with authenticated fetch,
event subscription, theme access, and i18n
Widgets are stored in workspace at frontend/widgets/{id}/ and served via
the API. Layout config is stored at frontend/layout.json. The agent can
create/edit both using existing memory_write/memory_read tools.
Gateway handlers now reference ironclaw_frontend::assets constants instead
of include_str!() with local paths, completing the separation.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: address CI failures — license, rust-version, formatting, manifest warnings
- Add license = "MIT OR Apache-2.0" to ironclaw_frontend Cargo.toml (cargo-deny)
- Fix rust-version to 1.92 to match other crates
- Log warning for invalid widget manifests instead of silent skip
- Run cargo fmt across all files
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(frontend): structured data cards + chat renderer API for rich message rendering
Agent responses containing JSON/structured data (like mission results,
status objects) now render as styled cards with labeled fields, status
badges, and monospaced IDs instead of raw text.
Built-in rendering:
- Detects inline JSON objects (including Python-style single quotes)
- Renders as data cards with key-value rows
- Status/state fields get colored badges (success/error/pending)
- UUIDs rendered in monospace
Extensible via widgets:
- IronClaw.registerChatRenderer({ id, match, render, priority })
- First matching renderer wins (priority ordering)
- Renderer gets the content element to mutate in place
Also adds ChatRenderer variant to WidgetSlot enum.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(frontend): hash-based URL navigation for page refresh persistence
Navigation state is now encoded in window.location.hash so refreshing
the page (or sharing a URL) restores the current view:
#/chat → chat tab, assistant thread
#/chat/{threadId} → specific conversation
#/memory/{path/to/file} → memory browser with file open
#/jobs/{jobId} → job detail view
#/routines/{id} → routine detail view
#/settings/{subtab} → settings sub-tab (extensions, etc.)
#/logs → logs tab
Hooked into all navigation functions: switchTab, switchThread,
switchToAssistant, createNewThread, readMemoryFile, openJobDetail,
closeJobDetail, openRoutineDetail, closeRoutineDetail,
switchSettingsSubtab.
Thread restore is deferred until loadThreads() completes (async),
then the pending thread ID is matched against the loaded thread list.
Browser back/forward buttons work via hashchange listener.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(frontend): auto-open README.md when first visiting Memory tab
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(frontend): preserve URL hash across page refresh
Two bugs caused the hash to reset on Cmd+R:
1. Auth URL cleanup (replaceState) stripped the hash fragment —
now preserves it via cleaned.hash
2. restoreFromHash() called switchTab() which called updateHash()
overwriting the full hash before the detail was restored —
now suppresses hash updates during the entire restore sequence
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(frontend): seed frontend/README.md with customization guide for agent
The agent didn't know it could customize the frontend via workspace writes.
Now seeds frontend/README.md on first boot with a guide covering:
- Layout config (branding, colors, tab order) via frontend/layout.json
- Custom CSS via frontend/custom.css with common variable names
- Widget creation (manifest + index.js + style.css)
- API endpoints
Also seeds frontend/.config with skip_indexing: true so frontend assets
aren't chunked/embedded for search.
When a user says "change the color scheme to red", the agent can now
discover frontend/README.md via memory_tree, read the guide, and write
the appropriate layout.json or custom.css.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(frontend): wire workspace-aware serving for index.html and style.css
The index_handler and css_handler now read from workspace to apply
frontend customizations on page load:
- index_handler: reads frontend/layout.json, discovers widgets in
frontend/widgets/*, reads frontend/custom.css, then calls
assemble_index() to inject branding colors, layout config,
widget scripts, and custom CSS into the base HTML.
Falls back to embedded HTML if no customizations exist.
- css_handler: appends frontend/custom.css from workspace after
the embedded base stylesheet.
This completes the end-to-end flow:
Agent writes frontend/layout.json → user refreshes → sees changes
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(frontend): wire up remaining widget system gaps
Audit-driven fixes for the widget extension system:
1. Widget tab panel ID: panels now get id="tab-{widgetId}" so
switchTab() can find and activate them
2. Widget JS auth: inline widget JS in assembled HTML instead of
<script src> to protected endpoint (browser script tags can't
send Authorization headers)
3. Layout config: fully implement tab ordering, default_tab,
chat.suggestions, chat.image_upload application
4. SSE event forwarding: wrap EventSource.addEventListener to
intercept all named events and dispatch to widget subscribers
via IronClaw.api._dispatch()
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(frontend): XSS prevention, widget queue drain, code-block false positives
Security (2 XSS fixes):
1. HTML-escape branding title in assemble_index() to prevent
<script>alert(1)</script> injection via layout.json
2. Escape </script> in inlined widget JS to prevent script tag
breakout — uses <\/script> replacement
3. Escape widget IDs in HTML attributes via escape_html_attr()
Correctness:
4. Drain _widgetInitQueue after DOM is ready — widgets registered
before tab-bar exists now mount correctly instead of silently
failing
5. Skip inline <code> elements in upgradeInlineJson to prevent
false-positive JSON card rendering on code spans like
<code>{key: value}</code>
6. Document scope_css limitation with nested @media rules
Tests (13 new):
- XSS: title injection escaped, widget JS </script> breakout escaped,
widget ID attribute escaped
- Edge cases: escape_html basic, escape_html_attr quotes, missing
head/body tags, empty widget JS, whitespace-only custom CSS skipped
- Widget: at-rule not prefixed, declarations preserved, special chars
in widget ID, all slot variants round-trip, minimal manifest
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* style: fix clippy — collapsible if, while_let_on_iterator
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(ci): resolve frontend clippy and formatting failures
* fix(frontend): address PR review — XSS, scope_css, cache, dedup
Security (3 XSS gaps):
1. Layout JSON injected into <script>window.__IRONCLAW_LAYOUT__</script>
is now run through escape_tag_close() — serde_json does not escape `<`
or `/`, so a branding title containing `</script>` previously broke
out of the script tag. Case-insensitive, UTF-8 safe.
2. Widget CSS and custom CSS injected into <style> tags are now escaped
the same way against `</style>` breakouts.
3. New escape_tag_close() helper handles `</script`/`</style` uniformly
(case-insensitive with tail preserved, via char-boundary walk).
Correctness:
4. scope_css now tracks brace depth via a stack that distinguishes rule
lists from declaration blocks. Selectors nested inside @media,
@supports, @container, @layer, @document, @scope are recursively
scoped. @keyframes/@font-face/@page bodies pass through opaque so
inner keyframe selectors (0%, 100%) are not prefixed. The old
single-bool parser produced unbalanced output on any nested rule.
5. WidgetInstanceConfig.enabled now defaults to true (via serde_default
+ manual Default impl). A layout entry that omits `enabled` while
setting `config` no longer silently disables the widget.
6. build_frontend_html short-circuit replaced with a
layout_has_customizations() helper covering all branding/tabs/chat
fields. The old boolean missed subtitle, logo_url, favicon_url,
default_tab, image_upload.
7. Custom CSS is now served only via /style.css (css_handler). Removed
from FrontendBundle injection to prevent double-application.
8. Dead pub index_handler/css_handler/js_handler in
handlers/static_files.rs removed — routes use private handlers in
server.rs that need GatewayState.
9. Widget file path validation is now component-based via
is_safe_segment / is_safe_relative_path. Rejects `.`, `..`, empty,
`/`, `\`, NUL in any component, plus leading `/`. MIME detection is
case-insensitive and adds .mjs / .map.
10. Layout and widget-manifest parse errors now log tracing::warn!
instead of silently falling back.
Extension system follow-ups:
11. Extracted shared widget-loading helpers (load_widget_manifests,
load_resolved_widgets, read_widget_manifest) in handlers/frontend.rs.
frontend_widgets_handler and build_frontend_html both delegate, so
widget discovery exists in exactly one place.
12. New FrontendHtmlCache in GatewayState. Cache key is derived from the
updated_at of frontend/layout.json and the frontend/widgets/
directory (max child mtime) via a single list("frontend/") call.
A cache hit skips reading every widget manifest/JS/CSS per request.
Edits invalidate naturally because list() sees the newer timestamp.
Cache survives rebuild_state() by cloning the Arc.
13. upgradeInlineJson rewritten without the nested-quantifier regex. New
_findJsonCandidates does a linear bracket scan that respects string
literals and fast-skips <code>/<pre> regions. Three hard caps bound
worst-case work (MAX_PARA_LEN=20000, MAX_SCAN=5000,
MAX_CANDIDATES=32), eliminating the catastrophic-backtracking risk.
Tests (29 new):
- bundle.rs: 5 — layout JSON / widget CSS / custom CSS <script>/<style>
breakouts, escape_tag_close case-insensitive, multi-byte safety
- widget.rs: 5 — @media inner selector scoped, nested @supports+@media,
@keyframes passthrough, sibling rules in @media, complex mix brace
balance
- layout.rs: 3 — enabled defaults true, Default impl enabled,
explicit false respected
- handlers/frontend.rs: 4 — segment allows/rejects, relative path
allows/rejects (traversal, backslash, encoded separators)
Quality gate:
- cargo fmt clean
- cargo clippy --all --benches --tests --examples --all-features →
zero warnings
- cargo test --lib -p ironclaw_frontend -p ironclaw → 4171 main +
43 frontend tests pass
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: post-merge — PairingStore::new_noop, CLI snapshot, docs
Merge of origin/staging surfaced three small follow-ups:
1. src/channels/wasm/wrapper.rs — PairingStore::new() signature changed
in staging to take (db, cache). Switch the test call site to
PairingStore::new_noop() to match other tests in the file.
2. src/cli/snapshots/..long_help_output_without_import.snap — accept
the new snapshot. Clap's render_long_help for --auto-approve now
emits an indented blank line between the short and long description;
this test was already failing on staging tip (see Staging CI run
24021660555) so the snapshot update was needed regardless of this PR.
3. src/workspace/seeds/FRONTEND.md — address new copilot comments:
- Placeholder is `{id}` (matches API path segment and manifest id
field), not `{name}`.
- Only `slot: "tab"` is actually mounted by the browser runtime.
Trim the slot list to what's implemented and mention
IronClaw.registerChatRenderer() for inline rendering. The extra
WidgetSlot variants stay in the Rust API for forward compatibility
but are no longer advertised to users until mounting is wired.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* refactor: rename ironclaw_frontend → ironclaw_gateway, .system/gateway/ workspace
Two coupled renames to align frontend assets with the broader `.system/`
namespace introduced by other in-progress work:
1. Workspace folder: `frontend/` → `.system/gateway/`
- layout.json, custom.css, widgets/{id}/, README.md, .config all
move under `.system/gateway/`
- LAYOUT_PATH and WIDGETS_DIR are now constants in the handler so a
future move is a one-line change
- is_config_path test updated to use the new path
- FRONTEND.md seed rewritten to point at `.system/gateway/`
- Cache key doc comments updated to match
- No legacy or migration shim — this never shipped to prod
2. Crate: `ironclaw_frontend` → `ironclaw_gateway`
- Matches how the surrounding subsystem is called (`channels/web` is
"the gateway"). Cleaner mental model: workspace folder, crate name,
and module name all align.
- Directory renamed via `git mv` so history is preserved.
- Cargo.toml workspace member + dependency updated; package name
updated; description tweaked to "gateway frontend assets".
- All `use ironclaw_frontend::` imports rewritten in server.rs and
handlers/frontend.rs.
- Doctest in widget.rs updated to use the new crate name.
- Cargo.lock regenerated.
The HTTP API paths stay as `/api/frontend/*` since they're a public
surface; only the internal workspace path and crate name moved.
Quality gate:
- cargo fmt clean
- cargo clippy --all --benches --tests --examples --all-features →
zero warnings
- cargo test -p ironclaw_gateway → 43 unit + 1 doctest pass
- cargo test --lib -p ironclaw → 4228 pass (8 unrelated IPv6/DNS
validation failures, also failing on clean post-merge baseline)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(gateway): per-request CSP nonce for inlined widget scripts
Copilot review caught that `assemble_index()` injects two kinds of inline
`<script>` blocks (the layout-config script and per-widget module scripts),
but the gateway's CSP sets `script-src 'self' …CDNs…` with no
`'unsafe-inline'` and no nonce — so the browser silently blocks every
injected script the moment any customization is enabled. The widget
runtime would never execute on a customized index page.
Fix uses a per-request CSP nonce (W3C standard pattern):
- `crates/ironclaw_gateway/src/bundle.rs`
- New `NONCE_PLACEHOLDER` sentinel constant, re-exported from the crate root
- `assemble_index()` stamps `nonce="__IRONCLAW_CSP_NONCE__"` on every
injected `<script>` tag (both the layout-config script and each
widget's module script)
- Inline `<style>` blocks deliberately do NOT carry a nonce — the
gateway's CSP allows `'unsafe-inline'` for `style-src`, so adding
one would be dead weight; pinned with a regression test
- Three new tests verify the placeholder appears on layout + widget
scripts and is absent on widget styles
- `src/channels/web/server.rs`
- Static CSP layer now reads from a single `BASE_CSP` constant so the
static and per-response variants stay in lock-step
- New `build_csp_with_nonce(nonce)` produces the same CSP with
`'nonce-{nonce}'` added to script-src, preserving the explicit CDN
list and the strict `style-src 'self' 'unsafe-inline' …` policy
- New `generate_csp_nonce()` returns 16 random bytes hex-encoded via
OsRng — same primitive `tokens_create_handler` already uses
- `index_handler` now returns `Response` (not `impl IntoResponse`) so
it can branch:
- Workspace has no customizations → serve embedded `INDEX_HTML`
unchanged; the static CSP layer applies (no inline scripts to
authorize anyway)
- Workspace has customizations → generate fresh nonce, replace
placeholder in cached HTML, and emit a per-response
`Content-Security-Policy` header with the nonce. Setting the
header here suppresses the global `if_not_present` layer for this
response only.
- Two new unit tests pin the nonce-source position in script-src and
the format/uniqueness of `generate_csp_nonce()`
The HTML cache still works because the cached HTML contains the
placeholder (not the actual nonce); per-request substitution preserves
caching while the browser still sees a unique nonce on every page load.
Quality gate:
- cargo fmt clean
- cargo clippy --all --benches --tests --examples --all-features →
zero warnings
- cargo test -p ironclaw_gateway → 46 pass (+3 nonce tests)
- cargo test --lib -p ironclaw → 4238 pass (+2 CSP tests)
Refs: PR #1725 review by copilot-pull-request-reviewer
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(gateway): wire ko.js asset through ironclaw_gateway::assets
The merge of staging brought in a Korean i18n pack referenced via
include_str!("static/i18n/ko.js") in src/channels/web/server.rs.
After the gateway extraction the static/ directory moved into
crates/ironclaw_gateway/static/, so the legacy include_str! path
no longer resolved. Add I18N_KO_JS to ironclaw_gateway::assets and
make the i18n_ko_handler reference it like the other language packs.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* test(e2e): add Playwright coverage for chat-driven frontend customization
Adds two end-to-end scenarios for the widget extension system shipped in
PR #1725, both driven by talking to the agent in chat:
1. **Tab bar to left side panel.** The user asks the agent to move the
tab bar; the mock LLM emits a `memory_write` tool call writing
`.system/gateway/custom.css`, and after a reload the test asserts the
served stylesheet contains the overlay, the computed flex-direction
of `.tab-bar` is `column`, and the bar is now taller than it is wide.
2. **Workspace-data widget.** The user asks the agent to create a
"Skills" widget that renders workspace skills. Two chat turns write
`.system/gateway/widgets/skills-viewer/manifest.json` and `index.js`
into the workspace. After a reload the test verifies the new tab
button appears in `.tab-bar`, switches to it, waits for the widget's
`data-testid="skills-viewer-root"` to mount, and asserts the widget
actually fetched `/api/skills` (no `skills-viewer-error` marker) and
that the panel carries the `data-widget="skills-viewer"` attribute
the gateway runtime stamps for CSS isolation.
Both tests share a `clean_customizations` fixture that wipes the
workspace overlay files before and after each run so the session-scoped
gateway server stays isolated across tests in the file (`memory_write`
treats empty content as effectively cleared, and the gateway skips
empty / unparseable widget files silently).
Supporting changes:
- **mock_llm.py**: three new `TOOL_CALL_PATTERNS` (`customize: move
tab bar to left`, `customize: create skills viewer manifest`,
`customize: install skills viewer code`) that emit one
`memory_write` call per turn — the existing one-tool-per-response
shape is preserved.
- **app.js (`_addWidgetTab`)**: fix a latent bug where widget tabs
would be queued forever because the function looked for a
`.tab-content` / `#tab-content` element that the gateway HTML never
ships. The built-in tab panels live as siblings of `.tab-bar` inside
`#app`, so we now resolve the parent off the first existing
`.tab-panel` (with `#app` as a final fallback). Without this fix the
Skills widget tab never mounts and the second scenario can't pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* test(e2e): support multi tool calls per response in mock_llm
The mock LLM previously emitted at most one tool call per assistant
turn. That shape silently bypasses the v2 engine and CodeAct dispatch
paths, where a single response can fan out into several parallel tool
calls (or several Python helper invocations from one script). Tests
written against that constraint were either contorted into multiple
chat turns or quietly failed to cover multi-call regressions.
Changes:
- ``TOOL_CALL_PATTERNS`` args functions may now return ``list[dict]``
instead of a single ``dict``. Each item is its own
``{"tool_name", "arguments"}`` pair, so one trigger can mix several
tools in one response. ``_normalize_tool_calls`` always wraps the
return value into a list so the dispatcher stays shape-agnostic.
- ``match_tool_call`` returns ``list[dict] | None``.
- ``_tool_call_response`` and ``_stream_tool_call`` now accept either a
single dict (legacy callers) or a list. The streaming path emits
per-tool-call header + arguments chunks with distinct ``index``
values, exercising clients' per-index merging logic the same way real
providers force them to.
- ``_find_tool_results`` collects every fresh ``role: tool`` message
after the most recent user turn (not just the first), and the
chat-completion summary path renders a multi-line acknowledgment
when more than one tool ran in a single turn. The single-result
helper is kept as a thin shim for the special-response path.
- The PR #1725 customization scenario is consolidated: instead of
three separate triggers (one memory_write each), the
``customize: install skills viewer widget`` trigger now emits *both*
the manifest and ``index.js`` writes in one assistant turn. The
``customize: move tab bar to left`` trigger stays single-call to
cover the legacy code path. The Playwright test in
``test_widget_customization.py`` is updated to a single chat turn
for the widget install — if the v2 engine ever drops the second
parallel call, the test will fail because the new tab can't mount
without both files.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(gateway): address PR #1725 review feedback
Four issues raised in the 2026-04-07 review pass:
1. **Widget id / directory mismatch** (`src/channels/web/handlers/frontend.rs`).
`read_widget_manifest` now rejects widgets whose `manifest.id` does
not match the on-disk directory name. The loader uses the directory
name to compute file paths (`{WIDGETS_DIR}{dir}/index.js`) while the
layout-config gating and the public
`/api/frontend/widget/{id}/{*file}` endpoint key off `manifest.id`.
When those drift, code can be mounted from one folder under a
different id and the file API silently 404s — a correctness footgun
for widget authors and a path-confusion attack surface for the
serving handler. Fix lives in the shared helper so both
`load_resolved_widgets` and `load_widget_manifests` get it. Adds
regression tests for both the rejection and the matching path.
2/3. **`memory_write` doc examples used the wrong parameter name**
(`src/workspace/seeds/FRONTEND.md`). The seeded customization guide
showed `memory_write path=".system/gateway/..."`, but the actual tool
parameter is `target` (`src/tools/builtin/memory.rs`). As written the
examples wouldn't work if copy-pasted into a tool call. Both
examples (layout.json + custom.css) updated to `target=`.
4. **`css_handler` allocated on the hot path** (`src/channels/web/server.rs`).
The handler always called `assets::STYLE_CSS.to_string()` in the
no-overlay branches, copying the entire embedded stylesheet on
every request. Switched the local to `Cow<'static, str>` so the
common path borrows the static string and only the overlay branch
pays for an owned `format!`.
Quality gate:
- `cargo fmt` clean
- `cargo clippy --no-default-features --features libsql --tests` zero warnings
- `cargo test --no-default-features --features libsql --lib channels::web::handlers::frontend` — 6 passed (4 existing + 2 new regression tests)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(gateway): address PR #1725 paranoid-architect review
Five issues raised in the 2026-04-07 review pass:
1. **High — `</style>` breakout XSS in branding CSS-vars injection**
(`crates/ironclaw_gateway/src/bundle.rs`). Every other inline injection
point in `assemble_index()` runs through `escape_tag_close`, but the
branding `<style>` block formatted directly. A hostile color value
containing `</style>` could close the tag early and inject HTML. Now
wraps `css_vars` in `escape_tag_close(&css_vars, "</style")` for
defense in depth, with a regression test in
`test_assemble_index_branding_style_breakout_escaped`.
2. **Medium — CSS property injection via unvalidated branding colors**
(`crates/ironclaw_gateway/src/layout.rs`). `to_css_vars()` interpolated
`primary` / `accent` strings raw into `--color-primary: {};`, letting
a hostile `layout.json` break out of the `:root {}` block (e.g.
`red; } .chat-input[value^="s"] { background: url(...) }`). Added
`is_safe_css_color()` validator that accepts hex literals, modern
functional notation including `rgb(0 0 0 / 50%)`, and bare named
colors, while rejecting `;`, `{}`, `<>`, quotes, backslash, `*`
(handles both `/*` and `*/` comment markers), `url(...)`, and unknown
functions. `to_css_vars()` silently drops invalid values so the rest
of the branding config still applies. Six new unit tests cover the
accepted forms, the injection vectors, and the `to_css_vars` drop.
3. **Medium — CSP policy duplication risks silent drift**
(`src/channels/web/server.rs`). `BASE_CSP` and `build_csp_with_nonce`
re-hardcoded every directive independently, so adding a `connect-src`
to one would silently leave the other on the old policy. Extracted
per-directive constants (`STYLE_SRC`, `FONT_SRC`, `CONNECT_SRC`,
`IMG_SRC`, `FRAME_SRC`, `FORM_ACTION`) and built both flavors via a
single `build_csp(nonce: Option<&str>)` helper. `BASE_CSP_HEADER` is
now a `LazyLock<HeaderValue>` (with a safe minimal fallback to honor
the no-`.expect()` rule on the request path). Added two regression
tests: `test_base_and_nonce_csp_agree_outside_script_src` strips the
`script-src` directive from both flavors and asserts byte equality,
and `test_base_csp_header_matches_build_csp_none` locks the lazy
header to `build_csp(None)`.
4. **Medium — `_wipe_customizations` ignored HTTP status**
(`tests/e2e/scenarios/test_widget_customization.py`). The cleanup
posts now assert `status_code == 200` with `resp.text` in the
message, so an auth/server failure surfaces immediately instead of
bleeding leftover workspace state into the next test.
5. **Drive-by — pre-existing flake in `test_telegram_token_colon_preserved
_in_validation_url`** (`src/extensions/manager.rs`). The test reads
`IRONCLAW_TEST_TELEGRAM_API_BASE_URL` via `telegram_bot_api_url`
without taking the `lock_env()` mutex, so when a parallel test holds
the override the read races and the assertion sees
`http://127.0.0.1:.../bot…` instead of `https://api.telegram.org/`.
The new tests in this PR changed scheduling enough to surface the
race on every run. Fixed by acquiring the same `ScopedEnvVar` lock
and clearing the override inside the test, making it deterministic.
Quality gate:
- `cargo fmt` clean
- `cargo clippy --no-default-features --features libsql --tests` zero warnings
- `cargo test --no-default-features --features libsql --lib` — 4284 passed
- `cargo test -p ironclaw_gateway` — 50 unit + 1 doctest passed (was 46)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* ci: nudge workflows for
|
||
|
Illia Polosukhin
|
af9b59a284 |
feat: unified tool dispatch + schema-validated workspace (#2049)
* feat(workspace): add JSON Schema validation to document metadata Add a `schema` field to `DocumentMetadata` that enables automatic content validation on workspace writes. When a document or its folder `.config` carries a JSON Schema, all write operations (write, append, patch, write_to_layer, append_to_layer) validate content against it before persisting. This is the foundation for typed system state (settings, extension configs, skill manifests) stored as workspace documents. Builds on the metadata infrastructure from #1723 — schema is inherited via the existing `.config` chain (folder → document → defaults). Refs: #640, #1894, #1937 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(tools): add channel-agnostic ToolDispatcher with audit trail Introduce `ToolDispatcher` — a universal entry point for executing tools from any caller (gateway, CLI, routine engine, WASM channels). Creates lightweight system jobs for FK integrity, records ActionRecords, and returns ToolOutput. This is a third entry point alongside v1's Worker::execute_tool() and v2's EffectBridgeAdapter::execute_action(). DispatchSource::Channel(String) is intentionally string-typed — channels are interchangeable extensions that can appear at runtime. Also adds JobContext::system() factory and create_system_job() to both PostgreSQL and libSQL backends. Refs: #640 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(workspace): settings-as-workspace-documents with dual-write adapter Add WorkspaceSettingsAdapter that implements SettingsStore by reading/ writing workspace documents at _system/settings/{key}.json. During migration, dual-writes to both the legacy settings table and workspace. Reads prefer workspace, falling back to the legacy table. Known setting keys (llm_backend, selected_model, tool_permissions.*, etc.) get JSON Schemas stored in document metadata — writes are validated automatically by Phase 0's schema validation. Also adds settings_schemas.rs with compile-time schema registry and settings_path() helper. Refs: #640, #1937 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(gateway): wire ToolDispatcher into GatewayState Add tool_dispatcher field to GatewayState with with_tool_dispatcher() builder method. Create and wire the dispatcher in main.rs when both tool_registry and database are available. All 16 GatewayState construction sites updated. Per-handler migration (routing mutations through ToolDispatcher instead of direct DB calls) is deferred to follow-up PRs — each handler has complex ownership checks, cache refresh, and response types. Refs: #640 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(tools): add system introspection tools (tools_list, version) Add SystemToolsListTool and SystemVersionTool as proper Tool implementations that replace hardcoded /tools and /version commands. Registered at startup via register_system_tools(). Available in both v1 and v2 engines — no is_v1_only_tool filter to worry about. Refs: #640 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(workspace): extension and skill state schemas and path helpers Add workspace path helpers and JSON Schemas for storing extension configs, extension state, and skill manifests under _system/extensions/ and _system/skills/. This establishes the workspace document structure that ExtensionManager and SkillRegistry will use as a durable persistence backend (read-through cache pattern). Runtime state (active MCP connections, WASM runtimes) stays in memory. Only durable config and activation state moves to workspace documents. Refs: #640, #1741 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR review feedback and CI failures CI fixes: - deny.toml: allow MIT-0 license required by jsonschema - workspace/document.rs: #[allow(dead_code)] on system path constants pending follow-up phases that consume them - workspace/settings_adapter.rs: remove unused chrono::Utc import - workspace/settings_adapter.rs: collapse nested if into && form Review fixes (gemini-code-assist): - tools/dispatch.rs: await save_action directly instead of fire-and-forget tokio::spawn so short-lived CLI callers cannot drop audit records before they are persisted; surface errors via tracing::warn - tools/dispatch.rs: remove DispatchSource::Agent variant — sequence_num=0 with a reused job_id would violate UNIQUE(job_id, sequence_num). Agent callers must use Worker::execute_tool() which manages sequence numbers atomically against the agent's existing job - workspace/settings_adapter.rs: validate content against the schema BEFORE the first workspace write so the initial document creation cannot bypass schema enforcement (subsequent writes are validated by the workspace resolved-metadata path established after the first write) Refs: #2049 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: unify all machine state under .system/ Rename the workspace prefix from `_system/` to `.system/` (Unix dot-prefix convention for hidden internal state) and migrate v2 engine state from `engine/` to `.system/engine/` so all machine-managed state lives under one root. New layout: .system/ ├── settings/ (per-user settings as workspace docs) ├── extensions/ (extension config + activation state) ├── skills/ (skill manifests) └── engine/ ├── README.md (auto-generated index) ├── knowledge/ (lessons, skills, summaries, specs, issues) ├── orchestrator/ (Python orchestrator versions, failures, overlays) ├── projects/ (project files + nested missions/) └── runtime/ (threads, steps, events, leases, conversations) The inner `.runtime/` dot-prefix is dropped under `.system/engine/` since `.system/` itself is the hidden marker; no double-hiding needed. The `ENGINE_PREFIX` constant in `workspace::document::system_paths` is declared as the canonical convention; bridge `store_adapter` continues to define per-subdirectory constants below it for ergonomic interpolation. No legacy migration code — pre-production rename. Refs: #2049 * fix(pr-2049): security, correctness, and robustness fixes from review Critical security: - dispatch.rs: redact sensitive params before persisting ActionRecord (was leaking plaintext secrets into the audit log for tools with sensitive_params()) - settings_schemas.rs: validate settings keys against path traversal (reject /, \, .., leading ., empty, length > 128, non-alphanumeric); wire validation into all settings_adapter read/write/delete paths Data correctness: - history/store.rs + libsql/jobs.rs: write status as JobState::Completed .to_string() ('completed' snake_case) instead of 'Completed'; system jobs were round-tripping as Pending in parse_job_state() - settings_adapter.rs: fix .system/.config metadata to set skip_versioning: false (was true) — descendants inherit this via find_nearest_config, so the previous value silently disabled versioning for ALL .system/** documents, contradicting the audit- trail intent - workspace/mod.rs: add resolve_metadata_in_scope; use it in write_to_layer / append_to_layer so non-primary layer writes resolve schema/indexing/versioning from the target layer's .config chain instead of the primary user_id's. Also pass &scope (not &self.user_id) to maybe_save_version so versions are attributed to the correct scope Pipeline parity: - dispatch.rs: add SafetyLayer to ToolDispatcher; mirror Worker pipeline (prepare_tool_params -> validator -> redact -> timeout -> sanitize output) so dispatch path gets the same safety guarantees as the agent worker. Sanitized output is now stored in ActionRecord.output_sanitized instead of duplicating raw JSON Robustness: - settings_adapter.rs: propagate update_metadata errors in ensure_system_config and write_to_workspace (was silently ignored via let _ =, leaving schemas/skip_indexing unenforced) - settings_adapter.rs: set_all_settings now collects the first workspace write error and returns it after the legacy write completes, so partial-migration state is observable - settings_schemas.rs: rewrite llm_custom_providers schema to match CustomLlmProviderSettings (id/name/adapter/base_url/default_model/ api_key/builtin instead of stale name/protocol/base_url/model) Build: - Cargo.toml: jsonschema with default-features = false to avoid pulling a second reqwest major version Docs: - db/mod.rs: docstring for create_system_job uses 'completed' snake_case - workspace/document.rs: clarify .system/ versioning ("by default ARE versioned; individual files may opt out via skip_versioning") - settings_adapter.rs: clarify per-key reads prefer workspace, aggregate reads stay on legacy during migration - tools/builtin/system.rs: trim doc to match implemented scope (system_tools_list, system_version) - channels/web/mod.rs: move stale 'sweep tasks managed by with_oauth' comment back to oauth_sweep_shutdown line Refs: #2049 * docs+ci: enforce 'everything goes through tools' principle Document the core design principle from #2049 in two places so future contributors (human and AI) discover it during development: - CLAUDE.md: new "Everything Goes Through Tools" section near the "Adding a New Channel" guide. Includes the rule, the rationale (audit trail, safety pipeline parity, channel-agnostic surface, agent parity), and a pointer to the detailed rule file. - .claude/rules/tools.md: full pattern with required/forbidden examples, the list of layers that ARE exempt (Worker::execute_tool, v2 EffectBridgeAdapter, tool implementations themselves, background engine jobs, read-aggregation queries), and how to annotate intentional exceptions. Also extends `paths` to cover src/channels/** and src/cli/** so it surfaces when those files are edited. Enforce with a new pre-commit safety check (#7) in scripts/pre-commit-safety.sh: - Scans newly added lines under src/channels/web/handlers/*.rs and src/cli/*.rs for direct touches of state.{store, workspace, workspace_pool, extension_manager, skill_registry, session_manager}. - Suppress with a trailing `// dispatch-exempt: <reason>` comment on the same line, matching the existing `// safety:` convention. - Only checks added lines (`+` in the diff), so existing untouched handlers don't trip the check during incremental migration. The check fires only for new code: handlers that haven't been migrated yet (52 existing direct accesses across 12 handler files) won't break unmodified, but any new line that bypasses the dispatcher will be flagged at commit time. Refs: #2049 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(pr-2049): address Copilot review on workspace schema layer - workspace::extension_state: extension/skill path helpers now reuse the canonical name validators (`canonicalize_extension_name`, `validate_skill_name`) instead of a weak `replace('/', "_")`. Names containing `..`, `\`, NUL, or other escapes are now rejected at the helper boundary, eliminating a path-traversal foothold for callers. Helpers return `Result<String, PathError>`. Regression tests added. - workspace::settings_adapter::ensure_system_config: now idempotent across upgrades. If `.system/.config` already exists with stale metadata (e.g. an older `skip_versioning: true` from before fix #3042846635), it is repaired to the expected inherited values instead of being left silently broken. Regression test added. - workspace::settings_adapter::write_to_workspace: lazily seeds `.system/.config` via a `OnceCell`, so callers no longer need to remember to invoke `ensure_system_config()` at startup before any setting write. Regression test added. - workspace::settings_adapter::delete_setting: workspace delete failures are now logged via `tracing::warn!` instead of being silently dropped. We still don't propagate the error — the legacy table is the source of truth during migration and a stale workspace doc is recoverable on the next write — but partial-delete state is now observable. - workspace::schema: documented why we don't cache compiled validators yet (settings/extension/skill writes are not a hot path; revisit if schema validation moves into a frequent write path). [skip-regression-check] schema.rs change is doc-only. * fix(pr-2049): address 4 remaining review issues 1. tool_dispatcher dropped during gateway startup src/channels/web/mod.rs: rebuild_state was initializing tool_dispatcher to None, so every subsequent with_* call zeroed the dispatcher the first caller injected. Preserve it across rebuild_state like every other field. Regression test: tool_dispatcher_survives_subsequent_with_calls. 2. WorkspaceSettingsAdapter not wired into runtime src/app.rs: Build the adapter in build_all() when workspace+db are both present, eagerly call ensure_system_config(), expose on AppComponents as settings_store, and thread it into init_extensions(...) so register_permission_tools and upgrade_tool_list receive it instead of the raw db. src/main.rs: SIGHUP handler prefers the adapter over raw db. src/workspace/mod.rs: re-export WorkspaceSettingsAdapter. 3. changed_by regression on layered writes src/workspace/mod.rs: write_to_layer and append_to_layer were passing the target layer's scope as changed_by, so version history attributed layered edits to the layer name instead of the actor. Pass self.user_id while keeping metadata resolution in the target scope. Regression test: layered_writes_record_actor_in_changed_by. 4. Legacy engine/ paths invisible after upgrade src/bridge/store_adapter.rs: Add migrate_legacy_engine_paths(), called at the start of load_state_from_workspace(), which scans list_all() for engine/... documents and rewrites them to .system/engine/... Idempotent: skips rewrites when the new path already exists, deletes the legacy duplicate either way. Three regression tests in #[cfg(all(test, feature = "libsql"))] module. Quality gate: cargo fmt, cargo clippy --all --all-features zero warnings, cargo test --all-features --lib 4313 passed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(e2e): use PUT for settings write in ownership test test_settings_written_and_readable was sending POST /api/settings/{key} but the route has been PUT since #4 (Feb 2026) — the test was returning 405 Method Not Allowed. Switch to httpx.put() so it matches the current route registration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(pr-2049): address second round of review feedback Addresses the remaining unresolved PR #2049 review comments from serrrfirat and ilblackdragon. ## Changes ### ToolDispatcher — integration coverage + log level - src/tools/dispatch.rs: add two libsql-gated integration tests for the full dispatch pipeline: (a) persist an ActionRecord with sensitive params redacted in the audit row while the tool still sees the raw value, sanitized output populated; (b) honor the per-tool execution_timeout() and record a failure action. - Tests use a raw-SQL helper to find system-category jobs since list_agent_jobs_for_user intentionally filters them out. - Replace warn! with debug! on audit persistence failure — dispatch is reachable from interactive CLI/REPL sessions where warn!/info! output corrupts the terminal UI (CLAUDE.md Code Style → logging). ### WorkspaceSettingsAdapter — log level - src/workspace/settings_adapter.rs: same warn! → debug! fix on the delete_setting workspace failure path, for the same REPL reason. ### Schema validation — surface all errors - src/workspace/schema.rs: switch from jsonschema::validate to validator_for + iter_errors so users fixing a malformed setting see every violation in one round instead of playing whack-a-mole. Also distinguishes "invalid schema" from "invalid content" errors. - Regression tests: multiple_errors_are_all_reported and invalid_schema_is_distinguished_from_invalid_content. ### create_system_job — started_at + row growth docs - src/db/libsql/jobs.rs and src/history/store.rs: include started_at in the INSERT (set to the same instant as created_at/completed_at) so duration queries don't see NULL and "started but not completed" filters don't misclassify these rows. Fixed in both backends. - Add doc comments on both impls warning about row growth per dispatch call. Deleting rows would violate "LLM data is never deleted" (CLAUDE.md); if listing-query performance becomes a concern, prefer a partial index (WHERE category != 'system') over deletion. ### Lib test repair - src/channels/web/server.rs: extensions_setup_submit_handler Err branch now sets resp.activated = Some(false) so clients and the regression test see an explicit `false` rather than `null`. Also rename the test's fake channel to snake_case (test_failing_channel) so it matches the canonicalize-extension-names behavior from PR #2129 — previously the test was passing a dashed name and getting "Capabilities file not found" instead of the intended activation failure. ## Not addressed (false positive / deferred) - dispatch.rs:177 output_raw/output_sanitized swap — verified against ActionRecord::succeed(Option<String>, Value, Duration) and the worker's call site at job.rs:704; argument order is correct. - settings_adapter.rs:186 TOCTOU window — author self-classified as "Low / completeness" and no other code path writes to .system/settings/** without going through write_to_workspace. - schema.rs recompilation caching — deferred per earlier review. ## Quality gate - cargo fmt - cargo clippy --all --benches --tests --examples --all-features zero warnings - cargo test --all-features --lib: 4387 passed, 0 failed, 3 ignored Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(pr-2049): address third round of review feedback Addresses unresolved comments from serrrfirat's "Paranoid Architect Review" and Copilot's third pass on the engine-state migration. ## src/workspace/settings_adapter.rs ### HIGH — Cross-tenant data leak through owner-scoped Workspace `Workspace` is constructed for a single user_id at AppBuilder time. Without gating, `set_setting("user_B", key, val)` would dual-write into the **owner's** workspace, and a subsequent `user_A.get_setting(...)` would return user_B's value: a real cross-user data leak. Fix: - Add `gate_user_id` field set to `workspace.user_id()` at construction. - All `SettingsStore` methods that touch the workspace now check `workspace_allowed_for(user_id)` first; non-owner callers fall through to the legacy table only — preserving their pre-#2049 behavior. - This matches the long-term plan: per-user settings live in the legacy table until a per-user `WorkspaceSettingsAdapter` (one per WorkspacePool entry) is wired up; admin/global settings go through the workspace-backed path so they pick up schema validation. Regression test: `workspace_settings_are_owner_gated_in_multi_tenant_mode` asserts (a) owner's workspace doc is not overwritten by a non-owner write, (b) each user reads back their own legacy value, and (c) a non-owner with no legacy entry must NOT see the owner's workspace value bleeding through. ### MEDIUM — Dual-write order Reverse `set_setting` and `set_all_settings` to write legacy first, workspace second. The legacy table is the source of truth during migration (it backs aggregate `list_settings` reads), so writing it first guarantees those readers always see a consistent value even if the workspace write fails. Failed workspace writes are self-healing on the next per-key read-miss. ### MEDIUM — `ensure_system_config_lazy` double-execution race Replace the manual `get()`/`set()` pattern with `OnceCell::get_or_try_init`. Two concurrent first-callers no longer both run `ensure_system_config()`. Functionally equivalent (idempotent either way) but no longer wasteful. ## src/bridge/store_adapter.rs ### MEDIUM — Migration drops document metadata (S3) `migrate_legacy_engine_paths` previously copied only `doc.content`, silently dropping the `metadata` column. Now calls `ws.update_metadata(new_doc.id, &doc.metadata)` after each write to preserve schema/skip_indexing/hygiene flags. Logged-not-fatal: content has already been moved, metadata loss is recoverable. Regression test: `migration_preserves_document_metadata` seeds a doc with custom metadata and asserts it survives the rewrite. ### MEDIUM — `ws.exists()` swallowed transient errors (Copilot) `unwrap_or(false)` on the existence check could cause the migrator to overwrite an existing `.system/engine/...` doc when storage hiccups. Now propagates the error (counts as failed step + `continue`), per Copilot's exact suggested patch. ### LOW — `list_all()` runs every startup (Copilot) Add a cheap preflight: `ws.list("engine")` first; only fall through to the recursive `list_all()` discovery when the directory listing returns at least one entry. Steady-state startups (post-migration) skip the full workspace scan entirely. Regression test: `migration_preflight_skips_full_scan_when_no_legacy_paths` asserts unrelated and already-migrated documents are untouched. ### MEDIUM — Counter undercount on `already_present` (S5) When `already_present` is true the legacy duplicate is still deleted, but the previous code skipped the `migrated += 1` increment, undercounting in debug logs. Fixed: `migrated` now counts every successful path migration including the already-present case. ### Documented — Version-history loss is acceptable scope (C1) Read-write-delete pattern means `memory_document_versions.document_id ON DELETE CASCADE` drops the legacy doc's version chain. Documented in the function-level doc comment as intentional + bounded: - v2 engine state is runtime state (rewritten on every mutation), not user-curated data - v2 was newly introduced in this PR — no production deployment with pre-existing curated history at risk - A path-preserving rename op would need new trait methods on both backends; out of scope for fix-forward. If a future caller needs history-preserving rename, it should be added to the storage layer properly, not bolted onto migration. ## Quality gate - cargo fmt - cargo clippy --all --benches --tests --examples --all-features zero warnings - cargo test --all-features --lib: 4390 passed, 0 failed, 3 ignored (+3 new tests on top of round 2) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(pr-2049): address fourth round of review feedback Two latent issues flagged by serrrfirat in the latest review pass: 1. **Null schema permanently locks documents** (`src/workspace/schema.rs`). `serde_json` deserializes a metadata field of `"schema": null` as `Some(Value::Null)`, not `None`, so the upstream `if let Some(schema) = &metadata.schema` check passes through to `validate_content_against_schema`. There, `validator_for(Value::Null)` errors out and every subsequent write to that document is blocked — a latent DoS. Added an explicit `schema.is_null()` early-return guard at the top of the validator, plus a regression test (`null_schema_is_treated_as_no_op`) that asserts even non-JSON content passes when the schema is null. 2. **System job titles were raw source labels** (`src/history/store.rs`, `src/db/libsql/jobs.rs`). `create_system_job` set `title = source`, so any UI rendering `agent_jobs.title` would display dispatched system jobs as `channel:gateway` / `system` / etc. instead of a human-readable label. Both PostgreSQL and libSQL backends now write `format!("System: {source}")`. Updated the two dispatch integration tests that pinned the old format. Schema-recompilation comment (`schema.rs:47`) was acknowledged as "acceptable for now" by the reviewer; existing NOTE in the source already documents the caching trade-off and upgrade path, so no code change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(pr-2049): address fifth round of review feedback Eight comments from Copilot + serrrfirat. Real fixes for the load-bearing gaps; doc clarifications for the rest where the existing behavior is intentional. **Real code changes** - `src/tools/dispatch.rs` — enforce `tool.parameters_schema()` (JSON Schema) in the dispatch path. Previously the SafetyLayer validator only checked for injection patterns; channel/CLI/routine callers could pass arbitrary shapes and only discover the mismatch (or worse, silently malformed behavior) inside the tool itself. Now we run `jsonschema::validate(&tool.parameters_schema(), &normalized_params)` after the injection check, with a permissive-empty-schema fast path so tools that haven't yet declared a schema aren't penalised. Regression test `dispatch_rejects_params_violating_tool_schema` asserts a required-field violation is rejected before the tool is invoked. - `src/workspace/settings_adapter.rs` — `write_to_workspace` now calls `schema_for_key(key)` once and reuses the resolved schema for both pre-write validation and post-write metadata persistence (was called twice). Eliminates duplicate work and removes a theoretical divergence window if the schema registry ever became non-deterministic. - `src/workspace/settings_adapter.rs` — `ensure_system_config` now also rewrites the `.config` document content when its metadata is repaired, not just the metadata column. The metadata column is the inheritance source of truth, but having the doc's content silently diverge from it confuses anyone reading the doc directly to understand which inherited flags are active. - `src/error.rs` + `src/workspace/settings_schemas.rs` — new `WorkspaceError::InvalidPath { path, reason }` variant. Path/key rejection (path-traversal, character set, length) now surfaces as `InvalidPath`, not `SchemaValidation` — callers and downstream UIs can distinguish "your settings *key* has bad characters" from "your settings *value* failed JSON-Schema validation" without string-matching error messages. `validate_settings_key` returns the new variant; the one match site in `settings_adapter.rs::write_to_workspace` is updated. Regression test `validate_settings_key_returns_invalid_path_variant`. **Documentation-only fixes** - `src/tools/dispatch.rs` — clarify in the `dispatch()` doc-comment that `sanitize_tool_output` runs only against the persisted ActionRecord payload, NOT against the value returned to the caller. This mirrors `Worker::execute_tool` (the agent loop also receives the raw output so reasoning can be reproduced from history). Channels that forward dispatcher output to end users must run their own boundary sanitization at the channel edge. - `src/history/store.rs` + `src/db/libsql/jobs.rs` — `create_system_job` doc updated to explicitly state that system job timestamps do NOT reflect tool execution time (the row is INSERTed before the tool runs, with all three timestamps pinned to "now"). Consumers that need execution duration must read `job_actions.duration_ms` for the associated action rows. Restructuring to a two-phase INSERT+UPDATE was rejected: the audit row must be durable even if the dispatcher panics mid-tool, and the second write would double per-dispatch DB cost. - `src/workspace/schema.rs` — added baseline regression test `moderately_complex_schema_compiles_within_budget` that pins schema compile + validate latency for a moderately deep nested schema at <500ms wall-clock. Guards against orders-of-magnitude regressions from a future `jsonschema` upgrade or accidentally pathological schema construction. Hard limits on schema complexity are deferred (the real defense today is keeping schema-bearing paths under `.system/`, which is system-controlled). **Acknowledged, no change** - libSQL `create_system_job` unbounded row growth — already documented as intentional in the existing comment block, with the mitigation path spelled out (partial index on `WHERE category != 'system'` for listing queries). Rate-limiting dispatch would silently drop user-initiated actions, which is worse than unbounded retention. The "LLM data is never deleted" rule (CLAUDE.md) explicitly applies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
980d60ea45 |
[codex] Stabilize auth readiness and gate flows (#2050)
* Unify extension readiness and refresh dynamic tool leases * Fix v2 OAuth refresh and scope legacy credential fallback * Stabilize auth readiness and gate flows * Tighten auth token submission and OAuth fallback * Expose tool registry database handle * Handle expired runtime credentials in auth preflight * Fix E2E regressions on extension lifecycle branch * Normalize OAuth auth descriptors and flow launchers * Address review feedback on gate routing and latent actions * Apply formatter cleanup in tests * Address auth API review follow-ups * Generalize Google auth fallback and bundle alias metadata * Skip MCP OAuth when Authorization header is configured * Re-emit pending approval gates on follow-up * Open OAuth auth links in a new tab * Move shared OAuth runtime into auth module * Fix CI lint failures after staging merge * Unify OAuth resume and user greeting lifecycle * Ignore E2E virtualenv * Repair staging-merge build break in extension lifecycle paths The previous merge of staging into extension-lifecycle (commit |
||
|
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> |
||
|
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> |
||
|
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>
|
||
|
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>
|
||
|
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> |
||
|
Illia Polosukhin
|
62d16e69ac |
fix(mcp): handle 400 auth errors, clear auth mode after OAuth, trim tokens (#1158)
* fix(mcp): handle 400 auth errors, clear auth mode after OAuth, trim tokens Three bugs prevented MCP server authentication (e.g. GitHub MCP) from working correctly: 1. **400 treated as auth-required**: GitHub's MCP endpoint returns 400 "Authorization header is badly formatted" instead of 401 when auth is missing. Broadened auth detection in activate_mcp, send_request, and discover_via_401 to also match 400+authorization errors. 2. **Auth mode not cleared after OAuth callback**: The OAuth callback handler and setup submit handler did not call clear_auth_mode(), leaving pending_auth on the thread. The next user message was intercepted as a token instead of triggering an LLM turn. 3. **Token trimming**: Tokens with leading/trailing whitespace or newlines produced malformed Authorization headers. Now trimmed before storage (configure) and before use (build_request_headers). Adds E2E tests with a mock MCP server (JSON-RPC + OAuth discovery + DCR + token exchange) covering install -> activate -> OAuth callback -> LLM turn lifecycle, plus a GitHub-style 400 error variant. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(mcp): add TTL to PendingAuth and clear auth mode on all failure paths Auth mode (pending_auth on a Thread) had no timeout and several code paths that failed to clear it, causing user messages to be swallowed indefinitely. This adds defense-in-depth: - Add created_at + 5-minute TTL to PendingAuth; auto-clear on next message if expired (safety net for edge cases like user closing browser mid-OAuth) - Clear auth mode on OAuth callback failure paths (unknown/consumed state, expired flow) - Move clear_auth_mode before configure() match in setup_submit so it runs on failure too (addresses Copilot review feedback) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(ci): exclude test hunks from unwrap/assert pre-commit check The pre-commit safety script only excluded files in tests/ but not #[cfg(test)] mod tests blocks inside src/ files. Use the git diff @@ hunk header context (which includes the enclosing function name) to detect and skip test hunks. Also removes unnecessary // safety: comments from test assertions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: restore formatting in test assertions The replace_all edit that removed // safety: comments collapsed newlines. Restore proper line breaks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address Copilot review - tighten pre-commit filter, document TTL sync - pre-commit-safety.sh: only exclude `mod tests` hunks (not `fn test_*`) to avoid hiding unwrap/assert in production functions like test_server() - session.rs: extract AUTH_MODE_TTL_SECS constant and add doc comment linking to OAUTH_FLOW_EXPIRY to prevent silent drift [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(mcp): return error on expired auth input, clear auth on all OAuth paths - When auth mode TTL expires and the user sends a message (possibly a pasted token), return an explicit "expired, please retry" response instead of forwarding the content to the LLM/history - Add clear_auth_mode() to all early-return paths in oauth_callback_handler (provider error, missing state/code, no extension manager) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
27e21fdabe |
feat: add pre-push git hook with delta lint mode (#833)
* feat: add pre-push git hook with delta lint mode Add pre-push hook and CI quality gate scripts: - .githooks/pre-push: runs quality gate before push - scripts/ci/quality_gate.sh: baseline fmt + clippy correctness + tests - scripts/ci/delta_lint.sh: clippy warnings filtered to changed lines only - Updated dev-setup.sh to install pre-push hook Supports environment-gated modes: - IRONCLAW_STRICT_LINT=1: deny all clippy warnings - IRONCLAW_STRICT_DELTA_LINT=1: deny warnings only on changed lines Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use git rev-parse for SCRIPT_DIR, add python3 check - Fix SCRIPT_DIR resolution in pre-push hook to work correctly with symlinks by using git rev-parse --show-toplevel - Add python3 availability check in delta_lint.sh Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: delta lint stderr handling, --locked flag, path normalization - Stop suppressing clippy stderr; capture it and show compilation errors if clippy produces no JSON output - Add --locked flag to clippy for lockfile consistency - Use repo root (via git rev-parse) for path normalization instead of os.getcwd() which may differ from repo root Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: dynamically detect upstream base branch in delta_lint.sh Instead of hard-coding `origin/main`, derive the base ref by checking `refs/remotes/origin/HEAD`, then falling back to `origin/main` and `origin/master`. If none can be resolved, skip delta lint gracefully with a warning and exit 0. Addresses PR #833 review feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: re-trigger CI after adding skip-regression-check label Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR #833 review feedback for delta lint - Pass remote name ($1) from pre-push hook to delta_lint.sh - Accept optional remote name arg, fall back to dynamic detection - Treat error-level diagnostics as always blocking - Check span overlap [line_start, line_end] vs changed ranges - Handle +++ /dev/null (file deletions) in parse_diff - Catch git merge-base failure with graceful skip - Add CLIPPY_STDERR to EXIT trap cleanup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: drop -D warnings from delta lint, scope pre-push tests to --lib 1. Remove `-D warnings` from the clippy invocation in delta_lint.sh. With -D warnings, all warnings are promoted to error level in JSON output, which bypasses the delta filter entirely (errors are always blocking). The Python filter already handles the blocking decision for warnings based on changed-line overlap. 2. Scope pre-push tests to `cargo test --lib` (unit tests only) instead of the full test suite. Full integration tests can take minutes and will train developers to use --no-verify. The full suite runs in CI. Skip tests entirely with IRONCLAW_PREPUSH_TEST=0. Addresses zmanian's review feedback on PR #833. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Henry Park
|
fda5160940 |
Make no-panics CI check test-aware (#1160)
* Make no-panics check test-aware * Handle proc-macro test attrs in no-panics check * Pin Python for no-panics CI job |
||
|
Illia Polosukhin
|
7776d267f8 |
ci: enforce no .unwrap(), .expect(), or assert!() in production code (#1087)
Add a diff-based CI job and pre-commit hook check that block panic-inducing calls (.unwrap(), .expect(), assert!, assert_eq!, assert_ne!) from entering production Rust code. debug_assert is excluded (compiled out in release). False positives can be suppressed with an inline `// safety: <reason>` comment. - pre-commit-safety.sh: add check 6 (PANIC) for staged diffs - code_style.yml: add `no-panics` job, wire into roll-up gate - check-boundaries.sh: extend check 2 to also catch assert!() Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
febed1e12e |
feat: add cargo-deny for supply chain safety (#834)
* feat: add cargo-deny for supply chain safety Add dependency auditing via cargo-deny to catch license violations, security advisories, and untrusted sources. Integrates into CI as a parallel job alongside clippy, and into the local quality gate script. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use cargo-deny action in CI, improve quality gate script - Use EmbarkStudios/cargo-deny-action@v2 instead of cargo install for faster CI execution - Fix quality_gate_strict.sh to check for cargo-deny availability instead of suppressing stderr Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add missing Unlicense and CDLA-Permissive-2.0 to license allowlist Add Unlicense (used by aho-corasick, memchr, etc.) and CDLA-Permissive-2.0 (used by webpki-roots) to prevent cargo deny check from failing on the current dependency tree. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: trigger CI after retargeting PR to staging Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use valid cargo-deny v0.19 syntax for unmaintained advisories The `unmaintained` field in [advisories] accepts "all", "workspace", "transitive", or "none" — not "warn". Use "workspace" to flag unmaintained direct dependencies without failing on transitive ones. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: re-trigger CI after adding skip-regression-check label Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: migrate deny.toml [licenses] to version 2 format Remove deprecated `unlicensed` and `default` fields, add `version = 2`. In v2, all licenses are denied unless explicitly in the allow list, making these fields redundant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: ignore pre-existing advisories in deny.toml with justification Add known RUSTSEC IDs to the ignore list so cargo-deny CI passes. Each advisory is documented with mitigation context. Dependency upgrades to resolve these should be tracked separately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback for cargo-deny integration - quality_gate_strict.sh: fail hard when cargo-deny is not installed instead of silently skipping, and let set -e handle check failures - deny.toml: remove empty [graph].targets so cargo-deny checks all platforms instead of only the runner's default target Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(deny.toml): correct serde_yml advisory comment to reflect direct dependency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: tighten clippy-windows check in roll-up job Change from checking only `== "failure"` to checking `!= "success" && != "skipped"`. This ensures any unexpected result (e.g., cancelled) also blocks the merge, while still allowing the expected "skipped" state for non-main PRs. Addresses zmanian's review feedback on PR #834. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: cd to repo root in strict gate, deny wildcard versions - quality_gate_strict.sh: add `cd` to repo root so the script works when invoked from any working directory. - deny.toml: change `wildcards = "allow"` to `"deny"` to catch `*` version requirements in dependencies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Henry Park
|
81f7b64994 |
fix(ci): disambiguate WASM bundle filenames to prevent tool/channel collision (#964)
* fix(ci): disambiguate WASM bundle filenames to prevent tool/channel collision When a tool and channel share the same name (e.g. slack, telegram), the CI build produced identical bundle filenames, causing the second to overwrite the first. Both manifests then pointed to the wrong binary. Prefix bundle filenames with the extension kind (tool-slack-... vs channel-slack-...) and parse the prefix when patching manifests, so each manifest receives the correct artifact URL and SHA256. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(registry): add installer tests for tool/channel name disambiguation Regression tests for the CI artifact collision fix (PR #964). Verifies: - extract_tar_gz rejects archives with wrong wasm name (the collision bug) - Tool bundle extracts slack-tool.wasm correctly - Channel bundle extracts slack.wasm correctly - Tool and channel manifests install to separate directories Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): add kind validation and filter non-WASM checksum entries - Validate .kind is "tool" or "channel" before using in build-wasm-extensions (hard error) - Filter checksums.txt to *-wasm32-wasip2.tar.gz entries before parsing, avoiding noisy warnings from binary artifact entries in build-local-artifacts - Add kind validation with warning+skip in both checksum-parsing loops Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix rustfmt formatting in installer tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
14aadd3063 |
refactor: make src/llm/ self-contained for crate extraction (#767)
* refactor: make src/llm/ self-contained for crate extraction Move LlmError, LLM config types, and OAuth callback helpers into src/llm/ so the module has zero `use crate::` imports outside of crate::llm. This prepares the module for extraction into a standalone workspace crate. - Move LlmError enum from src/error.rs to src/llm/error.rs - Move LlmConfig, NearAiConfig, RegistryProviderConfig, BedrockConfig, CacheRetention, OAUTH_PLACEHOLDER from src/config/llm.rs to src/llm/config.rs - Move OAuth callback utilities (callback_url, bind_callback_listener, wait_for_callback, landing_html, etc.) from src/cli/oauth_defaults.rs to src/llm/oauth_helpers.rs - Remove session.rs dependency on crate::bootstrap (inline default path) - Add cache_retention field to RegistryProviderConfig, resolve from env in config/llm.rs instead of reading env var in llm/mod.rs - Add Check 6 to scripts/check-boundaries.sh enforcing LLM isolation - All original locations re-export for backward compatibility [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR #767 review — session path bug and boundary check 1. Fix SessionConfig::default() usage in setup wizard: the fallback at wizard.rs:995 now constructs SessionConfig with the real default_session_path() instead of a relative "session.json", which would write auth tokens to the CWD instead of ~/.ironclaw/. 2. Widen check-boundaries.sh Check 6 to catch all `crate::` references (not just `use crate::` imports). Pre-existing inline references (16 occurrences) are reported as warnings; only new `use crate::` imports are hard violations. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR #767 review and audit findings in src/llm/ PR review fixes: - Reject wildcard addresses (0.0.0.0, ::) in OAuth callback listener to prevent session token exposure on all interfaces - Fix boundary check comment-stripping that could hide real violations (use sed to strip inline comments before matching) Audit fixes: - Fix UTF-8 byte-index slicing panic in recording.rs hint extraction - Add effective_model_name() delegation to RetryProvider and SmartRoutingProvider for consistency with other wrappers - Add calculate_cost() delegation to CachedProvider and RecordingLlm - Deduplicate retry loop logic in RetryProvider via generic helper - Replace hardcoded /tmp path in recording tests with tempfile Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
3b57d5bec9 |
chore: add reviewer-feedback guardrails (CLAUDE.md, pre-commit hook, skill) (#665)
* chore: add reviewer-feedback guardrails (CLAUDE.md, pre-commit hook, skill) Analysis of ~50 PRs from the past week identified 10 recurring themes in Copilot and Gemini code review comments. This change addresses them at development time through three layers: 1. CLAUDE.md additions (7 new rules): - Transaction safety for multi-step DB operations - UTF-8 string safety (no byte-index slicing) - Case-insensitive comparisons for paths/media types - Decorator/wrapper trait method delegation - Sensitive data redaction in logs/SSE - tempfile crate for test temporary files - Trust boundaries for worker container data 2. Pre-commit hook (scripts/pre-commit-safety.sh): Mechanical checks for unsafe byte slicing, case-sensitive extension comparisons, hardcoded /tmp paths, unredacted tool parameter logging, and non-transactional DB operations. Installed via dev-setup.sh alongside existing commit-msg hook. 3. Review checklist skill (skills/review-checklist/SKILL.md): Activates on "review"/"merge" keywords. Covers the judgment-based items that can't be linted: transaction safety, SSRF validation, approval checks, decorator delegation, test quality, and doc accuracy. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback on pre-commit-safety.sh - Cache diff output in variable to avoid ~10 redundant git diff calls (Gemini) - Add early exit when no .rs files are changed (Gemini) - Fix header comment: list all 5 checks, not just 4 (Copilot) - Fix check 2 comment: only mentions file extensions, not media types (Copilot) - Add resolve_base_ref() with fallback candidates instead of hardcoded origin/main for standalone mode (Copilot) - TX check: use -W (function context) to reduce false positives, honor // safety: suppression, print triggering lines (Copilot) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Zaki Manian
|
45ec691f4c |
Improve test infrastructure: StubChannel, gateway helpers, security tests, search edge cases (#623)
* feat(testing): add StubChannel test double for Channel trait Adds StubChannel to src/testing.rs alongside StubLlm. Supports message injection via mpsc sender, response/status capture, and configurable health check toggling. Includes handle methods for use after ownership transfer to ChannelManager. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(testing): wire StubChannel into TestHarnessBuilder Add with_stub_channel() builder method that creates a StubChannel pre-registered in a ChannelManager. Tests can inject messages via the sender and verify routing through the manager. The channel field on TestHarness is Optional, defaulting to None for backward compat. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: gate external-service tests behind integration feature flag Replace silent try_connect() skip pattern with explicit feature gating. cargo test now runs only self-contained tests. cargo test --features integration runs tests requiring PostgreSQL. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(channels): add ChannelManager unit tests using StubChannel Cover add/start_all stream merging, respond routing, unknown channel errors, health_check_all with mixed health, empty-channels error path, and injection channel merging -- all via StubChannel test double. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: document test tier separation (unit/integration/live) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: add architecture boundary check script Grep-based checks for three architecture boundaries: - Direct database driver usage (tokio_postgres/libsql) outside src/db/ - .unwrap()/.expect() in production code (warning only) - Direct std::env::var reads outside config layer (warning only) The DB driver check is a hard violation; the other two are warnings for gradual cleanup. Run with: bash scripts/check-boundaries.sh Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(search): add RRF edge case tests for empty inputs, limits, and config modes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(security): add regression tests for skill installer ZIP and SSRF protections Add 11 regression tests covering the security controls in skill_tools: ZIP extraction safety: - Valid SKILL.md extraction works correctly - Non-SKILL.md entries are ignored (returns error) - Path traversal entries (../../SKILL.md) do not match - Nested path entries (subdir/SKILL.md) do not match - Oversized entries (>1MB uncompressed) are rejected SSRF prevention: - Loopback addresses (127.0.0.1) are blocked - Private ranges (10.x, 172.16.x, 192.168.x) are blocked - Link-local addresses (169.254.x) are blocked - Public IPs (8.8.8.8, 1.1.1.1) are allowed - IPv4-mapped IPv6 unwrapping logic works correctly - Metadata endpoints and .internal/.local hostnames are blocked - Normal hostnames (github.com, clawhub.dev) are allowed Also documents a known gap: url::Url::host_str() returns bracketed IPv6 addresses that std::net::IpAddr cannot parse, so IPv4-mapped IPv6 URLs currently bypass IP-based checks in validate_fetch_url. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(testing): extract TestGatewayBuilder to eliminate gateway test duplication Both ws_gateway_integration.rs and openai_compat_integration.rs manually constructed GatewayState with 19+ fields. Extracted to a shared builder in src/channels/web/test_helpers.rs that provides sensible defaults and lets tests override only what they need. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add implementation plans for testing batches 1 and 2 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): close IPv6 SSRF bypass in validate_fetch_url validate_fetch_url used host_str() which returns bracketed IPv6 (e.g. "[::ffff:7f00:1]") that IpAddr::parse() cannot handle, silently skipping IP-based SSRF checks for all IPv6 URLs. Switch to url::Host enum matching to extract proper IpAddr values without string parsing. IPv4-mapped IPv6 addresses like ::ffff:127.0.0.1 are now correctly unwrapped and blocked. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(skills): add activation criteria limits enforcement tests Adds test_activation_criteria_enforce_limits to verify that enforce_limits() correctly trims excess patterns (>5), keywords (>20), and tags (>10), and filters out short keywords/tags (<3 chars). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(wasm): add security regression tests for WASM tool loader Add 6 tests covering: tool name path separator rejection, empty name rejection, nonexistent file handling, invalid WASM bytes rejection, dotfile discovery behavior, and subdirectory non-recursion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: address PR review feedback - Remove plan files from repo (ilblackdragon review) - Replace CLAUDE.md test tier rules with pointer to check-boundaries.sh - Add Check 4 to check-boundaries.sh: enforces integration tests are gated behind the 'integration' feature flag Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: add try_connect silent-skip pattern check to check-boundaries.sh Check 5 catches try_connect() and similar silent-skip patterns in integration tests. Tests should use feature gates to fail loudly when prerequisites are missing, not silently return. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): harden skill fetch SSRF checks * fix(scripts): use bash arrays in check-boundaries.sh tier violation check Refactor Check 4 in check-boundaries.sh to use bash arrays and printf instead of string concatenation with echo -e. This is more robust with special characters in filenames and avoids portability concerns with echo -e. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
04c5c3fe9f |
feat: WASM extension versioning with WIT compat checks (#592)
* feat: add WASM extension versioning with WIT compat checks and CI enforcement Phase 1 — WIT Versioning & Compatibility Checks: - Version WIT packages as `package near:agent@0.2.0;` - Add `semver` crate for version parsing and comparison - Add `WIT_TOOL_VERSION` / `WIT_CHANNEL_VERSION` host constants - Add `version` and `wit_version` fields to capabilities schemas - Add `wit_version` column to `wasm_tools` DB table (both backends) - Add load-time `check_wit_version_compat()` with semver rules - Add `IncompatibleWitVersion` error variants for tools and channels - Enhance instantiation errors with WIT version mismatch hints - Update all 14 capabilities JSON and 14 registry JSON files Phase 2 — Upgrade-in-Place & Channel DB Storage: - Change tool store to DELETE-before-INSERT (one version per extension) - Create `wasm_channels` table (PostgreSQL migration + libSQL schema) - Add `WasmChannelStore` trait with PostgreSQL and libSQL backends - Add `extension_info` tool showing version, WIT version, and status - Wire `ExtensionInfoTool` into tool registry (7 extension tools) Phase 3 — CI Version-Bump Enforcement: - Add `scripts/check-version-bumps.sh` checking WIT/tool/channel versions - Add `version-check` CI job (PR-only) to `.github/workflows/test.yml` - Support `[skip-version-check]` label/commit message bypass Includes 7 regression tests for WIT version compatibility checking and 2 integration tests for WIT version annotation verification. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback for WASM extension versioning - Wrap PostgreSQL DELETE+INSERT in transactions for both tool and channel store() methods to prevent data loss on partial failure (Gemini, Copilot) - Rename StoredWasmChannelWithBinary.tool → .channel (copy-paste fix) - Remove unused WasmError::IncompatibleWitVersion variant (dead code) - Map channel loader WIT mismatch to IncompatibleWitVersion instead of generic Config error, simplify variant to single String message - Fix extension_info description to match actual returned fields - Add schema test for ExtensionInfoTool matching existing test pattern - Fix CI script to fail fast on git errors instead of silent bypass [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Illia Polosukhin
|
46218ec794 |
test: add WIT compatibility tests for WASM extensions (#586)
* test: add WIT compatibility tests for all WASM tools and channels Adds CI and integration tests to catch WIT interface breakage across all 14 WASM extensions (10 tools + 4 channels). Previously, changing wit/tool.wit or wit/channel.wit could silently break guest-side tools that weren't rebuilt until release time. Three new pieces: 1. scripts/build-wasm-extensions.sh — builds all WASM extensions from source by reading registry manifests. Used by CI and locally. 2. tests/wit_compat.rs — integration tests that compile and instantiate each .wasm binary against the current wasmtime host linker with stubbed host functions. Catches added/removed/renamed WIT functions, signature mismatches, and missing exports. Skips gracefully when artifacts aren't built so `cargo test` still passes standalone. 3. .github/workflows/test.yml — new wasm-wit-compat CI job that builds all extensions then runs instantiation tests on every PR. Added to the branch protection roll-up. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix rustfmt formatting in wit_compat tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback on WIT compat tests - Switch build script from python3 to jq for JSON parsing, consistent with release.yml and avoids python3 dependency (#1, #7) - Use dirs::home_dir() instead of HOME env var for portability (#2) - Filter extensions by manifest "kind" field instead of path (#3) - Replace .flatten() with explicit error handling in dir iteration (#4, #5) - Split stub_tool_host_functions into stub_shared_host_functions + tool-only tool-invoke stub, since tool-invoke is not in channel WIT (#6) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Zaki Manian
|
b4b19738a8 |
Trajectory benchmarks and e2e trace test rig (#553)
* refactor: extract shared assertion helpers to support/assertions.rs Move 5 assertion helpers from e2e_spot_checks.rs to a shared module. Add assert_all_tools_succeeded and assert_tool_succeeded for eliminating false positives in E2E tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add tool output capture via tool_results() accessor Extract (name, preview) from ToolResult status events in TestChannel and TestRig, enabling content assertions on tool outputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: correct tool parameters in 3 broken trace fixtures - tool_time.json: add missing "operation": "now" for time tool - robust_correct_tool.json: same fix - memory_full_cycle.json: change "path" to "target" for memory_write Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add tool success and output assertions to eliminate false positives Every E2E test that exercises tools now calls assert_all_tools_succeeded. Added tool output content assertions where tool results are predictable (time year, read_file content, memory_read content). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: capture per-tool timing from ToolStarted/ToolCompleted events Record Instant on ToolStarted and compute elapsed duration on ToolCompleted, wiring real timing data into collect_metrics() instead of hardcoded zeros. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: add RAII CleanupGuard for temp file/dir cleanup in tests Replace manual cleanup_test_dir() calls and inline remove_file() with Drop-based CleanupGuard that ensures cleanup even if a test panics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add Drop impl and graceful shutdown for TestRig Wrap agent_handle in Option so Drop can abort leaked tasks. Signal the channel shutdown before aborting for future cooperative shutdown. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: replace agent startup sleep with oneshot ready signal Use a oneshot channel fired in Channel::start() instead of a fixed 100ms sleep, eliminating the race condition on slow systems. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: replace fragile string-matching iteration limit with count-based detection Use tool completion count vs max_tool_iterations instead of scanning status messages for "iteration"/"limit" substrings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use assert_all_tools_succeeded for memory_full_cycle test Remove incorrect comment about memory_tree failing with empty path (it actually succeeds). Omit empty path from fixture and use the standard assert_all_tools_succeeded instead of per-tool assertions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: promote benchmark metrics types to library code Move TraceMetrics, ScenarioResult, RunResult, MetricDelta, and compare_runs() from tests/support/metrics.rs to src/benchmark/metrics.rs. Existing tests use re-export for backward compatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add Scenario and Criterion types for agent benchmarking Scenario defines a task with input, success criteria, and resource limits. Criterion is an enum of programmatic checks (tool_used, response_contains, etc.) evaluated without LLM judgment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add initial benchmark scenario suite (12 scenarios across 5 categories) Scenarios cover tool_selection, tool_chaining, error_recovery, efficiency, and memory_operations. All loaded from JSON with deserialization validation test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add benchmark runner with BenchChannel and InstrumentedLlm BenchChannel is a minimal Channel implementation for benchmarks. InstrumentedLlm wraps any LlmProvider to capture per-call metrics. Runner creates a fresh agent per scenario, evaluates success criteria, and produces RunResult with timing, token, and cost metrics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add baseline management, reports, and benchmark entry point - baseline.rs: load/save/promote benchmark results - report.rs: format comparison reports with regression detection - benchmark_runner.rs: integration test with real LLM (feature-gated) - Add benchmark feature flag to Cargo.toml Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: apply cargo fmt to benchmark module Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): add multi-turn scenario types with setup, judge, ResponseNotContains Add BenchScenario, Turn, TurnAssertions, JudgeConfig, ScenarioSetup, WorkspaceSetup, SeedDocument types for multi-turn benchmark scenarios. Add ResponseNotContains criterion variant. Add TurnAssertions::to_criteria() converter for backward compat with existing evaluation engine. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): add JSON scenario loader with recursive discovery and tag filter Add load_bench_scenarios() for the new BenchScenario format with recursive directory traversal and tag-based filtering. Create 4 initial trajectory scenarios across tool-selection, multi-turn, and efficiency categories. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): multi-turn runner with workspace seeding and per-turn metrics Add run_bench_scenario() that loops over BenchScenario turns, seeds workspace documents, collects per-turn metrics (tokens, tool calls, wall time), and evaluates per-turn assertions. Add TurnMetrics to metrics.rs and clear_for_next_turn() to BenchChannel. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): add LLM-as-judge scoring with prompt formatting and score parsing Create judge.rs with format_judge_prompt, parse_judge_score, and judge_turn. Wire into run_bench_scenario for turns with judge config -- scores below min_score fail the turn. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): add CLI subcommand (ironclaw benchmark) Add BenchmarkCommand with --tags, --scenario, --no-judge, --timeout, --update-baseline flags. Wire into Command enum and main.rs dispatch. Feature-gated behind benchmark flag. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): per-scenario JSON output with full trajectory Add save_scenario_results() that writes per-scenario JSON files alongside the run summary. Each scenario gets its own file with turn_metrics trajectory. Update CLI to use new output format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): add ToolRegistry::retain_only and wire tool filtering in scenarios Add a retain_only() method to ToolRegistry that filters tools down to a given allowlist. Wire this into run_bench_scenario() so that when a scenario specifies a tools list in its setup, only those tools are available during the benchmark run. Includes two tests for the new method: one verifying filtering works and one verifying empty input is a no-op. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): wire identity overrides into workspace before agent start Add seed_identity() helper that writes identity files (IDENTITY.md, USER.md, etc.) into the workspace before the agent starts, so that workspace.system_prompt() picks them up. Wire it into run_bench_scenario() after workspace seeding. Include a test that verifies identity files are written and readable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): add --parallel and --max-cost CLI flags Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(benchmark): use feature-conditional snapshot names for CLI help tests Prevents snapshot conflicts between default (no benchmark) and all-features (with benchmark) builds by using separate snapshot names per feature set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): parallel execution with JoinSet and budget cap enforcement Replace sequential loop in run_all_bench() with parallel execution using JoinSet + semaphore when config.parallel > 1. Add budget cap enforcement that skips remaining scenarios when max_total_cost_usd is exceeded. Track skipped count in RunResult.skipped_scenarios and display it in format_report(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): add tool restriction and identity override test scenarios Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: fix formatting for Phase 3 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): add SkillRegistry::retain_only and wire skill filtering in scenarios Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(benchmark): add --json flag for machine-readable output Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: add GitHub Actions benchmark workflow (manual trigger) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(benchmark): remove in-tree benchmark harness, keep retain_only utilities Move benchmark-specific code out of ironclaw in preparation for the nearai/benchmarks trajectory adapter. This removes: - src/benchmark/ (runner, scenarios, metrics, judge, report, etc.) - src/cli/benchmark.rs and the Benchmark CLI subcommand - benchmarks/ data directory (scenarios + trajectories) - .github/workflows/benchmark.yml - The "benchmark" Cargo feature flag What remains: - ToolRegistry::retain_only() and SkillRegistry::retain_only() - Test support types (TraceMetrics, InstrumentedLlm) inlined into tests/support/ instead of re-exporting from the deleted module Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add README for LLM trace fixture format Documents the trajectory JSON format, response types, request hints, directory structure, and how to write new traces. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(test): unify trace format around turns, add multi-turn support Introduce TraceTurn type that groups user_input with LLM response steps, making traces self-contained conversation trajectories. Add run_trace() to TestRig for automatic multi-turn replay. Backward-compatible: flat "steps" JSON is deserialized as a single turn transparently. Includes all trace fixtures (spot, coverage, advanced), plan docs, and new e2e tests for steering, error recovery, long chains, memory, and prompt injection resilience. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): fix CI failures after merging main - Fix tool_json fixture: use "data" parameter (not "input") to match JsonTool schema - Fix status_events test: remove assertion for "time" tool that isn't in the fixture (only "echo" calls are used) - Allow dead_code in test support metrics/instrumented_llm modules (utilities for future benchmark tests) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Working on recording traces and testing them * feat(test): add declarative expects to trace fixtures, split infra tests Add TraceExpects struct with 9 optional assertion fields (response_contains, tools_used, all_tools_succeeded, etc.) that can be declared in fixture JSON instead of hand-written Rust. Add verify_expects() and run_recorded_trace() so recorded trace tests become one-liners. Split trace infra tests (deserialization, backward compat) into tests/trace_format.rs which doesn't require the libsql feature gate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(test): add expects to all trace fixtures, simplify e2e tests Add declarative expects blocks to all 19 trace fixture JSONs across spot/, coverage/, advanced/, and root directories. Update all 8 e2e test files to use verify_trace_expects() / run_and_verify_trace(), replacing ~270 lines of hand-written assertions with fixture-driven verification. Tests that check things beyond expects (file content on disk, metrics, event ordering) keep those extra assertions alongside the declarative ones. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): adapt tests to AppBuilder refactor, fix formatting Update test files to work with refactored TestRigBuilder that uses AppBuilder::build_all() (removing with_tools/with_workspace methods). Update telegram_check fixture to use tool_list instead of echo. Fix cargo fmt issues in src/llm/mod.rs and src/llm/recording.rs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(test): deduplicate support unit tests into single binary Support modules (assertions, cleanup, test_channel, test_rig, trace_llm) had #[cfg(test)] mod tests blocks that were compiled and run 12 times — once per e2e test binary that declares `mod support;`. Extracted all 29 support unit tests into a dedicated `tests/support_unit_tests.rs` so they run exactly once. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix trailing newlines in support files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(test): unify trace types and fix recorded multi-turn replay Import shared types (TraceStep, TraceResponse, TraceToolCall, RequestHint, ExpectedToolResult, MemorySnapshotEntry, HttpExchange*) from ironclaw::llm::recording instead of redefining them in trace_llm.rs. Fix the flat-steps deserializer to split at UserInput boundaries into multiple turns, instead of filtering them out and wrapping everything into a single turn. This enables recorded multi-turn traces to be replayed as proper multi-turn conversations via run_trace(). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): fix CI failures - unused imports and missing struct fields - Add #[allow(unused_imports)] on pub use re-exports in trace_llm.rs (types are re-exported for downstream test files, not used locally) - Add `..` to ToolCompleted pattern in test_channel.rs to match new `error` and `parameters` fields Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): fix CI failures after merging main - Add missing `error` and `parameters` fields to ToolCompleted constructors in support_unit_tests.rs - Add `..` to ToolCompleted pattern match in support_unit_tests.rs - Add #[allow(dead_code)] to CleanupGuard, LlmTrace impl, and TraceLlm impl (only used behind #[cfg(feature = "libsql")]) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Adding coverage running script * fix(test): address review feedback on E2E test infrastructure - Increase wait_for_responses polling to exponential backoff (50ms-500ms) and raise default timeout from 15s to 30s to reduce CI flakiness (#1) - Strengthen prompt_injection_resilience test with positive safety layer assertion via has_safety_warnings(), enable injection_check (#2) - Add assert_tool_order() helper and tools_order field in TraceExpects for verifying tool execution ordering in multi-step traces (#3) - Document TraceLlm sequential-call assumption for concurrency (#6) - Clean up CleanupGuard with PathKind enum instead of shotgun remove_file + remove_dir_all on every path (#8) - Fix coverage.sh: default to --lib only, fix multi-filter syntax, add COV_ALL_TARGETS option - Add coverage/ to .gitignore - Remove planning docs from PR [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review - use HashSet in retain_only, improve skill test - Use HashSet for O(N+M) lookup in SkillRegistry::retain_only and ToolRegistry::retain_only instead of linear scan - Strengthen test_retain_only_empty_is_noop in SkillRegistry to pre-populate with a skill before asserting the no-op behavior [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): revert incorrect safety layer assertion in injection test The safety layer sanitizes tool output, not user input. The injection test sends a malicious user message with no tools called, so the safety layer never fires. Reverted to the original test which correctly validates the LLM refuses via trace expects. Also fixed case-sensitive request hint ("ignore" -> "Ignore") to suppress noisy warning. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: clean stale profdata before coverage run Adds `cargo llvm-cov clean` before each run to prevent "mismatched data" warnings from stale instrumentation profiles. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix formatting in retain_only test [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com> |
||
|
Illia Polosukhin
|
f60c91e9a7 |
ci: enforce regression tests for fix commits (#517)
* ci: enforce regression tests for fix commits Add a commit-msg hook and CI workflow that require test changes alongside bug fix commits, ensuring every fix includes a regression test that would have caught the bug. - scripts/commit-msg-regression.sh: local git hook (blocks fix commits without test changes; exempts static/docs-only; bypass via [skip-regression-check] marker) - .github/workflows/regression-test-check.yml: CI mirror on PRs (checks title + commit messages; skip via label) - scripts/dev-setup.sh: install hook in step 6 - .github/scripts/create-labels.sh: add skip-regression-check label - CLAUDE.md: document regression test policy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback on regression test enforcement - Use here-strings instead of echo|grep to avoid misinterpreting special characters in variables - Use git diff -W (whole-function context) to detect edits inside existing test functions, not just new #[test] attributes - Honor [skip-regression-check] in commit messages in CI (not just the PR label) - Use git rev-parse --git-path hooks for worktree-safe hook install [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update .github/workflows/regression-test-check.yml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Illia Polosukhin
|
ffb1cc9be8 |
refactor: architecture improvements for contributor velocity (#198)
* refactor: split large files and consolidate test stubs for contributor velocity - Extract 7 Database sub-traits (ConversationStore, JobStore, SandboxStore, RoutineStore, ToolFailureStore, SettingsStore, WorkspaceStore) with Database as a supertrait combining them all - Split libsql_backend.rs (2769 lines) into src/db/libsql/ directory with one file per sub-trait implementation - Split config.rs (1753 lines) into src/config/ directory with 16 domain files - Consolidate 3 duplicate test LLM stubs into shared StubLlm in src/testing.rs - Split server.rs handlers into src/channels/web/handlers/ directory - Extract main.rs init phases into AppBuilder (src/app.rs) - Add developer setup script (scripts/dev-setup.sh) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: move heartbeat test from examples/ to tests/ Convert standalone example binary into a proper #[ignore] integration test, matching the convention of the other integration tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix rustfmt formatting for CI Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review comments from Copilot - tunnel.rs: replace .ok().flatten() with ? to propagate env var errors - secrets.rs: remove misleading "process-wide cache" comment - database.rs: use uppercase "DATABASE_URL" in error key - testing.rs: gate harness tests with #[cfg(feature = "libsql")] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Illia Polosukhin <ilblacdragon@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Ilgın Kanat
|
115b7f38fe |
DM pairing + Telegram channel improvements (#17)
* feat: Implement DM pairing for channels - Introduced a new pairing system to manage direct messages from unknown senders. - Added `PairingStore` to handle pending requests and allowlist management. - Implemented CLI commands for listing and approving pairing requests. - Updated Telegram channel to utilize the new pairing logic, including workspace paths for storing pairing data. - Enhanced WASM channel integration to support pairing functionality. This feature enhances security by requiring approval for unknown senders before they can interact with the agent. * Enhance Telegram channel support with media captioning and DM pairing features - Added support for media captions in Telegram messages, allowing for richer content handling. - Updated message processing to utilize either text or caption, improving message flexibility. - Enhanced DM pairing functionality to include approval and listing capabilities for direct messages. - Updated feature parity documentation to reflect new capabilities and improvements in Telegram integration. * Update README and BUILDING_CHANNELS documentation for Telegram channel integration - Enhanced README with instructions for building and running the Telegram channel, including a note on running `./scripts/build-all.sh` for full releases. - Added detailed steps in BUILDING_CHANNELS.md for building and deploying the Telegram channel, emphasizing the need to run `./channels-src/telegram/build.sh` before building the main crate to ensure updated WASM is included. - Updated CLI module to expose a new command for pairing with store functionality. * Implement build script for Telegram channel WASM and enhance pairing error handling - Added a new `build.rs` script to automate the compilation of the Telegram channel's WASM binary from source, ensuring reproducible builds and emphasizing supply chain security by preventing committed binaries. - Updated `BUILDING_CHANNELS.md` to reflect the new build process and the importance of not committing compiled binaries. - Enhanced error handling in the pairing approval process to include rate limiting for failed attempts, improving security and user feedback. * Remove Telegram channel WASM binary file as part of the build process cleanup, ensuring no committed binaries are present in the repository. |
