Skip to content

feat(reborn): capability-policy availability dimension (epic #5261) - #5349

Closed
zetyquickly wants to merge 29 commits into
mainfrom
feat/capability-policy-availability
Closed

zetyquickly wants to merge 29 commits into
mainfrom
feat/capability-policy-availability

Conversation

@zetyquickly

@zetyquickly zetyquickly commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Part of epic #5261 (capability policy — four-dimension model). This PR carries the availability dimension.

What this adds

Builds on the capability-policy engine PR (#5344) plus #4544 (scoped-lifecycle installation store). It is the only slice that depends on #4544.

Scope / isolation

Availability-only. This branch deliberately does not:

  • contain the control-plane files (local_user_directory, capability_user_policy_routes),
  • modify serve.rs or the ironclaw_reborn_webui_ingress crate.

Those belong to the separate control-plane PR.

Notes for review

Gates

  • cargo fmt
  • cargo clippy -p ironclaw_reborn_composition --features capability-policy,webui-v2-beta --tests -- -D warnings (clean)
  • cargo clippy -p ironclaw_reborn_composition --tests -- -D warnings (feature off, clean)
  • cargo build -p ironclaw_reborn_cli --features webui-v2-beta,capability-policy
  • cargo test -p ironclaw_reborn_composition --features capability-policy,webui-v2-beta --lib capability_surface_policy (11 passed) and --lib capability_admin_routes (3 passed)

🤖 Generated with Claude Code


Role-model expansion (#5261)

This PR now also carries role-aware availability (Owner/Admin get the full default surface, bypassing per-user+tenant hides) and builtin-capability governance (builtin.shell etc. are governable).

serrrfirat and others added 23 commits June 8, 2026 13:54
…ifecycle-admin

# Conflicts:
#	Cargo.lock
#	FEATURE_PARITY.md
#	crates/ironclaw_product_workflow/src/lib.rs
#	crates/ironclaw_product_workflow_storage/Cargo.toml
#	crates/ironclaw_product_workflow_storage/src/lib.rs
#	crates/ironclaw_product_workflow_storage/tests/durable_ledger_contract.rs
Settled Reborn-stack design for admin-shared tools/skills + per-user auth: one account type (User = secrets + tools + memory), Owner/Admin/Member roles, the four-dimension capability policy (availability/configuration/identity/approval) resolving capability default -> tenant -> user, plus a prior-art/compose-with map anchored to #4628 (#4544, #1626, #3289/#4354, #4527) and verified live-code seams.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First independent slice of the #4628 continuation: the four-dimension policy vocabulary and the pure precedence cascade, with no dependency on #4544. Availability / IdentityMode (user-keyed vs admin-keyed) / config / approval (reusing the existing PermissionMode). resolve_effective_policy() is an order-independent, most-specific-wins fold with deep-merged config. PolicyResolver async port is the seam a #4544-backed adapter fills. Depends only on ironclaw_host_api; clippy -D warnings clean; 8 unit tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pability defaults

A CapabilityDefaultPolicySource trait + in-memory StaticCapabilityDefaultPolicySource supplying the per-capability default policy (architecture doc §7) keyed by CapabilityId, over a conservative global fallback (hidden + ask). Sources the default without adding a field to the 49-construction-site CapabilityDescriptor, keeping the slice #4544-independent. clippy -D warnings clean; 3 new tests (11 total).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rebase #4544 onto current main (106 commits of drift). Resolved 2 conflicts: product_workflow/src/lib.rs export list (kept both LifecycleSearchExtensionSummary from main and lifecycle_package_kind_label from #4544); FEATURE_PARITY.md (kept #4544's scoped-lifecycle clause on Hosted MCP, main's newer #5256 rows for NEAR AI MCP and Tool policies).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er (#5266)

Gives the Reborn WebChat-v2 stack a typed user role so the facade can gate admin operations — the prerequisite for admin-grants-permissions in epic #5261.

- ironclaw_host_api: new UserRole { Owner > Admin > Member } + UserStatus authority enums (alongside UserId/TenantId); wire-stable snake_case, as_str/parse with least-privilege fallback, is_admin/is_owner.

- ironclaw_reborn_identity: surface role/status on UserRecord + the persisted StoredUser (serde-default so pre-existing records rehydrate to member/active); re-export the enums so crate::UserRole resolves.

- WebuiAuthentication carries role (env-bearer operator authenticates as Owner; default Member; session/OIDC populate from the persisted record in a follow-up). WebUiAuthenticatedCaller carries role + is_admin() (role.is_admin() || operator_webui_config — operator flag kept as a compat shim). The admin gate USAGE lands with the grant/revoke methods in #5268.

clippy -D warnings clean across host_api/identity/product_workflow/composition; host_api role + identity tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`ScopedLifecyclePolicyCapabilitySurfaceResolver` derives the per-(tenant,
user) capability allow-set from #4544's scoped-lifecycle installations
(admin-shared -> all users in the tenant; user-private -> owner only;
disabled excluded), mapping each installed package to its model-visible
capability ids via a `PackageCapabilitySource` seeded from the first-party
extension catalog (`visible_capability_ids`, manifest `Visibility::Model`).

Fail-closed but graceful: the resolver never returns `Err` (which would
abort the turn at host construction). No resolvable user, a user with no
grants, or a store read failure (including a not-yet-created installation
set) all deny every capability while the turn still runs.

Wired into `build_reborn_runtime`'s local-dev branch behind a new
`capability-policy` feature (compiles the resolver + the
`ironclaw_product_workflow_storage` dep) and *activated* per-runtime by
`IRONCLAW_REBORN_CAPABILITY_POLICY` (default off, mirroring the
`HooksActivationConfig` master-flag-default-off pattern). With the feature
absent or the flag off, local-dev keeps the historical `AllowAll` surface,
so existing flows and all current tests are unchanged.

Tests: 8 module unit tests (admin-shared/user-private visibility, disabled
exclusion, no-user + store-failure graceful deny, package->cap mapping,
real first-party catalog seeding, principal precedence). Full composition
lib suite (852) green under `--features capability-policy`; clippy clean in
default and `--all-features` states.

Part of #5261. Depends on #4544.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…§17)

Design §1-§14 holds; §17 captures what's built (epic #5261 child issues) and the
corrections implementation forced: first-party integrations are WASM extensions
not MCP (§7/§12 fixed); per-user availability is a CapabilityPolicyDelta (#4544
can_be_mutated_by forbids an admin writing UserPrivate for others, so #4544 is
tenant-shared + per-user rides deltas); the resolver is feature+IRONCLAW_REBORN_CAPABILITY_POLICY
gated (default off); the scoped-lifecycle store roots under the /tenants durable
mount (the /engine default has no backend); #5272 reworked to REST-created users;
approval has no admin path yet; Reborn admin gate is host_api::UserRole not
src/ownership.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…5273) (#5288)

The storage + resolution foundation for the configuration / identity / approval
dimensions. Adds, in `ironclaw_capability_policy`:

- `CapabilityPolicyDeltaStore` — durable store of per-(tenant, scope, capability)
  `CapabilityPolicyDelta` rows (the admin grants). Keyed with an explicit tenant
  (PolicyScope carries none) so deltas never leak across tenants;
  upsert/delete/`deltas_for`/`list_subject_deltas`. The admin REST surface
  (#5268) writes here; the resolver reads. Mirrors the #4544 store shape.
- `InMemoryCapabilityPolicyDeltaStore` — in-memory backend for tests / local-dev
  (durable filesystem / libSQL backend is the follow-on).
- `StoreBackedPolicyResolver` — implements the #5262 `PolicyResolver` port by
  folding the capability default (#5263 `CapabilityDefaultPolicySource`) with the
  subject's stored deltas via `resolve_effective_policy` into an
  `EffectivePolicy`. Resolution is live (reads the store every call).

`deltas_for` pre-filters to the subject (tenant-wide row + the subject's own user
row; project scope is dormant in v1). Availability in the resulting
`EffectivePolicy` is the policy view (default + deltas); combining it with the
installation view (#4544 / #5267) — plus injecting config, applying the identity
ownership filter, and merging the approval admin layer — is the enforcement layer
(the remaining #5273 work).

Tests: 6 (upsert/read-back/delete, tenant-wide vs user-only visibility, cross-
tenant isolation, default->tenant->user fold incl. config deep-merge + approval
override, default-only when no deltas, subject-scoped listing). Crate green under
`--all-features` clippy `-D warnings`.

Part of #5261. Depends on #5262, #5263.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…r, identity/config/approval enforcement (epic #5261)

Slices the #4544-INDEPENDENT capability-policy engine out of the tested
integration branch onto #5262 (main + the ironclaw_capability_policy crate).

Contains:
- durable FilesystemCapabilityPolicyDeltaStore (libSQL/filesystem) + its
  contract test, behind the storage crate's new ironclaw_capability_policy dep;
- StoreBackedPolicyResolver wired via capability_policy_engine.rs
  (local_dev_capability_policy_delta_store + build_capability_policy_resolver);
- the identity/config/approval dispatch seams (PolicyResolverConfigSource,
  PolicyResolverAdminApprovalSource) wired through factory.rs + local_dev.rs.

Builds on #5262. Independent of #4544: no scoped-lifecycle store,
capability_surface_policy, or control-plane route modules are present, and the
crate compiles + tests pass without any #4544 file. The acting-principal
derivation the config seam needs is inlined into the engine so it carries no
dependency on the availability surface resolver.

Enforces three of the four capability-policy dimensions (identity, config,
approval); availability stays AllowAll on this branch and lands in a separate
availability PR. Gated by the `capability-policy` feature + the
IRONCLAW_REBORN_CAPABILITY_POLICY env toggle (off by default).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…min' into feat/capability-policy-availability

# Conflicts:
#	Cargo.lock
#	crates/ironclaw_product_workflow_storage/Cargo.toml
#	crates/ironclaw_reborn_composition/Cargo.toml
#	crates/ironclaw_reborn_composition/src/lib.rs
…ycle store + dispatch resolver combine (epic #5261)

Builds on the capability-policy engine (#5344) + #4544's scoped-lifecycle
installation store. Adds the availability dimension of the four-dimension
capability model:

- #4544's scoped-lifecycle install store (merged in): the durable
  installed-package surface, disjoint from the engine's delta store.
- The dispatch-seam surface resolver (capability_surface_policy.rs,
  ScopedLifecyclePolicyCapabilitySurfaceResolver): availability = installed
  AND policy-available — it intersects the installed set from #4544's store
  with the engine's shared EffectivePolicy.available, behind the SINGLE
  shared policy resolver constructed once in factory.rs. runtime.rs wires it
  in, falling back to AllowAll when capability_policy is not activated.
- The availability admin REST surface (#5268, capability_admin_routes.rs):
  admin-gated tenant-wide extension install/list/uninstall writing into the
  same scoped-lifecycle store the resolver reads.

This is the only slice that depends on #4544. Availability-only: it does not
contain the control-plane files (local_user_directory, capability_user_policy_routes)
and does not modify serve.rs or the ingress crate — those are the
control-plane branch's job.

Note: the role-based admin gate (WebUiAuthenticatedCaller::is_admin /
UserRole) lands with the user-role work (#5270), which is not in this slice's
base. ensure_admin gates on the operator-config admin bit until #5270 merges
(see TODO(#5261) in capability_admin_routes.rs). The CLI capability-policy
feature forwards only to the composition crate; the ingress-crate forward is
added by the control-plane branch (#5272).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 26, 2026 16:23
@github-actions github-actions Bot added scope: docs Documentation scope: dependencies Dependency updates labels Jun 26, 2026
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5349 June 26, 2026 16:23 Destroyed
@github-actions github-actions Bot added size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Jun 26, 2026
@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a capability-policy stack: policy model/store, scoped lifecycle storage, admin approval and identity-mandate enforcement, capability-surface resolution from installed packages, feature-gated runtime wiring, and updated parity/architecture docs.

Changes

Capability policy and scoped lifecycle

Layer / File(s) Summary
Policy model and store
Cargo.toml, crates/ironclaw_capability_policy/*, crates/ironclaw_product_workflow_storage/src/capability_policy_delta.rs, crates/ironclaw_product_workflow_storage/tests/durable_capability_policy_delta_contract.rs
Adds the capability-policy crate, delta types, precedence fold, in-memory store, filesystem-backed delta store, and durable delta-store tests.
Scoped lifecycle domain
crates/ironclaw_product_workflow/src/lib.rs, crates/ironclaw_product_workflow/src/lifecycle.rs, crates/ironclaw_product_workflow/src/scoped_lifecycle.rs
Adds scoped lifecycle IDs, ownership and visibility rules, effective installation resolution, and public exports.
Scoped lifecycle storage primitives
crates/ironclaw_product_workflow_storage/AGENTS.md, crates/ironclaw_product_workflow_storage/Cargo.toml, crates/ironclaw_product_workflow_storage/src/lib.rs, crates/ironclaw_product_workflow_storage/src/scoped_lifecycle.rs, crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/*
Adds scoped lifecycle storage module wiring, virtual paths, entry encoding, and record parsing.
Scoped lifecycle store implementation
crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/store.rs
Adds the filesystem-backed scoped lifecycle store, reservation and tombstone handling, and feature-gated backend wrappers.
Scoped lifecycle store contracts
crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/store/tests.rs, crates/ironclaw_product_workflow_storage/tests/durable_ledger_contract.rs
Adds scoped lifecycle store tests for CAS behavior, stale updates, tombstone cleanup, and durable reopen cases.
Policy approvals and credentials
crates/ironclaw_host_runtime/src/*, crates/ironclaw_reborn_composition/src/profile_approval_authorization.rs, crates/ironclaw_reborn_composition/src/local_dev_authorization.rs, crates/ironclaw_reborn_composition/src/product_auth_runtime_credentials.rs, crates/ironclaw_reborn_composition/src/product_auth_runtime_credentials/tests*, crates/ironclaw_reborn_composition/src/product_auth_durable/*
Threads capability_id into credential requests and adds admin approval plus identity-mandate enforcement.
Capability surface resolver
crates/ironclaw_reborn_composition/Cargo.toml, crates/ironclaw_reborn_composition/src/lib.rs, crates/ironclaw_reborn_composition/src/capability_policy_engine.rs, crates/ironclaw_reborn_composition/src/capability_surface_policy.rs, crates/ironclaw_reborn_composition/src/runtime.rs
Adds the capability-policy modules, scoped-lifecycle-backed availability resolver, and local-dev resolver selection.
Dispatch wiring and feature gates
crates/ironclaw_loop_support/Cargo.toml, crates/ironclaw_loop_support/src/*, crates/ironclaw_reborn_cli/Cargo.toml, crates/ironclaw_reborn_composition/src/factory.rs, crates/ironclaw_reborn_composition/src/product_live_adapters.rs, crates/ironclaw_reborn_composition/src/runtime/local_dev/*, crates/ironclaw_reborn_composition/tests/product_live_adapters.rs
Threads optional policy config sources through loop dispatch, local-dev factories, and product-live adapter creation.
Parity docs and plan
FEATURE_PARITY.md, V1_REBORN_PARITY_AUDIT.md, docs/plans/2026-06-24-capability-policy-architecture.md
Updates parity, audit, and architecture docs for the new capability-policy and scoped-lifecycle stack.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

Possibly related PRs

  • nearai/ironclaw#4588 — Touches crates/ironclaw_loop_support/src/capability_port.rs and the same dispatch seam.
  • nearai/ironclaw#4779 — Modifies the same local-dev capability port wiring and factory path.
  • nearai/ironclaw#5063 — Changes the same approval-gating path in profile_approval_authorization.rs.

Suggested reviewers

  • think-in-universe
  • serrrfirat
  • hanakannzashi

Poem

Rust spun a policy loom at dusk,
Scoped lifecycles stirred from dust.
Deltas merged and gates stood tall,
Dispatch chose the right tool for all,
And quiet crates hummed, tight and just.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is detailed, but it omits most required template sections and never gives an explicit linked-issue field or checklist entries. Add the template sections for Change Type, Linked Issue, Validation, Security Impact, Trust-Boundary, Database Impact, Blast Radius, Rollback, and Review Follow-Through.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed Uses Conventional Commits style and accurately summarizes the capability-policy availability change.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the capability policy architecture for IronClaw Reborn, establishing a four-dimension policy model (Availability, Configuration, Identity, and Approval) to govern capability execution. It adds the ironclaw_capability_policy crate, implements scoped lifecycle package installations (supporting admin-shared and user-private scopes), and provides durable storage backends (libSQL/Postgres) for policy deltas and installations. Additionally, it integrates these policy dimensions into the agent loop dispatch, credential resolution, and approval authorization processes, and exposes admin REST endpoints for managing tenant-wide extensions. The feedback highlights a potential key collision risk in capability_admin_routes.rs when sanitizing package IDs with simple hyphen replacement and concatenation, suggesting the use of structured keys or injective length-prefixed encoding to prevent collisions.

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 +197 to +204
let sanitized: String = package
.id
.as_str()
.chars()
.map(|c| if c.is_ascii_alphanumeric() { c } else { '-' })
.collect();
ScopedLifecycleInstallationId::new(format!("admin-shared-{sanitized}"))
.map_err(|_| CapabilityAdminError::BadRequest)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Sanitizing the package ID by replacing non-alphanumeric characters with hyphens (-) and concatenating it with a prefix introduces a risk of collision. For example, two distinct package IDs like web_search and web-search would both sanitize to web-search, resulting in the same ScopedLifecycleInstallationId (admin-shared-web-search).

To prevent key collisions, use structured keys instead of string concatenation with a separator. If structured keys are too costly to implement due to existing architecture, consider adding explicit collision detection and logging at the point of key generation, or use an injective length-prefixed encoding rather than simple concatenation with a delimiter to eliminate the risk of separator-collision attacks.

Suggested change
let sanitized: String = package
.id
.as_str()
.chars()
.map(|c| if c.is_ascii_alphanumeric() { c } else { '-' })
.collect();
ScopedLifecycleInstallationId::new(format!("admin-shared-{sanitized}"))
.map_err(|_| CapabilityAdminError::BadRequest)
ScopedLifecycleInstallationId::new(Scope::AdminShared, package.id.clone())
.map_err(|_| CapabilityAdminError::BadRequest)
References
  1. To prevent key collisions, use structured keys instead of string concatenation with a separator.
  2. When a theoretically ideal solution (like using structured keys to avoid collisions) is too costly to implement due to existing architecture, consider addressing the most severe symptom (like silent failures) at a more practical layer. For example, instead of rewriting a string-keyed registry, add explicit collision detection and logging at the point of key generation.
  3. When generating deterministic identifiers or hashes from multiple string components, use an injective length-prefixed encoding rather than simple concatenation with a delimiter to eliminate the risk of separator-collision attacks or accidental collisions.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces the “availability” slice of the four-dimension Reborn capability-policy model (#5261) by adding durable scoped-lifecycle installation storage (admin-shared vs user-private) and wiring policy-driven availability/config/identity/approval seams into local-dev runtime composition behind the capability-policy feature flag and IRONCLAW_REBORN_CAPABILITY_POLICY activation gate.

Changes:

  • Adds a new ironclaw_capability_policy crate plus durable delta-store implementation in ironclaw_product_workflow_storage.
  • Adds scoped-lifecycle installation ownership model + durable stores (libSQL/Postgres/filesystem) for admin/user package installation resolution.
  • Wires dispatch seams for availability surface resolution, config deep-merge into capability inputs, identity enforcement at credential resolution, and admin-layer approval precedence.

Reviewed changes

Copilot reviewed 48 out of 49 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
FEATURE_PARITY.md Updates parity note for hosted MCP/extensions lifecycle foundation
crates/ironclaw_reborn_composition/tests/product_live_adapters.rs Adapts test config for optional policy config source
crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs Threads optional policy config source through local-dev tests
crates/ironclaw_reborn_composition/src/runtime/local_dev/shell_tests.rs Threads optional policy config source through shell tests
crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs Adds optional policy config source into refreshing local-dev capability port
crates/ironclaw_reborn_composition/src/runtime/local_dev.rs Builds and passes policy config source when policy is active
crates/ironclaw_reborn_composition/src/runtime.rs Wires scoped-lifecycle + policy resolver into capability surface resolution (local-dev path)
crates/ironclaw_reborn_composition/src/profile_approval_authorization.rs Adds admin approval source seam + precedence logic + tests
crates/ironclaw_reborn_composition/src/product_live_adapters.rs Adds policy config source plumbing into product-live capability factory
crates/ironclaw_reborn_composition/src/product_auth_runtime_credentials/tests/duplicate_selection.rs Updates credential resolver requests to include capability_id
crates/ironclaw_reborn_composition/src/product_auth_runtime_credentials.rs Enforces identity dimension at credential resolution (optional policy)
crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs Updates durable auth tests to include capability_id in requests
crates/ironclaw_reborn_composition/src/product_auth_durable/interactions.rs Adds TODO for identity provisioning tagging once capability/policy is threaded
crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs Adds TODO for identity provisioning tagging once capability/policy is threaded
crates/ironclaw_reborn_composition/src/local_dev_authorization.rs Threads optional admin approval policy source into local-dev authorizer
crates/ironclaw_reborn_composition/src/lib.rs Exposes capability admin route mount types behind feature gates
crates/ironclaw_reborn_composition/src/factory.rs Constructs and shares single policy delta store + resolver handles; threads into services
crates/ironclaw_reborn_composition/src/capability_policy_engine.rs New engine module: delta store root, resolver builder, config + approval adapters, activation gate
crates/ironclaw_reborn_composition/Cargo.toml Adds capability-policy feature and optional deps
crates/ironclaw_reborn_cli/Cargo.toml Forwards capability-policy feature to composition crate
crates/ironclaw_product_workflow/src/scoped_lifecycle.rs New scoped lifecycle ownership model + effective resolution logic
crates/ironclaw_product_workflow/src/lifecycle.rs Adds lifecycle_package_kind_label helper
crates/ironclaw_product_workflow/src/lib.rs Exports scoped lifecycle types and lifecycle_package_kind_label
crates/ironclaw_product_workflow_storage/tests/durable_ledger_contract.rs Extends durable contract tests to cover scoped lifecycle store reopen semantics
crates/ironclaw_product_workflow_storage/tests/durable_capability_policy_delta_contract.rs New durable contract tests for capability policy delta store + resolver fold
crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/store/tests.rs New unit tests for filesystem scoped lifecycle store CAS/tombstone behavior
crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/store.rs New filesystem/libsql/postgres scoped lifecycle installation stores
crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/paths.rs New path derivation helpers for scoped lifecycle store
crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/entries.rs New entry (de)serialization + tombstone helpers for scoped lifecycle store
crates/ironclaw_product_workflow_storage/src/scoped_lifecycle.rs New scoped lifecycle storage module glue + error mapping helpers
crates/ironclaw_product_workflow_storage/src/lib.rs Exports new storage adapters (scoped lifecycle + capability policy delta)
crates/ironclaw_product_workflow_storage/src/capability_policy_delta.rs New filesystem-backed capability policy delta store implementation
crates/ironclaw_product_workflow_storage/Cargo.toml Adds deps needed for new stores (hex, ironclaw_capability_policy)
crates/ironclaw_product_workflow_storage/AGENTS.md Updates crate purpose/boundaries to include new workflow ports
crates/ironclaw_loop_support/src/lib.rs Re-exports LoopCapabilityConfigSource
crates/ironclaw_loop_support/src/capability_port.rs Adds policy config deep-merge into dispatch path + replay protections + tests
crates/ironclaw_loop_support/Cargo.toml Adds dependency on ironclaw_capability_policy for deep-merge utility
crates/ironclaw_host_runtime/src/wasm_credentials.rs Threads capability_id into runtime credential resolution requests
crates/ironclaw_host_runtime/src/obligations.rs Adds capability_id to credential account request and threads through call sites
crates/ironclaw_capability_policy/src/store.rs New policy delta store trait + in-memory store + store-backed resolver
crates/ironclaw_capability_policy/src/lib.rs New policy vocabulary + resolver interface + deep-merge implementation
crates/ironclaw_capability_policy/Cargo.toml New crate manifest
Cargo.toml Adds ironclaw_capability_policy to workspace members
Cargo.lock Adds lock entries for new crate and deps

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

Comment on lines +288 to +290
if gate_policy.effects_force_approval(&gate_effects) {
return require_approval();
}
Comment on lines +139 to +152
let mut found = Vec::new();
for scope in subject_scopes(subject) {
for delta in self.deltas_in_scope(&subject.tenant_id, &scope).await? {
// Defense-in-depth: the path scheme already isolates by scope,
// but re-check capability + subject applicability before
// returning a row to the resolver.
if &delta.capability == capability
&& scope_applies_to_subject(&delta.scope, subject)
{
found.push(delta);
}
}
}
Ok(found)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 15

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/profile_approval_authorization.rs (1)

288-303: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep Disabled as a deny before the hard-floor gate.

Line 288 returns RequireApproval before line 303 checks ToolPermissionOverride::Disabled, so a disabled hard-floor capability (for example Financial) becomes approvable instead of denied. Move the disabled check above the hard-floor return and add a regression for Financial + Disabled.

Proposed precedence fix
+    let tool_override = settings
+        .tool_override(&context.resource_scope, &descriptor.id)
+        .await;
+    if matches!(tool_override, Some(ToolPermissionOverride::Disabled)) {
+        return Decision::Deny {
+            reason: DenyReason::PolicyDenied,
+        };
+    }
+
     if gate_policy.effects_force_approval(&gate_effects) {
         return require_approval();
     }
@@
-    let tool_override = settings
-        .tool_override(&context.resource_scope, &descriptor.id)
-        .await;
-    if matches!(tool_override, Some(ToolPermissionOverride::Disabled)) {
-        return Decision::Deny {
-            reason: DenyReason::PolicyDenied,
-        };
-    }
🤖 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/profile_approval_authorization.rs`
around lines 288 - 303, The precedence in
profile_approval_authorization::authorization should keep
ToolPermissionOverride::Disabled as an outright deny before any hard-floor
approval path. Move the tool_override disabled check in the approval decision
flow so it is evaluated before gate_policy.effects_force_approval and the
require_approval() return, ensuring a disabled capability like Financial cannot
be escalated to RequireApproval. Add a regression test around the approval logic
in this module covering Financial plus Disabled to verify it remains denied.
🤖 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_capability_policy/src/store.rs`:
- Around line 116-137: The scope keying and applicability logic is duplicated
privately in store.rs, so move the canonical scope encoder/matcher into the
shared ironclaw_capability_policy API and have both stores use it. Expose
reusable helpers for scope_key and scope_applies_to_subject (or equivalent) from
the policy crate, then update the store implementation to call those shared
functions instead of re-deriving the tenant/project/user matching rules locally.

In `@crates/ironclaw_loop_support/src/capability_port.rs`:
- Around line 1723-1788: The admin policy merge is happening after the input has
already been wrapped by host_runtime_input_for_capability, which puts policy
keys at the wrong nesting level for SandboxProcessPlan-wrapping capabilities.
Update the flow in the capability dispatch path so the deep merge via
ironclaw_capability_policy::deep_merge_into runs on the raw capability input
before any wrapping, then validate the merged payload with
prepare_provider_arguments_with_detail against capability.parameters_schema, and
finally apply host_runtime_input_for_capability only once to produce the final
leased input. Keep the existing fail-open handling around
policy_config_source::config_for and preserve the InvalidInput failure path for
merged-input validation errors.

In `@crates/ironclaw_product_workflow_storage/src/capability_policy_delta.rs`:
- Around line 134-152: deltas_for currently scans all deltas in a subject scope
and filters in memory, which makes resolution depend on unrelated rows and
unnecessary deserialization. Update CapabilityPolicyDeltaStore::deltas_for to
read the exact leaf entry for each subject scope using the existing
tenant/scope/capability path logic instead of calling deltas_in_scope, while
keeping list_subject_deltas on the broader directory scan path. Preserve the
existing subject applicability check and capability match in deltas_for after
loading the specific leaf.

In `@crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/store.rs`:
- Around line 97-118: The create/delete flow in scoped lifecycle storage can
leave a stale installation-id reservation behind when the tombstone write
partially succeeds, so update the `store.rs` lifecycle logic to prevent
package-slot reuse while an old reservation still exists. In
`reserve_installation_id`, the package-path check used before creating a new
install must also account for any live reservation tied to the same slot, and
the delete path that writes the package tombstone and clears the installation-id
reservation must be made atomic or fail in a way that leaves the slot blocked.
Adjust the related `load_installation` mismatch handling so it cannot observe a
newly reused package slot with an old reservation still present.
- Around line 429-437: Treat a never-written tenant prefix as empty state in
list_installations instead of an error. Update list_versioned_installations in
store.rs to detect FilesystemError::NotFound from filesystem.query and return an
empty result (matching the handling already used in capability_policy_delta)
rather than mapping it through scoped_lifecycle_filesystem_error as transient.
Keep the logic localized around scoped_lifecycle_tenant_installations_path and
the query loop so brand-new tenants return [].

In `@crates/ironclaw_product_workflow/src/scoped_lifecycle.rs`:
- Around line 325-337: The default implementation of
list_effective_installations in scoped_lifecycle.rs is currently deriving
effective installs from list_installations(&subject.tenant_id), which scans the
whole tenant and then filters later. Update this resolver to perform a
subject-scoped read using the store’s existing admin_shared plus the subject’s
user_private lookup, and return EffectiveScopedLifecycleInstallations directly
from that narrower data path. Keep the change anchored around
list_effective_installations and
resolve_effective_scoped_lifecycle_installations so the hot-path dispatch-seam
lookup no longer depends on tenant-wide listing.
- Around line 21-66: `ScopedLifecycleInstallationId` is using hand-written serde
logic instead of the canonical validated-newtype pattern. Update the type to
derive `Serialize`/`Deserialize` with `#[serde(try_from = "String", into =
"String")]`, and add `TryFrom<String>` plus `From<ScopedLifecycleInstallationId>
for String` so serialization goes through the existing `new` validation path.
Remove the manual `Serialize`/`Deserialize` impls and keep
`ScopedLifecycleInstallationId::new` as the single validation entry point.

In `@crates/ironclaw_reborn_composition/src/capability_admin_routes.rs`:
- Around line 293-314: The list_extensions_handler response is currently using
list_effective_installations, which includes the admin’s own UserPrivate rows
instead of only tenant-wide AdminShared entries. Update the handler to read the
tenant installations from CapabilityAdminRouteConfig.installations and filter
the results to Extension ownership marked AdminShared before building
ListResponse and ExtensionSummary, so the admin UI only exposes tenant-shared
availability.
- Around line 194-205: The admin_shared_installation_id helper currently
sanitizes LifecyclePackageRef ids by replacing every non-alphanumeric character
with -, which can make different package ids map to the same admin-shared-*
installation id. Update this function to derive the
ScopedLifecycleInstallationId in a collision-free way by preserving the package
id losslessly or by using the validated LifecyclePackageRef as the identifier
source, so upserts and uninstalls in capability_admin_routes.rs always target
the correct shared row.

In `@crates/ironclaw_reborn_composition/src/capability_policy_engine.rs`:
- Around line 28-37: The private principal derivation in principal_user_id
duplicates the authority-subject logic and can drift from the
config/availability seam. Move this derivation into a single shared helper that
both capability_policy_engine and the availability/config path call, or add a
regression test that asserts they produce the same subject from the same
TurnScope and TurnActor inputs. Keep the existing actor-first, explicit-owner
fallback behavior, but make sure the shared symbol is the only source of truth.

In `@crates/ironclaw_reborn_composition/src/capability_surface_policy.rs`:
- Around line 239-263: The `principal_user_id` helper currently falls back from
`TurnActor` to `TurnScope` ownership, which can let availability/config and
approval/identity resolve different policy subjects on delegated turns. Update
`principal_user_id` to fail closed when the actor-derived user and
`scope.explicit_owner_user_id()` would not resolve to the same `UserId`, and
return `None` in that mismatch case so the policy surface cannot split authority
across `PolicySubject` sources.

In `@crates/ironclaw_reborn_composition/src/product_auth_runtime_credentials.rs`:
- Around line 598-600: The non-Configured account branch in the credential stage
currently returns AuthRequired for all identities, but AdminKeyed mandates
should treat this as a Backend issue instead. Update the logic in
product_auth_runtime_credentials::CredentialStage so the non-configured check
distinguishes IdentityMode::AdminKeyed from other modes and maps
shared-admin/non-configured accounts to Backend rather than AuthRequired. Add a
regression test covering an AdminKeyed mandate with a non-configured
shared-admin account to verify the stage returns Backend and does not trigger a
re-auth gate.

In
`@crates/ironclaw_reborn_composition/src/product_auth_runtime_credentials/tests.rs`:
- Around line 1335-1339: The resolver double in resolve currently ignores both
PolicySubject and CapabilityId, so add caller-level coverage that exercises the
real call site and verifies the expected tenant/user/capability flow instead of
only stubbing the fake. Update one or more tests around the code that builds the
policy request (for example the path using request.capability_id and the acting
subject) so the assertions check the specific PolicySubject and CapabilityId
values passed into resolve and fail if those inputs are wrong or omitted.

In `@crates/ironclaw_reborn_composition/src/runtime.rs`:
- Around line 2856-2870: The capability-policy seeding in runtime.rs uses only
first-party assets, which can miss filesystem-installed extensions that the
runtime/admin route already exposes. Update the policy resolver setup around
AvailableExtensionCatalog::from_first_party_assets_with_nearai_mcp_config and
StaticPackageCapabilitySource::from_catalog to seed from the same assembled
catalog/registry the runtime uses, or alternatively block unmapped installs
earlier at the admin boundary. Keep the fix localized to the runtime capability
source initialization so installed extensions contribute the correct dispatch
allow-set.

In `@docs/plans/2026-06-24-capability-policy-architecture.md`:
- Around line 524-535: Update Appendix A so the admin-gate reference matches the
§17 correction: replace the `src/ownership::UserRole` guidance with
`ironclaw_host_api::UserRole` as the reborn admin gate, and explicitly mark
`src/ownership` as the legacy V1 tree. Keep the surrounding guidance in the
`Concept | Status — reuse / extend` table aligned with this symbol change so
readers do not wire new work against the wrong crate.

---

Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/profile_approval_authorization.rs`:
- Around line 288-303: The precedence in
profile_approval_authorization::authorization should keep
ToolPermissionOverride::Disabled as an outright deny before any hard-floor
approval path. Move the tool_override disabled check in the approval decision
flow so it is evaluated before gate_policy.effects_force_approval and the
require_approval() return, ensuring a disabled capability like Financial cannot
be escalated to RequireApproval. Add a regression test around the approval logic
in this module covering Financial plus Disabled to verify it remains denied.
🪄 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: ded77c43-fb99-4446-814e-5cd7fd1b0be6

📥 Commits

Reviewing files that changed from the base of the PR and between 185ce88 and b3a9432.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock, !**/Cargo.lock
📒 Files selected for processing (48)
  • Cargo.toml
  • FEATURE_PARITY.md
  • V1_REBORN_PARITY_AUDIT.md
  • crates/ironclaw_capability_policy/Cargo.toml
  • crates/ironclaw_capability_policy/src/lib.rs
  • crates/ironclaw_capability_policy/src/store.rs
  • crates/ironclaw_host_runtime/src/obligations.rs
  • crates/ironclaw_host_runtime/src/wasm_credentials.rs
  • crates/ironclaw_loop_support/Cargo.toml
  • crates/ironclaw_loop_support/src/capability_port.rs
  • crates/ironclaw_loop_support/src/lib.rs
  • crates/ironclaw_product_workflow/src/lib.rs
  • crates/ironclaw_product_workflow/src/lifecycle.rs
  • crates/ironclaw_product_workflow/src/scoped_lifecycle.rs
  • crates/ironclaw_product_workflow_storage/AGENTS.md
  • crates/ironclaw_product_workflow_storage/Cargo.toml
  • crates/ironclaw_product_workflow_storage/src/capability_policy_delta.rs
  • crates/ironclaw_product_workflow_storage/src/lib.rs
  • crates/ironclaw_product_workflow_storage/src/scoped_lifecycle.rs
  • crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/entries.rs
  • crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/paths.rs
  • crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/store.rs
  • crates/ironclaw_product_workflow_storage/src/scoped_lifecycle/store/tests.rs
  • crates/ironclaw_product_workflow_storage/tests/durable_capability_policy_delta_contract.rs
  • crates/ironclaw_product_workflow_storage/tests/durable_ledger_contract.rs
  • crates/ironclaw_reborn_cli/Cargo.toml
  • crates/ironclaw_reborn_composition/Cargo.toml
  • crates/ironclaw_reborn_composition/src/capability_admin_routes.rs
  • crates/ironclaw_reborn_composition/src/capability_policy_engine.rs
  • crates/ironclaw_reborn_composition/src/capability_surface_policy.rs
  • crates/ironclaw_reborn_composition/src/factory.rs
  • crates/ironclaw_reborn_composition/src/lib.rs
  • crates/ironclaw_reborn_composition/src/local_dev_authorization.rs
  • crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs
  • crates/ironclaw_reborn_composition/src/product_auth_durable/interactions.rs
  • crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs
  • crates/ironclaw_reborn_composition/src/product_auth_runtime_credentials.rs
  • crates/ironclaw_reborn_composition/src/product_auth_runtime_credentials/tests.rs
  • crates/ironclaw_reborn_composition/src/product_auth_runtime_credentials/tests/duplicate_selection.rs
  • crates/ironclaw_reborn_composition/src/product_live_adapters.rs
  • crates/ironclaw_reborn_composition/src/profile_approval_authorization.rs
  • crates/ironclaw_reborn_composition/src/runtime.rs
  • crates/ironclaw_reborn_composition/src/runtime/local_dev.rs
  • crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs
  • crates/ironclaw_reborn_composition/src/runtime/local_dev/shell_tests.rs
  • crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs
  • crates/ironclaw_reborn_composition/tests/product_live_adapters.rs
  • docs/plans/2026-06-24-capability-policy-architecture.md

Comment on lines +116 to +137
/// Stable per-row key component for a scope: `tenant` / `project:<id>` /
/// `user:<id>`. Matches the store's `(tenant, scope, capability)` keying so an
/// upsert replaces the delta at the same scope+capability.
fn scope_key(scope: &PolicyScope) -> String {
match scope {
PolicyScope::Tenant => "tenant".to_string(),
PolicyScope::Project { project_id } => format!("project:{}", project_id.as_str()),
PolicyScope::User { user_id } => format!("user:{}", user_id.as_str()),
}
}

/// `true` when a delta at `scope` applies to `subject`: the tenant-wide row, or
/// the subject's own user row.
fn scope_applies_to_subject(scope: &PolicyScope, subject: &PolicySubject) -> bool {
match scope {
PolicyScope::Tenant => true,
PolicyScope::User { user_id } => user_id == &subject.user_id,
// Project scope is dormant in v1 (default project == tenant) and the
// subject carries no project id to match against.
PolicyScope::Project { .. } => false,
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Promote the scope-key/applicability rules into the shared policy API.

These two helpers define the durable lookup contract, but keeping them private here forces the filesystem store to re-derive them verbatim in another crate. Any later change in only one copy will make persisted rows unreadable or visible under one backend but not the other. Expose one canonical encoder/matcher from ironclaw_capability_policy and have both stores consume it.

🤖 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_capability_policy/src/store.rs` around lines 116 - 137, The
scope keying and applicability logic is duplicated privately in store.rs, so
move the canonical scope encoder/matcher into the shared
ironclaw_capability_policy API and have both stores use it. Expose reusable
helpers for scope_key and scope_applies_to_subject (or equivalent) from the
policy crate, then update the store implementation to call those shared
functions instead of re-deriving the tenant/project/user matching rules locally.

Comment on lines +1723 to +1788
let mut input = host_runtime_input_for_capability(&request.capability_id, input)?;
// Configuration dimension (#5261): deep-merge the admin policy
// config into the model-supplied input on the FIRST (non-replay)
// dispatch only, with admin keys winning (model input is the base,
// the policy config overlays). The replay branch above reuses the
// already-merged leased payload, so it must NOT re-merge — re-merging
// would diverge from the approval-leased fingerprint.
//
// Fail-OPEN is structural here: a `config_for` fault must NOT end the
// turn (an `Err` maps to a terminal `HostUnavailable`, see
// `.claude/rules/agent-loop-capabilities.md`). We match the result
// explicitly — `Ok(Some)` merges, `Ok(None)` skips, and `Err` is
// logged at `debug!` (dispatch path: never `info!`/`warn!`) before
// continuing with the un-merged model input.
if let Some(config_source) = &self.policy_config_source {
match config_source
.config_for(&self.run_context, &request.capability_id)
.await
{
Ok(Some(config)) => {
ironclaw_capability_policy::deep_merge_into(&mut input, &config);
// The admin patch is overlaid AFTER the input was
// validated/normalized against the capability schema, so
// re-validate the merged payload exactly like the original
// input. A validation failure is model-visible and
// recoverable (the model can adjust its request), so route
// it to `Failed { InvalidInput }` — never an `Err` that
// would end the turn.
input = match prepare_provider_arguments_with_detail(
&input,
&capability.parameters_schema,
"capability input",
) {
Ok(input) => input,
Err(error) => {
let result = Ok(CapabilityOutcome::Failed(CapabilityFailure {
error_kind: CapabilityFailureKind::InvalidInput,
safe_summary: error.error.safe_summary,
detail: error.detail,
}));
guard.commit();
self.record_loop_completed(
&idempotency_key,
requested_invocation_id,
result.clone(),
)?;
return result;
}
};
// Re-apply the sandbox-plan wrapping so the merged payload
// is normalized like the original (process-sandbox plans
// round-trip through `SandboxProcessPlan` validation).
input = host_runtime_input_for_capability(&request.capability_id, input)?;
}
Ok(None) => {}
Err(error) => {
tracing::debug!(
capability_id = request.capability_id.as_str(),
error = %error,
"policy config source faulted; continuing with un-merged \
input (fail-open, #5261 D5)"
);
}
}
}
(input, capability.estimate.clone())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect host_runtime_input_for_capability to confirm wrap semantics / idempotency.
fd -t f capability_port.rs --full-path crates/ironclaw_loop_support/src \
  --exec ast-grep run --pattern 'fn host_runtime_input_for_capability($$$) { $$$ }' --lang rust {}
rg -nP '\bfn host_runtime_input_for_capability\b' -C40 --type=rust

Repository: nearai/ironclaw

Length of output: 153


🏁 Script executed:

#!/bin/bash
# Find host_runtime_input_for_capability definition
cd crates/ironclaw_loop_support/src
ast-grep outline capability_port.rs | grep -A5 host_runtime_input_for_capability
# Search for the function definition
ast-grep run --pattern 'fn host_runtime_input_for_capability($$$) { $$$ }' --lang rust capability_port.rs
# If pattern fails, grep for function signature
rg -n '^fn host_runtime_input_for_capability|pub fn host_runtime_input_for_capability' -A 5 capability_port.rs

Repository: nearai/ironclaw

Length of output: 803


🏁 Script executed:

#!/bin/bash
# Read full implementation of host_runtime_input_for_capability
sed -n '2009,2045p' crates/ironclaw_loop_support/src/capability_port.rs | cat -n
# Also check what happens when deserializing already-wrapped plan
rg -n -A 20 "SandboxProcessPlan" crates/ironclaw_loop_support/src/capability_port.rs | head -60

Repository: nearai/ironclaw

Length of output: 5079


Admin policy config merges at incorrect nesting level for host_runtime_input_for_capability-wrapping capabilities

The current flow at lines 1723-1775 incorrectly deep-merges the admin config into the already-wrapped value, causing admin keys to apply at the SandboxProcessPlan level instead of the tool-argument level. This violates the principle that admin patches must target the original schema.

Current (broken) flow for process-sandbox:

  1. Line 1723: wrap raw input → SandboxProcessPlan
  2. Lines 1738-1740: deep-merge admin config into wrapped value (wrong nesting)
  3. Lines 1741-1758: re-validate wrapped value against parameters_schema (semantically incorrect)
  4. Line 1775: re-wrap already-wrapped value (unnecessary)

Correct flow:

  1. Deep-merge admin config into raw input (pre-wrap)
  2. Validate merged input against parameters_schema
  3. Single wrap via host_runtime_input_for_capability

For process-sandbox capabilities, the admin patch targets tool arguments, but currently lands in the SandboxProcessPlan structure. Re-wrapping an already-wrapped SandboxProcessPlan may also fail validation or produce unexpected results.

Refactor the merge logic to apply admin config before the first host_runtime_input_for_capability call, then perform a single wrap after validation.

🤖 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_loop_support/src/capability_port.rs` around lines 1723 -
1788, The admin policy merge is happening after the input has already been
wrapped by host_runtime_input_for_capability, which puts policy keys at the
wrong nesting level for SandboxProcessPlan-wrapping capabilities. Update the
flow in the capability dispatch path so the deep merge via
ironclaw_capability_policy::deep_merge_into runs on the raw capability input
before any wrapping, then validate the merged payload with
prepare_provider_arguments_with_detail against capability.parameters_schema, and
finally apply host_runtime_input_for_capability only once to produce the final
leased input. Keep the existing fail-open handling around
policy_config_source::config_for and preserve the InvalidInput failure path for
merged-input validation errors.

Comment on lines +134 to +152
async fn deltas_for(
&self,
subject: &PolicySubject,
capability: &CapabilityId,
) -> Result<Vec<CapabilityPolicyDelta>, PolicyError> {
let mut found = Vec::new();
for scope in subject_scopes(subject) {
for delta in self.deltas_in_scope(&subject.tenant_id, &scope).await? {
// Defense-in-depth: the path scheme already isolates by scope,
// but re-check capability + subject applicability before
// returning a row to the resolver.
if &delta.capability == capability
&& scope_applies_to_subject(&delta.scope, subject)
{
found.push(delta);
}
}
}
Ok(found)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Use exact leaf reads in deltas_for; scope scans couple unrelated records.

deltas_for is on the live resolver path, but it currently lists and deserializes every delta under the tenant/user scope before filtering by capability. That means one malformed unrelated row can make all policy resolution for that scope fail, and every dispatch cost grows with total scoped deltas. The path scheme already gives you the exact (tenant, scope, capability) leaf, so keep directory scans for list_subject_deltas and read the specific leaf here.

🤖 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_product_workflow_storage/src/capability_policy_delta.rs`
around lines 134 - 152, deltas_for currently scans all deltas in a subject scope
and filters in memory, which makes resolution depend on unrelated rows and
unnecessary deserialization. Update CapabilityPolicyDeltaStore::deltas_for to
read the exact leaf entry for each subject scope using the existing
tenant/scope/capability path logic instead of calling deltas_in_scope, while
keeping list_subject_deltas on the broader directory scan path. Preserve the
existing subject applicability check and capability match in deltas_for after
loading the specific leaf.

Comment on lines +97 to +118
let (cas, reserved_id_version) = match existing.as_ref() {
Some(existing) => (CasExpectation::Version(existing.version), None),
None => {
let cas = match self.package_path_state(&path).await? {
PackagePathState::Absent => CasExpectation::Absent,
PackagePathState::Tombstone(version) => CasExpectation::Version(version),
PackagePathState::Occupied => {
return Err(scoped_lifecycle_invalid_request(
"scoped lifecycle installation package already exists for ownership",
));
}
};
let reservation_path = scoped_lifecycle_installation_id_path(
&self.root,
installation.tenant_id(),
&installation.installation_id,
)?;
let reserved_id_version = self
.reserve_installation_id(&reservation_path, &installation)
.await?;
(cas, reserved_id_version)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Stale installation-id reservations can survive a failed delete and poison later installs.

Lines 197-214 persist the package tombstone before tombstoning the installation-id reservation. If that second CAS fails, the call returns Err, but a later create at Lines 100-117 only checks package_path_state, so it can reuse the package slot under a new installation_id while the old reservation still points at it. load_installation then trips the mismatch transient at Lines 412-420, and the old id stays wedged. Please make the two writes atomic, or block package-slot reuse until any prior live reservation for that slot is cleared. As per path instructions, "Fail loud" includes avoiding poisoned downstream state after boundary-write failures.

Also applies to: 173-215, 387-423

🤖 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_product_workflow_storage/src/scoped_lifecycle/store.rs`
around lines 97 - 118, The create/delete flow in scoped lifecycle storage can
leave a stale installation-id reservation behind when the tombstone write
partially succeeds, so update the `store.rs` lifecycle logic to prevent
package-slot reuse while an old reservation still exists. In
`reserve_installation_id`, the package-path check used before creating a new
install must also account for any live reservation tied to the same slot, and
the delete path that writes the package tombstone and clears the installation-id
reservation must be made atomic or fail in a way that leaves the slot blocked.
Adjust the related `load_installation` mismatch handling so it cannot observe a
newly reused package slot with an old reservation still present.

Source: Path instructions

Comment on lines +429 to +437
let path = scoped_lifecycle_tenant_installations_path(&self.root, tenant_id)?;
let mut installations = Vec::new();
let mut offset = 0_u64;
loop {
let entries = self
.filesystem
.query(&path, &Filter::All, Page::new(offset, Page::MAX_LIMIT))
.await
.map_err(|error| scoped_lifecycle_filesystem_error("list installations", error))?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Treat an untouched tenant prefix as empty state, not a transient failure.

Some backends return FilesystemError::NotFound for a never-written query prefix. crates/ironclaw_product_workflow_storage/src/capability_policy_delta.rs:61-74 already handles that as empty, but list_versioned_installations maps the same case to Transient, so list_installations can fail for a brand-new tenant instead of returning [].

Suggested fix
-            let entries = self
-                .filesystem
-                .query(&path, &Filter::All, Page::new(offset, Page::MAX_LIMIT))
-                .await
-                .map_err(|error| scoped_lifecycle_filesystem_error("list installations", error))?;
+            let entries = match self
+                .filesystem
+                .query(&path, &Filter::All, Page::new(offset, Page::MAX_LIMIT))
+                .await
+            {
+                Ok(entries) => entries,
+                Err(FilesystemError::NotFound { .. }) => break,
+                Err(error) => {
+                    return Err(scoped_lifecycle_filesystem_error(
+                        "list installations",
+                        error,
+                    ))
+                }
+            };
📝 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
let path = scoped_lifecycle_tenant_installations_path(&self.root, tenant_id)?;
let mut installations = Vec::new();
let mut offset = 0_u64;
loop {
let entries = self
.filesystem
.query(&path, &Filter::All, Page::new(offset, Page::MAX_LIMIT))
.await
.map_err(|error| scoped_lifecycle_filesystem_error("list installations", error))?;
let path = scoped_lifecycle_tenant_installations_path(&self.root, tenant_id)?;
let mut installations = Vec::new();
let mut offset = 0_u64;
loop {
let entries = match self
.filesystem
.query(&path, &Filter::All, Page::new(offset, Page::MAX_LIMIT))
.await
{
Ok(entries) => entries,
Err(FilesystemError::NotFound { .. }) => break,
Err(error) => {
return Err(scoped_lifecycle_filesystem_error(
"list installations",
error,
))
}
};
🤖 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_product_workflow_storage/src/scoped_lifecycle/store.rs`
around lines 429 - 437, Treat a never-written tenant prefix as empty state in
list_installations instead of an error. Update list_versioned_installations in
store.rs to detect FilesystemError::NotFound from filesystem.query and return an
empty result (matching the handling already used in capability_policy_delta)
rather than mapping it through scoped_lifecycle_filesystem_error as transient.
Keep the logic localized around scoped_lifecycle_tenant_installations_path and
the query loop so brand-new tenants return [].

Comment on lines +239 to +263
/// The acting principal for capability availability: the turn's actor (the user
/// driving it) first, then the explicit thread owner. A shared (room-agent)
/// account resolves to its own `UserId` here when a turn is driven as it.
/// Returns `None` for an ownerless / actor-fallback turn → the resolver fails
/// closed to an empty allow-set.
///
/// SUBJECT INVARIANT (epic #5261): all four policy dimensions must resolve the
/// same `PolicySubject` for a given turn or an admin grant applies
/// inconsistently. Availability and configuration key off this helper
/// (actor-first, then explicit owner); approval (`PolicyResolverAdminApprovalSource`)
/// and identity (`resolve_identity_mandate`) key off the dispatch
/// `ResourceScope.user_id`. Those agree whenever the actor IS the acting user —
/// which is every path the hand-driven Acme walkthrough exercises (you drive as
/// each user directly, including the shared `engineering@` account). They could
/// diverge only on a future delegated turn where `actor != resource_scope owner`;
/// aligning the dispatch `ResourceScope` with this helper for that case is a
/// deferred follow-up (not exercised by the milestone).
pub(crate) fn principal_user_id<'a>(
scope: &'a TurnScope,
actor: Option<&'a TurnActor>,
) -> Option<&'a UserId> {
actor
.map(|actor| &actor.user_id)
.or_else(|| scope.explicit_owner_user_id())
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Fail closed when actor and owner would resolve different policy subjects.

Lines 245-255 document that availability/config can key off actor while identity/approval key off ResourceScope.user_id. That means a delegated turn can expose tools under one user’s availability policy while enforcing credentials/approval under another user. Until all dimensions share one subject, reject the mismatch instead of applying split authority.

🤖 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/capability_surface_policy.rs` around
lines 239 - 263, The `principal_user_id` helper currently falls back from
`TurnActor` to `TurnScope` ownership, which can let availability/config and
approval/identity resolve different policy subjects on delegated turns. Update
`principal_user_id` to fail closed when the actor-derived user and
`scope.explicit_owner_user_id()` would not resolve to the same `UserId`, and
return `None` in that mismatch case so the policy surface cannot split authority
across `PolicySubject` sources.

Source: Coding guidelines

Comment on lines 598 to 600
if account.status != CredentialAccountStatus::Configured {
return Err(CredentialStageError::AuthRequired);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Map non-configured AdminKeyed accounts to Backend.

Line 598 returns AuthRequired for every non-Configured account, but under IdentityMode::AdminKeyed the user cannot fix provisioning; this contradicts the AdminKeyed “no re-auth gate” contract in Lines 548-551.

Proposed fix
         if account.status != CredentialAccountStatus::Configured {
+            #[cfg(feature = "capability-policy")]
+            if matches!(identity_mandate, Some(IdentityMode::AdminKeyed)) {
+                tracing::debug!(
+                    capability = %request.capability_id.as_str(),
+                    "admin-keyed capability account is not configured; unavailable"
+                );
+                return Err(CredentialStageError::Backend);
+            }
             return Err(CredentialStageError::AuthRequired);
         }

Please add a regression test for an AdminKeyed mandate with a non-configured shared-admin account. As per coding guidelines, “Every bug fix must include a regression test (#[test] or #[tokio::test]) that reproduces the original failure.”

🤖 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/product_auth_runtime_credentials.rs`
around lines 598 - 600, The non-Configured account branch in the credential
stage currently returns AuthRequired for all identities, but AdminKeyed mandates
should treat this as a Backend issue instead. Update the logic in
product_auth_runtime_credentials::CredentialStage so the non-configured check
distinguishes IdentityMode::AdminKeyed from other modes and maps
shared-admin/non-configured accounts to Backend rather than AuthRequired. Add a
regression test covering an AdminKeyed mandate with a non-configured
shared-admin account to verify the stage returns Backend and does not trigger a
re-auth gate.

Source: Coding guidelines

Comment on lines +1335 to +1339
async fn resolve(
&self,
_subject: &PolicySubject,
_capability: &CapabilityId,
) -> Result<EffectivePolicy, PolicyError> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the policy key inputs in the resolver double.

The fake ignores PolicySubject and CapabilityId, so these tests would still pass if the resolver stopped using request.capability_id or used the wrong acting subject. Make at least one caller-level test verify the expected tenant/user/capability. As per path instructions, “Test through the caller: when a helper gates a side effect, require a test driving the real call site.”

🤖 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/product_auth_runtime_credentials/tests.rs`
around lines 1335 - 1339, The resolver double in resolve currently ignores both
PolicySubject and CapabilityId, so add caller-level coverage that exercises the
real call site and verifies the expected tenant/user/capability flow instead of
only stubbing the fake. Update one or more tests around the code that builds the
policy request (for example the path using request.capability_id and the acting
subject) so the assertions check the specific PolicySubject and CapabilityId
values passed into resolve and fail if those inputs are wrong or omitted.

Source: Path instructions

Comment thread crates/ironclaw_reborn_composition/src/runtime.rs
Comment on lines +524 to +535

| Concept | Status — reuse / extend |
|---|---|
| Tenant / User / roles / per-user secrets | **exist (#1626)** — `TenantId`/`UserId`, RBAC, admin secrets, multi-tenant isolation. Add `kind(person\|shared)`. **Admin gate today = `src/ownership::UserRole` (Owner/Admin/Regular) + `AdminScope`** — reuse it; `ProjectRole(Owner/Editor/Viewer)` is the separate `ironclaw_projects` model (see §16) |
| Project + default project | `ProjectRecord` exists; **add** `is_default` + auto-create on tenant init |
| Memory (per-user) | exists; **compose with #5205** (memory as a userland extension) |
| Identity (user-keyed / admin-keyed) | **exists (#3289 / #4354)** — product-auth, per-user OAuth, account-scoped staging + MRU selection. **Wire to it**; don't add new keying |
| Approval | persistent approval-policy port + `AlwaysAllow`, and **#5195** (always-allow persisted as tool settings) — wire the `approval` field |
| Availability + ownership + effective resolution | **in-flight (#4544)** scoped-lifecycle ownership + effective package-set resolution, and **#5256** user-scoped tool settings — **extend these** |
| Enforcement | `ToolDispatcher::dispatch` — consult the resolver once per call |
| Shared-account auth | SSO via IdP (Google Workspace); no IronClaw operator list (reconcile vs #4354 — §1.5) |
| **Extend / add (the genuinely new work)** | the **config / identity / approval** dimensions layered on #4544's package-set; the **tenant publish surface + precedence**; **`PolicyAdmin` + WebUI**; per-capability **`default_policy`** in manifests; lower toward declarative **#4120 / #3036**. **Not** a parallel policy engine. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Appendix A contradicts §17 correction 7 on admin gate location.

§17 correction 7 (line 500-503) states: "Reborn admin gate = ironclaw_host_api::UserRole (#5266), not src/ownership." But Appendix A line 527 still says: "Admin gate today = src/ownership::UserRole (Owner/Admin/Regular) + AdminScope — reuse it".

Since §17 explicitly wins over prior sections, Appendix A needs to be updated to reference ironclaw_host_api::UserRole and note that src/ownership is the legacy V1 tree. This prevents readers from wiring against the wrong crate.

🤖 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 `@docs/plans/2026-06-24-capability-policy-architecture.md` around lines 524 -
535, Update Appendix A so the admin-gate reference matches the §17 correction:
replace the `src/ownership::UserRole` guidance with
`ironclaw_host_api::UserRole` as the reborn admin gate, and explicitly mark
`src/ownership` as the legacy V1 tree. Keep the surrounding guidance in the
`Concept | Status — reuse / extend` table aligned with this symbol change so
readers do not wire new work against the wrong crate.

zetyquickly and others added 3 commits June 26, 2026 09:42
…le home (#5261)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…min REST moves to control plane (#5261)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5349 June 26, 2026 16:47 Destroyed
@railway-app

railway-app Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-5349 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Jun 26, 2026 at 9:41 pm

zetyquickly and others added 3 commits June 26, 2026 14:32
…#5266 #5261)

Foundation for the capability-policy role model:
- UserRole::rank() (Owner=2 > Admin=1 > Member=0) + outranks() (strict) in
  ironclaw_host_api — no derived Ord (variant order would invert privilege).
- TurnActor gains role (#[serde(default)] → Member for legacy/channel actors)
  + with_role(); WebUiAuthenticatedCaller::actor() now carries the caller's
  role to dispatch (it was dropped before), so the availability resolver can
  be role-aware.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…#5267 #5261)

- Role-aware dispatch surface: the availability resolver reads the acting role
  (now carried on TurnActor) and, for Owner/Admin, returns the full installed +
  builtin set BYPASSING the per-capability policy intersection (admins/owner are
  capped by neither per-user nor tenant-scope hides). Members keep installed AND
  EffectivePolicy.available, fail-closed. Read-time bypass (demotion re-applies
  stored hides next turn).
- Builtin governance: the resolver now seeds builtin capability-ids (builtin.shell,
  ...) from the registry snapshot (minus installable-extension caps) into the
  base allow-set, so builtins are available-by-default AND hideable per-user. They
  were previously excluded (only installed extensions seeded), so policy-on denied
  all builtins.
+5 unit tests (admin bypass of user/tenant hides; builtin default-available;
member builtin-hide drops it; member installed∩policy holds).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 26, 2026 21:35
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5349 June 26, 2026 21:35 Destroyed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 59 out of 60 changed files in this pull request and generated 1 comment.

Comment on lines +227 to +231
pub(crate) fn capability_policy_activated() -> bool {
std::env::var("IRONCLAW_REBORN_CAPABILITY_POLICY")
.map(|value| matches!(value.trim(), "1" | "true" | "yes" | "on"))
.unwrap_or(false)
}

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

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/capability_surface_policy.rs (1)

208-217: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Mark intentional boundary-error fallbacks with silent-ok.

These store/policy read failures are intentionally converted into fail-closed allowlists. Repo Fail-loud guidance requires an inline // silent-ok: <reason> when a Result-backed boundary read fallback is acceptable.

Suggested annotation
             Err(error) => {
+                // silent-ok: scoped-lifecycle lookup failure fails closed by denying all capabilities for this turn.
                 tracing::debug!(
                     Err(error) => {
+                        // silent-ok: policy resolver failure fails closed by denying only this capability for this turn.
                         tracing::debug!(

As per coding guidelines, “When a fallback is genuinely acceptable, it must be justified inline with a // silent-ok: <reason> comment naming the operation.”

Also applies to: 263-273

🤖 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/capability_surface_policy.rs` around
lines 208 - 217, Add an inline `// silent-ok: ...` justification to the
intentional fail-closed fallback in the `Err(error)` branch of the
scoped-lifecycle installation lookup, directly above the
`tracing::debug!`/`CapabilityAllowSet::Allowlist(BTreeSet::new())` return. Make
the same annotation for the matching store/policy read fallback at the other
cited block, so the boundary-read-to-deny-all behavior is explicitly documented
where the `Result` is handled.

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.

Inline comments:
In `@crates/ironclaw_reborn_composition/src/runtime.rs`:
- Around line 2894-2903: The builtin capability discovery in the runtime is
incorrectly using `registry_snapshot.capabilities()` minus `installable_caps`,
which can misclassify third-party or filesystem capabilities as builtins and
keep them always installed. Update this logic in `runtime.rs` to derive
`builtin_capabilities` from an explicit host-builtin marker/source, or filter
against the full admin-installable package-to-capability map instead of the
first-party-only catalog. Keep the change localized around
`shared_extension_registry`, `extension_registry`, and the
`builtin_capabilities` collection so availability fails closed and scoped
lifecycle authority is preserved.

---

Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/capability_surface_policy.rs`:
- Around line 208-217: Add an inline `// silent-ok: ...` justification to the
intentional fail-closed fallback in the `Err(error)` branch of the
scoped-lifecycle installation lookup, directly above the
`tracing::debug!`/`CapabilityAllowSet::Allowlist(BTreeSet::new())` return. Make
the same annotation for the matching store/policy read fallback at the other
cited block, so the boundary-read-to-deny-all behavior is explicitly documented
where the `Result` is handled.
🪄 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: 2d91fa7e-a6e1-4390-8b9c-4e1e0267e253

📥 Commits

Reviewing files that changed from the base of the PR and between 20c508b and d9fddd7.

📒 Files selected for processing (14)
  • crates/ironclaw_host_api/src/lib.rs
  • crates/ironclaw_host_api/src/role.rs
  • crates/ironclaw_product_workflow/src/webui_inbound.rs
  • crates/ironclaw_reborn_composition/src/capability_surface_policy.rs
  • crates/ironclaw_reborn_composition/src/runtime.rs
  • crates/ironclaw_reborn_composition/src/slack_channel_routes.rs
  • crates/ironclaw_reborn_composition/src/slack_channel_routes/allowed/tests.rs
  • crates/ironclaw_reborn_composition/src/slack_host_beta.rs
  • crates/ironclaw_reborn_composition/src/slack_personal_binding_pairing_serve.rs
  • crates/ironclaw_reborn_composition/src/webui_serve.rs
  • crates/ironclaw_reborn_identity/src/filesystem_store.rs
  • crates/ironclaw_reborn_identity/src/filesystem_store/record.rs
  • crates/ironclaw_reborn_identity/src/lib.rs
  • crates/ironclaw_turns/src/scope.rs

Comment on lines +2894 to +2903
let registry_snapshot = local_runtime
.shared_extension_registry
.as_ref()
.map(|shared| shared.snapshot())
.unwrap_or_else(|| Arc::clone(&local_runtime.extension_registry));
let builtin_capabilities: Vec<CapabilityId> = registry_snapshot
.capabilities()
.map(|descriptor| descriptor.id.clone())
.filter(|capability_id| !installable_caps.contains(capability_id))
.collect();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Don’t infer builtins by subtracting a first-party catalog from the active registry.

installable_caps is first-party-only, but registry_snapshot.capabilities() is the active registry. Any filesystem/third-party capability present in the registry and absent from that catalog is now classified as builtin_capabilities and seeded as always-installed, bypassing scoped-lifecycle availability. Derive builtins from an explicit host-builtin source/package marker, or subtract the full admin-installable package→capability map.
As per path instructions, availability must fail closed and preserve scoped authority at dispatch seams.

🤖 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/runtime.rs` around lines 2894 - 2903,
The builtin capability discovery in the runtime is incorrectly using
`registry_snapshot.capabilities()` minus `installable_caps`, which can
misclassify third-party or filesystem capabilities as builtins and keep them
always installed. Update this logic in `runtime.rs` to derive
`builtin_capabilities` from an explicit host-builtin marker/source, or filter
against the full admin-installable package-to-capability map instead of the
first-party-only catalog. Keep the change localized around
`shared_extension_registry`, `extension_registry`, and the
`builtin_capabilities` collection so availability fails closed and scoped
lifecycle authority is preserved.

Source: Path instructions

@rpelevin

Copy link
Copy Markdown

I would treat availability as the first executable authority gate, not as a convenience filter.

The review point about builtin capability discovery is the right boundary to harden. If the resolver decides that something is builtin by subtracting one catalog from another, a third-party or filesystem capability can accidentally become always installed. That would bypass the scoped lifecycle store and make availability weaker than the admin install surface.

The regression I would want:

  1. create a first-party builtin capability, an admin-installed package capability, a user-private package capability, and a filesystem or third-party capability with overlapping names;
  2. assert only the real builtin is present without a scoped-lifecycle install record;
  3. assert admin-shared availability appears for the right tenant and user set;
  4. assert user-private availability appears only for the owning user;
  5. assert policy-available but not installed still produces no dispatch capability;
  6. assert installed but policy-hidden still produces no dispatch capability;
  7. assert store or policy-read failure returns an empty allowlist with an explicit fail-closed reason, not a fallback allow.

That keeps availability ahead of approval, identity, and config. A user approval preference, an admin grant, or a credential path should never resurrect a capability that was not actually visible through the same install-plus-policy resolver used by dispatch.

I would also make the fail-closed comments part of the contract, not just lint cleanup: if a boundary read fails, the receipt should explain that the capability was unavailable because the resolver could not prove the installed and policy-available pair.

Boundary: architecture and regression-test feedback only; no claim about using this project, running this branch, validating implementation behavior, implementation correctness, merge readiness, security review, production readiness, partnership, customer interest, official alignment, NearAI usage, Ironclaw usage, conformance certification, or Neura usage.

@zetyquickly

Copy link
Copy Markdown
Contributor Author

ai slop + pelevin

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-5349 — d9fddd7e Deployed Jun 26, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: dependencies Dependency updates scope: docs Documentation size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants