Wire Reborn auth consumers through staged credentials

[serrrfirat]

Summary

Refs #4176.

This PR wires the runtime credential consumer paths that depend on host-owned staged credentials:

• adds a composition-owned GSuite credential stager that resolves the selected Google product-auth account and satisfies InjectSecretOnce before first-party host HTTP egress consumes the StagedObligation;
• projects recoverable first-party credential failures through DispatchError::AuthRequired so the host runtime can return RuntimeCapabilityOutcome::AuthRequired instead of a generic failed runtime dispatch;
• fixes MCP HTTP/SSE credential planning to plan the real tools/call JSON-RPC body once before handshake and thread that validated plan into the eventual tools/call send;
• documents the open-question decisions for GSuite dynamic selection, GitHub manual-token-first modeling, server-scoped MCP account selection, recovery semantics, refresh ownership, and credential target scope.

Open-question decisions

• GSuite stays dynamic through CredentialAccountService; static runtime_credentials are deferred until account bindings are stable.
• GitHub first-party WASM remains github_token manifest-driven with manual-token product-auth as the first account model; the existing staged host HTTP path is covered by github_wasm_runtime_contract.
• MCP auth selection is composition-owned and server-scoped; MCP protocol code only consumes host-planned staged credential injections.
• Multiple matching accounts return account-selection-required; missing/expired/revoked/refresh-failed credentials map to auth recovery/AuthRequired; backend failures fail closed.
• Refresh stays provider-specific behind CredentialAccountService::refresh_account.
• Header/query targets are preferred; path placeholders remain compatibility-only where an upstream protocol requires them.

Tests

• cargo test -p ironclaw_reborn_composition --test gsuite
• cargo test -p ironclaw_mcp
• cargo test -p ironclaw_host_runtime first_party_adapter_maps_handler_auth_required_to_dispatch_auth_required
• cargo test -p ironclaw_host_runtime --test github_wasm_runtime_contract
• cargo test -p ironclaw_first_party_extensions -p ironclaw_host_api -p ironclaw_capabilities
• git diff --check

[gemini-code-assist]

Code Review

This pull request introduces runtime credential staging for GSuite first-party capabilities and refactors MCP HTTP/SSE credential planning to ensure handshakes remain credential-free. Key feedback includes addressing a potential build failure in offline or airgapped deployments where product auth ports are resolved unconditionally, safely unwrapping those ports during GSuite handler registration, using exhaustive matching instead of a catch-all wildcard when mapping dispatch errors, and optimizing PlannedMcpJsonRpc by storing constructed headers directly to avoid redundant session ID lookups.

[serrrfirat]

Multi-Agent Code Review

Intent: Wire Reborn auth consumers through staged credentials, covering GSuite credential stager, AuthRequired projection, and MCP HTTP/SSE credential planning.

Stats: 11 findings → 0 Critical, 2 High, 7 Medium, 2 Low, 0 Nit
Reviewers: security, bugs, performance, tests, conventions (5/5 OK)
Head: 1f2e04735

🐛 Bugs (2)

• [High] crates/ironclaw_host_runtime/src/services.rs:226 — stage_secret_error maps SecretExpired/LeaseRevoked to Backend instead of AuthRequired
• [Medium] crates/ironclaw_host_runtime/src/services.rs:216 — consume() errors collapsed to Backend hide expired/revoked credentials

🧪 Tests (7)

• [High] crates/ironclaw_host_runtime/src/services.rs:204 — stage_secret_once AuthRequired/Backend error branches untested
• [Medium] crates/ironclaw_host_runtime/src/production.rs:1638 — dispatch_kind_to_failure pin test omits new AuthRequired variant
• [Medium] crates/ironclaw_host_api/src/dispatch.rs:186 — DispatchError::AuthRequired -> DispatchFailureKind contract untested
• [Medium] crates/ironclaw_first_party_extensions/src/gsuite/handlers.rs:110 — post-refresh stage_credential failure path untested
• [Medium] crates/ironclaw_host_runtime/src/production.rs:983 — required_secrets propagation through auth_required_outcome from FirstParty path untested
• [Low] crates/ironclaw_first_party_extensions/src/gsuite/handlers.rs:245 — GsuiteDispatchError::auth_requirement per-reason branches untested
• [Low] crates/ironclaw_capabilities/src/error.rs:114 — FromDispatchError::AuthRequired preserves required_secrets untested

📐 Conventions (2)

• [Medium] crates/ironclaw_capabilities/src/host.rs:1642 — Obligation-derived AuthorizationRequiresAuth silently fills required_secrets with Vec::new()
• [Medium] crates/ironclaw_first_party_extensions/src/gsuite/handlers.rs:245 — GsuiteDispatchError::auth_requirement collapses Recovery projection to empty default with no rationale

---

Posted by code-review skill v1. Multi-agent review with intent analyzer + 5 parallel reviewers (deduplicated, confidence ≥ 50).

[serrrfirat]

Fix notes (commit 62851fd1)

Addressed the Medium-and-higher findings from the multi-agent review above.

🐛 Bug fixes

stage_secret_error misrouted recoverable credential conditions (services.rs:226+:220)

stage_secret_error only matched is_unknown_secret() → AuthRequired; everything else fell through to Backend. SecretExpired, LeaseRevoked, and LeaseExpired are all user-actionable re-auth conditions (their stable_reason() returns CredentialExpired/CredentialRevoked) — mapping them to Backend meant the runtime auth gate never fired for expired or revoked credentials, defeating the AuthRequired projection this PR introduces.

Fixed: stage_secret_error now also matches is_expired() and is_revoked(). The consume() failure path was also hardcoded to Backend; it now routes through the same classifier.

📐 Convention fixes

host.rs:1642 — required_secrets: Vec::new() without rationale

The obligation-path CapabilityObligationError::AuthRequired conversion is a unit variant with no payload to forward. Added a // silent-ok: comment per .claude/rules/error-handling.md naming why the empty list is structurally inevitable here (dispatch-path conversion in error.rs forwards the real list).

handlers.rs:245 — Recovery projection collapse undocumented

GsuiteAuthRequirement intentionally only carries required_secrets; the richer recovery context (provider, accounts, reason) is preserved via reason() and consumed separately by the runtime for telemetry/user-facing state. Added a doc-comment on auth_requirement() naming this split and why each variant maps the way it does.

🧪 New tests (356 LoC, 7 files)

Test	What it pins
stage_secret_once_returns_auth_required_for_unknown_handle	UnknownSecret → AuthRequired on lease
stage_secret_once_returns_auth_required_for_expired_secret	SecretExpired → AuthRequired on lease
stage_secret_once_returns_auth_required_for_revoked_lease_on_consume	LeaseRevoked → AuthRequired on consume
stage_secret_once_returns_auth_required_for_expired_lease_on_consume	LeaseExpired → AuthRequired on consume
stage_secret_once_returns_backend_on_secret_store_failure	BackendMisconfigured → Backend
dispatch_kind_to_failure_pins_… (extended)	DispatchFailureKind::AuthRequired → RuntimeFailureKind::Authorization
dispatch_errors_preserve_typed_failure_kind (extended)	DispatchError::AuthRequired → DispatchFailureKind::AuthRequired
dispatch_failure_kind_display_preserves_stable_literals (extended)	DispatchFailureKind::AuthRequired.as_str() == "AuthRequired"
from_dispatch_auth_required_preserves_required_secrets	From<DispatchError::AuthRequired> forwards non-empty required_secrets
from_dispatch_auth_required_preserves_empty_required_secrets	Same for empty list
auth_requirement_classifies_each_credential_dispatch_reason	Pins all 6 GsuiteCredentialDispatchReason variants against auth_requirement()

All PR-body test suites green post-fix.

[serrrfirat]

Code Review — Pass 2 (e56e714)

Reviewers: Security · Bugs · Performance · Tests · Conventions
Findings: 18 total · 2 High · 9 Medium · 5 Low · 2 Nit
Inline: 13 · Summary-only: 5

---

🔴 High

#	File	Finding
B1	services/runtime_adapters.rs:323	AuthRequired swallowed by Resource when reconcile fails after handler error — see inline
T1	services/tests/first_party_runtime_adapter.rs:54	required_secrets not asserted; reservation cleanup not verified — see inline

🟠 Medium (summary-only)

• B2 runtime_adapters.rs — serde_json::to_vec(&result.output) failure at the success path propagates OutputDecode without releasing the reservation. Every exit path must call release_first_party_reservation or governor.reconcile. Add cleanup in the map_err closure. (conf 97)
• T6 services.rs — RegisteredRuntimeHealth::missing_runtime_backends on an empty-available list never tested. Add tests::services::registered_runtime_health_reports_missing_backends_when_available_is_empty. (conf 93)
• T7 gsuite/handlers.rs — merge_attendees has no test for the no-email addition path (appended unconditionally) or empty existing list. Add tests::handlers::merge_attendees_appends_addition_without_email_field and ::merge_attendees_with_empty_existing_list. (conf 93)

🔵 Nit (summary-only)

• P2 ironclaw_mcp/src/lib.rs:573 — credential_injections Vec cloned for preflight validation because McpJsonRpcMethod::credential_injections consumes it. Extractable as a borrow-based validate_tools_call_credential_injections(&[...]) helper to avoid the clone. (conf 90)
• C4 services.rs:228 — Doc comment on stage_secret_error says "pub(crate) for testing" but the function is called on production paths at lines 214 and 219. Update to "pub(crate) so it can be unit-tested directly; used in production by stage_secret_once." (conf 80)

---

Reviewers failed: none. Intent confidence: high.

[abbyshekit]

Review — wire Reborn auth consumers through staged credentials

Combined pass (Claude Code + Codex 5.5 gpt-5.5). The staging→egress handoff, scope isolation, enum exhaustiveness and fail-closed mapping are verified correct, and it compiles on default + libsql/postgres. But two wired-behavior bugs (both Codex, both verified) break auth-resume and the add_attendees capability.

High — AuthRequired persists Failed, not BlockedAuth, so auth-resume can't resume (Codex P1, verified)

On the dispatch-error path, crates/ironclaw_capabilities/src/host.rs:473 unconditionally calls fail_run_if_configured(..., "Dispatch") and then return Err(CapabilityInvocationError::from(error)). When a first-party handler returns DispatchError::AuthRequired (e.g. a GSuite staging miss), the caller gets an auth gate (AuthorizationRequiresAuth), but the run was already marked Failed. The correct transition exists — apply_run_state_transition_if_configured (helpers.rs:169-196) maps AuthorizationRequiresAuth → block_auth(..., "AuthRequired") → RunStatus::BlockedAuth — but the dispatch path doesn't use it. So turn-blocking / auth-resume paths that rely on BlockedAuth can't resume the blocked invocation. Fix: on the dispatch-error path, route AuthRequired through apply_run_state_transition_if_configured (BlockAuth) instead of the unconditional fail_run("Dispatch").

High — one-shot staging breaks the two-request add_attendees capability (Codex P2, verified)

crates/ironclaw_first_party_extensions/src/gsuite/handlers.rs:85 stages the credential once before capability_execution. But execute_add_attendees (handlers.rs:336-374) performs two egress calls in one execution — a GET (line 342) then a PATCH (line 367). The staged-obligation store is one-shot (take removes the secret on first read), so the GET consumes it and the PATCH fails with a missing required credential. Stage before each runtime HTTP request (or restage before the PATCH) so the capability works with the production stager. (The AuthExpired refresh branch already restages at line 112; the normal GET→PATCH path doesn't.)

Medium — staged secret leaks on pre-egress error (Claude)

gsuite/handlers.rs:85-87 stages before capability_execution(...); if input parsing fails there (or any pre-egress error), the staged secret is left in RuntimeSecretInjectionStore with no explicit cleanup — it only clears via the short TTL. The egress-boundary discard_staged_secret_injections only fires once egress is invoked. Not exploitable (scope+capability+handle keyed, TTL-bounded, not stealable), but a real cleanup gap. Stage after capability_execution succeeds, or discard_for_capability on the handler's error paths. (Fixing the P2 restaging should subsume this.)

Low

• crates/ironclaw_reborn_composition/src/factory.rs:492 — require_product_auth_runtime_ports(&services)? is now called unconditionally in build_local_dev; benign (local-dev always wires egress + secret store), but the error string ("Google OAuth provider backend requires host runtime HTTP egress") is misleading if it fires without OAuth configured.
• crates/ironclaw_host_api/src/dispatch.rs:698-708 — DispatchError::Debug redacts required_secrets names, but SecretHandle/RuntimeAuthGate derive value-revealing Debug elsewhere; inconsistent intent (no real leak — never serialized/logged).

What's good

Correct staging→egress handoff (the same secret_injection_store Arc survives all builder transforms incl. process-port rebinding); thorough fail-closed mapping (missing/expired/revoked → AuthRequired, backend → closed); MCP plan-once-and-reuse removes the old double-plan nondeterminism; scope isolation verified (tenant+user+agent+project+mission+thread+invocation); compiles on default + libsql/postgres; also fixes a pre-existing reservation leak on the OutputDecode path; no production unwrap/expect/panic.

Coverage gaps

A test that a dispatch-level AuthRequired persists BlockedAuth and is resumable (the P1); an add_attendees end-to-end through the production stager asserting the PATCH still has its credential (the P2); the pre-egress staging-leftover cleanup; the refresh path staging the second handle and egress consuming the refreshed value; a production-wiring (libsql) test that a composition-staged GSuite secret is readable by host_runtime egress through the captured ports.

_{Codex 5.5 caught both the auth-resume run-state bug (P1) and the two-request staging break (P2) — both verified against source; the cleanup-gap and nits are from my pass.}

[henrypark133]

Code Review (multi-agent)

Intent: Wire Reborn runtime credential consumers through staged credentials (auth propagation + MCP credential planning for first-party host HTTP egress).

Stats: 6 findings (from 9 raw, 6 after dedup). Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, pattern-refactor. Reviewers failed: none. Body-only: 0.

No findings from: security, bugs, conventions, maintainability, pattern-refactor.

performance

1. Low auth_requirement() allocates a Vec on every is_auth_required() call (handlers.rs:248-272, conf 72) — anchor: crates/ironclaw_first_party_extensions/src/gsuite/handlers.rs:248
   • is_auth_required() is self.auth_requirement().is_some(), so every call runs the full match and allocates/clones a Vec only to discard it. Called at least once per failed first-party dispatch (runtime_adapters.rs:316).
2. Low std::Mutex guards session_ids in async MCP client (lib.rs:501-538, conf 58) — anchor: crates/ironclaw_mcp/src/lib.rs:501
   • McpHostHttpClientState uses std::sync::Mutex for session_ids; current_session_id() and capture_session_id() lock synchronously in async context. Lock is not held across .await (not a correctness bug), but under concurrent MCP dispatch sharing one Arc, all callers serialize on this lock per handshake and per session-id capture. PR adds the session_ids map + cleanup drop handler.

tests

1. Medium auth_requirement() misses Recovery(Configured) -> None branch (handlers.rs:250-258, conf 90) — anchor: crates/ironclaw_first_party_extensions/src/gsuite/handlers.rs:253
   • auth_requirement_classifies_each_credential_dispatch_reason exercises Recovery(SetupRequired) (returns Some(Vec::new())) but never constructs Recovery(Configured) to assert auth_requirement() returns None. The Configured branch (handlers.rs:253) suppresses the auth gate for backend infrastructure failures; a regression would incorrectly trigger the user-facing re-auth prompt for a backend config error.
   • Also flagged by: performance/Low
2. Medium Custom Debug impl for DispatchError::AuthRequired has no redaction test (dispatch.rs:230-250, conf 85) — anchor: crates/ironclaw_host_api/src/dispatch.rs:230
   • The PR adds a manual fmt::Debug for DispatchError that deliberately omits required_secrets from AuthRequired output (replacing with "[N handle(s) redacted]"). This is a log-safety invariant; a regression would leak secret handle identifiers into tracing logs. No test asserts format!("{:?}", ...) excludes the handle name.
   • Also flagged by: maintainability/Low
3. Medium stage_credential failure on the refresh-retry path is not exercised (handlers.rs:112-114, conf 80) — anchor: crates/ironclaw_first_party_extensions/src/gsuite/handlers.rs:112
   • GsuiteExecutor::dispatch calls stage_credential twice: after initial resolution (line 82) and after token refresh retry (line 112). Tests cover staging failure on the first call but use noop_credential_stager() on the refresh path. No test drives a 401-triggered refresh where the post-refresh staging call fails, leaving that error path untested.
   • Also flagged by: conventions/Medium

local-patterns

1. Nit // silent-ok: annotation used on a non-error-swallowing code path (host.rs:1643-1648, conf 75) — anchor: crates/ironclaw_capabilities/src/host.rs:1643
   • // silent-ok: is established in this repo exclusively for paths that swallow/ignore an error result (.ok(), unwrap_or_default(), warn-and-continue). Here it annotates a match arm that constructs required_secrets: Vec::new() — no error is swallowed. A future reader grepping silent-ok to audit error silencing hits a false positive.

[serrrfirat]

Pass-2 review findings addressed (7690a38a9)

All 18 findings from the second review pass fixed. Summary:

Bugs fixed

B1 — AuthRequired swallowed by Resource when reconcile fails (runtime_adapters.rs)
account_or_release_failed_first_party_execution)?; was propagating via ?, returning Resource instead of AuthRequired when the resource governor failed to reconcile. Changed to if let Err(...) = ... { warn!(...) } so the auth signal survives accounting failures.

B2 — Reservation leaked on output serialization failure (runtime_adapters.rs)
serde_json::to_vec(&result.output) could fail (OOM/serde) and return OutputDecode without releasing the reservation. Added release_first_party_reservation to the map_err closure — every exit path now either reconciles or releases.

Security fixes

S1 — Recovery(Configured) wrongly triggered AuthRequired gate (gsuite/handlers.rs)
Catch-all Recovery(_) arm in auth_requirement() returned Some(vec![]) for every CredentialRecoveryKind, including Configured (a backend infra failure). Now returns None for Configured; all other recovery kinds (SetupRequired, ReauthorizeRequired, AccountSelectionRequired) retain Some(vec![]).

S2 — DispatchError Debug leaked secret handle names into logs (dispatch.rs)
Replaced derive(Debug) with a manual Debug impl that prints "[N handle(s) redacted]" for required_secrets instead of the actual handle identifiers.

S3 — stage_secret_error missed LeaseConsumed / UnknownLease (services.rs)
Both have user-actionable stable_reason values (CredentialExpired, MissingCredential) but were mapping to Backend. Added is_consumed() and is_unknown_lease() to the AuthRequired predicate to align with the store's own taxonomy.

Convention / quality fixes

• map_stage_error Backend staging failure tagged BackendAuth (not HostApi) to match map_credential_error's category scheme
• stage_secret_error doc comment corrected — function is used in production by stage_secret_once, not test-only
• gsuite_error doc comment documents that CredentialRecoveryProjection context is intentionally dropped at the composition boundary (structured recovery hints deferred to follow-up)
• validate_tools_call_credential_injections(&[...]) extracted in mcp/lib.rs — removes the .clone() on the credential injections Vec before the MCP handshake

Tests added (23 new)

Crate	Tests
ironclaw_host_runtime (adapter)	required_secrets forwarded through AuthRequired; reservation released on auth-required failure; specific handle forwarded; panic handler → Backend + reservation released
ironclaw_host_runtime (first_party)	kind() returns None for AuthRequired; with_usage preserves required_secrets
ironclaw_host_runtime (services)	stage_secret_error covers LeaseConsumed and UnknownLease; RegisteredRuntimeHealth empty-available, dedup, all-available
ironclaw_reborn_composition	gsuite_error projects stage AuthRequired → FirstPartyCapabilityError::AuthRequired; stage Backend → Dispatch{Backend}
ironclaw_host_api	event_kind() pinned as "auth_required" for both empty and non-empty handle lists
ironclaw_first_party_extensions	merge_attendees empty-existing, no-email addition, no-additions edge cases

All PR-body test suites green. Pre-existing first_party_builtin_tools failures unaffected.

[serrrfirat]

Addressed the review comments in 4fb4cc6a8.

Fixed Review Feedback

Reviewer	Direct quote	Fix
henrypark133	"never constructs Recovery(Configured) to assert auth_requirement() returns None"	Already fixed in 78e0294a9 — added explicit Recovery(Configured) → None assertion in auth_requirement_classifies_each_credential_dispatch_reason.
henrypark133	"No test asserts format!("{:?}", ...) excludes the handle name"	Added dispatch_error_auth_required_debug_redacts_required_secrets in host_api_contract.rs — asserts handle name absent and redaction count present.
henrypark133	"No test drives a 401-triggered refresh where the post-refresh staging call fails"	Added gsuite_handler_stage_credential_failure_on_refresh_retry_returns_auth_required using a new FailOnNthCallStager test double that fails on the 2nd call; verifies auth_required() is true and only one egress request was made.
henrypark133	"all callers serialize on this lock per handshake and per session-id capture"	Added doc comment explaining why std::sync::Mutex is appropriate: O(1) hold time, never held across .await, key includes invocation_id so concurrent invocations use disjoint map entries.
henrypark133	"A future reader grepping silent-ok to audit error silencing hits a false positive"	Replaced // silent-ok: with a plain explanatory comment. silent-ok: is reserved for error-swallowing paths; this site constructs Vec::new() and swallows nothing.

CI

Clippy result_large_err failure fixed in d6ff73182: boxed CredentialRecoveryProjection in GsuiteCredentialDispatchReason::Recovery to reduce the variant from 240 bytes to pointer-sized (0 warnings on base branch, 30 introduced by this PR).

Validation

• cargo fmt --check ✓
• cargo test -p ironclaw_host_api -p ironclaw_capabilities -p ironclaw_first_party_extensions -p ironclaw_mcp ✓ (all green)

[serrrfirat]

Addressed abbyshekit and henrypark133 review comments in d324be3dc and prior commits.

Fixed Review Feedback

Reviewer	Direct quote	Fix
abbyshekit	"dispatch path doesn't use it. So turn-blocking / auth-resume paths that rely on BlockedAuth can't resume the blocked invocation"	P1 fixed: both dispatch paths in host.rs (~L473, ~L810) now call apply_run_state_transition_if_configured instead of unconditional fail_run_if_configured("Dispatch"). AuthRequired → BlockedAuth, not Failed. Pinned by capability_host_blocks_auth_when_dispatch_returns_auth_required.
abbyshekit	"GET consumes it and the PATCH fails with a missing required credential"	P2 fixed: GsuiteCredentialStager passed through CapabilityExecution::execute into execute_add_attendees; re-staged before PATCH with network bytes accounting. Pinned by add_attendees_restages_credential_before_patch.
abbyshekit	"staged secret is left in RuntimeSecretInjectionStore with no explicit cleanup"	Fixed (Medium, subsumed by P2): moved stage_credential to after capability_execution() parse succeeds on both initial and refresh-retry paths. Parse failure no longer leaves a staged credential behind.
abbyshekit	"require_product_auth_runtime_ports ... error string ... is misleading"	Fixed (Low1): updated error message to name both prerequisites (HTTP egress + secret store) without tying it to Google OAuth.
abbyshekit	"DispatchError::Debug redacts required_secrets names, but SecretHandle/RuntimeAuthGate derive value-revealing Debug elsewhere"	Noted (Low2): acknowledged inconsistency; no secret leaks (never serialized/logged), deferring uniform redaction audit to a follow-up.
henrypark133	"never constructs Recovery(Configured) to assert auth_requirement() returns None"	Already fixed in 78e0294a9 — added explicit Recovery(Configured) → None assertion in auth_requirement_classifies_each_credential_dispatch_reason.
henrypark133	"No test asserts format!("{:?}", ...) excludes the handle name"	Added dispatch_error_auth_required_debug_redacts_required_secrets in host_api_contract.rs — asserts handle name absent and redaction count present.
henrypark133	"No test drives a 401-triggered refresh where the post-refresh staging call fails"	Added gsuite_handler_stage_credential_failure_on_refresh_retry_returns_auth_required using FailOnNthCallStager; verifies auth_required() true and only one egress request made.

Validation

• cargo test -p ironclaw_capabilities -p ironclaw_first_party_extensions -p ironclaw_host_api ✓ (all green)
• cargo test -p ironclaw_host_runtime --lib ✓ (164 passed)
• cargo fmt --check ✓