fix(reborn): auto-activate web-access and Brave-backed web_search so agents discover real web search - #6232
Open
pranavraja99 wants to merge 6 commits into
Open
fix(reborn): auto-activate web-access and Brave-backed web_search so agents discover real web search#6232pranavraja99 wants to merge 6 commits into
pranavraja99 wants to merge 6 commits into
Conversation
Contributor
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6232
July 18, 2026 06:02
Destroyed
github-actions
Bot
added
size: L
📝 WalkthroughSummary by CodeRabbit
WalkthroughLocal runtime construction now bundles and bootstraps ChangesFirst-party extension bootstrap
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant build_local_runtime
participant bootstrap_web_search_brave
participant AdminSecretProvisioner
participant RebornLocalExtensionManagementPort
participant bootstrap_web_access
build_local_runtime->>bootstrap_web_search_brave: bootstrap web_search
bootstrap_web_search_brave->>AdminSecretProvisioner: provision brave_api_key
bootstrap_web_search_brave->>RebornLocalExtensionManagementPort: install and activate web_search
build_local_runtime->>bootstrap_web_access: bootstrap web-access when available
bootstrap_web_access->>RebornLocalExtensionManagementPort: inspect, install, and activate web-access
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces auto-activation for the zero-config 'web-access' extension during runtime composition, eliminating the need for manual discovery. It adds a new 'web_access_bootstrap' module, integrates it into the local runtime build process, and updates the test suite to reflect this auto-bootstrapped behavior. Feedback on the changes suggests avoiding an unnecessary clone of the 'UserId' string in the bootstrap module by passing it as a reference.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+72
to
+126
| let caller: UserId = extension_management.tenant_operator_user_id().clone(); | ||
| let package_ref = | ||
| LifecyclePackageRef::new(LifecyclePackageKind::Extension, WEB_ACCESS_EXTENSION_ID) | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web-access package ref is invalid: {error}"), | ||
| })?; | ||
|
|
||
| let phase = extension_management | ||
| .project(package_ref.clone(), &caller) | ||
| .await | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web-access extension projection failed: {error}"), | ||
| })? | ||
| .phase; | ||
|
|
||
| match phase { | ||
| LifecyclePhase::Discovered | LifecyclePhase::Installed => {} | ||
| LifecyclePhase::Active => return Ok(WebAccessBootstrapOutcome::AlreadyActive), | ||
| LifecyclePhase::Removed => { | ||
| tracing::debug!( | ||
| "web-access was explicitly removed; preserving that state rather than \ | ||
| re-installing it" | ||
| ); | ||
| return Ok(WebAccessBootstrapOutcome::SkippedPreservedRemoved); | ||
| } | ||
| LifecyclePhase::Disabled => { | ||
| tracing::debug!( | ||
| "web-access is explicitly disabled; preserving that state rather than \ | ||
| re-activating it" | ||
| ); | ||
| return Ok(WebAccessBootstrapOutcome::SkippedDisabled); | ||
| } | ||
| other => { | ||
| tracing::debug!( | ||
| phase = ?other, | ||
| "web-access is not in an auto-activatable phase; skipping bootstrap" | ||
| ); | ||
| return Ok(WebAccessBootstrapOutcome::SkippedNonActivatable); | ||
| } | ||
| } | ||
|
|
||
| if phase == LifecyclePhase::Discovered { | ||
| extension_management | ||
| .install(package_ref.clone(), &caller) | ||
| .await | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web-access extension install failed: {error}"), | ||
| })?; | ||
| } | ||
| extension_management | ||
| .activate(package_ref, ExtensionActivationMode::Static, &caller) | ||
| .await | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web-access extension activation failed: {error}"), | ||
| })?; |
Contributor
There was a problem hiding this comment.
The caller identity (UserId) is cloned unnecessarily. Since project, install, and activate all accept &UserId, we can pass a reference directly and avoid cloning the heap-allocated UserId string.
let caller = extension_management.tenant_operator_user_id();
let package_ref =
LifecyclePackageRef::new(LifecyclePackageKind::Extension, WEB_ACCESS_EXTENSION_ID)
.map_err(|error| RebornBuildError::InvalidConfig {
reason: format!("web-access package ref is invalid: {error}"),
})?;
let phase = extension_management
.project(package_ref.clone(), caller)
.await
.map_err(|error| RebornBuildError::InvalidConfig {
reason: format!("web-access extension projection failed: {error}"),
})?
.phase;
match phase {
LifecyclePhase::Discovered | LifecyclePhase::Installed => {}
LifecyclePhase::Active => return Ok(WebAccessBootstrapOutcome::AlreadyActive),
LifecyclePhase::Removed => {
tracing::debug!(
"web-access was explicitly removed; preserving that state rather than \
re-installing it"
);
return Ok(WebAccessBootstrapOutcome::SkippedPreservedRemoved);
}
LifecyclePhase::Disabled => {
tracing::debug!(
"web-access is explicitly disabled; preserving that state rather than \
re-activating it"
);
return Ok(WebAccessBootstrapOutcome::SkippedDisabled);
}
other => {
tracing::debug!(
phase = ?other,
"web-access is not in an auto-activatable phase; skipping bootstrap"
);
return Ok(WebAccessBootstrapOutcome::SkippedNonActivatable);
}
}
if phase == LifecyclePhase::Discovered {
extension_management
.install(package_ref.clone(), caller)
.await
.map_err(|error| RebornBuildError::InvalidConfig {
reason: format!("web-access extension install failed: {error}"),
})?;
}
extension_management
.activate(package_ref, ExtensionActivationMode::Static, caller)
.await
.map_err(|error| RebornBuildError::InvalidConfig {
reason: format!("web-access extension activation failed: {error}"),
})?;![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs`:
- Around line 79-120: Replace the caller-masked project preflight in web_access
bootstrap with a host-only lifecycle check that distinguishes never-installed,
removal-tombstone, disabled, and privately owned states without changing
ownership; only auto-install or activate eligible states and preserve removed,
disabled, and private installations. In
crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs
lines 79-120, update the phase/installation flow accordingly. In
crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs
lines 606-695, add production-caller rebuild tests proving removal stays
removed, disabled stays disabled, and a member-private installation is neither
activated nor promoted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1054c3e8-6ecd-4b79-89dd-be0a9b21b624
📒 Files selected for processing (5)
crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rscrates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rscrates/ironclaw_reborn_composition/src/extension_host/mod.rscrates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rscrates/ironclaw_reborn_composition/src/factory.rs
Comment on lines
+79
to
+120
| let phase = extension_management | ||
| .project(package_ref.clone(), &caller) | ||
| .await | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web-access extension projection failed: {error}"), | ||
| })? | ||
| .phase; | ||
|
|
||
| match phase { | ||
| LifecyclePhase::Discovered | LifecyclePhase::Installed => {} | ||
| LifecyclePhase::Active => return Ok(WebAccessBootstrapOutcome::AlreadyActive), | ||
| LifecyclePhase::Removed => { | ||
| tracing::debug!( | ||
| "web-access was explicitly removed; preserving that state rather than \ | ||
| re-installing it" | ||
| ); | ||
| return Ok(WebAccessBootstrapOutcome::SkippedPreservedRemoved); | ||
| } | ||
| LifecyclePhase::Disabled => { | ||
| tracing::debug!( | ||
| "web-access is explicitly disabled; preserving that state rather than \ | ||
| re-activating it" | ||
| ); | ||
| return Ok(WebAccessBootstrapOutcome::SkippedDisabled); | ||
| } | ||
| other => { | ||
| tracing::debug!( | ||
| phase = ?other, | ||
| "web-access is not in an auto-activatable phase; skipping bootstrap" | ||
| ); | ||
| return Ok(WebAccessBootstrapOutcome::SkippedNonActivatable); | ||
| } | ||
| } | ||
|
|
||
| if phase == LifecyclePhase::Discovered { | ||
| extension_management | ||
| .install(package_ref.clone(), &caller) | ||
| .await | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web-access extension install failed: {error}"), | ||
| })?; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not bootstrap from the caller-masked projection.
project() reports Discovered both after removal (the installation and manifest are deleted) and for a private installation hidden from the operator. The bootstrap therefore resurrects explicitly removed Web Access on restart and can convert another member’s private installation into a tenant-shared installation through install().
crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs#L79-L120: use a host-only lifecycle preflight that distinguishes never-installed, removal tombstone, disabled, and private-owned states without changing ownership.crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs#L606-L695: add caller-level rebuild tests proving removal remains removed, disabled remains disabled, and a member-private install is neither activated nor promoted.
As per coding guidelines, lifecycle states must remain distinct, restart reconstruction must revalidate ownership, and removal/restart behavior must be tested through the production caller.
📍 Affects 2 files
crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs#L79-L120(this comment)crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs#L606-L695
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs`
around lines 79 - 120, Replace the caller-masked project preflight in web_access
bootstrap with a host-only lifecycle check that distinguishes never-installed,
removal-tombstone, disabled, and privately owned states without changing
ownership; only auto-install or activate eligible states and preserve removed,
disabled, and private installations. In
crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs
lines 79-120, update the phase/installation flow accordingly. In
crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs
lines 606-695, add production-caller rebuild tests proving removal stays
removed, disabled stays disabled, and a member-private installation is neither
activated nor promoted.
Source: Coding guidelines
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 2 | 0 | 2 | 05bd583dfcd8 |
Head: 05bd583dfcd8dce4fff19c400c9ed60b0e09c615
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Changes requested: the bootstrap cannot distinguish a truly absent extension from a removal or masked private installation, so it can override opt-outs and widen private visibility on restart.
Findings
Blocking: 2 / Notes: 0
Blocking findings
1. ❌ [HIGH] Preserve removal across restarts
Location: crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs:113-115
A successful removal deletes the installation (and then its manifest), so the next startup's project returns Discovered. This branch consequently installs and activates web-access again. An operator who explicitly removed web access will therefore have its network capability restored after restart, contrary to the promised Removed preservation. Persist/query an explicit removal tombstone (or otherwise distinguish it from absence) and add a remove → restart regression test.
2. ❌ [MEDIUM] Do not promote masked private installs at boot
Location: crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs:79-80
project deliberately masks a foreign private installation as Discovered. Bootstrap then treats that result as absent and calls install as the tenant operator; the install policy converts an existing InstallationOwner::Users row to Tenant. Thus a restart silently turns a member's private web-access installation into a tenant-wide tool, despite the SkippedNonActivatable comment describing the opposite behavior. Use host-visible state that can distinguish masked private ownership from absence, and skip rather than promote it; cover this restart scenario in tests.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
| } | ||
| } | ||
|
|
||
| if phase == LifecyclePhase::Discovered { |
Contributor
There was a problem hiding this comment.
Removal deletes the installation and manifest, so the next startup projects this as Discovered and this branch reinstalls it. That overrides an explicit remove after restart; preserve a durable opt-out/tombstone and test remove → restart.
| reason: format!("web-access package ref is invalid: {error}"), | ||
| })?; | ||
|
|
||
| let phase = extension_management |
Contributor
There was a problem hiding this comment.
A foreign private installation is intentionally masked by project as Discovered. The subsequent operator install promotes that existing private row to tenant ownership, so a restart widens a user's private tool to all users. Distinguish this state from true absence and skip it.
|
🚅 Deployed to the ironclaw-pr-6232 environment in ironclaw-ci-preview
|
reborn treats every first-party integration (Gmail, Slack, GitHub, web-access, ...) as an opt-in extension-catalog entry. `extension_search`/ `extension_install`/`extension_activate` are always in the model's tool list (hardcoded CORE_TOOL_NAMES in the progressive-disclosure catalog — this is not a tool-disclosure gap), but nothing in the base prompt ever points the model at that catalog, so it never has a reason to go looking for `web-access`. Any production reborn agent asked a question needing current information silently falls back to whatever raw tools it already has (e.g. `http`) with no way to find a URL. `web-access` is already a zero-config, trust-policy-approved first-party extension (Exa-MCP-backed search + get_content, no credentials). Add `web_access_bootstrap::bootstrap_web_access`, called once from `build_local_runtime` right after `bootstrap_nearai_mcp` (same call site, same install-then-activate-against-current-phase pattern, but simpler: no credential-submit step and no durable-storage gate, since there's nothing to configure). Respects an explicit prior Disabled/Removed choice rather than overriding it. Verified end-to-end against a GAIA benchmark run (nearai/nearai-bench): before this change, tasks needing a real web lookup scored 0 (blind `http`, no way to discover a search capability); after, the same tasks show `web-access.search`/`web-access.get_content` in the trace and pass. Updated two tests whose assumptions the new default-active behavior changes: `local_dev_web_access_installs_activates_and_dispatches_through_host_runtime` no longer needs to install/activate manually (asserts the auto-bootstrap guarantee directly), and `local_dev_extension_lifecycle_tools_manage_visible_extension_surface` (which uses web-access as its example for the generic install/activate/ remove capability-handler round trip) now asserts the already-active starting state before doing remove-then-reinstall, using the real tenant-operator identity for the ownership-sensitive calls.
…figured reborn's own registry already ships a complete, versioned web_search WASM tool (registry/tools/web_search.json, tagged "default") that calls the real Brave Search API — but it was never wired into ironclaw_reborn_composition's extension catalog at all (from_first_party_assets_with_nearai_mcp_config had no web_search_package()), so it was unreachable from reborn regardless of whether an operator configured BRAVE_API_KEY. Vendor the tool's assets (manifest.toml + input/output schemas + prompt doc + the prebuilt wasm32-wasip2 binary, sha256-verified against the registry's declared hash) under ironclaw_first_party_extensions/assets/web-search/, matching the existing bundled-extension convention (github, slack, google-*), and register web_search_package() in the catalog. Add web_search_bootstrap::bootstrap_web_search_brave, mirroring web_access_bootstrap's install-then-activate pattern but gated on a real credential (BRAVE_API_KEY) the way bootstrap_nearai_mcp is: seeds the brave_api_key secret via the existing AdminSecretProvisioner when configured, then installs/activates web_search for the tenant-operator identity unless the user already explicitly disabled or removed it. web_search and web-access share the same model-facing "web_search" display name (see ironclaw_runner::tool_disclosure's capability-id aliasing), so only one may be active at a time without a name collision. build_local_runtime now prefers Brave whenever BRAVE_API_KEY is configured — a real, quota'd backend beats Exa's zero-config but rate-shared free MCP tier — and falls back to bootstrap_web_access's existing zero-config Exa path otherwise, preserving prior behavior for anyone without a Brave key. The manifest's runtime_credentials entry omits `source` (defaults to RuntimeCredentialRequirementSource::SecretHandle), which package_runtime_credential_auth_requirements does not surface as an OAuth requirement — so activation does not block on it, matching how github/gmail/ etc.'s OAuth-gated credentials differ from a bare secret-store lease. The actual credential injection into the outbound X-Subscription-Token header happens at HTTP-egress time via the manifest's declared audience + target, already-existing host-runtime machinery this change doesn't touch. Also rebases the earlier web-access auto-activation commit onto current main and fixes two unrelated call sites in extension_lifecycle_capabilities.rs that had drifted against approval_test_support::invoke_json_with_local_dev_approval's current (4-arg, no TrustDecision) signature, plus reborn_runner.rs's rename of RebornRuntimeProfileOptions on the benchmarks side. Verified: cargo test -p ironclaw_reborn_composition --features test-support --lib -> 1643 passed, 0 failed. cargo clippy -p ironclaw_reborn_composition --features test-support --tests -- -D warnings -> clean. cargo fmt --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
secret_owner_scope (the dispatch-time credential pre-flight) only ever checks the invocation caller's own (tenant, user) scope, then falls back to tenant_shared_managed_scope() -- never an arbitrary third scope. Provisioning the bootstrapped brave_api_key secret under the bootstrap caller's own owner_scope left it invisible to every real capability invocation, which kept surfacing AuthRequired instead of ever calling Brave. Provision under owner_scope.tenant_shared_managed_scope() instead, matching every other secret-store write in this crate. Also threads a real RebornLocalRuntimeIdentity through the regression test (matching ExecutionContext::local_default's tenant/agent) and adds an end-to-end invocation assertion that the credential gate itself clears, bounded by a short timeout so a slow/failing egress attempt can't turn the test into a multi-minute hang.
DenyWasmHostSecrets (unconditional false) was the only implementation of WasmHostSecrets anywhere in the codebase, so any WASM tool that pre-flights its own credential via near::agent::host::secret_exists before attempting a call (as the web_search Brave tool does) could never proceed, even once the secret was genuinely provisioned and visible to the separate WasmRuntimeCredentialProvider / dispatch-time AuthRequired pre-flight paths. Add WasmRuntimeSecretsAdapter, resolved via the same secret_owner_scope helper (own scope, falling back to tenant-shared managed scope) that already backs those other two paths, and swap it into host_for_scope per-invocation alongside the existing HTTP adapter swap. Thread a new secret_store: Option<Arc<dyn SecretStore>> through WasmRuntimeAdapter:: new/try_new from the one real call site (services/builder.rs). WasmHostSecrets::exists is a synchronous guest-facing call; bridged to the async secret store via Handle::current().block_on(...), safe here because WASM guest execution runs inside its own spawn_blocking context (see wasm_execution.rs), not a normal async worker thread.
# Conflicts: # crates/ironclaw_host_runtime/src/services/runtime_adapters.rs # crates/ironclaw_reborn_composition/src/extension_host/available_extensions.rs # crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rs # crates/ironclaw_reborn_composition/src/factory.rs
pranavraja99
force-pushed
the
fix/reborn-web-access-default-active
branch
from
July 23, 2026 00:58
05bd583 to
bcf92cf
Compare
github-actions
Bot
added
scope: docs
pranavraja99
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/ironclaw_reborn_composition/src/extension_host/available_extensions.rs (2)
1892-1943: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExtend the asset-ref integrity test to cover
web_search.
bundled_first_party_manifest_asset_refs_are_packagedvalidates every capability'sinput_schema_ref/output_schema_ref/prompt_doc_refresolves to a packaged asset for each listed id, but the newweb_searchid isn't in the list — a broken schema/prompt path for the new capability would ship undetected.✅ Proposed fix
let mut extension_ids = vec![ "github", "notion", "web-access", + "web_search", NEARAI_EXTENSION_ID,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/extension_host/available_extensions.rs` around lines 1892 - 1943, Extend the extension_ids list in bundled_first_party_manifest_asset_refs_are_packaged to include the web_search extension id, using its existing identifying constant if available, so its capability schema and prompt asset references are validated.
1802-1807: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAdd
"web_search"toreserved_host_bundled_extension_id.Every other bundled first-party id (
github,notion,web-access,slack_bot,NEARAI_EXTENSION_ID, gsuite) is reserved so filesystem discovery can't load a same-idInstalledLocalmanifest that would thencatalog.extend()-replace the compiled-inHostBundledentry. The newweb_searchid is missing, leaving the freshly-bundled Brave tool (with real credential injection) open to the exact trust-laundering this function exists to block.🔒 Proposed fix
pub(crate) fn reserved_host_bundled_extension_id(extension_id: &ExtensionId) -> bool { matches!( extension_id.as_str(), - "github" | "notion" | "web-access" | "slack_bot" | NEARAI_EXTENSION_ID + "github" | "notion" | "web-access" | "web_search" | "slack_bot" | NEARAI_EXTENSION_ID ) || is_gsuite_extension_id(extension_id) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/extension_host/available_extensions.rs` around lines 1802 - 1807, Add the "web_search" identifier to the match list in reserved_host_bundled_extension_id, preserving the existing reservations and gsuite handling so filesystem discovery cannot replace the bundled HostBundled entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_host_runtime/src/services/runtime_adapters.rs`:
- Around line 917-921: Restrict WASM secret probing to manifest-declared handles
by threading the capability’s declared runtime-credential handle set into
WasmRuntimeSecretsAdapter from the surrounding dispatch path. Update the
adapter’s exists/access checks to return false before calling
secret_owner_scope() for undeclared names, while preserving allowed owner-scoped
access and tenant-shared fallback. Add caller-level WASM dispatch tests covering
both allowed cases and denial of undeclared handles.
In
`@crates/ironclaw_reborn_composition/src/extension_host/web_search_bootstrap.rs`:
- Around line 97-199: The phase check in bootstrap_web_search_brave must stop
relying on project(), which hides removal tombstones and foreign-private
ownership. In
crates/ironclaw_reborn_composition/src/extension_host/web_search_bootstrap.rs#L97-199,
add a host-only lifecycle preflight that distinguishes never-installed,
removal-tombstone, disabled, and foreign-private-owned states before
provisioning brave_api_key or installing/activating; preserve removed and
private states without ownership changes. Apply the same preflight to
bootstrap_web_access in
crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs#L69-128.
Add caller-level restart coverage for both extensions proving explicit removal
remains removed and member-private installs are neither activated nor promoted.
In `@crates/ironclaw_reborn_composition/src/factory.rs`:
- Around line 7100-7209: Add the missing conditional compilation gate to the
test containing the bootstrap_web_search_brave regression coverage, matching the
gate used by its surrounding local-development tests. Keep the tenant-shared
scope assertions and arbitrary-caller credential-gate invocation unchanged,
while ensuring the test only compiles and runs in the intended configuration.
- Around line 7065-7099: Add a `#[cfg(any(feature = "libsql", feature =
"postgres"))]` attribute to the
`local_dev_web_search_brave_is_auto_installed_and_activated_when_configured`
test, matching the durable-backend gating used by sibling tests, so it is
excluded when `admin_secret_provisioner` is unavailable.
---
Outside diff comments:
In
`@crates/ironclaw_reborn_composition/src/extension_host/available_extensions.rs`:
- Around line 1892-1943: Extend the extension_ids list in
bundled_first_party_manifest_asset_refs_are_packaged to include the web_search
extension id, using its existing identifying constant if available, so its
capability schema and prompt asset references are validated.
- Around line 1802-1807: Add the "web_search" identifier to the match list in
reserved_host_bundled_extension_id, preserving the existing reservations and
gsuite handling so filesystem discovery cannot replace the bundled HostBundled
entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 29523368-a0c9-4e3b-9a4b-f4366245b8ee
⛔ Files ignored due to path filters (1)
crates/ironclaw_first_party_extensions/assets/web-search/wasm/web_search_tool.wasmis excluded by!**/*.wasm,!**/*.wasm
📒 Files selected for processing (14)
crates/ironclaw_first_party_extensions/assets/web-search/manifest.tomlcrates/ironclaw_first_party_extensions/assets/web-search/prompts/web_search/search.mdcrates/ironclaw_first_party_extensions/assets/web-search/schemas/web_search/search.input.v1.jsoncrates/ironclaw_first_party_extensions/assets/web-search/schemas/web_search/search.output.v1.jsoncrates/ironclaw_host_runtime/src/services.rscrates/ironclaw_host_runtime/src/services/builder.rscrates/ironclaw_host_runtime/src/services/runtime_adapters.rscrates/ironclaw_reborn_composition/src/extension_host/available_extensions.rscrates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rscrates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rscrates/ironclaw_reborn_composition/src/extension_host/mod.rscrates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rscrates/ironclaw_reborn_composition/src/extension_host/web_search_bootstrap.rscrates/ironclaw_reborn_composition/src/factory.rs
Comment on lines
+917
to
+921
| match &self.secret_store { | ||
| Some(store) => host.with_secrets(Arc::new(WasmRuntimeSecretsAdapter { | ||
| store: Arc::clone(store), | ||
| scope: scope.clone(), | ||
| })), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict WASM secret probes to manifest-declared handles.
Line 918 installs a scope-wide oracle: exists() accepts any valid handle, while web_search only declares brave_api_key in manifest.toml Lines 22-24. An untrusted WASM extension can probe other caller or tenant-shared secret names. Thread the capability’s declared runtime-credential handles into WasmRuntimeSecretsAdapter and return false before secret_owner_scope() for undeclared names. Add caller-level WASM dispatch tests for allowed owner-scoped access, allowed tenant-shared fallback, and denied undeclared handles.
As per coding guidelines, credentials must remain host-side and be injected only at the narrowest egress boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_host_runtime/src/services/runtime_adapters.rs` around lines
917 - 921, Restrict WASM secret probing to manifest-declared handles by
threading the capability’s declared runtime-credential handle set into
WasmRuntimeSecretsAdapter from the surrounding dispatch path. Update the
adapter’s exists/access checks to return false before calling
secret_owner_scope() for undeclared names, while preserving allowed owner-scoped
access and tenant-shared fallback. Add caller-level WASM dispatch tests covering
both allowed cases and denial of undeclared handles.
Source: Coding guidelines
Comment on lines
+97
to
+199
| pub(crate) async fn bootstrap_web_search_brave( | ||
| api_key: Option<SecretString>, | ||
| extension_management: &Arc<RebornLocalExtensionManagementPort>, | ||
| admin_secret_provisioner: Option<&Arc<dyn AdminSecretProvisioner>>, | ||
| owner_scope: &ironclaw_host_api::ResourceScope, | ||
| ) -> Result<WebSearchBootstrapOutcome, RebornBuildError> { | ||
| let Some(api_key) = api_key else { | ||
| return Ok(WebSearchBootstrapOutcome::NotConfigured); | ||
| }; | ||
|
|
||
| let caller: UserId = extension_management.tenant_operator_user_id().clone(); | ||
| let package_ref = | ||
| LifecyclePackageRef::new(LifecyclePackageKind::Extension, WEB_SEARCH_EXTENSION_ID) | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web_search package ref is invalid: {error}"), | ||
| })?; | ||
|
|
||
| let phase = extension_management | ||
| .project(package_ref.clone(), &caller) | ||
| .await | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web_search extension projection failed: {error}"), | ||
| })? | ||
| .phase; | ||
|
|
||
| match phase { | ||
| LifecyclePhase::Discovered | LifecyclePhase::Installed => {} | ||
| LifecyclePhase::Active => return Ok(WebSearchBootstrapOutcome::AlreadyActive), | ||
| LifecyclePhase::Removed => { | ||
| tracing::debug!( | ||
| "web_search was explicitly removed; preserving that state rather than \ | ||
| re-installing it" | ||
| ); | ||
| return Ok(WebSearchBootstrapOutcome::SkippedPreservedRemoved); | ||
| } | ||
| LifecyclePhase::Disabled => { | ||
| tracing::debug!( | ||
| "web_search is explicitly disabled; preserving that state rather than \ | ||
| re-activating it" | ||
| ); | ||
| return Ok(WebSearchBootstrapOutcome::SkippedDisabled); | ||
| } | ||
| other => { | ||
| tracing::debug!( | ||
| phase = ?other, | ||
| "web_search is not in an auto-activatable phase; skipping bootstrap" | ||
| ); | ||
| return Ok(WebSearchBootstrapOutcome::SkippedNonActivatable); | ||
| } | ||
| } | ||
|
|
||
| // Seed the secret before activation so the tool's declared credential | ||
| // injection has something to lease. Best-effort: a provisioner is only | ||
| // wired up on backends with durable secret-store crypto; if it's absent | ||
| // we still install/activate (the tool itself fails closed per-call with | ||
| // a clear "Brave API key not found" message rather than silently | ||
| // pretending to work), so a missing provisioner is not fatal here. | ||
| // | ||
| // Provision under the TENANT-SHARED scope, not the bootstrap caller's own | ||
| // (tenant, user) pair: `web_search` installs Tenant-owned (same as | ||
| // `web-access`), and the host's dispatch-time credential pre-flight | ||
| // (`secret_owner_scope`) only ever checks the model-invocation caller's | ||
| // own scope, then falls back to `caller_scope.tenant_shared_managed_scope()` | ||
| // — never an arbitrary third scope. Provisioning under the bootstrap's | ||
| // own owner scope left the secret invisible to every real invocation | ||
| // (dispatch-time pre-flight always reported it absent, surfacing | ||
| // AuthRequired instead of ever calling Brave). | ||
| if let Some(provisioner) = admin_secret_provisioner { | ||
| let handle = SecretHandle::new(BRAVE_API_KEY_SECRET_NAME).map_err(|error| { | ||
| RebornBuildError::InvalidConfig { | ||
| reason: format!("brave_api_key secret handle is invalid: {error}"), | ||
| } | ||
| })?; | ||
| let shared_scope = owner_scope.tenant_shared_managed_scope(); | ||
| provisioner | ||
| .put( | ||
| &shared_scope.tenant_id, | ||
| &shared_scope.user_id, | ||
| handle, | ||
| api_key, | ||
| ) | ||
| .await | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("brave_api_key secret provisioning failed: {error}"), | ||
| })?; | ||
| } | ||
|
|
||
| if phase == LifecyclePhase::Discovered { | ||
| extension_management | ||
| .install(package_ref.clone(), &caller) | ||
| .await | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web_search extension install failed: {error}"), | ||
| })?; | ||
| } | ||
| extension_management | ||
| .activate(package_ref, ExtensionActivationMode::Static, &caller) | ||
| .await | ||
| .map_err(|error| RebornBuildError::InvalidConfig { | ||
| reason: format!("web_search extension activation failed: {error}"), | ||
| })?; | ||
| Ok(WebSearchBootstrapOutcome::Activated) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Both auto-bootstrap modules branch on a project() phase that cannot represent "removed" or "foreign-private", so neither actually honors an explicit removal or protects a private install across a restart.
project() (see extension_lifecycle.rs's phase_for_activation_state) only returns Discovered | Installed | Disabled | Active; removal deletes the installation row so it is indistinguishable from never-installed, and a foreign owner's private install is masked to Discovered too. Fixing only one of these two near-identical modules leaves the same restart-time resurrection/ownership-widening hazard in the other.
crates/ironclaw_reborn_composition/src/extension_host/web_search_bootstrap.rs#L97-L199: replace theproject()-based phase check with a host-only lifecycle preflight that distinguishes never-installed, removal-tombstone, disabled, and foreign-private-owned states without changing ownership, before deciding to install/activate or provisionbrave_api_key.crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs#L69-L128: apply the same host-only preflight here (this is the already-flagged, still-unresolved instance).
Add a caller-level restart test proving an explicit removal stays removed and a member-private install is neither activated nor promoted, for both extensions.
📍 Affects 2 files
crates/ironclaw_reborn_composition/src/extension_host/web_search_bootstrap.rs#L97-L199(this comment)crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs#L69-L128
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/ironclaw_reborn_composition/src/extension_host/web_search_bootstrap.rs`
around lines 97 - 199, The phase check in bootstrap_web_search_brave must stop
relying on project(), which hides removal tombstones and foreign-private
ownership. In
crates/ironclaw_reborn_composition/src/extension_host/web_search_bootstrap.rs#L97-199,
add a host-only lifecycle preflight that distinguishes never-installed,
removal-tombstone, disabled, and foreign-private-owned states before
provisioning brave_api_key or installing/activating; preserve removed and
private states without ownership changes. Apply the same preflight to
bootstrap_web_access in
crates/ironclaw_reborn_composition/src/extension_host/web_access_bootstrap.rs#L69-128.
Add caller-level restart coverage for both extensions proving explicit removal
remains removed and member-private installs are neither activated nor promoted.
Comment on lines
+7065
to
+7099
| #[tokio::test] | ||
| async fn local_dev_web_search_brave_is_auto_installed_and_activated_when_configured() { | ||
| let dir = tempfile::tempdir().expect("tempdir"); | ||
| let owner = "local-dev-web-search-owner"; | ||
| // Match `ExecutionContext::local_default`'s tenant/agent below (both | ||
| // resolve to `LOCAL_DEFAULT_TENANT_ID`/`LOCAL_DEFAULT_AGENT_ID` when no | ||
| // explicit identity is threaded through) — this is exactly the | ||
| // scope-mismatch bug this test now guards against: the bootstrap | ||
| // must provision the secret under the SAME (tenant, user) a real | ||
| // capability invocation will look it up under, not whatever internal | ||
| // fallback identity composition happens to default to. | ||
| let runtime_identity = RebornLocalRuntimeIdentity { | ||
| tenant_id: ironclaw_host_api::TenantId::new(ironclaw_host_api::LOCAL_DEFAULT_TENANT_ID) | ||
| .expect("valid tenant id"), | ||
| agent_id: ironclaw_host_api::AgentId::new(ironclaw_host_api::LOCAL_DEFAULT_AGENT_ID) | ||
| .expect("valid agent id"), | ||
| }; | ||
| let services = build_reborn_services( | ||
| RebornBuildInput::local_dev(owner, dir.path().join("local-dev")) | ||
| .with_local_runtime_identity( | ||
| runtime_identity.tenant_id.clone(), | ||
| runtime_identity.agent_id.clone(), | ||
| ), | ||
| ) | ||
| .await | ||
| .expect("local-dev services build"); | ||
| let local_runtime = services.local_runtime.as_ref().expect("local runtime"); | ||
| let extension_management = local_runtime | ||
| .extension_management | ||
| .as_ref() | ||
| .expect("extension management"); | ||
| let admin_secret_provisioner = local_runtime | ||
| .admin_secret_provisioner | ||
| .as_ref() | ||
| .expect("admin secret provisioner"); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Missing #[cfg(any(feature = "libsql", feature = "postgres"))] on this test — will panic on a no-durable-backend build.
admin_secret_provisioner is only ever Some when the durable backend path populates it (lines 1725-1737); the no-durable build leaves it None. This test unconditionally does local_runtime.admin_secret_provisioner.as_ref().expect("admin secret provisioner") at lines 7096-7099 with no cfg gate, unlike every sibling durable-dependent test in this file (e.g. local_dev_nearai_mcp_auto_bootstraps_from_injected_config at line 7618-7619, resolve_local_dev_secret_master_key_rejects_malformed_file_with_path_context at line 6534-6535). Under cargo test --no-default-features --features test-support this .expect() panics.
🐛 Proposed fix
+ #[cfg(any(feature = "libsql", feature = "postgres"))]
#[tokio::test]
async fn local_dev_web_search_brave_is_auto_installed_and_activated_when_configured() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[tokio::test] | |
| async fn local_dev_web_search_brave_is_auto_installed_and_activated_when_configured() { | |
| let dir = tempfile::tempdir().expect("tempdir"); | |
| let owner = "local-dev-web-search-owner"; | |
| // Match `ExecutionContext::local_default`'s tenant/agent below (both | |
| // resolve to `LOCAL_DEFAULT_TENANT_ID`/`LOCAL_DEFAULT_AGENT_ID` when no | |
| // explicit identity is threaded through) — this is exactly the | |
| // scope-mismatch bug this test now guards against: the bootstrap | |
| // must provision the secret under the SAME (tenant, user) a real | |
| // capability invocation will look it up under, not whatever internal | |
| // fallback identity composition happens to default to. | |
| let runtime_identity = RebornLocalRuntimeIdentity { | |
| tenant_id: ironclaw_host_api::TenantId::new(ironclaw_host_api::LOCAL_DEFAULT_TENANT_ID) | |
| .expect("valid tenant id"), | |
| agent_id: ironclaw_host_api::AgentId::new(ironclaw_host_api::LOCAL_DEFAULT_AGENT_ID) | |
| .expect("valid agent id"), | |
| }; | |
| let services = build_reborn_services( | |
| RebornBuildInput::local_dev(owner, dir.path().join("local-dev")) | |
| .with_local_runtime_identity( | |
| runtime_identity.tenant_id.clone(), | |
| runtime_identity.agent_id.clone(), | |
| ), | |
| ) | |
| .await | |
| .expect("local-dev services build"); | |
| let local_runtime = services.local_runtime.as_ref().expect("local runtime"); | |
| let extension_management = local_runtime | |
| .extension_management | |
| .as_ref() | |
| .expect("extension management"); | |
| let admin_secret_provisioner = local_runtime | |
| .admin_secret_provisioner | |
| .as_ref() | |
| .expect("admin secret provisioner"); | |
| #[cfg(any(feature = "libsql", feature = "postgres"))] | |
| #[tokio::test] | |
| async fn local_dev_web_search_brave_is_auto_installed_and_activated_when_configured() { | |
| let dir = tempfile::tempdir().expect("tempdir"); | |
| let owner = "local-dev-web-search-owner"; | |
| // Match `ExecutionContext::local_default`'s tenant/agent below (both | |
| // resolve to `LOCAL_DEFAULT_TENANT_ID`/`LOCAL_DEFAULT_AGENT_ID` when no | |
| // explicit identity is threaded through) — this is exactly the | |
| // scope-mismatch bug this test now guards against: the bootstrap | |
| // must provision the secret under the SAME (tenant, user) a real | |
| // capability invocation will look it up under, not whatever internal | |
| // fallback identity composition happens to default to. | |
| let runtime_identity = RebornLocalRuntimeIdentity { | |
| tenant_id: ironclaw_host_api::TenantId::new(ironclaw_host_api::LOCAL_DEFAULT_TENANT_ID) | |
| .expect("valid tenant id"), | |
| agent_id: ironclaw_host_api::AgentId::new(ironclaw_host_api::LOCAL_DEFAULT_AGENT_ID) | |
| .expect("valid agent id"), | |
| }; | |
| let services = build_reborn_services( | |
| RebornBuildInput::local_dev(owner, dir.path().join("local-dev")) | |
| .with_local_runtime_identity( | |
| runtime_identity.tenant_id.clone(), | |
| runtime_identity.agent_id.clone(), | |
| ), | |
| ) | |
| .await | |
| .expect("local-dev services build"); | |
| let local_runtime = services.local_runtime.as_ref().expect("local runtime"); | |
| let extension_management = local_runtime | |
| .extension_management | |
| .as_ref() | |
| .expect("extension management"); | |
| let admin_secret_provisioner = local_runtime | |
| .admin_secret_provisioner | |
| .as_ref() | |
| .expect("admin secret provisioner"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_reborn_composition/src/factory.rs` around lines 7065 - 7099,
Add a `#[cfg(any(feature = "libsql", feature = "postgres"))]` attribute to the
`local_dev_web_search_brave_is_auto_installed_and_activated_when_configured`
test, matching the durable-backend gating used by sibling tests, so it is
excluded when `admin_secret_provisioner` is unavailable.
Comment on lines
+7100
to
+7209
| let owner_scope = local_dev_nearai_mcp_owner_scope( | ||
| UserId::new(owner).expect("valid owner"), | ||
| Some(&runtime_identity), | ||
| ) | ||
| .expect("owner scope"); | ||
|
|
||
| // Without a configured key, `build_reborn_services` above already ran | ||
| // `bootstrap_web_search_brave(None, ...)` — assert it left `web_search` | ||
| // untouched (still just `Discovered`, matching the fallback-to-Exa | ||
| // behavior the mutual-exclusion logic in `build_local_runtime` relies | ||
| // on) before exercising the configured path directly. | ||
| let web_search_ref = | ||
| LifecyclePackageRef::new(LifecyclePackageKind::Extension, "web_search") | ||
| .expect("valid ref"); | ||
| let phase_before = extension_management | ||
| .project( | ||
| web_search_ref.clone(), | ||
| extension_management.tenant_operator_user_id_for_test(), | ||
| ) | ||
| .await | ||
| .expect("project web_search before bootstrap") | ||
| .phase; | ||
| assert_eq!(phase_before, LifecyclePhase::Discovered); | ||
|
|
||
| let outcome = crate::extension_host::web_search_bootstrap::bootstrap_web_search_brave( | ||
| Some(secrecy::SecretString::from("test-brave-key".to_string())), | ||
| extension_management, | ||
| Some(admin_secret_provisioner), | ||
| &owner_scope, | ||
| ) | ||
| .await | ||
| .expect("brave bootstrap succeeds"); | ||
| assert_eq!( | ||
| outcome, | ||
| crate::extension_host::web_search_bootstrap::WebSearchBootstrapOutcome::Activated | ||
| ); | ||
| assert!(!outcome.leaves_web_access_available()); | ||
|
|
||
| let phase_after = extension_management | ||
| .project( | ||
| web_search_ref, | ||
| extension_management.tenant_operator_user_id_for_test(), | ||
| ) | ||
| .await | ||
| .expect("project web_search after bootstrap") | ||
| .phase; | ||
| assert_eq!(phase_after, LifecyclePhase::Active); | ||
|
|
||
| // Must be provisioned under the TENANT-SHARED scope, not the | ||
| // bootstrap caller's own (tenant, user) — `secret_owner_scope` (the | ||
| // dispatch-time credential pre-flight in ironclaw_host_runtime) only | ||
| // ever checks the real invocation caller's own scope, then falls back | ||
| // to `caller_scope.tenant_shared_managed_scope()`, never an arbitrary | ||
| // third scope. Provisioning anywhere else leaves the secret invisible | ||
| // to every real capability invocation regardless of who calls it — | ||
| // this regressed once already (dispatch-time pre-flight kept | ||
| // reporting the secret absent and surfacing AuthRequired, so Brave | ||
| // was never actually called end-to-end despite `web_search` | ||
| // reporting `Active`). | ||
| let shared_scope = owner_scope.tenant_shared_managed_scope(); | ||
| let secrets = admin_secret_provisioner | ||
| .list(&shared_scope.tenant_id, &shared_scope.user_id) | ||
| .await | ||
| .expect("list secrets"); | ||
| assert!( | ||
| secrets | ||
| .iter() | ||
| .any(|meta| meta.handle.as_str() == "brave_api_key"), | ||
| "expected brave_api_key to be seeded at the tenant-shared scope, got: {secrets:?}" | ||
| ); | ||
|
|
||
| // End-to-end regression guard for the scope bug itself: invoke | ||
| // `web_search.search` as an ARBITRARY caller (not the bootstrap/tenant- | ||
| // operator identity) and assert the credential pre-flight does not | ||
| // gate it on `AuthRequired`. Before the tenant-shared-scope fix this | ||
| // always returned `AuthRequired` regardless of who called it, because | ||
| // the secret was provisioned under a scope no real invocation could | ||
| // ever read back. Whatever happens after the credential check (a real | ||
| // network call to Brave with a fake key, which can be slow to fail) | ||
| // is out of scope here — bounded by a short timeout so a slow/hanging | ||
| // egress attempt can't turn this into a multi-minute test; a timeout | ||
| // still proves the credential gate itself was cleared. | ||
| let context = web_search_context("web_search.search"); | ||
| enable_global_auto_approve_for_context(local_runtime, &context).await; | ||
| let invocation = services | ||
| .host_runtime | ||
| .as_ref() | ||
| .expect("host runtime") | ||
| .invoke_capability(RuntimeCapabilityRequest::new( | ||
| context, | ||
| CapabilityId::new("web_search.search").unwrap(), | ||
| ResourceEstimate::default(), | ||
| serde_json::json!({ "query": "ironclaw reborn" }), | ||
| )); | ||
| match tokio::time::timeout(std::time::Duration::from_secs(10), invocation).await { | ||
| Ok(result) => { | ||
| let outcome = result.expect("runtime invocation completes"); | ||
| assert!( | ||
| !matches!(outcome, RuntimeCapabilityOutcome::AuthRequired(_)), | ||
| "expected the brave_api_key credential pre-flight to resolve \ | ||
| (regardless of what the actual HTTP call then does), got: {outcome:?}" | ||
| ); | ||
| } | ||
| Err(_) => { | ||
| // Timed out past the credential gate (presumably stuck on the | ||
| // real network call with a fake key) — the gate itself was | ||
| // cleared, which is all this test asserts. | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Rest of the test — scope-mismatch regression coverage is solid.
The tenant-shared-scope assertion (7148-7169) and the arbitrary-caller end-to-end credential-gate check (7171-7209) directly guard the exact scope bug called out in the comments. Contingent on the missing cfg gate fix above being applied so this actually runs where it's meant to.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_reborn_composition/src/factory.rs` around lines 7100 - 7209,
Add the missing conditional compilation gate to the test containing the
bootstrap_web_search_brave regression coverage, matching the gate used by its
surrounding local-development tests. Keep the tenant-shared scope assertions and
arbitrary-caller credential-gate invocation unchanged, while ensuring the test
only compiles and runs in the intended configuration.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_reborn_composition/src/factory.rs (1)
7065-7208: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDrive configured Brave bootstrap through the runtime build
Configuration is read by a
BRAVE_API_KEYoverride/env helper inbootstrap_web_search_brave; this test bypasses that gate by calling the helper withSome(...), and it still uses fake real Brave egress while accepting a timeout as credential-pre-flight success. Configure Brave through a test helper/input (or hermetically via the env override) sobuild_local_runtimechooses Brave, then use controlled egress/assert an execution failure instead ofAuthRequired.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/factory.rs` around lines 7065 - 7208, Update local_dev_web_search_brave_is_auto_installed_and_activated_when_configured to configure BRAVE_API_KEY through the build test input or scoped environment override, then exercise build_reborn_services/build_local_runtime without directly calling bootstrap_web_search_brave. Replace the real Brave request and timeout with controlled egress, asserting the invocation reaches execution and fails there rather than returning AuthRequired, while preserving the activation and tenant-shared secret assertions.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/factory.rs`:
- Around line 7065-7208: Update
local_dev_web_search_brave_is_auto_installed_and_activated_when_configured to
configure BRAVE_API_KEY through the build test input or scoped environment
override, then exercise build_reborn_services/build_local_runtime without
directly calling bootstrap_web_search_brave. Replace the real Brave request and
timeout with controlled egress, asserting the invocation reaches execution and
fails there rather than returning AuthRequired, while preserving the activation
and tenant-shared secret assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2a46ef10-4bd6-49ae-b62c-476fe07aca39
📒 Files selected for processing (1)
crates/ironclaw_reborn_composition/src/factory.rs
29 tasks
# Conflicts: # crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rs # crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs # crates/ironclaw_reborn_composition/src/factory.rs # crates/ironclaw_reborn_composition/src/factory/tests.rs
pranavraja99
force-pushed
the
fix/reborn-web-access-default-active
branch
from
July 24, 2026 06:18
2d9ffac to
a395ca9
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reborn treats every first-party integration (Gmail, Slack, GitHub,
web-access, ...) as an opt-in extension-catalog entry.extension_search/extension_install/extension_activateare always in the model's tool list (they're in the hardcodedCORE_TOOL_NAMESprogressive-disclosure allowlist — this is not a tool-disclosure gap), but nothing in the base prompt/toolset ever points the model at that catalog, so it never has a reason to go looking forweb-access(orweb_search, added below). Any production reborn agent asked a question needing current information silently falls back to whatever raw tools it already has (e.g.http) with no way to discover a URL.web-accessis already a zero-config, trust-policy-approved first-party extension (Exa-MCP-backed search +get_content, no credentials needed). This PR auto-installs and activates it once at runtime-composition time, the same waybootstrap_nearai_mcpalready auto-activates thenearaiextension when NEAR AI credentials are configured — just simpler, since there's no credential-submit step or durable-storage gate to worry about.web_access_bootstrap::bootstrap_web_access(new): checks the current lifecycle phase for the runtime's tenant-operator identity, installs (ifDiscovered) then activates (if not alreadyActive) — and respects an explicit priorDisabled/Removedchoice rather than overriding it.build_local_runtime, immediately after the existingbootstrap_nearai_mcpcall (same call site, same pattern).Update: real, credentialed Brave Search (
web_search), gated onBRAVE_API_KEYweb-access's Exa MCP backend is zero-config but rate-shared/free-tier.web_searchis a separate, already-registry-published, fully-built first-party WASM tool that calls the real Brave Search API — it just had no auto-activation path either, so an operator who setBRAVE_API_KEYnever actually got it used.web_search_bootstrap::bootstrap_web_search_brave(new): same install-then-activate pattern asweb_access_bootstrap, but gated on a real credential — only runs ifBRAVE_API_KEYis set (web_search_bootstrap_api_key_from_envreturnsNonefor unset/empty, and the bootstrap short-circuits toNotConfiguredwith no install/activate at all in that case).web-accessandweb_searchshare the same model-facingweb_searchdisplay name (seeironclaw_runner::tool_disclosure), so only one may be active —build_local_runtimetries the Brave bootstrap first and only falls back tobootstrap_web_access(Exa) whenWebSearchBootstrapOutcome::leaves_web_access_available()is true (i.e. no key configured).brave_api_keysecret is provisioned under the tenant-shared scope (owner_scope.tenant_shared_managed_scope()), not the bootstrap caller's own(tenant, user)— the dispatch-time credential pre-flight (secret_owner_scope) only ever checks the real invocation caller's own scope, then falls back to the tenant-shared scope, never an arbitrary third scope. Provisioning anywhere else left the secret invisible to every real capability invocation (silentAuthRequiredregardless of who called it).WasmHostSecretsimplementation (newWasmRuntimeSecretsAdapterinironclaw_host_runtime): a WASM guest's ownsecret-exists/secret-gethost calls (e.g.web_search's ownnear::agent::host::secret_existspre-flight before attempting its Brave HTTP call) are a separate mechanism from the existingWasmRuntimeCredentialProvider(which only injects credentials into the tool's own outbound HTTP headers).DenyWasmHostSecrets(unconditionalfalse) was the only implementation anywhere in the codebase, soweb_searchcould never proceed past its own credential check even once the secret was genuinely provisioned and visible everywhere else. Fixed by resolving via the samesecret_owner_scopehelper that already backs the other two paths, swapped intohost_for_scopeper-invocation alongside the existing HTTP adapter swap.Verification (Brave update)
Verified end-to-end against a real 165-task GAIA benchmark run (via
nearai/nearai-bench), not just unit tests:web_search.searchcalls across the run, 99.9% succeeded (1,404/1,405) — genuine Brave Search API responses (arxiv, USGS, Nature, MapQuest, etc.), not stubs or errors.BRAVE_API_KEYunset → falls back toweb-access/Exa correctly (existing behavior preserved, regression-tested).ironclaw_reborn_compositiontest suite passes (1387 unit tests + all integration binaries),cargo check --workspace --all-targets,cargo clippy -- -D warningsclean,cargo fmt --checkclean.Verification (original web-access/Exa fix)
Verified end-to-end against a real benchmark (GAIA, via
nearai/nearai-bench) rather than just unit tests:httpwith no way to find a URL to fetch.web-access.search→web-access.get_contentcalls, and it passes.Full
ironclaw_reborn_compositiontest suite passes (1562 unit tests + all integration binaries), pluscargo check --workspace.Two existing tests needed updates since they assumed
web-accessstarts un-installed:local_dev_web_access_installs_activates_and_dispatches_through_host_runtime→ renamed..._is_auto_installed_activated_and_dispatches_through_host_runtime; now asserts the auto-bootstrap guarantee directly instead of manually installing/activating first.local_dev_extension_lifecycle_tools_manage_visible_extension_surface(usesweb-accessas its example for the generic install/activate/remove capability-handler round trip): now asserts the already-active starting state, then does a remove → reinstall → reactivate round trip (still exercises all four lifecycle capability handlers), using the real tenant-operator identity for the ownership-sensitive remove/reinstall calls (the auto-bootstrapped install is Tenant-owned, not owned by the generic test-fixture caller the file's other tests use).Test plan
cargo test -p ironclaw_reborn_composition --features test-support— 1387 passed, 0 failedcargo test -p ironclaw_host_runtime— all passedcargo check --workspace --all-targetscargo clippy -p ironclaw_reborn_composition -p ironclaw_host_runtime -p ironclaw_first_party_extensions --features test-support --tests -- -D warnings— cleancargo fmt --checkKnown follow-up: this branch is currently based on an older
mainsnapshot and shows as conflicting against currentmain— needs a rebase before merge (tracked separately; the underlying architecture moved the extension-package catalog to a new self-contained per-package module system inironclaw_first_party_extensions::packages, which this PR'sweb_searchpackage will need to move into as well).🤖 Generated with Claude Code