Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,10 @@ path = "tests/integration/webui_v2_router_smoke.rs"
name = "reborn_integration_wiring_parity"
path = "tests/integration/wiring_parity.rs"

[[test]]
name = "reborn_omp_registration"
path = "tests/integration/reborn_omp_registration.rs"

Comment on lines +486 to +489

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: List existing [[test]] target names whose path is under tests/integration/.
set -euo pipefail
python3 - <<'PY'
import re, pathlib
text = pathlib.Path("Cargo.toml").read_text()
for block in re.findall(r'\[\[test\]\]([^\[]*)', text):
    name = re.search(r'name\s*=\s*"([^"]+)"', block)
    path = re.search(r'path\s*=\s*"([^"]+)"', block)
    if name and path and path.group(1).startswith("tests/integration/"):
        print(f"{name.group(1)}  <-  {path.group(1)}")
PY

Repository: nearai/ironclaw

Length of output: 4949


🏁 Script executed:

#!/bin/bash
# Search for the repo testing invariant mentioning reborn_integration_<name>.
set -euo pipefail
rg -n --hidden --glob '!target/**' 'integration-test binary|integration-test binaries|reborn_integration_<name>|reborn_integration_' .

Repository: nearai/ironclaw

Length of output: 47202


Rename the integration-test target to match the repo invariant.

.claude/rules/testing.md:30, tests/integration/CLAUDE.md:161, scripts/ci/reborn-coverage-int-tier-tests.sh:9, and docs/internal/testing-playbook.md:267 require integration tests under tests/integration/ be registered as name = "reborn_integration_<name>". reborn_omp_registration violates that invariant; update the target name and any matching references to reborn_integration_omp_registration.

🤖 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 `@Cargo.toml` around lines 486 - 489, Rename the Cargo integration-test target
from reborn_omp_registration to reborn_integration_omp_registration, and update
every matching reference in the repository, including CI scripts and testing
documentation, while preserving the existing test path.

Source: Coding guidelines

[profile.release]
strip = true # Remove debug symbols from release binaries

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,12 @@ fn reborn_contracts_crates_carry_a_checked_size_ceiling() {
// sandbox transport now exposes graceful lifecycle release. This is
// contract vocabulary; execution and provider cleanup remain in the
// sandbox runtime lane.
("ironclaw_host_api", 18_799),
// Raised 18_799 -> 18_808 (2026-08-08, #7392 slice 3): the additive
// `provider_tool_name` override field on CapabilityDescriptor (serde
// default None + skip_serializing_if) plus its doc comment — pure
// declarative vocabulary, 9 lines. Count read from this test's own
// failure message.
("ironclaw_host_api", 18_808),
// 14_479 -> 13_949 (2026-08-07, #7157): downward re-capture after the
// delivery-heuristic vocabulary (stored trigger delivery targets and
// their run-profile plumbing) left this crate with the two-lane
Expand Down Expand Up @@ -1768,6 +1773,34 @@ fn provider_tool_names_stay_at_model_protocol_boundaries() {
"crates/ironclaw_host_api/src/ids.rs",
"crates/ironclaw_safety/src/lib.rs",
"crates/ironclaw_safety/src/provider_validation.rs",
// Issue #7392 provider-name resolver: the additive declarative field
// (`provider_tool_name`) lives on the capability declaration and the
// runtime descriptor as plain serialized data, validated as
// `ProviderToolName` only where it becomes provider-protocol identity
// (manifest parse + the loop boundary). These files are declarative
// storage/propagation, not routing or product logic.
"crates/ironclaw_host_api/src/capability.rs",
"crates/ironclaw_extension_registry/src/v2.rs",
"crates/ironclaw_extension_registry/src/v3.rs",
"crates/ironclaw_extension_registry/src/package.rs",
"crates/ironclaw_extension_registry/src/hosted_mcp_discovery.rs",
// Host-owned manifest/descriptor builders that carry the additive
// declarative field through to the descriptor (same storage role as
// the registry mappings above).
"crates/ironclaw_extension_host/src/active.rs",
"crates/ironclaw_extension_host/src/generic_host.rs",
"crates/ironclaw_extension_host/src/hosted_mcp_manifest.rs",
"crates/ironclaw_extension_host/src/mcp.rs",
"crates/ironclaw_extension_manager/src/admin_configuration_capability.rs",
"crates/ironclaw_extension_manager/src/extension_lifecycle_capabilities.rs",
"crates/ironclaw_extension_manager/src/operator_config_capability.rs",
"crates/ironclaw_extension_manager/src/skill_auto_activate_capability.rs",
"crates/ironclaw_extension_manager/src/ironhub/capabilities.rs",
"crates/ironclaw_host_runtime/src/first_party_tools/mod.rs",
// Test-support-gated omp registration seam (issue #7392 slice 3):
// builds the builtin package plus the five omp capabilities with
// their exact-name overrides; compiled out of production binaries.
"crates/ironclaw_host_runtime/src/first_party_tools/omp.rs",
Comment on lines +1800 to +1803

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

The allowlist justification claims a gate that does not exist.

The comment states the omp registration seam is "Test-support-gated" and "compiled out of production binaries". crates/kernel/ironclaw_host_runtime/src/first_party_tools/omp.rs carries no #[cfg] gate, its module doc at Lines 22-28 states the surface "is enabled in PRODUCTION builds", and crates/app/ironclaw_composition/src/factory.rs Lines 1180-1184 calls omp_coding_package from the production factory.

This allowlist is a provider-protocol boundary control. A future reviewer reading this entry will conclude the seam cannot reach a shipped binary. Record the real justification: an unconditional, temporary production registration seam for the #7392 benchmark arm, to be removed at cutover.

📝 Proposed fix
-        // Test-support-gated omp registration seam (issue `#7392` slice 3):
-        // builds the builtin package plus the five omp capabilities with
-        // their exact-name overrides; compiled out of production binaries.
+        // ⚠️ TEMPORARY omp registration seam (issue `#7392` slice 3): builds
+        // the builtin package plus the five omp capabilities with their
+        // exact-name overrides. NOT feature-gated — it ships in production
+        // builds for the /benchmark arm and is removed at cutover.
         "crates/ironclaw_host_runtime/src/first_party_tools/omp.rs",
📝 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
// Test-support-gated omp registration seam (issue #7392 slice 3):
// builds the builtin package plus the five omp capabilities with
// their exact-name overrides; compiled out of production binaries.
"crates/ironclaw_host_runtime/src/first_party_tools/omp.rs",
// ⚠️ TEMPORARY omp registration seam (issue `#7392` slice 3): builds
// the builtin package plus the five omp capabilities with their
// exact-name overrides. NOT feature-gated — it ships in production
// builds for the /benchmark arm and is removed at cutover.
"crates/ironclaw_host_runtime/src/first_party_tools/omp.rs",
🤖 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/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs`
around lines 1800 - 1803, Update the comment above the omp.rs allowlist entry to
remove the incorrect “Test-support-gated” and “compiled out of production
binaries” claims, and describe it instead as an unconditional temporary
production registration seam for the `#7392` benchmark arm that must be removed at
cutover.

// Host loop/run/thread protocol structs that preserve exact model
// provider names for tool-result roundtrips and historical replay.
// The provider-tool-call DTOs live in the `capability` submodule of the
Expand Down
21 changes: 21 additions & 0 deletions crates/app/ironclaw_composition/src/builtin_capability_policy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,27 @@ effects = ["dispatch_capability", "read_filesystem", "write_filesystem"]
mounts = "workspace"
network = "default"

# ⚠️ TEMPORARY benchmark override (issue #7392 bench arm): grants for the
# omp-parity capability ids so the /benchmark surface advertises read/write/edit
# alongside glob/grep. Revert at cutover alongside the omp.rs override.
[[grants]]
capability = "builtin.read"
effects = ["dispatch_capability", "read_filesystem", "write_filesystem"]
mounts = "workspace"
network = "default"

[[grants]]
capability = "builtin.write"
effects = ["dispatch_capability", "read_filesystem", "write_filesystem"]
mounts = "workspace"
network = "default"

[[grants]]
capability = "builtin.edit"
effects = ["dispatch_capability", "read_filesystem", "write_filesystem"]
mounts = "workspace"
network = "default"

[[grants]]
capability = "builtin.list_dir"
effects = ["dispatch_capability", "read_filesystem", "write_filesystem"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ fn local_host_shell_authorization_inputs(
resource_profile: None,
origin_gate_matrix: None,
standard_op: None,
provider_tool_name: None,
};
let policy = builtin_capability_policy().expect("capability policy");
let grants = policy.builtin_grants(
Expand Down Expand Up @@ -429,6 +430,7 @@ async fn trace_commons_authorize_decision(
resource_profile: None,
origin_gate_matrix: None,
standard_op: None,
provider_tool_name: None,
};
let policy = Arc::new(builtin_capability_policy().expect("capability policy"));
let provider_id = ExtensionId::new(BUILTIN_FIRST_PARTY_PROVIDER).expect("provider id");
Expand Down
43 changes: 40 additions & 3 deletions crates/app/ironclaw_composition/src/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ use ironclaw_extension_manager::{
};
use ironclaw_extension_registry::{
ExtensionInstallationStore, ExtensionInstallationStorePort, ExtensionLifecycleService,
ExtensionRegistry, ManifestSource, SharedExtensionRegistry,
ExtensionPackage, ExtensionRegistry, ManifestSource, SharedExtensionRegistry,
};
use ironclaw_filesystem::ScopedFilesystem;
#[cfg(test)]
Expand Down Expand Up @@ -1115,13 +1115,29 @@ fn production_builtin_extension_registry(
process_backend: ProcessBackendKind,
memory_package: Option<&ironclaw_extension_registry::ExtensionPackage>,
) -> Result<ExtensionRegistry, RebornBuildError> {
let mut registry = ExtensionRegistry::new();
let package =
builtin_first_party_package_for_process_backend(process_backend).map_err(|error| {
RebornBuildError::InvalidConfig {
reason: format!("built-in first-party package is invalid: {error}"),
}
})?;
let package = extend_builtin_package(package)?;
let mut registry = ExtensionRegistry::new();
registry
.insert(package)
.map_err(|error| RebornBuildError::InvalidConfig {
reason: format!("built-in first-party registry is invalid: {error}"),
})?;
insert_bound_memory_package(&mut registry, memory_package)?;
Ok(registry)
}

/// The host-owned extension packages layered on top of the built-in
/// first-party package (extension lifecycle, IronHub, admin configuration,
/// operator configuration, skill auto-activation) — shared by the stock and
/// omp-extended builtin registries so the omp arm differs ONLY in the five
/// omp capabilities, never in the extension layers.
fn extend_builtin_package(package: ExtensionPackage) -> Result<ExtensionPackage, RebornBuildError> {
let package = extend_builtin_first_party_package(package).map_err(|error| {
RebornBuildError::InvalidConfig {
reason: format!("extension lifecycle package is invalid: {error}"),
Expand All @@ -1147,10 +1163,31 @@ fn production_builtin_extension_registry(
reason: format!("skill auto-activation package is invalid: {error}"),
}
})?;
Ok(package)
}

/// Issue #7392 slice 3 seam (test-support only): the built-in first-party
/// registry whose coding capabilities are the omp-extended package
/// (`ironclaw_host_runtime::omp_coding_package`) — the stock surface plus
/// the five omp tools under the exact `read`/`write`/`edit`/`glob`/`grep`
/// names. The extension layers and the bound memory package ride on top
/// exactly as in production.
#[cfg(any(test, feature = "test-support"))]
pub(crate) fn omp_extended_builtin_extension_registry(
process_backend: ProcessBackendKind,
memory_package: Option<&ironclaw_extension_registry::ExtensionPackage>,
) -> Result<ExtensionRegistry, RebornBuildError> {
let package = ironclaw_host_runtime::omp_coding_package(process_backend).map_err(|error| {
RebornBuildError::InvalidConfig {
reason: format!("omp-extended built-in package is invalid: {error}"),
}
})?;
let package = extend_builtin_package(package)?;
let mut registry = ExtensionRegistry::new();
registry
.insert(package)
.map_err(|error| RebornBuildError::InvalidConfig {
reason: format!("built-in first-party registry is invalid: {error}"),
reason: format!("omp-extended first-party registry is invalid: {error}"),
})?;
insert_bound_memory_package(&mut registry, memory_package)?;
Ok(registry)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ pub(super) async fn build_backend_production(
network_http_egress_for_test,
#[cfg(any(test, feature = "test-support"))]
trust_fixture_extensions_for_test,
#[cfg(any(test, feature = "test-support"))]
omp_coding_tools_for_test,
} = context;
let deployment_is_local_single_user = matches!(
production_wiring.runtime_policy.deployment,
Expand Down Expand Up @@ -478,6 +480,13 @@ pub(super) async fn build_backend_production(
Arc::new(crate::outbound::MutableOutboundDeliveryTargetRegistry::default());
let skill_auto_activate_learned = Arc::new(AtomicBool::new(true));
let process_backend = production_wiring.runtime_policy.process_backend;
#[cfg(any(test, feature = "test-support"))]
let extension_registry = if omp_coding_tools_for_test {
omp_extended_builtin_extension_registry(process_backend, resolved_memory.package.as_ref())?
} else {
production_builtin_extension_registry(process_backend, resolved_memory.package.as_ref())?
};
#[cfg(not(any(test, feature = "test-support")))]
let extension_registry =
production_builtin_extension_registry(process_backend, resolved_memory.package.as_ref())?;
let extension_registry = Arc::new(extension_registry);
Expand Down Expand Up @@ -590,6 +599,17 @@ pub(super) async fn build_backend_production(
trigger_active_run_lookup,
process_backend,
)?;
#[cfg(any(test, feature = "test-support"))]
if omp_coding_tools_for_test {
// The omp handler adapter replaces the stock builtin handler for
// `builtin.glob`/`builtin.grep` and adds `builtin.read`/`write`/`edit`
// (issue #7392 slice 3 registration seam).
ironclaw_host_runtime::insert_omp_coding_handlers(&mut first_party_registry).map_err(
|error| RebornBuildError::InvalidConfig {
reason: format!("omp first-party handlers are invalid: {error}"),
},
)?;
}
if let (Some(package), Some(handler)) = (
resolved_memory.package.as_ref(),
resolved_memory.tool_handler.as_ref(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub(super) async fn build_production_shaped(
network_http_egress_for_test,
#[cfg(any(test, feature = "test-support"))]
trust_fixture_extensions_for_test,
#[cfg(any(test, feature = "test-support"))]
omp_coding_tools_for_test,
memory_binding_policy,
memory_provider_connection,
..
Expand Down Expand Up @@ -94,6 +96,8 @@ pub(super) async fn build_production_shaped(
network_http_egress_for_test,
#[cfg(any(test, feature = "test-support"))]
trust_fixture_extensions_for_test,
#[cfg(any(test, feature = "test-support"))]
omp_coding_tools_for_test,
};
match storage {
RebornStorageInput::Disabled => Err(RebornBuildError::InvalidConfig {
Expand Down Expand Up @@ -373,6 +377,11 @@ pub(super) struct RebornProductionBuildContext {
pub(super) network_http_egress_for_test: Option<Arc<dyn ironclaw_network::NetworkHttpEgress>>,
#[cfg(any(test, feature = "test-support"))]
pub(super) trust_fixture_extensions_for_test: bool,
/// Issue #7392 slice 3 seam: select the omp-extended built-in package +
/// handlers. Test-support only; default `false` keeps the stock surface
/// byte-identical.
#[cfg(any(test, feature = "test-support"))]
pub(super) omp_coding_tools_for_test: bool,
}

fn production_wiring(
Expand Down
20 changes: 20 additions & 0 deletions crates/app/ironclaw_composition/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,13 @@ pub struct RebornHostBindings {
/// `InstalledLocal` (#5459).
#[cfg(any(test, feature = "test-support"))]
pub(crate) trust_fixture_extensions_for_test: bool,
/// Issue #7392 slice 3 seam: when `true`, the built-in first-party
/// package and handler registry are replaced by the omp-extended
/// variants (exact `read`/`write`/`edit`/`glob`/`grep` model surface).
/// Test-support only; the harness never sets it by default and the field
/// ships zero bytes in production builds.
#[cfg(any(test, feature = "test-support"))]
pub(crate) omp_coding_tools_for_test: bool,
pub(crate) product_auth_ports: Option<RebornProductAuthServicePorts>,
/// `first_party`-runtime extension factories the binary assembles
/// (extension-runtime P2). Empty until concrete extension crates extract
Expand Down Expand Up @@ -842,6 +849,17 @@ impl RebornHostBindings {
self
}

/// Select the omp-extended built-in first-party package + handler
/// registry (issue #7392 slice 3): the stock builtin surface plus the
/// five omp capabilities under the exact `read`/`write`/`edit`/`glob`/
/// `grep` names. Test-support only; the default surface is
/// byte-identical to today.
#[cfg(any(test, feature = "test-support"))]
pub fn with_omp_coding_tools_for_test(mut self) -> Self {
self.omp_coding_tools_for_test = true;
self
}

/// Inject Reborn-native product-auth service ports.
///
/// Production callers should provide durable implementations here. The
Expand Down Expand Up @@ -928,6 +946,8 @@ impl RebornHostBindings {
network_http_egress_for_test: None,
#[cfg(any(test, feature = "test-support"))]
trust_fixture_extensions_for_test: false,
#[cfg(any(test, feature = "test-support"))]
omp_coding_tools_for_test: false,
product_auth_ports: None,
native_extension_factories: Vec::new(),
channel_extension_bindings: Vec::new(),
Expand Down
1 change: 1 addition & 0 deletions crates/app/ironclaw_composition/src/product_capability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,7 @@ mod tests {
resource_profile: None,
origin_gate_matrix: None,
standard_op: None,
provider_tool_name: None,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,32 @@ fn assert_tool_succeeded(outcome: &ToolOutcome, label: &str) {
);
}

/// Assert the tool completed with a recoverable failure whose model-visible
/// diagnostic contains `needle`. The omp glob/grep engines (⚠️ TEMPORARY
/// benchmark override, issue #7392) error with the pinned `Path not found`
/// text when the caller's workspace root does not exist yet — the v1 tools
/// returned empty results for the same input.
fn assert_tool_failed_containing(outcome: &ToolOutcome, label: &str, needle: &str) {
let Resolution::Done(done) = &outcome.resolution else {
panic!("{label} should complete, got {:?}", outcome.resolution);
};
let ironclaw_host_api::resolution::ToolVerdict::RecoverableFailure { diagnostic, .. } =
&done.verdict
else {
panic!(
"{label} should fail recoverably, got {:?}",
done.verdict
);
};
let text = diagnostic
.model_visible_text()
.unwrap_or_else(|| panic!("{label} failure must carry free-text cause, got {diagnostic:?}"));
assert!(
text.contains(needle),
"{label} failure diagnostic must contain {needle:?}, got {text:?}"
);
}

async fn read_composed_path(services: &RebornRuntimeStores, path: &str) -> Option<String> {
let filesystem = services
.local_runtime_for_test()
Expand Down Expand Up @@ -369,15 +395,16 @@ async fn fresh_caller_reads_an_empty_workspace_then_writes_into_it() {
"newcomer",
"builtin_glob",
ironclaw_host_runtime::GLOB_CAPABILITY_ID,
serde_json::json!({ "pattern": "**/*" }),
// ⚠️ TEMPORARY benchmark override (issue #7392): `builtin.glob` now
// dispatches to the omp glob engine, whose pinned schema takes `path`
// (glob/file/dir to search) — there is no `pattern` field.
serde_json::json!({ "path": "**/*" }),
)
.await;
assert_tool_succeeded(&globbed, "glob on a fresh caller's workspace");
let globbed_output = globbed.output.expect("glob returns output"); // safety: test-only assertion in #[cfg(test)] module.
assert_eq!(
globbed_output["files"].as_array().map(Vec::len),
Some(0),
"a fresh caller's glob matches nothing, got {globbed_output}"
assert_tool_failed_containing(
&globbed,
"glob on a fresh caller's workspace",
"Path not found",
);

let grepped = invoke_workspace_tool_as(
Expand All @@ -388,12 +415,10 @@ async fn fresh_caller_reads_an_empty_workspace_then_writes_into_it() {
serde_json::json!({ "pattern": "anything" }),
)
.await;
assert_tool_succeeded(&grepped, "grep on a fresh caller's workspace");
let grepped_output = grepped.output.expect("grep returns output"); // safety: test-only assertion in #[cfg(test)] module.
assert_eq!(
grepped_output["files"].as_array().map(Vec::len),
Some(0),
"a fresh caller's grep matches nothing, got {grepped_output}"
assert_tool_failed_containing(
&grepped,
"grep on a fresh caller's workspace",
"Path not found",
);

// The first write from that same fresh caller must succeed and become
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ impl HostRuntime for StubHostRuntime {
resource_profile: None,
origin_gate_matrix: None,
standard_op: None,
provider_tool_name: None,
},
description_trust: Default::default(),
access: VisibleCapabilityAccess::Available,
Expand Down
Loading