feat(common): apply ExtensionName newtype to fan-out sites (PR 2/2) by ilblackdragon · Pull Request #2617 · nearai/ironclaw

ilblackdragon · 2026-04-18T05:57:37Z

Summary

Follow-up to #2611. Migrates the remaining stringly-typed extension_name and credential_name fields to the newtypes introduced in ironclaw_common::identity.

Stacks on #2611 — base is feat/identity-newtypes-pr1. Rebase onto staging once PR 1 merges.

Newly typed

Wire boundary (serde transparent — JSON unchanged)

AppEvent::OnboardingState.extension_name
AppEvent::GateRequired.extension_name (Option<ExtensionName>)
AppEvent::ExtensionStatus.extension_name

Internal types

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, the selected_auth_prompt tuple
resolve_extension_for_action() and resolve_auth_gate_display_name() return types
emit_auth_required_status() / Session::enter_auth_mode() / event_from_configure_result() parameters
normalize_extension_name() return type

Behavioral change

PendingAuthPrompt::new is now infallible: it accepts ExtensionName directly since the identity validator already 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 now.

Conversion discipline

ExtensionName::new(\"...\").unwrap() at test construction sites (validates and self-documents).
ExtensionName::from_trusted(...) where a trusted upstream String (DB row, registry entry, legacy field) is being adapted. Every from_trusted call site is an explicit audit trail of where a typed boundary is being crossed.

Test plan

cargo fmt --check
cargo clippy --all --benches --tests --examples --all-features — zero warnings
cargo test --lib — 5034/5034
cargo test --test engine_v2_gate_integration — 26/26
cargo test -p ironclaw_common — 40/40
Integration + e2e smoke after merge

Left for a later PR

parse_credential_name → Option<CredentialName> — the parser has narrower anti-injection semantics (rejects hyphens rather than canonicalizing) that differ from CredentialName::new. Wants a deliberate ::strict_parse constructor before we convert it.
Secrets store name: String — stays String at the DB boundary.

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.

gemini-code-assist

Code Review

This pull request replaces raw strings with a dedicated ExtensionName newtype for extension identifiers across the codebase, including events, session management, and authentication flows. Feedback points out a security vulnerability where user input bypasses validation via from_trusted in the web server. Additionally, several renames are suggested for functions and parameters to align with the new type, and concerns were raised about the semantic misuse of ExtensionName for non-authentication action names.

gemini-code-assist · 2026-04-18T06:00:04Z

            let mut onboarding_event =
                crate::channels::web::onboarding::event_from_configure_result(
-                    name.clone(),
+                    ironclaw_common::ExtensionName::from_trusted(name.clone()),


Using from_trusted for user-provided input (from req.name) bypasses the validation logic of the ExtensionName newtype. At the public API boundary, ExtensionName::new() must be used to validate against path traversal characters (/, , .., \0) to prevent security vulnerabilities and ensure type invariants are maintained, as from_trusted is reserved for trusted upstream sources.

References

Prefer creating a newtype wrapper that validates on construction ('Parse, Don't Validate') to ensure type safety and centralize validation at system boundaries.

Path traversal characters (/, , .., \0) in extension names must be validated at the public API boundary for all operations (install, activate, remove) to prevent security vulnerabilities.

Fixed in 7af6a7e — agreed, this was a real security gap. extensions_setup_submit_handler now validates the URL path segment with ExtensionName::new at entry and returns 400 on failure; all three downstream from_trusted wraps in the same handler are gone. Added a regression test (test_extensions_setup_submit_rejects_path_traversal_name) that drives the handler with malformed path segments (..%2Ftraversal, slash%2Fname, BadCase, has%20space, trailing_) and asserts 400 before the value reaches extension lookup.

Also codified this as a general rule (a3d489f): src/channels/web/CLAUDE.md now lists the setup/configure-route validation requirement, and a new pre-commit check (CREDNAME in scripts/pre-commit-safety.sh) flags CredentialName references in web-layer code so the next violation trips at commit time.

gemini-code-assist · 2026-04-18T06:00:04Z

    tools: &crate::tools::ToolRegistry,
    pending: &PendingGate,
-) -> String {
+) -> ironclaw_common::ExtensionName {


The function resolve_auth_gate_display_name now returns an ExtensionName (an identifier/slug) rather than a human-readable display name. Consider renaming this function to resolve_auth_gate_extension_name to better reflect its return type and avoid confusion with actual display names (e.g., those found in PendingOAuthLaunchParams).

Renamed in 7af6a7e to resolve_auth_gate_extension_name. Agreed — the old name was a leftover from when the return type was String; "display name" conflicted with the actual human-readable display name (e.g., PendingOAuthLaunchParams.display_name).

gemini-code-assist · 2026-04-18T06:00:04Z

+        // Non-authentication gates don't use this name; return
        // something innocuous.
-        pending.action_name.clone()
+        ironclaw_common::ExtensionName::from_trusted(pending.action_name.clone())


Returning pending.action_name wrapped in ExtensionName::from_trusted for non-authentication gates is semantically incorrect, as an action name is not an extension identifier. This weakens the type safety benefits of the 'Parse, Don't Validate' pattern. Consider if this branch should return an Option<ExtensionName> or if the caller should be structured to avoid this call for non-auth gates.

References

Prefer creating a newtype wrapper that validates on construction ('Parse, Don't Validate') to ensure type safety and centralize validation at system boundaries.

Fixed in 7af6a7e. The function now returns Option<ExtensionName> and short-circuits to None for non-Authentication gates instead of fabricating an ExtensionName from pending.action_name. Callers thread the Option through: send_pending_gate_status takes Option<&ExtensionName> and only uses it on the Authentication arm (with a warn! log if upstream plumbing ever violates the invariant), and the GateRequired SSE event's extension_name is now a direct .clone() of the Option — no more matches!(...).then(|| ...) dance.

gemini-code-assist · 2026-04-18T06:00:04Z

    message: &IncomingMessage,
    pending: &PendingGate,
-    auth_display_name: &str,
+    auth_display_name: &ironclaw_common::ExtensionName,


The parameter auth_display_name should be renamed to extension_name (or similar) to match its new type ExtensionName and its usage in StatusUpdate::AuthRequired. Using 'display_name' for an identifier slug is misleading.

Renamed in 7af6a7e — auth_display_name → extension_name, consistent with both the ExtensionName type and the StatusUpdate::AuthRequired.extension_name field it feeds.

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.

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.

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()`.

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.

…ypes-pr2 # Conflicts: # src/channels/web/server.rs # src/extensions/manager.rs

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.

Copilot

Pull request overview

Migrates remaining extension_name (and some related auth-flow fields) from String to the ironclaw_common::identity::ExtensionName newtype across the gateway, bridge, channels, and tests, while keeping wire JSON unchanged.

Changes:

Replace stringly-typed extension identifiers with ExtensionName in SSE/AppEvent DTOs, StatusUpdate/TUI events, OAuth pending flow types, and auth-mode state.
Harden /api/extensions/{name}/setup submit by validating the path segment as an ExtensionName at handler entry; add regression coverage and web-layer identity linting.
Update tests and supporting docs to match the new typed boundaries.

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/e2e_live.rs	Adjusts assertions to work with `ExtensionName` (`as_str()`).
src/extensions/manager.rs	Wraps/broadcasts extension identifiers as `ExtensionName`; updates OAuth flow handling comparisons; updates tests.
src/channels/web/types.rs	Updates `PendingGateInfo.extension_name` to `Option<ExtensionName>` and fixes related tests.
src/channels/web/server.rs	Switches many auth/extension flows to `ExtensionName`; validates setup-submit path segment; adds regression test.
src/channels/web/responses_api.rs	Updates tests for `PendingGateInfo` to use `ExtensionName`.
src/channels/web/platform/static_files.rs	Comment clarifications; adds safety note on string slicing.
src/channels/web/onboarding.rs	Updates `event_from_configure_result` to accept `ExtensionName`.
src/channels/web/CLAUDE.md	Documents boundary rules for identity types in web layer and canonical resolver usage.
src/channels/wasm/wrapper.rs	Updates WASM channel tests to use `ExtensionName`.
src/channels/channel.rs	Changes `StatusUpdate::{AuthRequired,AuthCompleted}.extension_name` to `ExtensionName`.
src/bridge/router.rs	Changes auth-gate extension resolution to return typed `ExtensionName` and plumbs through to status events.
src/bridge/auth_manager.rs	Updates pending OAuth launch params to carry `ExtensionName`.
src/bridge/CLAUDE.md	Adds bridge module documentation emphasizing centralized auth/identity resolution.
src/auth/oauth.rs	Types `PendingOAuthFlow.extension_name` as `ExtensionName`.
src/auth/mod.rs	Types `PendingOAuthLaunchParams.extension_name` as `ExtensionName`.
src/agent/thread_ops.rs	Updates auth token submission path to pass `&str` from `ExtensionName`.
src/agent/session.rs	Types pending-auth state/prompts with `ExtensionName`; makes `PendingAuthPrompt::new` infallible.
src/agent/dispatcher.rs	Types parsed/persisted auth prompt extension names; updates normalization and persistence plumbing.
scripts/pre-commit-safety.sh	Adds a new pre-commit check preventing `CredentialName` usage in web-layer code.
crates/ironclaw_tui/src/event.rs	Types TUI auth events with `ExtensionName`.
crates/ironclaw_tui/Cargo.toml	Adds `ironclaw_common` dependency to support `ExtensionName` in the TUI.
crates/ironclaw_common/src/event.rs	Updates `AppEvent`/`OnboardingStateDto` extension fields to `ExtensionName` (serde-transparent).
Cargo.lock	Records the new dependency edge (`ironclaw_tui` → `ironclaw_common`).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-18T09:53:56Z

+    // Resolver returns a trusted identity string — either a real extension
+    // name from the manager, the provider-extension hint off the tool, or
+    // the credential-name fallback when no extension owns the action.
+    ironclaw_common::ExtensionName::from_trusted(raw)


resolve_extension_name_for_auth_flow can return the raw parameters["name"] string for tool_install/tool_activate without validation (see src/bridge/auth_manager.rs:334-345). Wrapping that output with ExtensionName::from_trusted means untrusted/user-influenced values (including path separators, uppercase, spaces, etc.) can flow through as an ExtensionName. Consider validating/canonicalizing here (e.g., ExtensionName::new(&raw) with a fallback to other resolution branches or to credential_fallback) instead of using from_trusted on this path.

Suggested change

// Resolver returns a trusted identity string — either a real extension

// name from the manager, the provider-extension hint off the tool, or

// the credential-name fallback when no extension owns the action.

ironclaw_common::ExtensionName::from_trusted(raw)

// The auth-flow resolver may surface user-influenced values for some

// actions, so validate/canonicalize before promoting to ExtensionName.

match ironclaw_common::ExtensionName::new(&raw) {

Ok(extension_name) => extension_name,

Err(error) => {

debug!(

action_name,

raw = %raw,

credential_fallback,

%error,

"Rejected invalid extension name resolved for auth flow; falling back"

);

ironclaw_common::ExtensionName::new(credential_fallback).unwrap_or_else(

|fallback_error| {

debug!(

credential_fallback,

%fallback_error,

"Credential fallback is not a canonical extension name; preserving legacy trusted fallback"

);

ironclaw_common::ExtensionName::from_trusted(

credential_fallback.to_string(),

)

},

)

}

}

Fixed in c813caa — you're right, and I went further: the fix belongs at the resolver, not the call site. AuthManager::resolve_extension_name_for_auth_flow now returns ExtensionName directly. Branch 1 (the user-influenced tool_install / tool_activate / tool_auth name param) validates via ExtensionName::new and falls through on failure instead of short-circuiting with an unvalidated string. Branches 2–4 (provider-extension hint, canonicalized action name, caller credential fallback) use from_trusted — their sources are typed upstream state.

Consolidating at the resolver matches the "one resolver, no re-derivation" rule in src/bridge/CLAUDE.md and drops the from_trusted wrap at both callers (router.rs resolve_extension_for_action, server.rs pending_gate_extension_name).

Copilot · 2026-04-18T09:53:57Z

+        // The resolver returns a trusted identity string — either a real
+        // extension name from the manager, the provider-extension hint off
+        // the tool, or the credential-name fallback when no extension owns
+        // the action. Either way it's sourced from typed upstream state.
+        return Some(ironclaw_common::ExtensionName::from_trusted(resolved));


resolved ultimately comes from AuthManager::resolve_extension_name_for_auth_flow, which may return an unvalidated parameters["name"] value for tool_install/tool_activate. Converting it via ExtensionName::from_trusted allows malformed/path-traversal strings to be treated as a typed ExtensionName. Prefer validating/canonicalizing (e.g., ExtensionName::new(&resolved) and returning None/fallback on error) rather than using from_trusted here.

Fixed in c813caa (same root fix as Copilot's sibling comment on router.rs:109). The resolver now returns a validated ExtensionName, so the from_trusted wrap here collapses to a direct Some(resolver.await).

Copilot · 2026-04-18T09:53:57Z

    let pending = pending?;
-    let validated = PendingAuthPrompt::new(
-        pending.extension_name,
-        pending.instructions,
-        pending.auth_url,
-        pending.setup_url,
-        pending.awaiting_token,
-    )?;
    Some((
-        validated.extension_name.clone(),
+        pending.extension_name.clone(),
        ParsedAuthData {
-            extension_name: Some(validated.extension_name),
-            instructions: validated.instructions,
-            auth_url: validated.auth_url,
-            setup_url: validated.setup_url,
-            awaiting_token: validated.awaiting_token,
+            extension_name: Some(pending.extension_name),


PendingAuthPrompt is Deserialized via #[serde(transparent)] ExtensionName, which does not validate on deserialization. With the removal of the re-validation step, an old/invalid persisted prompt can now be restored and propagated as a supposedly-typed ExtensionName. Consider re-validating/canonicalizing here (e.g., ExtensionName::new(pending.extension_name.as_str())) and returning None on failure to preserve the previous safety behavior for legacy rows.

Fixed in c813caa. restore_selected_auth_prompt now re-validates pending.extension_name via ExtensionName::new and drops + warn!s the prompt on failure. This restores the pre-PR2 behaviour (PendingAuthPrompt::new(String, ...) rejected empty strings on restore) and upgrades it to the full identity invariant — uppercase, path-separators, consecutive underscores, etc. are all rejected now.

Added regression test test_restore_selected_auth_prompt_rejects_invalid_legacy_row that forges three legacy rows (empty / uppercase / ../traversal) straight through serde and asserts each is dropped.

Copilot · 2026-04-18T09:53:57Z

+    /// Create a new `PendingAuthPrompt` from an already-validated
+    /// [`ExtensionName`]. Infallible — the identity type carries the
+    /// non-empty invariant that this constructor used to check.


The docstring/contract for PendingAuthPrompt::new is now inconsistent: the type no longer trims/validates, and ExtensionName can be constructed via from_trusted or deserialization without enforcing non-empty. Please update the comment to avoid implying a non-empty invariant that isn't guaranteed in all construction paths (and/or document which callers are required to validate).

Fixed in c813caa. Refreshed the PendingAuthPrompt struct doc + field doc to match the new contract:

The non-empty / canonical-form invariant lives in ExtensionName itself, validated at its own construction sites (::new / TryFrom).

#[serde(transparent)] deserialize does not re-validate — explicitly called out in the struct doc.

Rehydration sites (specifically restore_selected_auth_prompt in dispatcher.rs) re-run ExtensionName::new so legacy invalid rows are dropped.

Paired with fix C so the doc now matches the code, not the prior stringly-typed contract.

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.

…ypes-pr2

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 c813caa. - src/channels/web/server.rs:3831 — test used `.as_deref()` on the function's Option<ExtensionName> return; switched to `.as_ref().map(|n| n.as_str())` matching the pattern from the adjacent test. No new logic — just adapting two staging landings to the typed surface PR #2617 introduces. The validation behaviour for the fallback path is already locked in by the identity-layer tests in `ironclaw_common::identity` (rejects_path_traversal, rejects_uppercase, etc.) and by the regression test added in c813caa (test_restore_selected_auth_prompt_rejects_invalid_legacy_row). [skip-regression-check] — type adaptation to unblock CI, no behaviour change needing its own regression test. Clippy with `-D warnings` clean, 5074 lib tests pass.

…earai#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 (nearai#2561, nearai#2473, nearai#2512, nearai#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 nearai#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 nearai#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 nearai#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 nearai#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 nearai#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 (nearai#2561, nearai#2473, nearai#2512, nearai#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 nearai#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 nearai#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 c813caa. - src/channels/web/server.rs:3831 — test used `.as_deref()` on the function's Option<ExtensionName> return; switched to `.as_ref().map(|n| n.as_str())` matching the pattern from the adjacent test. No new logic — just adapting two staging landings to the typed surface PR nearai#2617 introduces. The validation behaviour for the fallback path is already locked in by the identity-layer tests in `ironclaw_common::identity` (rejects_path_traversal, rejects_uppercase, etc.) and by the regression test added in c813caa (test_restore_selected_auth_prompt_rejects_invalid_legacy_row). [skip-regression-check] — type adaptation to unblock CI, no behaviour change needing its own regression test. Clippy with `-D warnings` clean, 5074 lib tests pass. * fix(auth): extract shared resolver; wrapper delegates instead of duplicating Addresses two Copilot comments on PR nearai#2617 that surfaced the same architectural issue: the no-auth-manager fallback in `pending_gate_extension_name` had grown a three-branch copy of the resolver's precedence that quietly skipped branch 3 (canonicalize action_name + check `ExtensionManager::extension_info`). Exactly the duplicate-resolution drift the "one resolver" rule in `src/bridge/CLAUDE.md` warns against — four prior identity bugs (nearai#2561, nearai#2473, nearai#2512, nearai#2574) were the same pattern. - Extracted `pub(crate) async fn resolve_auth_flow_extension_name` to `src/bridge/auth_manager.rs` as the single site of the four-branch precedence. Takes `Option<&ToolRegistry>` + `Option<&ExtensionManager>` so both the `AuthManager` method (which passes its own fields) and the web wrapper (which passes `state.tool_registry` / `state.extension_manager`) share identical logic. - `AuthManager::resolve_extension_name_for_auth_flow` is now a 1-block delegator. - `pending_gate_extension_name` in `web/server.rs` drops its inline fallback entirely and calls the shared free function from both branches. The bare-test-harness path now runs branch 3 (canonicalize + installed-extension check) that it previously missed. - Updated `src/bridge/CLAUDE.md` to document the free function as the single authority, the three approved wrappers as thin delegators, and the return type as `ExtensionName` (was stale `String` from the pre-c813caa9 era). Regression coverage: the existing `resolve_extension_name_for_auth_flow_prefers_installed_channel_name` test passes unchanged — it exercises branch 3 through the method, which now reaches it via the extracted free function. * Merge remote-tracking branch 'origin/staging' into feat/identity-newtypes-pr2 Picks up nearai#2644 (platform/ extraction) and nearai#2645 (features/oauth/ move). Manual resolutions: - src/channels/web/server.rs: staging removed 720 lines of OAuth callback code (moved to features/oauth/mod.rs in nearai#2645). My PR 2 ExtensionName changes to two of those functions (oauth_callback_handler, slack_relay_oauth_callback_handler) ported to the new location. - src/bridge/auth_manager.rs: extended the shared resolver's branch-1 action pattern to include 'tool-activate' and 'tool-auth' variants, matching staging's new pending_gate_extension_name_uses_install_parameters_for_hyphenated_activate_tool test expectation. Underscore + hyphen variants for all three actions. No new PR 2 logic — just aligning the type surface with two staging refactors. 5074 lib tests pass (+1 vs previous — the new staging hyphenated-tool test). Clippy -D warnings clean. * fix(web): address PR nearai#2617 round-3 review feedback Two Copilot findings from the 2026-04-18 review: 1. `/api/extensions/{name}/{activate,remove,setup}` handlers accepted `Path<String>` and forwarded it to the extension manager without validating path-traversal, invalid characters, or case — only `extensions_setup_submit_handler` had the `ExtensionName::new` guard. Applied the same boundary validation to all three siblings. 2. `restore_pending_auth_mode` took `extension_name: &str` and re-wrapped it with `ExtensionName::from_trusted`, re-introducing an unvalidated string boundary even though every caller already held an `ExtensionName` (`pending_auth.extension_name`). Changed the helper to accept `&ExtensionName` so the identity stays typed end-to-end; `from_trusted` is no longer needed here. Regression: added `test_extensions_sibling_handlers_reject_path_traversal_name` covering activate / remove / setup-GET with the same malformed slugs the setup-submit test already locks in (path traversal, slash in segment, uppercase, space, trailing underscore). Drives the handlers through axum routing so the boundary is exercised end-to-end. * fix(ci): adapt replay_outcome to ExtensionName after staging merge Staging nearai#2621 added `tests/support/replay_outcome.rs`, which destructures `StatusUpdate::{AuthRequired,AuthCompleted}.extension_name` into a `String` field of `EventSummary`. This PR made those `StatusUpdate` fields `ExtensionName`, so the post-merge build breaks in the replay snapshot gate and all-features clippy jobs. Convert to `String` at the destructure via `ExtensionName::into()` so the `EventSummary` shape (and the persisted `.snap` files) stay unchanged. The test-support / snapshot wire format is a legitimate String boundary per `.claude/rules/types.md`.

* docs(rules): add review-driven guidance for Claude Code Synthesizes recurring patterns from ~30 merged PRs, 147 bot review comments (Copilot/Gemini), human reviews, and ~50 issues filed in the past 2 weeks. Each rule cites the motivating PR/issue numbers. New files: - error-handling.md — silent-failure taxonomy (unwrap_or_default, .ok()?, poisoned caches), persist-then-reload atomicity, channel-edge error mapping. (nearai#2526, nearai#2633, nearai#2653, nearai#2673, nearai#2546, nearai#2407, nearai#2408) - agent-evidence.md — side-effect claims must cite tool evidence, empty-fast outputs are errors, external-effect tools must read back, setup UI round-trip. (nearai#2544, nearai#2580, nearai#2582, nearai#2541, nearai#2545, nearai#2411, nearai#2543, nearai#2586) - lifecycle.md — discovery vs. activation, terminal auth rejection, list_installed vs. list_active, deactivation unwinds, snapshot rehydrate must re-validate. (nearai#2556, nearai#2557, nearai#2558, nearai#2564, nearai#2419, PR nearai#2617, PR nearai#2631) Extended: - types.md — from_trusted boundary rule, validated-newtype template with shared validate(&str), serde(try_from) required for validated types, wire-stable enums (no Debug; serde alias for migrations), canonical wire-contract field naming. (PR nearai#2685, nearai#2681, nearai#2687, nearai#2678, nearai#2669, nearai#2665, nearai#2683, nearai#2702) - safety-and-sandbox.md — every new ingress scans pre-transform/pre- injection, bounded resources (interners/streams/fan-out caps), cache keys must be complete. (nearai#2491, nearai#2676, nearai#2470, nearai#2633, nearai#2673, nearai#2710, PR nearai#2702) - review-discipline.md — PR scope discipline, guardrail scripts are code (regression tests, grouped-import parsing, CI has_code inclusion), absolute-path ban in committed docs, stale comments after refactors. (PR nearai#2668, nearai#2628, nearai#2680, nearai#2687, nearai#2647, nearai#2689, nearai#2701) All new files carry paths: frontmatter so they auto-load only on matching files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(rules): split agent-evidence into prompt + code rule agent-evidence.md mixed two concerns: runtime agent instruction (what the LLM should do when concluding a turn) and code-enforcement rules (what the dispatcher, engine, and tools must implement). Rules under .claude/rules/ only guide Claude Code when editing the repo — the runtime agent never reads them. Splits the two: - crates/ironclaw_engine/prompts/codeact_postamble.md — new section "Evidence before claiming side effects". Sits next to the existing "FINAL() answer quality" guidance; loaded via include_str! in executor/prompt.rs (no Rust change needed). - .claude/rules/tool-evidence.md — renamed from agent-evidence.md, keeps only the code invariants (engine v2 side-effect gate, empty-fast ToolError::EmptyResult, external-effect tools must read back, setup UI round-trip). Prompt tests pass unchanged; the postamble addition is pure text. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * prompt: tighten evidence rule to FINAL() claims only, not tool use Live-test validation of the "Evidence before claiming side effects" section (added in the prior commit) showed it inhibited legitimate tool use. With the original wording, `zizmor_scan_v2` live-recording timed out at 302s with zero responses; reverting the postamble restored healthy behavior (88s run, 8 shell calls including `cargo install zizmor` and full workflow analysis). The original phrasing conflated two things: what the agent should claim and what tools it should call. The rule is only about the claim. Re-tunes the section to: - Open with an explicit "this does not restrict tool calls" scope. - Drop the "<1ms = failure" heuristic (too broad — normal tools like `tool_info(schema)` are legitimately fast). - Drop the full enumeration of forbidden side-effect verbs; keep the rule narrower and clearer. - Shorten the code example (remove redundant early-return). Re-tuned run: agent is active (shell calls, real reasoning), live recording completes in ~9s. The remaining test failure is a pre-existing assertion bug (exact `t == "shell"` match against tool strings that now carry arguments like `"shell(cmd)"`) — reproduces with the old postamble too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(live): fix tool-name assertions + re-record zizmor traces The two `zizmor_scan*` live tests had four broken tool-name assertions that silently failed to match: `tools.iter().any(|t| t == "shell")` against a tool list that now contains `"shell(cmd)"` strings (tool events carry args via `format_action_display_name` in `src/bridge/router.rs`). Two of the four were negative assertions checking for the absence of `tool_install` recovery loops — those silently passed even when a recovery loop actually ran. `sandbox_live_e2e.rs:203` already used the correct `t == "shell" || t.starts_with("shell(")` pattern; applied it consistently to all four sites. Verified live: - `IRONCLAW_LIVE_TEST=1 cargo test --test e2e_live -- zizmor_scan --ignored --test-threads=1` → 2 passed, 0 failed, 51.78s. Agent installs and runs zizmor end-to-end, producing real findings (exit code 14, dangerous triggers, excessive permissions, etc.). Traces re-recorded with the tuned postamble (commit 50d8517) and scrubbed: replaced `/home/illia/.cargo/bin/zizmor` with `/home/user/.cargo/bin/zizmor` per the developer-local-path ban in `.claude/rules/review-discipline.md`. No credentials, PII, or high-entropy secrets in either trace (only git SHAs from zizmor's workflow analysis output). Replay still passes: `cargo test --test e2e_live -- zizmor_scan --ignored` → 2/2 ok. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(replay): update zizmor_scan_v2 insta snapshot The engine v2 replay-snapshot gate (`engine_v2_tests::snapshot_zizmor_scan_v2`) failed against the re-recorded trace from 1691efe because the old snapshot encoded a broken run: - final_state: Failed - Missing Assistant message role - 3 issues: thread_failure (error), no_response (warning), llm_error (error) - 6 tool calls that never produced a final answer The new trace completes cleanly: - final_state: Done - System / User / Assistant roles present - 1 issue: mixed_mode (info) - 3 shell tool calls + successful `FINAL()` with real findings The snapshot was pinning a regression. Regenerated with `INSTA_UPDATE=always cargo test --test e2e_engine_v2 -- snapshot_zizmor_scan_v2`; passes on replay. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(rules): address PR nearai#2714 review feedback - review-discipline: reword "Doc Absolute Paths" as a review convention (pre-commit only scans .rs; the rule misleadingly claimed enforcement). - safety-and-sandbox: broaden `paths:` frontmatter to include the actual ingress owners (`bridge`, `channels`, `workspace`, `agent`, engine crate) so the rule auto-loads where it applies. - tool-evidence: mark the side-effect gate, empty-fast rule, and `unverified` flag as target/aspirational invariants — neither `ToolError::EmptyResult`, an `unverified` field on `ToolOutput`, nor a byte-count field on `ActionRecord` exist today. Point at concrete interim conventions (`ToolError::ExecutionFailed`, `unverified: true` in the JSON result body). - types: scope "Validated newtypes must gate Deserialize" to *new* types, document the `CredentialName`/`ExtensionName` exception (they intentionally use `#[serde(transparent)]` + derived `Deserialize` under the `serde_does_not_revalidate` test). Clarify the `from_trusted` trust boundary (trusted = typed upstream, untrusted = raw JSON field even if the field *name* is "registry entry"). Switch `new` template to `impl Into<String>` to avoid an unnecessary clone when an owned `String` is passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(rules): simplify types.md + split doc-hygiene; address review round 2 - types: collapse two templates into one canonical validated-newtype shape. New types use `#[serde(try_from = "String")]` with a shared `validate(&str)` helper — no more dual "transparent for some / try_from for others" guidance. `CredentialName`/`ExtensionName` are documented as the sole legacy exception (locked in by the `serde_does_not_revalidate` test); new code must not copy their `transparent` + `from_trusted` pattern. Removes the long "Using `from_trusted` safely" section and the separate "Validated newtypes must gate Deserialize" subsection that contradicted the Don'ts list. - doc-hygiene: new tiny rule file scoped to `**/*.md`, `**/*.py`, `docs/**` that carries the "no developer-local absolute paths in committed docs" convention. Removed from review-discipline.md where its `src/**/*.rs` scope meant the rule never loaded on the files it governed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(live): match hyphenated tool-install in attempted_relevant_tool The engine records `action_name` as the raw string the LLM emitted (`crates/ironclaw_engine/src/executor/structured.rs:381`), and the registry's lookup canonicalization only affects dispatch — not the name that reaches `StatusUpdate::ToolStarted`. The two other predicates in this file (`bad_recovery` at :420, `phase_b_recovery` at :531) already defend against both forms; this one should too, for consistency. Addresses PR nearai#2714 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

gemini-code-assist Bot reviewed Apr 18, 2026

View reviewed changes

ilblackdragon added 3 commits April 18, 2026 16:29

ilblackdragon force-pushed the feat/identity-newtypes-pr2 branch from 0e6ffab to cc7ea40 Compare April 18, 2026 08:10

ilblackdragon mentioned this pull request Apr 18, 2026

feat(common): CredentialName + ExtensionName newtypes (PR 1/2) #2611

Merged

7 of 8 tasks

Base automatically changed from feat/identity-newtypes-pr1 to staging April 18, 2026 09:14

ilblackdragon added 3 commits April 18, 2026 18:23

Merge remote-tracking branch 'origin/staging' into feat/identity-newt…

cd46201

…ypes-pr2 # Conflicts: # src/channels/web/server.rs # src/extensions/manager.rs

Copilot AI review requested due to automatic review settings April 18, 2026 09:48

github-actions Bot added scope: docs Documentation size: XL 500+ changed lines and removed size: L 200-499 changed lines labels Apr 18, 2026

Copilot started reviewing on behalf of ilblackdragon April 18, 2026 09:48 View session

Copilot AI reviewed Apr 18, 2026

View reviewed changes

ilblackdragon added 3 commits April 18, 2026 22:50

Merge remote-tracking branch 'origin/staging' into feat/identity-newt…

f8731de

…ypes-pr2

Copilot AI review requested due to automatic review settings April 18, 2026 14:14

ironclaw-ci Bot mentioned this pull request Apr 30, 2026

chore: promote staging to main (2026-04-30 16:44 UTC) #3121

Closed

-    // Resolver returns a trusted identity string — either a real extension
-    // name from the manager, the provider-extension hint off the tool, or
-    // the credential-name fallback when no extension owns the action.
-    ironclaw_common::ExtensionName::from_trusted(raw)
+    // The auth-flow resolver may surface user-influenced values for some
+    // actions, so validate/canonicalize before promoting to ExtensionName.
+    match ironclaw_common::ExtensionName::new(&raw) {
+        Ok(extension_name) => extension_name,
+        Err(error) => {
+            debug!(
+                action_name,
+                raw = %raw,
+                credential_fallback,
+                %error,
+                "Rejected invalid extension name resolved for auth flow; falling back"
+            );
+            ironclaw_common::ExtensionName::new(credential_fallback).unwrap_or_else(
+                |fallback_error| {
+                    debug!(
+                        credential_fallback,
+                        %fallback_error,
+                        "Credential fallback is not a canonical extension name; preserving legacy trusted fallback"
+                    );
+                    ironclaw_common::ExtensionName::from_trusted(
+                        credential_fallback.to_string(),
+                    )
+                },
+            )
+        }
+    }

Conversation

ilblackdragon commented Apr 18, 2026

Summary

Newly typed

Wire boundary (serde transparent — JSON unchanged)

Internal types

Behavioral change

Conversion discipline

Test plan

Left for a later PR

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants