refactor(composition): DeploymentConfig owns every deployment axis; no branching on mode - #6279
Merged
Merged
Conversation
…, not profile matches Phase 1 of #6274 — finishing `DeploymentConfig` as the main composition config (§4.4/§5.6 of the architecture-simplification note). Two composition sites derived enforcement authority by branching on a deployment mode rather than consuming resolved policy data: - `runtime.rs` skipped the model-budget accountant by matching `RebornCompositionProfile::LocalDevYolo`; - `RuntimeProfileApprovalGatePolicy` stored a `RuntimeProfile` and asked it `allows_minimal_approval_bypass()`. Both now read values classified once by the sanctioned resolver: `ironclaw_runtime_policy::{budget_enforcement, minimal_approval_bypass}` returning `BudgetEnforcement` / `MinimalApprovalBypass`. The classification lives in the one crate the guardrails already name as the only producer of `EffectiveRuntimePolicy`, so no consumer past the composition edge names a mode. The classifications key on `resolved_profile` (post-narrowing), which fixes a real gap: a tenant/org ceiling narrowing `LocalYolo` down to `LocalDev` previously left budgets unenforced and `Minimal` still bypassing gates, because the branch read the *requested* composition profile. Authority reductions now reach both axes. Also removes the `unwrap_or(RuntimeProfile::LocalDev)` fallback in `local_dev_approval_policy`, which invented a deployment profile to derive authority from when no policy was resolved, and defaulted the approval width to `AskDestructive`. The absent-policy path now fails closed to `AskAlways` with the bypass denied (`.claude/rules/error-handling.md`). Deliberately NOT stored as `EffectiveRuntimePolicy` fields: the resolver is the sole production constructor, and the other 99 construction sites are test fixtures, so stored fields would mean a 39-file mechanical diff per axis for a value that is a pure function of a field already serialized into the audit payload and the capability-surface digest. Regression tests (each fails before the change): - `org_ceiling_narrowing_restores_budget_enforcement` and `minimal_approval_bypass_tracks_the_resolved_profile` (resolver tier) - `org_ceiling_narrowing_yolo_away_restores_minimal_approval_gates` (gate-policy tier) - `absent_runtime_policy_fails_closed_to_ask_always_without_minimal_bypass` and `resolved_yolo_policy_allows_minimal_bypass_but_org_ceiling_removes_it` (driven through `local_dev_effects_require_approval`, the production caller) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…o branching on mode Phase 2 of #6274 — the pivot (§4.4/§5.6/§5.11 of the architecture- simplification note). `DeploymentConfig` covered three of seven composition profiles and held only the runtime-policy request. It now covers all seven and carries every axis that code used to obtain by matching a `RebornCompositionProfile`: - `RuntimeSubstrate` — None | Local | ProductionShaped - `TrafficPolicy` — Disabled | ValidateOnly | Serve { required_readiness, veto_on_production_blocking_diagnostic } - `ReadinessContract` — the (state, diagnostics) pair - `StorageShape` — None | LocalDevRoot | HostedSingleTenantPool | OperatorSupplied - event-store profile, hosted-extension-installation-state - the runtime-policy request, now `Option` (disabled and the production-shaped profiles carry an operator-supplied policy on `RebornBuildInput` instead), so `resolve()` returns `Result<Option<..>>` — "no request" stays distinguishable from "a request that failed". `DeploymentConfig::for_profile` is the one place a profile name becomes deployment data. Converted to read it: - `enforce_runtime_cutover_gate` — a seven-arm match, each arm spelling out its own readiness precondition, becomes one `TrafficPolicy` read. The pre-build live-traffic check and the gate now share `TrafficPolicy::live_traffic_refusal` so they cannot drift on wording or on which deployments may start. - `build_reborn_services` and `build_reborn_runtime`'s runtime-parts selection — dispatch on `RuntimeSubstrate`. - `check_production_scheduler_wake_wiring` — reads the substrate. - `readiness_contract_for_profile` — a field read. - The four `profile == HostedSingleTenant` storage-pairing guards in `factory.rs`/`input.rs` — read `StorageShape`. Modeling the pairing as a storage axis is what let these go; they were never really about the mode. - The six `RebornCompositionProfile` predicates delegate to the config, so there is one source of truth rather than seven parallel matches. Result: `runtime.rs` no longer names a profile variant at all outside comments — the new ratchet's shrink check caught that and the allowlist entry is already gone. New `reborn_deployment_mode_branching_ratchet` (§10) freezes the SET of production composition files naming a profile variant; a file entering fails, and a file leaving must be removed from the allowlist in the same PR. Definition of done is `{deployment.rs}`; each remaining entry documents why it is still there and what retires it. Named owner and a scanner self-test per §10. Tests: `every_composition_profile_maps_to_a_deployment_config`, `substrate_and_traffic_axes_replace_the_profile_predicates` (pins the predicates as delegations), `a_serving_deployment_requires_its_own_ readiness_state` (a constructor setting the two independently would make the deployment unstartable — pinned here rather than found at boot), `only_production_vetoes_on_a_production_blocking_diagnostic`, `deployments_without_a_policy_request_resolve_to_none`, `readiness_contract_travels_on_the_config`. The existing cutover-gate cases now drive the gate through a `DeploymentConfig`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6279
July 19, 2026 07:56
Destroyed
github-actions
Bot
added
size: XL
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesDeployment behavior is centralized in Deployment-driven composition
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BuildInput
participant DeploymentConfig
participant RuntimeBuilder
participant CutoverGate
participant RuntimeSubstrate
BuildInput->>DeploymentConfig: provide deployment axes
RuntimeBuilder->>DeploymentConfig: read traffic and substrate
RuntimeBuilder->>CutoverGate: validate live admission and readiness
CutoverGate-->>RuntimeBuilder: permit or reject startup
RuntimeBuilder->>RuntimeSubstrate: select runtime wiring
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the deployment configuration to be data-driven, replacing multiple profile-based branching matches with explicit configuration axes such as RuntimeSubstrate, StorageShape, TrafficPolicy, and ReadinessContract. It also introduces an anti-slippage ratchet test to prevent future branching on the profile enum. The review feedback correctly identifies a bug in the ratchet's comment and string stripping logic, which fails to account for Rust character literals and could lead to silent scan bypasses.
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
+111
to
+149
| let mut in_string = false; | ||
| let mut escaped = false; | ||
| while let Some(ch) = chars.next() { | ||
| if in_line_comment { | ||
| if ch == '\n' { | ||
| in_line_comment = false; | ||
| out.push('\n'); | ||
| } | ||
| continue; | ||
| } | ||
| if in_block_comment { | ||
| if ch == '*' && chars.peek() == Some(&'/') { | ||
| chars.next(); | ||
| in_block_comment = false; | ||
| } | ||
| continue; | ||
| } | ||
| if in_string { | ||
| if escaped { | ||
| escaped = false; | ||
| } else if ch == '\\' { | ||
| escaped = true; | ||
| } else if ch == '"' { | ||
| in_string = false; | ||
| } | ||
| continue; | ||
| } | ||
| match ch { | ||
| '/' if chars.peek() == Some(&'/') => { | ||
| chars.next(); | ||
| in_line_comment = true; | ||
| } | ||
| '/' if chars.peek() == Some(&'*') => { | ||
| chars.next(); | ||
| in_block_comment = true; | ||
| } | ||
| '"' => in_string = true, | ||
| _ => out.push(ch), | ||
| } |
Contributor
There was a problem hiding this comment.
The comment and string stripping logic does not account for Rust character literals (e.g., '"' or '''). If a file contains a character literal with a double quote, the state machine will incorrectly enter the in_string state and skip scanning the rest of the file (or until the next double quote), potentially leading to silent bypasses of the ratchet. Adding support for character literals fixes this issue.
let mut in_string = false;
let mut in_char = false;
let mut escaped = false;
while let Some(ch) = chars.next() {
if in_line_comment {
if ch == '\n' {
in_line_comment = false;
out.push('\n');
}
continue;
}
if in_block_comment {
if ch == '*' && chars.peek() == Some(&'/') {
chars.next();
in_block_comment = false;
}
continue;
}
if in_string {
if escaped {
escaped = false;
} else if ch == '\\' {
escaped = true;
} else if ch == '"' {
in_string = false;
}
continue;
}
if in_char {
if escaped {
escaped = false;
} else if ch == '\\' {
escaped = true;
} else if ch == '\'' {
in_char = false;
}
continue;
}
match ch {
'/' if chars.peek() == Some(&'/') => {
chars.next();
in_line_comment = true;
}
'/' if chars.peek() == Some(&'*') => {
chars.next();
in_block_comment = true;
}
'"' => in_string = true,
'\'' => in_char = true,
_ => out.push(ch),
}
Member
Author
There was a problem hiding this comment.
Confirmed and fixed — the inline lexer ignored char literals, so '"' opened a string and swallowed everything after it (hiding any DeploymentMode branch — a silent false negative). Rewrote it char-indexed (a single-char literal 'x' needs to peek two chars ahead to distinguish it from a lifetime 'a, which the Peekable form couldn't do), mirroring the shared ratchet_support stripper's char-vs-lifetime handling. The self-test now covers '"'-hides-a-following-branch and lifetime preservation.
Member
Author
There was a problem hiding this comment.
This is already handled — the stripper is char-indexed and explicitly distinguishes a char literal from a lifetime before it can enter the string state, so '"' never flips in_string.
See the '\'' arm just below the string arm:
- Escaped char literal
'\...'→ drop through the closing quote. - Single-char literal
'x'(incl.'"') → the quote two chars ahead proves it's a literal, not a lifetime, so all three chars are dropped. - Otherwise it's a lifetime (
'a) → emitted as-is.
Because that arm runs and consumes '"' as a 3-char unit, the interior " is never seen as a string opener.
This exact case is pinned by the self-test scanner_strips_comments_and_strings (added in response to your earlier review — the comment there reads "Regression (2026-07-19 gemini review): a char literal containing \" must not open a string and swallow a following branch"). It asserts:
let quote = '"';followed by aRebornCompositionProfile::LocalDevmatch → the branch after the char literal still survives stripping;- the
'"'char literal itself is dropped; - a real lifetime is preserved:
fn f<'a>(x: &'a str) {}still contains'aafter stripping.
Note the suggested '\'' => in_char = true state machine would regress the lifetime case: &'a str would enter in_char and swallow everything up to the next ', corrupting the scan. The char-vs-lifetime lookahead is deliberate. Declining this one as already-fixed-and-tested.
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
⚠️ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| 0 | 0 | 0 | 3a66d974aa05 |
Head: 3a66d974aa05867150d7ed312ee18429ca72d36b
Next: Human review or validation is required before merging.
Run details
Status: Current
Needs human: no
Needs validation: yes
Summary
Static stack-layer review found no concrete actionable defects in the deployment-axis refactor. Focused test execution could not start because the review environment has no cargo executable.
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.
|
🚅 Deployed to the ironclaw-pr-6279 environment in ironclaw-ci-preview
|
…har literals The inline stripper rolled its own Peekable lexer that ignored char literals: a flipped in_string and swallowed the rest of the file, silently hiding any DeploymentMode branch after it (a ratchet false negative). Rewrote it char-indexed (a char literal needs 2-char lookahead the Peekable form can't do), mirroring the shared ratchet_support stripper's char-vs-lifetime handling. Self-test extended with the -hides-a-branch regression plus lifetime preservation. Reported-by: gemini-code-assist (PR #6279 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6279
July 19, 2026 08:12
Destroyed
This was referenced Jul 19, 2026
Member
Author
|
✅ Ready for merge — DeploymentConfig Phase 2 (§4.4): enforcement axes resolved through the sanctioned classifier, no deployment-mode branching past the composition edge. Gemini's ratchet-lexer char-literal gap fixed (a |
Base automatically changed from
refactor/deployment-config-phase1-policy-values
to
main
July 19, 2026 19:24
# Conflicts: # crates/ironclaw_reborn_composition/src/runtime.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6279
July 19, 2026 19:49
Destroyed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_reborn_composition/src/lib.rs`:
- Line 33: Change the deployment module declaration in the composition crate
root from public to crate-private by using pub(crate) mod deployment, preserving
access for in-crate callers while removing it from the external API surface.
🪄 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: 58a7d3c7-c004-4ff3-b321-80ebca78011f
📒 Files selected for processing (10)
crates/ironclaw_architecture/tests/reborn_deployment_mode_branching_ratchet.rscrates/ironclaw_reborn_composition/src/deployment.rscrates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/input.rscrates/ironclaw_reborn_composition/src/lib.rscrates/ironclaw_reborn_composition/src/local_runtime_profile.rscrates/ironclaw_reborn_composition/src/readiness.rscrates/ironclaw_reborn_composition/src/root/profile.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/tests/core.rs
| mod blocked_auth_resume; | ||
| mod builtin_capability_policy; | ||
| mod deployment; | ||
| pub mod deployment; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Any consumer of the deployment module/types outside ironclaw_reborn_composition?
rg -nP --type=rust 'ironclaw_reborn_composition::deployment|use\s+.*deployment::(DeploymentConfig|RuntimeSubstrate|StorageShape|TrafficPolicy)' \
-g '!crates/ironclaw_reborn_composition/**'Repository: nearai/ironclaw
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## lib.rs\n'
cat -n crates/ironclaw_reborn_composition/src/lib.rs | sed -n '1,120p'
printf '\n## external consumers of deployment symbols\n'
rg -n --type=rust 'ironclaw_reborn_composition::deployment|use\s+.*deployment::(DeploymentConfig|RuntimeSubstrate|StorageShape|TrafficPolicy|ReadinessContract)' \
-g '!crates/ironclaw_reborn_composition/**' || true
printf '\n## internal consumers of deployment symbols\n'
rg -n --type=rust 'crate::deployment::(DeploymentConfig|RuntimeSubstrate|StorageShape|TrafficPolicy|ReadinessContract)|deployment::(DeploymentConfig|RuntimeSubstrate|StorageShape|TrafficPolicy|ReadinessContract)' \
crates/ironclaw_reborn_composition/src || trueRepository: nearai/ironclaw
Length of output: 6036
Keep deployment crate-private
crates/ironclaw_reborn_composition/src/lib.rs:33 exposes a lower-level substrate module that has no downstream consumers outside the crate. The composition root is supposed to expose only facade-shaped handles, so pub(crate) mod deployment; keeps the wiring available to in-crate call sites without widening the public surface.
🤖 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/lib.rs` at line 33, Change the
deployment module declaration in the composition crate root from public to
crate-private by using pub(crate) mod deployment, preserving access for in-crate
callers while removing it from the external API surface.
Source: Coding guidelines
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.63% — 313642 / 366296 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)
|
Member
Author
|
✅ Ready for merge — DeploymentConfig owns every deployment axis; no branching on mode. CI green 56/56, MERGEABLE. Rebased onto main after #6277 merged (GitHub auto-retargeted this PR's base main→main and it went CONFLICTING). Resolution:
Verification: clippy clean under the CI-representative Review comment: Gemini re-raised the char-literal stripping finding on the ratchet lexer. Declined with evidence — the Note: #6280 is stacked on this branch — after this lands it retargets to main. |
…d mechanism they are (#6280) * refactor(composition): de-prefix local_dev builder names to the shared mechanism they are Phase 3 of #6274 (§4.4.1 category 2: "mis-prefixed shared substrate — not local at all; de-prefix, don't configify"). These builders are not local-dev-specific. Both `build_local_dev_store_graph` variants are already called by the hosted single-tenant volume deployment as well as local-dev, and the rest are the ordinary shared substrate every deployment assembles. The `local_dev` prefix claimed a deployment mode that the code does not have. Renames (mechanical; no logic change): build_local_dev_store_graph -> build_local_runtime_store_graph build_local_dev_root_filesystem -> build_local_runtime_root_filesystem build_local_dev_secret_store -> build_secret_store build_local_dev_secret_store_for_test -> build_secret_store_for_test build_local_dev_approval_interaction_service* -> build_approval_interaction_service* build_local_dev_extension_management_for_test -> build_extension_management_for_test build_local_dev_skill_context_source_for_test -> build_skill_context_source_for_test type LocalDevWorkspaceFilesystems -> WorkspaceFilesystems The two `build_local_runtime_*` names keep a qualifier because they genuinely select the local runtime substrate (`RuntimeSubstrate::Local`) rather than the production-shaped one — that is a substrate distinction, not a deployment mode. `reborn_localdev_typename_ratchet`'s allowlist is already empty, so no ratchet entry retires here; the private `LocalDevWorkspaceFilesystems` alias was below its pub-visibility scan and is cleaned up for consistency. Call sites updated across the composition crate, its tests, the integration harness, `tests/integration/secrets.rs`, and `tests/integration/CLAUDE.md`. No new names collide with existing symbols (verified against HEAD). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(composition): RebornBuildInput carries the DeploymentConfig (#6282) Phase 4 of #6274. Completes the pivot: the build input carries the deployment as data instead of a profile name that consumers re-derive one from. `RebornBuildInput.profile: RebornCompositionProfile` becomes `deployment: DeploymentConfig`. `profile()` stays as a delegating accessor so no external caller changes; `deployment()` is the new read path. This removes the re-derivation Phase 2 had to sprinkle at each consumer: `build_reborn_services`, `build_reborn_runtime`, and the storage-shape guards called `DeploymentConfig::for_profile(profile, ..)` at the point of use. The config is now built once and carried. **The hazard that shaped the design.** A config built inside `RebornBuildInput::new` cannot know the operator's yolo host-access disclosure, so its policy request would carry `yolo_disclosure_acknowledged: false` and resolve fail-closed. Rather than paper over that, `new` takes an already-built `DeploymentConfig`, and `local_runtime_build_input_with_options` — the one place holding the operator's confirmation — builds it and hands it in. Pinned by `yolo_disclosure_reaches_both_the_carried_deployment_and_the_resolved_policy`, which resolves the *carried* config and asserts it reaches `RuntimeProfile::LocalYolo` / `ApprovalPolicy::Minimal`. `input.rs` now names zero profile variants, so its `reborn_deployment_mode_branching_ratchet` entry retires — the allowlist is down to four: deployment.rs (the terminal target state), factory.rs, local_runtime_profile.rs, readiness.rs. Also: - `local_dev_from_deployment` replaces the profile-name path, and its `debug_assert` is on the storage-shape axis rather than a list of profile names. - `with_deployment` is `#[cfg(test)]`: production builds the deployment at construction, and this only exists so a test can construct the deliberately mismatched deployment/storage pairing that drives the fail-closed guard in `build_reborn_services`. Tests: the two above plus `deployments_without_the_local_dev_storage_shape_are_rejected` (the helper's rejection is the storage-shape axis, not a profile list). NOT included, and not blocked on this: the single `build_runtime(cfg)` of §5.11, which merges the local and production store graphs into one backend-parameterized graph. That depends on Slice A store consolidation, which §9 gates behind Slice 0's reference-model property suite — infrastructure this repo does not have yet. Landing it before that oracle exists is exactly what §9 forbids. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6279
July 19, 2026 22:29
Destroyed
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 (2)
crates/ironclaw_reborn_composition/src/factory.rs (1)
1488-1491: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded profile names in errors contradict the deployment-axis ratchet.
The composition architecture dictates that behavior dispatches on deployment axes (like
storage_shape), not profile names. Hardcodingprofile=hosted-single-tenantin these errors will lie to the operator if another profile adopts this shape.
crates/ironclaw_reborn_composition/src/factory.rs#L1488-L1491: use the already-extracted{profile}binding (e.g.,format!("profile={profile} requires...")) instead of.to_string().crates/ironclaw_reborn_composition/src/input.rs#L377-L380: change the message to describe the required axis (e.g.,requires a deployment with the HostedSingleTenantPool storage shape; got profile={profile}).crates/ironclaw_reborn_composition/src/input.rs#L411-L414: apply the same axis-based description as above.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/factory.rs` around lines 1488 - 1491, The validation errors hardcode a profile name instead of describing the deployment axis. In crates/ironclaw_reborn_composition/src/factory.rs:1488-1491, update the error construction to interpolate the existing profile binding. In crates/ironclaw_reborn_composition/src/input.rs:377-380 and 411-414, revise both messages to require the HostedSingleTenantPool storage shape and report the actual profile, preserving the existing validation behavior.crates/ironclaw_reborn_composition/src/input.rs (1)
373-384: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract deployment resolution to avoid duplicate construction.
DeploymentConfig::for_profileis evaluated twice in these constructors. Bind it to a local variable to DRY up the configuration logic.
crates/ironclaw_reborn_composition/src/input.rs#L373-L384: bindlet deployment = DeploymentConfig::for_profile(profile, false);, use it in thestorage_shape()check, and pass it toSelf::new.crates/ironclaw_reborn_composition/src/input.rs#L407-L424: apply the exact same extraction for the config-and-env constructor.🤖 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/input.rs` around lines 373 - 384, In the constructor at crates/ironclaw_reborn_composition/src/input.rs lines 373-384, bind DeploymentConfig::for_profile(profile, false) to a local deployment variable, use it for the storage_shape() validation, and pass it to Self::new. Apply the same extraction in the config-and-env constructor at crates/ironclaw_reborn_composition/src/input.rs lines 407-424, preserving existing validation and construction behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/factory.rs`:
- Around line 1488-1491: The validation errors hardcode a profile name instead
of describing the deployment axis. In
crates/ironclaw_reborn_composition/src/factory.rs:1488-1491, update the error
construction to interpolate the existing profile binding. In
crates/ironclaw_reborn_composition/src/input.rs:377-380 and 411-414, revise both
messages to require the HostedSingleTenantPool storage shape and report the
actual profile, preserving the existing validation behavior.
In `@crates/ironclaw_reborn_composition/src/input.rs`:
- Around line 373-384: In the constructor at
crates/ironclaw_reborn_composition/src/input.rs lines 373-384, bind
DeploymentConfig::for_profile(profile, false) to a local deployment variable,
use it for the storage_shape() validation, and pass it to Self::new. Apply the
same extraction in the config-and-env constructor at
crates/ironclaw_reborn_composition/src/input.rs lines 407-424, preserving
existing validation and construction behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c2998bd5-ddfc-4024-b7e1-3c0349089439
📒 Files selected for processing (14)
crates/ironclaw_architecture/tests/reborn_deployment_mode_branching_ratchet.rscrates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/input.rscrates/ironclaw_reborn_composition/src/local_runtime_profile.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/test_support.rscrates/ironclaw_reborn_composition/src/test_support/local_dev_boot.rscrates/ironclaw_reborn_composition/src/test_support/mod.rscrates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rscrates/ironclaw_reborn_composition/src/test_support/skill_activation.rscrates/ironclaw_reborn_composition/tests/facade_factory.rstests/integration/CLAUDE.mdtests/integration/secrets.rstests/integration/support/harness/mod.rs
💤 Files with no reviewable changes (1)
- crates/ironclaw_architecture/tests/reborn_deployment_mode_branching_ratchet.rs
ilblackdragon
added a commit
that referenced
this pull request
Jul 20, 2026
Reconcile the capability-result collapse stack (host_api::Resolution as the single loop-facing capability result; CapabilityOutcome deleted; GateRecordStore + ReplayPayloadStore host-private stores) with 16 advancing main commits — DeploymentConfig owns every deployment axis (#6279), enforcement axes become resolved policy values (#6277), RebornServicesApi facade method-set freeze (#6292), hermetic NEARAI env tests (#6272), checkpoint stores over production impls (#6260), and the turn-state row store work (#6263). Single conflict: tests/integration/support/harness/mod.rs — both sides added fields to the same refresh-input struct literal. Resolved by unioning them: main's `trajectory_observer: None` + `extension_management` block AND the collapse's `gate_record_store` + `replay_payload_store`. Verified: `cargo build --workspace --all-features [--tests]` clean (no API drift), all 12 `ironclaw_architecture` ratchet binaries pass (collapse ratchet coexists with main's new deployment-mode-branching and facade-method-freeze ratchets). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ilblackdragon
added a commit
that referenced
this pull request
Jul 20, 2026
…uest-side) collapse plan (#6306) §14 status log was stale — it listed the §5.3 five-channel flip as "in flight on integration/reborn-flip-base" and said "CapabilityOutcome is retained for Stage 2b to delete", but that work has landed on main: - Move the §5.3 flip stack from "In flight" to "Merged"; record #6293 (Stage 2b — CapabilityOutcome + all result mirrors DELETED), #6299 (the stack squash-landed on main, reconciled with #6279/#6277/#6292/#6296), and #6303 (auth-gate setup fingerprint fix + injective encoding). - Fix the Slice C.1 bullet: the Resolution/Blocked/Suspension/HostFailure channel enums are now merged too. - Add the remaining work under "Not started": the Slice C down-path (request-side) collapse — the 9 request mirrors still frozen in FROZEN_COLLAPSE_DTOS — with the concrete risk-ordered slice sequence (D1 dispatch→Authorized, D2 authorize(&Invocation), D3 loop membrane mints Invocation, D4 resume/auth-resume, D5 security-milestone seal inline, D6 ratchet-to-empty + measure). Docs-only; the frozen contract (§1–§13) is unchanged, only the mutable §14 log. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 31, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 of #6274 — the pivot (§4.4 / §5.6 / §5.11).
Stacked on #6277 (Phase 1). Review that first; this branch contains it.
What changed
DeploymentConfigcovered three of seven composition profiles and held only the runtime-policy request. It now covers all seven and carries every axis code used to obtain by matching aRebornCompositionProfile:RuntimeSubstrateNone/Local/ProductionShapedTrafficPolicyDisabled/ValidateOnly/Serve { required_readiness, veto_on_production_blocking_diagnostic }ReadinessContract(state, diagnostics)pairStorageShapeNone/LocalDevRoot/HostedSingleTenantPool/OperatorSuppliedThe policy request is now
Option— disabled and the production-shaped profiles carry an operator-supplied policy onRebornBuildInputinstead — soresolve()returnsResult<Option<..>>. "No request" stays distinguishable from "a request that failed" rather than collapsing both into an absent policy.DeploymentConfig::for_profileis the one place a profile name becomes deployment data.Sites converted
enforce_runtime_cutover_gateTrafficPolicyreadbuild_reborn_servicesRuntimeSubstratebuild_reborn_runtimeruntime-partsRuntimeSubstratecheck_production_scheduler_wake_wiringmatches!(profile, Production | MigrationDryRun)readiness_contract_for_profileprofile == HostedSingleTenantstorage guardsStorageShapereadThe pre-build live-traffic check and the cutover gate now share
TrafficPolicy::live_traffic_refusal, so they cannot drift on wording or on which deployments may start — previously the same two refusals were spelled out in both places.Modeling the storage pairing as a
StorageShapeaxis is what let the four== HostedSingleTenantguards go. They were never really about the mode: they check that the operator paired the right storage handle with the deployment.Result:
runtime.rsno longer names a profile variant at all outside comments. The new ratchet's shrink check caught that, and its allowlist entry is already removed.New ratchet
reborn_deployment_mode_branching_ratchet(§10) freezes the set of production composition files naming a profile variant — set membership, not a count, so a swap is caught. A file entering fails; a file leaving must be removed from the allowlist in the same PR, so the debt only shrinks.Definition of done is
{deployment.rs}. Each remaining entry documents why it is still there and what retires it — most retire whenRebornBuildInputcarries aDeploymentConfiginstead of a profile. Named owner and a scanner self-test, per §10's requirements.This is deliberately coarser than "detect a
match": a line-based scan cannot reliably distinguishmatch p { P::X => }fromif p == P::Xfrommatches!(p, P::X | ..), and all three are the same debt.Scope note
RebornBuildInputstill carries aRebornCompositionProfilerather than aDeploymentConfig. That swap changes a public API consumed by the CLI, the integration harness, and the QA suites, and it is cleanly separable now that nothing branches on the profile — the architectural goal ("no branching on mode past the composition edge") is met by this PR. Tracked as the remaininginput.rs/local_runtime_profile.rsallowlist entries.Testing
New tests in
deployment.rs:every_composition_profile_maps_to_a_deployment_config—for_profilecovers every variantsubstrate_and_traffic_axes_replace_the_profile_predicates— pins the six predicates as thin delegations, so they cannot drift from the configa_serving_deployment_requires_its_own_readiness_state— a constructor settingrequired_readinessand the reported state independently would make the deployment permanently unstartable; pinned here rather than discovered at bootonly_production_vetoes_on_a_production_blocking_diagnostic— no other deployment inherits the vetodeployments_without_a_policy_request_resolve_to_nonereadiness_contract_travels_on_the_configThe existing cutover-gate cases now drive the gate through a real
DeploymentConfig.Same three pre-existing
llm_admin::nearai_mcp::tests::bootstrap_config_from_env_*failures as #6277 — environmental (NEARAI_API_KEYset in my shell), unrelated.One
arch-exempt: large_fileadded to the pre-existing ~6.9K-lineruntime/tests/core.rs, citing #6168; the change there only repoints existing cutover-gate cases plus one shared helper.🤖 Generated with Claude Code