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 c813caa9.

- 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 c813caa9
(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 #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
(#2561, #2473, #2512, #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 #2644 (platform/ extraction) and #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 #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 #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 #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`.

crates/ironclaw_common/src/event.rs

@@ -5,6 +5,7 @@
 //! frames, but other subsystems (agent loop, orchestrator, extensions)
 //! produce and consume them too.
 
+use crate::identity::ExtensionName;
 use serde::{Deserialize, Serialize};
 
 /// A single step in a plan progress update (SSE DTO).
@@ -64,7 +65,7 @@ impl OnboardingStateDto {
     /// post-pairing) from silently disagreeing when new fields land on
     /// `AppEvent::OnboardingState`.
     pub fn pairing_required(
-        extension_name: impl Into<String>,
+        extension_name: ExtensionName,
         request_id: Option<String>,
         thread_id: Option<String>,
         message: Option<String>,
@@ -72,7 +73,7 @@ impl OnboardingStateDto {
         onboarding: Option<serde_json::Value>,
     ) -> AppEvent {
         AppEvent::OnboardingState {
-            extension_name: extension_name.into(),
+            extension_name,
             state: Self::PairingRequired,
             request_id,
             message,
@@ -161,7 +162,7 @@ pub enum AppEvent {
     },
     #[serde(rename = "onboarding_state")]
     OnboardingState {
-        extension_name: String,
+        extension_name: ExtensionName,
         state: OnboardingStateDto,
         #[serde(skip_serializing_if = "Option::is_none")]
         request_id: Option<String>,
@@ -186,7 +187,7 @@ pub enum AppEvent {
         description: String,
         parameters: String,
         #[serde(skip_serializing_if = "Option::is_none")]
-        extension_name: Option<String>,
+        extension_name: Option<ExtensionName>,
         resume_kind: serde_json::Value,
         #[serde(skip_serializing_if = "Option::is_none")]
         thread_id: Option<String>,
@@ -281,7 +282,7 @@ pub enum AppEvent {
     /// Extension activation status change (WASM channels).
     #[serde(rename = "extension_status")]
     ExtensionStatus {
-        extension_name: String,
+        extension_name: ExtensionName,
         status: String,
         #[serde(skip_serializing_if = "Option::is_none")]
         message: Option<String>,
@@ -453,7 +454,7 @@ mod tests {
                 allow_always: false,
             },
             AppEvent::OnboardingState {
-                extension_name: String::new(),
+                extension_name: ExtensionName::from_trusted(String::new()),
                 state: OnboardingStateDto::AuthRequired,
                 request_id: None,
                 message: None,
@@ -524,7 +525,7 @@ mod tests {
                 thread_id: None,
             },
             AppEvent::ExtensionStatus {
-                extension_name: String::new(),
+                extension_name: ExtensionName::from_trusted(String::new()),
                 status: String::new(),
                 message: None,
             },
@@ -579,7 +580,7 @@ mod tests {
     #[test]
     fn pairing_required_constructor_sets_invariant_fields() {
         let event = OnboardingStateDto::pairing_required(
-            "telegram",
+            ExtensionName::new("telegram").unwrap(),
             Some("req-1".to_string()),
             Some("thread-1".to_string()),
             Some("Paired!".to_string()),
@@ -618,7 +619,14 @@ mod tests {
 
     #[test]
     fn pairing_required_constructor_serializes_to_onboarding_state_event() {
-        let event = OnboardingStateDto::pairing_required("telegram", None, None, None, None, None);
+        let event = OnboardingStateDto::pairing_required(
+            ExtensionName::new("telegram").unwrap(),
+            None,
+            None,
+            None,
+            None,
+            None,
+        );
         let json = serde_json::to_value(&event).unwrap();
         assert_eq!(json["type"], "onboarding_state");
         assert_eq!(json["state"], "pairing_required");

crates/ironclaw_tui/Cargo.toml

@@ -15,6 +15,7 @@ default = ["clipboard"]
 clipboard = ["dep:arboard", "dep:image"]
 
 [dependencies]
+ironclaw_common = { path = "../ironclaw_common", version = "0.2.0" }
 ratatui = { version = "0.29", features = ["crossterm"] }
 tui-textarea = { version = "0.7", features = ["crossterm"] }
 serde = { version = "1", features = ["derive"] }

crates/ironclaw_tui/src/event.rs

@@ -6,6 +6,7 @@
 
 use std::collections::VecDeque;
 
+use ironclaw_common::ExtensionName;
 use ratatui::crossterm::event::KeyEvent;
 
 /// A single log entry displayed in the TUI Logs tab.
@@ -280,13 +281,13 @@ pub enum TuiEvent {
 
     /// Extension needs user authentication.
     AuthRequired {
-        extension_name: String,
+        extension_name: ExtensionName,
         instructions: Option<String>,
     },
 
     /// Extension auth completed.
     AuthCompleted {
-        extension_name: String,
+        extension_name: ExtensionName,
         success: bool,
         message: String,
     },