fix: resolve staging CI test failures blocking promotion by serrrfirat · Pull Request #2574 · nearai/ironclaw

serrrfirat · 2026-04-17T09:59:09Z

Summary

Staging CI has been failing for 7 days (30+ orphaned promotion PRs, 64 commits stuck). This fixes all 3 categories of test failures:

Extension name resolution (server.rs): pending_gate_extension_name returned credential_name ("telegram_bot_token") instead of extension_name ("telegram") when auth_manager was unavailable. Added fallback extraction from install parameters.
CLI snapshots (2 .snap files): Regenerated stale snapshots — new profile subcommand, updated help descriptions for onboard/config/doctor/login.
E2E pairing tests (test_extensions.py): Relaxed exact dict equality to field check since frontend now sends optional thread_id in pairing approve bodies.

Test plan

pending_gate_extension_name_uses_install_parameters_for_post_install_auth passes (was failing in all 3 test configs)
cli::tests::test_help_output and test_long_help_output pass (was failing in all-features)
cargo fmt and cargo clippy clean
CI passes on this PR
Remaining E2E auth-card failures should resolve once extension_name fix is in staging

🤖 Generated with Claude Code

1. pending_gate_extension_name now extracts extension name from tool_install/tool_activate/tool_auth parameters even when auth_manager is unavailable, matching the AuthManager logic and returning "telegram" instead of "telegram_bot_token". 2. Updated CLI help snapshots to match new onboard/config/doctor/login descriptions and the addition of the profile subcommand. 3. Relaxed E2E pairing approve assertions to check only the code field, accommodating the new optional thread_id the frontend now sends. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

gemini-code-assist

Code Review

This pull request implements a fallback mechanism for extracting install parameters in the web server when the authentication manager is not wired up. It also enhances the CLI help documentation by adding more descriptive command summaries, a new 'Getting started' section, and including the 'profile' command. Additionally, E2E tests were updated to use more specific assertions for pairing code verification. I have no feedback to provide.

ilblackdragon

Code Review

PR: fix: resolve staging CI test failures blocking promotion
Verdict: Approve with nit

Extension name fallback (`server.rs`)

The new fallback block correctly mirrors AuthManager::resolve_extension_name_for_auth_flow() (lines 326-340). Both check the same tool names (tool_install | tool-install | tool_activate | tool_auth) and extract from parameters.get("name"). Good use of let-chains with matches! — idiomatic for this codebase.

Nit — trimming inconsistency: The fallback returns name.to_string() from the untrimmed as_str() value. The .map(str::trim) is only used in the filter predicate. The AuthManager has the same behavior, so this is consistent — but both callers would return " telegram " if the parameter had leading/trailing whitespace. Consider:

return name.trim().to_string(); // instead of name.to_string()

Low risk since parameter values are unlikely to have whitespace, and it matches the AuthManager's existing behavior.

Test coverage

Two good unit tests covering the install-parameter extraction and tool-registry fallback paths.
The tests exercise the helper directly with GatewayState, which is appropriate since there's no wrapper dropping inputs between engine_pending_gate_info and pending_gate_extension_name.
Minor gap: only tool_install is exercised — a parameterized test covering tool-install, tool_activate, and tool_auth would be more thorough, but the matches! logic is trivial so low risk.

E2E test relaxation (`test_extensions.py`)

Switching from exact dict equality to field check is the right approach — forward-compatible with additional optional fields the frontend may add.

Snapshots

Mechanical regeneration, looks correct.

Overall: well-scoped fix that correctly mirrors existing logic, has good test coverage, and unblocks the 7-day CI backlog. 👍

ilblackdragon

One more thought on the fallback duplication: rather than mirroring the AuthManager logic inline, could we ensure a SecretsStore is always available so AuthManager gets constructed? Looking at build_gateway_auth_manager, it returns None only when no secrets backend exists. If the test/CI configuration stood up a minimal in-memory or file-backed secrets store, auth_manager would always be Some(...) and this fallback path wouldn't be needed at all.

That would avoid the maintenance risk of the two implementations diverging if resolve_extension_name_for_auth_flow ever changes its logic.

…vailable, remove fallback duplication (#2574) - Remove inline fallback that duplicated AuthManager::resolve_extension_name_for_auth_flow() logic in pending_gate_extension_name(); auth_manager is now always wired in tests via a minimal InMemorySecretsStore-backed AuthManager - Fix trim inconsistency in AuthManager::resolve_extension_name_for_auth_flow() where the predicate trimmed whitespace but the return value did not Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

serrrfirat · 2026-04-17T11:19:39Z

Addressed both comments in 781980d:

Trim fix — applied in AuthManager::resolve_extension_name_for_auth_flow (the canonical resolver), so trimmed values propagate everywhere. No more inconsistency between filter predicate and return value.
Eliminated fallback duplication — removed the inline mirror block from pending_gate_extension_name. Tests now wire up a minimal AuthManager backed by InMemorySecretsStore, so they exercise the real code path. Single source of truth for extension name resolution.

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

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.

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

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.

…icating 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.

…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 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. * 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`.

Per .claude/rules/types.md, names that flow between modules and gate side effects should be typed identifiers, not `String`. The new enums from the #2557 fix were carrying `secret_name: String` because they inherited from `WebsocketRuntimeConfig::identify_secret_name: Option<String>` (the raw string parsed from capability JSON). That field is a valid boundary value, but everything below it in the auth-flow path should be typed. - `WebsocketAuthPreflight::MissingSecret { secret_name: String }` → `MissingCredential { credential_name: CredentialName }` - `WebsocketStartDecision::MissingAuth { secret_name: String }` → `MissingAuth { credential_name: CredentialName }` - `websocket_auth_preflight` now takes `Option<&CredentialName>`; name validation is lifted to the orchestrator where it belongs. - `websocket_start_decision` validates `config.identify_secret_name` via `CredentialName::new(...)` once, at the boundary. A syntactically invalid name surfaces as `MalformedConfig { reason }` with the real `IdentityError` embedded — not silently routed through "missing credential", which was the exact failure mode bug #2574 fixed in a different subsystem. Internal variables and log field names move from `secret_name` → `credential_name` to match the codebase convention around `CredentialName` and the existing `credential_name` fields in `bridge::auth_manager`, `bridge::router`, and `gate::mod`. Out of scope (larger follow-ups): - Capability-wire key `identify_secret_name` stays; it lives in every installed channel's `capabilities.json` (`channels-src/discord/...`) and renaming it is a breaking change to deployed WASM channels. - `SecretsStore::{exists, get_decrypted}(&str, &str)` stays; migrating the trait signature is a wider refactor that should not ride on this fix. - `WebsocketRuntimeConfig::identify_secret_name: Option<String>` stays as the boundary type that owns the raw-string-from-JSON contract. Added regression test `test_websocket_start_decision_malformed_config_with_invalid_credential_name` to pin the new "invalid name → MalformedConfig" behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…fatal closes (#2557) (#2707) * fix(wasm): gate websocket runtime on auth and stop reconnect loop on fatal closes (#2557) WASM channel runtimes spawned the websocket runtime on `connect_on_start` without verifying required credentials, and treated auth-rejected close frames as transient disconnects. When a Discord bot token was missing the runtime would connect, be rejected with 4003, and the outer reconnect loop retried forever (capped at 64s backoff) producing continuous network traffic and log spam. Three guards, all in `src/channels/wasm/wrapper.rs`: - `websocket_auth_preflight` + `websocket_start_decision` compose capability parsing and secret-presence into a typed decision; `Channel::start` matches on it and skips the spawn with a bounded `warn!` when the declared `identify_secret_name` is not present in the secrets store. Writing the secret and restarting the channel connects normally. - Runtime-entry guard: if `resolve_websocket_identify_message` returns `None` after preflight passed (credential revoked mid-flight), the task exits instead of connecting. - `classify_websocket_close_code` maps Discord-documented fatal auth codes (4003, 4004, 4010-4014) to `Terminal`; the read loop breaks `'reconnect` on terminal closes instead of falling through to backoff. Regression tests cover preflight, start-decision, and close-code classification. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: address bot feedback on #2557 websocket preflight Four distinct issues flagged by gemini-code-assist + copilot-pr-reviewer: 1. Close-code classification was global — 4000-series codes are application-defined (RFC 6455), so another provider using e.g. 4003 with different semantics would be incorrectly treated as terminal. Scope the Discord code table to Discord gateway hosts via `is_discord_gateway_host`; non-Discord URLs always return `Reconnect`. 2. Preflight silently allowed a capability that declares `identify_secret_name` but omits the `identify` template (no identify payload could ever be built even with a valid secret). Catch it in `websocket_start_decision` with a new `MalformedConfig { reason }` variant so the operator log points at the real cause. 3. `store.exists()` errors were mapped to `MissingSecret`, conflating a transient DB blip with a genuinely absent secret. Split the `Err` branch: log the store error and fail open (`Ready`); the runtime-entry guard still stops any spawn whose identify payload cannot be built. 4. Runtime-entry guard warning said "required auth secret unavailable" even when the true cause could be a missing `identify` template or a decrypt error. Rephrased to enumerate possible causes and include `has_identify_template` as a field. Tests: - `test_websocket_start_decision_malformed_config_without_identify_template` - `test_classify_websocket_close_code_non_discord_host_never_terminal` (covers plain non-Discord hosts and a host-suffix spoof) - existing close-code tests now pass the URL so the Discord-gated signature is exercised. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(wasm): use CredentialName newtype in websocket auth preflight Per .claude/rules/types.md, names that flow between modules and gate side effects should be typed identifiers, not `String`. The new enums from the #2557 fix were carrying `secret_name: String` because they inherited from `WebsocketRuntimeConfig::identify_secret_name: Option<String>` (the raw string parsed from capability JSON). That field is a valid boundary value, but everything below it in the auth-flow path should be typed. - `WebsocketAuthPreflight::MissingSecret { secret_name: String }` → `MissingCredential { credential_name: CredentialName }` - `WebsocketStartDecision::MissingAuth { secret_name: String }` → `MissingAuth { credential_name: CredentialName }` - `websocket_auth_preflight` now takes `Option<&CredentialName>`; name validation is lifted to the orchestrator where it belongs. - `websocket_start_decision` validates `config.identify_secret_name` via `CredentialName::new(...)` once, at the boundary. A syntactically invalid name surfaces as `MalformedConfig { reason }` with the real `IdentityError` embedded — not silently routed through "missing credential", which was the exact failure mode bug #2574 fixed in a different subsystem. Internal variables and log field names move from `secret_name` → `credential_name` to match the codebase convention around `CredentialName` and the existing `credential_name` fields in `bridge::auth_manager`, `bridge::router`, and `gate::mod`. Out of scope (larger follow-ups): - Capability-wire key `identify_secret_name` stays; it lives in every installed channel's `capabilities.json` (`channels-src/discord/...`) and renaming it is a breaking change to deployed WASM channels. - `SecretsStore::{exists, get_decrypted}(&str, &str)` stays; migrating the trait signature is a wider refactor that should not ride on this fix. - `WebsocketRuntimeConfig::identify_secret_name: Option<String>` stays as the boundary type that owns the raw-string-from-JSON contract. Added regression test `test_websocket_start_decision_malformed_config_with_invalid_credential_name` to pin the new "invalid name → MalformedConfig" behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: canonicalize identify_secret_name + flag asymmetric identify config Two correctness issues flagged by copilot-pr-reviewer on c4b9d1d. 1. **Canonicalization mismatch between preflight and runtime.** `CredentialName::new()` canonicalizes — it trims whitespace and folds `-` → `_`. Preflight checks existence via `credential_name.as_str()` (canonical), but `resolve_websocket_identify_message` later reads the raw `config.identify_secret_name` and passes that to the store. A capability declaring `"github-token"` against a store holding `"github_token"` would therefore pass preflight and fail in the spawn. Write the canonicalized form back to `config.identify_secret_name` before returning `Spawn(config)` so every downstream lookup uses the same string. Regression test `test_websocket_start_decision_spawn_canonicalizes_secret_name`. 2. **Asymmetric identify config passed through as Spawn.** The existing check caught `identify_secret_name` without `identify`, but the reverse (`identify` template present, `identify_secret_name` missing) slipped through. In that shape `resolve_websocket_identify_message` returns `None` because it needs a secret name, no Identify is ever sent, and the peer closes the connection — the exact #2557 spin this PR exists to prevent. Replaced the one-sided check with a `match (identify, credential_name)` so both asymmetric shapes surface as `MalformedConfig` with distinct reasons. Regression test `test_websocket_start_decision_malformed_config_without_secret_name`. No changes to wire contract, capability JSON key, or secrets-store trait. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: resolve 3 categories of staging CI test failures 1. pending_gate_extension_name now extracts extension name from tool_install/tool_activate/tool_auth parameters even when auth_manager is unavailable, matching the AuthManager logic and returning "telegram" instead of "telegram_bot_token". 2. Updated CLI help snapshots to match new onboard/config/doctor/login descriptions and the addition of the profile subcommand. 3. Relaxed E2E pairing approve assertions to check only the code field, accommodating the new optional thread_id the frontend now sends. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(web): address ilblackdragon review — ensure auth_manager always available, remove fallback duplication (nearai#2574) - Remove inline fallback that duplicated AuthManager::resolve_extension_name_for_auth_flow() logic in pending_gate_extension_name(); auth_manager is now always wired in tests via a minimal InMemorySecretsStore-backed AuthManager - Fix trim inconsistency in AuthManager::resolve_extension_name_for_auth_flow() where the predicate trimmed whitespace but the return value did not Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: allow clippy::too_many_arguments on register_startup_channels Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…i#2611) * 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.

…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`.

…fatal closes (nearai#2557) (nearai#2707) * fix(wasm): gate websocket runtime on auth and stop reconnect loop on fatal closes (nearai#2557) WASM channel runtimes spawned the websocket runtime on `connect_on_start` without verifying required credentials, and treated auth-rejected close frames as transient disconnects. When a Discord bot token was missing the runtime would connect, be rejected with 4003, and the outer reconnect loop retried forever (capped at 64s backoff) producing continuous network traffic and log spam. Three guards, all in `src/channels/wasm/wrapper.rs`: - `websocket_auth_preflight` + `websocket_start_decision` compose capability parsing and secret-presence into a typed decision; `Channel::start` matches on it and skips the spawn with a bounded `warn!` when the declared `identify_secret_name` is not present in the secrets store. Writing the secret and restarting the channel connects normally. - Runtime-entry guard: if `resolve_websocket_identify_message` returns `None` after preflight passed (credential revoked mid-flight), the task exits instead of connecting. - `classify_websocket_close_code` maps Discord-documented fatal auth codes (4003, 4004, 4010-4014) to `Terminal`; the read loop breaks `'reconnect` on terminal closes instead of falling through to backoff. Regression tests cover preflight, start-decision, and close-code classification. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: address bot feedback on nearai#2557 websocket preflight Four distinct issues flagged by gemini-code-assist + copilot-pr-reviewer: 1. Close-code classification was global — 4000-series codes are application-defined (RFC 6455), so another provider using e.g. 4003 with different semantics would be incorrectly treated as terminal. Scope the Discord code table to Discord gateway hosts via `is_discord_gateway_host`; non-Discord URLs always return `Reconnect`. 2. Preflight silently allowed a capability that declares `identify_secret_name` but omits the `identify` template (no identify payload could ever be built even with a valid secret). Catch it in `websocket_start_decision` with a new `MalformedConfig { reason }` variant so the operator log points at the real cause. 3. `store.exists()` errors were mapped to `MissingSecret`, conflating a transient DB blip with a genuinely absent secret. Split the `Err` branch: log the store error and fail open (`Ready`); the runtime-entry guard still stops any spawn whose identify payload cannot be built. 4. Runtime-entry guard warning said "required auth secret unavailable" even when the true cause could be a missing `identify` template or a decrypt error. Rephrased to enumerate possible causes and include `has_identify_template` as a field. Tests: - `test_websocket_start_decision_malformed_config_without_identify_template` - `test_classify_websocket_close_code_non_discord_host_never_terminal` (covers plain non-Discord hosts and a host-suffix spoof) - existing close-code tests now pass the URL so the Discord-gated signature is exercised. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(wasm): use CredentialName newtype in websocket auth preflight Per .claude/rules/types.md, names that flow between modules and gate side effects should be typed identifiers, not `String`. The new enums from the nearai#2557 fix were carrying `secret_name: String` because they inherited from `WebsocketRuntimeConfig::identify_secret_name: Option<String>` (the raw string parsed from capability JSON). That field is a valid boundary value, but everything below it in the auth-flow path should be typed. - `WebsocketAuthPreflight::MissingSecret { secret_name: String }` → `MissingCredential { credential_name: CredentialName }` - `WebsocketStartDecision::MissingAuth { secret_name: String }` → `MissingAuth { credential_name: CredentialName }` - `websocket_auth_preflight` now takes `Option<&CredentialName>`; name validation is lifted to the orchestrator where it belongs. - `websocket_start_decision` validates `config.identify_secret_name` via `CredentialName::new(...)` once, at the boundary. A syntactically invalid name surfaces as `MalformedConfig { reason }` with the real `IdentityError` embedded — not silently routed through "missing credential", which was the exact failure mode bug nearai#2574 fixed in a different subsystem. Internal variables and log field names move from `secret_name` → `credential_name` to match the codebase convention around `CredentialName` and the existing `credential_name` fields in `bridge::auth_manager`, `bridge::router`, and `gate::mod`. Out of scope (larger follow-ups): - Capability-wire key `identify_secret_name` stays; it lives in every installed channel's `capabilities.json` (`channels-src/discord/...`) and renaming it is a breaking change to deployed WASM channels. - `SecretsStore::{exists, get_decrypted}(&str, &str)` stays; migrating the trait signature is a wider refactor that should not ride on this fix. - `WebsocketRuntimeConfig::identify_secret_name: Option<String>` stays as the boundary type that owns the raw-string-from-JSON contract. Added regression test `test_websocket_start_decision_malformed_config_with_invalid_credential_name` to pin the new "invalid name → MalformedConfig" behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: canonicalize identify_secret_name + flag asymmetric identify config Two correctness issues flagged by copilot-pr-reviewer on c4b9d1d. 1. **Canonicalization mismatch between preflight and runtime.** `CredentialName::new()` canonicalizes — it trims whitespace and folds `-` → `_`. Preflight checks existence via `credential_name.as_str()` (canonical), but `resolve_websocket_identify_message` later reads the raw `config.identify_secret_name` and passes that to the store. A capability declaring `"github-token"` against a store holding `"github_token"` would therefore pass preflight and fail in the spawn. Write the canonicalized form back to `config.identify_secret_name` before returning `Spawn(config)` so every downstream lookup uses the same string. Regression test `test_websocket_start_decision_spawn_canonicalizes_secret_name`. 2. **Asymmetric identify config passed through as Spawn.** The existing check caught `identify_secret_name` without `identify`, but the reverse (`identify` template present, `identify_secret_name` missing) slipped through. In that shape `resolve_websocket_identify_message` returns `None` because it needs a secret name, no Identify is ever sent, and the peer closes the connection — the exact nearai#2557 spin this PR exists to prevent. Replaced the one-sided check with a `match (identify, credential_name)` so both asymmetric shapes surface as `MalformedConfig` with distinct reasons. Regression test `test_websocket_start_decision_malformed_config_without_secret_name`. No changes to wire contract, capability JSON key, or secrets-store trait. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions Bot added scope: channel/cli TUI / CLI channel scope: channel/web Web gateway channel size: M 50-199 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Apr 17, 2026

gemini-code-assist Bot reviewed Apr 17, 2026

View reviewed changes

ilblackdragon reviewed Apr 17, 2026

View reviewed changes

fix: allow clippy::too_many_arguments on register_startup_channels

b873eab

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

github-actions Bot added the scope: channel/wasm WASM channel runtime label Apr 17, 2026

ilblackdragon approved these changes Apr 17, 2026

View reviewed changes

ilblackdragon merged commit a6443dd into staging Apr 17, 2026
15 checks passed

ilblackdragon deleted the fix/staging-ci-tests branch April 17, 2026 13:51

github-actions Bot mentioned this pull request Apr 17, 2026

chore: promote staging to staging-promote/2874d2e9-24567658122 (2026-04-17 14:25 UTC) #2578

Merged

ilblackdragon mentioned this pull request Apr 18, 2026

refactor(events): collapse three PairingRequired emission sites into one constructor #2607

Merged

4 of 5 tasks

This was referenced Apr 18, 2026

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

Merged

feat(ci): AI-first CI workflows — review, interact, health monitor, smoke test #2459

Closed

serrrfirat mentioned this pull request Apr 20, 2026

Migrate from staging promotion bot to GitHub-native merge queue #2719

Closed

22 tasks

claude Bot mentioned this pull request Apr 22, 2026

chore: promote staging to staging-promote/d4d5263e-24759285758 (2026-04-22 05:02 UTC) #2821

Merged

ilblackdragon mentioned this pull request May 10, 2026

feat(reborn): add text-only loop driver host factory #3439

Merged

ilblackdragon mentioned this pull request May 10, 2026

feat(reborn): add ProductWorkflow and InboundTurnService facade (#3280) #3428

Merged

zmanian mentioned this pull request May 17, 2026

engine v2: expose channel-supplied thread/response ids to tools #3669

Closed

10 tasks done

zetyquickly mentioned this pull request Jun 2, 2026

engine v2: newtype client_thread_id / client_response_id on ThreadExecutionContext (Z2 follow-up from #3669) #4355

Closed

abbyshekit mentioned this pull request Jun 3, 2026

[codex] Port GSuite WASM capabilities to Reborn #4326

Merged

This was referenced Jun 18, 2026

fix(reborn): NEAR AI MCP ready state projection #4990

Merged

feat(admin): persist Engine V2 LLM usage #4989

Merged

claude Bot mentioned this pull request Jul 9, 2026

fix(webui-v2): render chat errors inline #5822

Merged

thisisjoshford mentioned this pull request Jul 25, 2026

Telegram setup via agent & extensions tab dead-ends on "admin must configure" #6671

Open

claude Bot mentioned this pull request Aug 6, 2026

feat(inspector): add prompt inspection and Prompt tab #7239

Merged

Navigation Menu

fix: resolve staging CI test failures blocking promotion - #2574

fix: resolve staging CI test failures blocking promotion#2574
ilblackdragon merged 3 commits into
stagingfrom
fix/staging-ci-tests

serrrfirat commented Apr 17, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

ilblackdragon left a comment

Uh oh!

ilblackdragon left a comment

Uh oh!

serrrfirat commented Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

serrrfirat commented Apr 17, 2026

Summary

Test plan

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

ilblackdragon left a comment

Choose a reason for hiding this comment

Code Review

Extension name fallback (server.rs)

Test coverage

E2E test relaxation (test_extensions.py)

Snapshots

Uh oh!

ilblackdragon left a comment

Choose a reason for hiding this comment

Uh oh!

serrrfirat commented Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Extension name fallback (`server.rs`)

E2E test relaxation (`test_extensions.py`)