refactor(composition): shrink deployment-mode branching ratchet 5->3 (#6274 Track 1) - #6387
Conversation
…(§4.4/#6274 Track 1) The `reborn_deployment_mode_branching_ratchet` allowlist tracks every file in `ironclaw_reborn_composition` still permitted to name a deployment-mode profile variant; the §4.4 target is `{deployment.rs}` (the one sanctioned profile->config edge). Two of the five entries were kept alive only by inline `#[cfg(test)]` fixtures — retire them by extracting the test modules to sibling `tests.rs` files (which the ratchet scanner excludes): - `webui/facade.rs` -> `webui/facade/tests.rs`: the `RebornCompositionProfile` literals were entirely in the trailing `#[cfg(test)] mod tests`. - `factory.rs` -> `factory/tests.rs`: same — all profile literals lived in the inline test module (production factory.rs names no variant; it reads `DeploymentConfig::storage_shape()`). Matches the crate's existing `mod auth_tests;` / `mod local_dev_host_tests;` sibling-file pattern. Shrinks factory.rs 8566->5599. Allowlist now `{deployment.rs (target), local_runtime_profile.rs, readiness.rs}`: - `local_runtime_profile.rs` — investigated for folding into `deployment.rs`; it keeps one production branch selecting the hosted-single-tenant-volume build input, whose `libsql`-feature requirement no `DeploymentConfig` axis yet captures. Cleanly retiring it needs a new config axis, so it's deferred to Phase 4 / §5.11 (the `build_runtime(cfg)` collapse), documented on the entry. - `readiness.rs` — profile is an operator-facing wire **label** (`RebornReadinessDiagnostic::profile`), not a branch; stays until that field is reshaped. Behavior-preserving: the extractions are pure moves (super-relative paths unchanged); `factory/tests.rs` carries an `arch-exempt: large_file` note. No production code changed. [skip-regression-check] test-module relocation + ratchet-allowlist trim; the architecture ratchet itself is the pinning test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
🔎 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. |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (2)
⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughFactory and WebUI test suites move into standalone modules. The added tests cover composition invariants, persistence, secrets, runtime integrations, skill boundaries, readiness diagnostics, operator authorization, tool catalogs, and WebUI skill lifecycle behavior. ChangesComposition regression coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
⚠️ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| 0 | 0 | 0 | 6e357cb5e2e6 |
Head: 6e357cb5e2e67c3e08412b83ba80b3bd5aaeaa11
Next: Human review or validation is required before merging.
Run details
Status: Current
Needs human: no
Needs validation: yes
Summary
No concrete defect found in static review. Production portions are byte-identical; the change relocates 96 test functions and tightens the ratchet allowlist. Toolchain validation could not run because cargo and rustfmt are unavailable.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_reborn_composition/src/webui/facade/tests.rs`:
- Line 1: The new tests.rs file exceeds the repository’s size threshold without
the required exemption or modularization. Either add a valid inline
`arch-exempt: large_file, <reason>, plan `#NNNN`` annotation to the test module,
or split the operator-catalog and skills test suites into separate modules.
🪄 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: f7e5701f-b025-4333-99e7-7cb4ebd7da89
📒 Files selected for processing (5)
crates/ironclaw_architecture/tests/reborn_deployment_mode_branching_ratchet.rscrates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/factory/tests.rscrates/ironclaw_reborn_composition/src/webui/facade.rscrates/ironclaw_reborn_composition/src/webui/facade/tests.rs
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 86.31% — 318346 / 368836 lines Per-crate breakdown (65 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)
|
|
🚅 Deployed to the ironclaw-pr-6387 environment in ironclaw-ci-preview
|
…ode-ratchet-shrink # Conflicts: # crates/ironclaw_reborn_composition/src/factory.rs
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y cutover, #6386 authorize() consolidation, #6408 outbound caller-scoping) Eighth fold. Dispositions follow the standing philosophy (never merge-as-is, never drop a feature); ledger entry lands in the PR body. - Tier B adopted wholesale: src/ + gateway/tui crates deleted, root package is the test-only ironclaw_reborn_integration_tests host, root Dockerfile is the Reborn image (de-migrated per owner decision D1 — this tree deletes ironclaw_reborn_migration; the D1 absence pin moves to the root Dockerfile), legacy test_rig/support files gone with their [[test]] entries. - #6386 authorize() consolidation: production side taken verbatim; the local-manifest trust test main relocated to ironclaw_capabilities::trust is re-expressed in this tree's manifest dialect (host_api sections + contracts registry arg). - #6408 outbound caller-scoping made structural on the generic lane: the OutboundDeliveryTargetOwner vocabulary + owner field are defined locally in composition (ironclaw_channel_host stays deleted), both registry paths keep main's entry_owned_by_caller filtering, and the generic channel provider stamps owners from the resolved resource (subject route / DM record user), never the caller. - #6374 trigger-fire access as config: main's TriggerFireAccessPolicy wiring and serve tests adopted; the LocalTriggerAccess* store lane stays deleted. - #6395 SSO/admin identity resolver: production-substrate branch adopted; the legacy libSQL identity fold stays out (greenfield blank-slate). - #6387 factory/facade test extraction: main's module topology adopted; this branch's test-module content three-way-merged into factory/tests.rs and webui/facade/tests.rs. - CI: package allowlist keeps this tree's crate set + main's root-package exclusion; bucket lanes for deleted crates removed (self-test repinned). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What & why
Track 1 of finishing
DeploymentConfigadoption (#6274): shrink thereborn_deployment_mode_branching_ratchetallowlist toward its §4.4 target of{deployment.rs}— the one sanctioned place a profile name becomes deploymentdata.
Two of the five allowlisted files were kept on the list only by inline
#[cfg(test)]fixtures that name aRebornCompositionProfilevariant. Theratchet scanner excludes
*tests.rsfiles, so extracting those test modules tosibling files retires the entries — no production change.
Changes
webui/facade.rs→webui/facade/tests.rs— the profile literals wereentirely in the trailing
#[cfg(test)] mod tests.factory.rs→factory/tests.rs— same; productionfactory.rsnames noprofile variant (it reads
DeploymentConfig::storage_shape()). Matches thecrate's existing
mod auth_tests;/mod local_dev_host_tests;pattern.Shrinks
factory.rs8566 → 5599 lines.{deployment.rs (TARGET), local_runtime_profile.rs, readiness.rs}.The two remaining entries (not retired — with rationale)
local_runtime_profile.rs— investigated for folding intodeployment.rs.It keeps one production branch selecting the hosted-single-tenant-volume
build input, whose
libsql-feature requirement noDeploymentConfigaxis yetcaptures (
hosted_single_tenant_volume()sets the sameStorageShape::LocalDevRootas
local_dev(), so no existing axis distinguishes it). Cleanly retiring itneeds a new config axis → deferred to Phase 4 / §5.11 (the
build_runtime(cfg)collapse). Documented on the allowlist entry.
readiness.rs— profile is an operator-facing wire label(
RebornReadinessDiagnostic::profile), not a branch; retires only if thatfield is reshaped.
Verification
cargo test -p ironclaw_architecture— green; the branching ratchet confirmsfactory.rs/webui/facade.rsno longer name a profile variant and thetrimmed allowlist matches reality (
only_shrinks+sorted_and_uniquepass).cargo test -p ironclaw_reborn_composition— green (30 suites; the relocatedtest modules compile and run unchanged).
cargo clippy -p ironclaw_reborn_composition -p ironclaw_architecture --all-targets -- -D warnings— clean.scripts/pre-commit-safety.sh— pass (factory/tests.rscarries anarch-exempt: large_filenote for the mechanical extraction).Behavior-preserving: the extractions are pure moves (
super-relative pathsunchanged). No production code changed.
🤖 Generated with Claude Code