W2 endorsed Reborn crate folds - #5874
Conversation
* Clean up OpenAI compat storage fold references * Update crates/ironclaw_reborn_openai_compat/tests/ref_store_contract.rs Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR removes three standalone crates ( ChangesCrate Consolidation and Layer Metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SrcAuthOauth as src/auth/oauth.rs
participant IronclawAuthOauth as ironclaw_auth::oauth
participant LoopbackOauth as ironclaw_auth::loopback_oauth
SrcAuthOauth->>IronclawAuthOauth: pub use re-export
IronclawAuthOauth->>LoopbackOauth: pub use bind_callback_listener, wait_for_callback
LoopbackOauth-->>IronclawAuthOauth: OAUTH_CALLBACK_PORT, OAuthCallbackError
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
scope: docs
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the workspace by folding several standalone crates into existing ones to simplify the crate structure. Specifically, ironclaw_oauth is folded into ironclaw_auth as a legacy loopback transport, ironclaw_skill_learning is integrated into ironclaw_skills::learning, and ironclaw_wasm_sandbox_core is merged into ironclaw_wasm::wasm_sandbox_core. Additionally, ironclaw_projects is retained as a standalone substrate crate. Feedback on the changes suggests correcting a historical reference in the ironclaw_wasm_limiter documentation and refining the dependency boundary tests to ignore comments when checking for forbidden imports to prevent false positives.
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.
| //! enforce identical limits; centralizing the impl prevents drift and | ||
| //! makes the dependency edge visible to `cargo check`, `cargo doc`, and | ||
| //! architecture-linting tests. `ironclaw_wasm_sandbox_core` previously kept a | ||
| //! architecture-linting tests. `ironclaw_wasm` previously kept a |
There was a problem hiding this comment.
The comment states that ironclaw_wasm previously kept a verbatim copy of the limiter. However, it was actually the folded ironclaw_wasm_sandbox_core (now ironclaw_wasm::wasm_sandbox_core) that kept the verbatim copy. To maintain historical accuracy and clarity, this should refer to ironclaw_wasm::wasm_sandbox_core instead of ironclaw_wasm.
| //! architecture-linting tests. `ironclaw_wasm` previously kept a | |
| //! architecture-linting tests. `ironclaw_wasm::wasm_sandbox_core` previously kept a |
| let source = std::fs::read_to_string(&module).expect("WASM sandbox core module is readable"); | ||
| for forbidden in [ | ||
| "ironclaw_product", | ||
| "ironclaw_dispatcher", | ||
| "ironclaw_extensions", | ||
| "ironclaw_filesystem", | ||
| "ironclaw_network", | ||
| "ironclaw_secrets", | ||
| "ironclaw_host_runtime", | ||
| "ironclaw_reborn_composition", | ||
| ] { | ||
| assert!( | ||
| !source.contains(forbidden), | ||
| "folded WASM sandbox core module must stay independent of product/runtime/app crates; \ | ||
| unexpected reference to `{forbidden}`" | ||
| ); | ||
| } |
There was a problem hiding this comment.
The substring check source.contains(forbidden) runs against the entire raw source file, including comments. This means any explanatory comments or documentation mentioning these forbidden crates (e.g., // This module must not depend on ironclaw_product_adapters) will trigger a test failure. To prevent false positives from documentation or comments, consider naively filtering out comment-only lines before performing the check.
let source = std::fs::read_to_string(&module).expect("WASM sandbox core module is readable");
let clean_source: String = source
.lines()
.filter(|line| !line.trim().starts_with("//"))
.collect::<Vec<_>>()
.join("\n");
for forbidden in [
"ironclaw_product",
"ironclaw_dispatcher",
"ironclaw_extensions",
"ironclaw_filesystem",
"ironclaw_network",
"ironclaw_secrets",
"ironclaw_host_runtime",
"ironclaw_reborn_composition",
] {
assert!(
!clean_source.contains(forbidden),
"folded WASM sandbox core module must stay independent of product/runtime/app crates; \
unexpected reference to `{forbidden}`"
);
}
e2916be to
e8a3872
Compare
|
🚅 Deployed to the ironclaw-pr-5874 environment in ironclaw-ci-preview
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs (1)
124-157: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider merging the two dependency-iteration loops.
Both loops iterate the same
workspace_dependency_names(package).filter(|dep| is_normal_dependency(dep))iterator. The first checks layer-matrix violations (plus theironclaw_agent_loopspecial rule); the second checks legacy-layer dependencies. Merging them into a single pass would avoid redundant iteration and make the relationship between the two checks clearer.This is a test file so the performance impact is negligible, but the duplication is a readability smell that will grow if a third cross-cutting check is added later.
🤖 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_architecture/tests/reborn_dependency_boundaries.rs` around lines 124 - 157, The dependency validation in reborn_dependency_boundaries.rs iterates over workspace_dependency_names(package).filter(|dep| is_normal_dependency(dep)) twice, which duplicates the same traversal. Merge the checks in the two loops into a single pass and keep both the ironclaw_agent_loop contracts-layer rule and the legacy-layer dependency rule in that combined flow. Use the existing helper symbols layer_matrix_exception, used_exceptions, and violations to preserve current behavior while making the cross-cutting checks easier to follow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs`:
- Around line 124-157: The dependency validation in
reborn_dependency_boundaries.rs iterates over
workspace_dependency_names(package).filter(|dep| is_normal_dependency(dep))
twice, which duplicates the same traversal. Merge the checks in the two loops
into a single pass and keep both the ironclaw_agent_loop contracts-layer rule
and the legacy-layer dependency rule in that combined flow. Use the existing
helper symbols layer_matrix_exception, used_exceptions, and violations to
preserve current behavior while making the cross-cutting checks easier to
follow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 188de88e-839d-4dac-be20-ef6b74dd2890
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (1)
crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.14% — 284130 / 333732 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
This branch was successfully deployed
1 participant
Summary
This is the combined W2 endorsed-folds PR for the remaining W2 crate-count reductions. It supersedes the individual draft PRs #5864, #5865, #5866, #5869, and #5870.
Note: #5868 (
ironclaw_reborn_openai_compat_storagecleanup) already merged into the W0 base branch, so this PR is rebased on top of it rather than duplicating that diff.This PR:
ironclaw_wasm_sandbox_coreintoironclaw_wasmironclaw_oauthintoironclaw_authironclaw_skill_learningintoironclaw_skillsironclaw_product_workflow_storageboundaryironclaw_projectsseparate for now, and only reconsiderironclaw_product_workflowas a future consumer-side target, never compositionironclaw_oauthcrateWhy this shape
We want fewer crates, but not a broad collapse. W0 added the mechanical guardrail first: every Reborn workspace crate must declare a layer and obey the allowlisted layer matrix. That means these W2 folds can remove low-value package boundaries without letting product/app/composition concerns drift downward into substrates.
Combining the remaining W2 work into one PR makes review easier than separate PRs because the reviewer can validate the net package graph once:
The commits are intentionally separated by fold/decision so reviewers can still inspect each move independently inside this one PR.
Reviewer guide
Fold WASM sandbox core into WASM runtime: pure domain-free WASM kernel code moves underironclaw_wasm::wasm_sandbox_core; product adapter code remains outsideironclaw_wasm.Fold OAuth loopback transport into auth: v1 loopback OAuth callback transport moves underironclaw_auth::loopback_oauth; Reborn OAuth/product auth ownership remains inironclaw_auth.Fold skill learning into skills: extraction/refinement logic and prompts move underironclaw_skills::learning; composition imports the owning skills crate directly.Clean up product workflow storage fold references: durable ledger storage docs/tests now point atironclaw_product_workflowinstead of the retired storage crate.Document projects crate W2 decision: keepsironclaw_projectsas a substrate entity/repository crate in W2.Remove folded OAuth crate coverage exemption: deletes the stale coverage exemption for a crate that no longer exists.Validation
cargo metadata --format-version 1 --no-deps | jq '[.packages[] | select(.name=="ironclaw_wasm_sandbox_core" or .name=="ironclaw_oauth" or .name=="ironclaw_skill_learning" or .name=="ironclaw_reborn_openai_compat_storage" or .name=="ironclaw_product_workflow_storage")] | map(.name)'->[]cargo test -p ironclaw_auth -p ironclaw_skills -p ironclaw_wasm -p ironclaw_wasm_product_adapters -p ironclaw_projectscargo test -p ironclaw_reborn_openai_compat -p ironclaw_architecturecargo check -p ironclaw_reborn_composition --features root-llm-providerironclaw_reborndead-code helpers and oneironclaw_reborn_compositionunused importcargo test -p ironclaw_product_workflow --features storage --test durable_scoped_ledger_contractcargo fmt --checkgit diff --check origin/codex/reborn-layer-allowlist-w0-main...HEAD