Extract operator and projection ownership - #6615
Merged
Merged
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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-6615
July 24, 2026 06:28
Destroyed
github-actions
Bot
added
scope: dependencies
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request introduces ChangesOperator and product ownership
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
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 |
Member
Author
|
@IronLoop requested for review. GitHub would not accept |
![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 |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | cfc70df72dc1 |
Head: cfc70df72dc1fd20b27563b81a127c08824e1062
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
The ownership move drops stream-level regression coverage for the auth-prompt projection path.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Restore auth projection stream tests
Location: crates/ironclaw_product/src/projection/turn_events.rs:414
The refactor deletes the seven turn_stream_auth tests without adding product-side replacements. The remaining prompt contract test invokes the helper directly, so it does not exercise the ProductRuntimeProjectionStream path that reaches this BlockedAuth branch. Rehome equivalent stream-level coverage for auth challenge scoping, manual/pairing/OAuth fallbacks, and lookup failures; otherwise regressions in the moved projection wiring can pass the product suite.
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.
| let gate_ref_str = gate_ref.as_str().to_string(); | ||
| match event.status { | ||
| TurnStatus::BlockedAuth => { | ||
| let view = auth_prompt_view_for_blocked_auth(BlockedAuthPromptRequest { |
Contributor
There was a problem hiding this comment.
The previous turn_stream_auth suite covered this stream-level BlockedAuth projection path, but it was deleted rather than moved. Please restore equivalent coverage in ironclaw_product; direct helper tests do not cover the stream wiring, auth scoping, fallback prompt variants, or lookup-error behavior.
|
🚅 Deployed to the ironclaw-pr-6615 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6615
July 24, 2026 06:42
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: 2
🤖 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_operator/src/llm_admin/nearai_login_serve.rs`:
- Around line 33-42: Update the comments in the constant initializers
NEARAI_CALLBACK_RATE_WINDOW_SECONDS and NEARAI_CALLBACK_RATE_MAX to remove the
SAFETY label, using invariant wording that reflects the non-zero literal
instead.
In `@crates/ironclaw_operator/src/llm_admin/provider_admin.rs`:
- Around line 539-545: Add an inline `// silent-ok: <reason>` annotation to the
intentional `Err(error)` fallback in the provider settings resolution flow
around `resolve_provider_config_from_env`, explicitly explaining why returning
`None` after debug logging is acceptable; otherwise propagate the error
consistently with `detect_env_llm` and `resolve_env_api_key`.
🪄 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: b71491fc-dd33-457e-ab80-319089fbb53f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (66)
Cargo.tomlcrates/ironclaw_architecture/tests/reborn_dependency_boundaries.rscrates/ironclaw_architecture/tests/reborn_extension_specificity.rscrates/ironclaw_host_api/src/lib.rscrates/ironclaw_host_api/src/operator_llm.rscrates/ironclaw_operator/Cargo.tomlcrates/ironclaw_operator/src/lib.rscrates/ironclaw_operator/src/llm_admin/active_model.rscrates/ironclaw_operator/src/llm_admin/llm_catalog.rscrates/ironclaw_operator/src/llm_admin/llm_config_service.rscrates/ironclaw_operator/src/llm_admin/llm_key_store.rscrates/ironclaw_operator/src/llm_admin/llm_reload.rscrates/ironclaw_operator/src/llm_admin/mod.rscrates/ironclaw_operator/src/llm_admin/nearai_login_serve.rscrates/ironclaw_operator/src/llm_admin/nearai_mcp.rscrates/ironclaw_operator/src/llm_admin/provider_admin.rscrates/ironclaw_operator/src/llm_admin/provider_admin_product_command.rscrates/ironclaw_operator/src/llm_admin/provider_repo.rscrates/ironclaw_operator/src/llm_admin/resolved_llm.rscrates/ironclaw_operator/src/operator_logs.rscrates/ironclaw_operator/src/operator_service_lifecycle.rscrates/ironclaw_operator/src/route_mounts.rscrates/ironclaw_product/Cargo.tomlcrates/ironclaw_product/src/lib.rscrates/ironclaw_product/src/projection.rscrates/ironclaw_product/src/projection/display_preview.rscrates/ironclaw_product/src/projection/live_progress.rscrates/ironclaw_product/src/projection/runtime_replay.rscrates/ironclaw_product/src/projection/tests.rscrates/ironclaw_product/src/projection/tests/cursor_validation.rscrates/ironclaw_product/src/projection/tests/display_preview.rscrates/ironclaw_product/src/projection/tests/display_preview_runtime.rscrates/ironclaw_product/src/projection/tests/failure_explanation.rscrates/ironclaw_product/src/projection/tests/live_progress_stream.rscrates/ironclaw_product/src/projection/tests/runtime_stream.rscrates/ironclaw_product/src/projection/tests/turn_stream.rscrates/ironclaw_product/src/projection/turn_events.rscrates/ironclaw_reborn_cli/Cargo.tomlcrates/ironclaw_reborn_cli/src/commands/config/set.rscrates/ironclaw_reborn_cli/src/commands/models.rscrates/ironclaw_reborn_cli/src/commands/onboard/llm_credentials.rscrates/ironclaw_reborn_cli/src/commands/serve.rscrates/ironclaw_reborn_cli/src/runtime/mod.rscrates/ironclaw_reborn_cli/tests/smoke.rscrates/ironclaw_reborn_composition/Cargo.tomlcrates/ironclaw_reborn_composition/src/llm_admin/active_model.rscrates/ironclaw_reborn_composition/src/llm_admin/llm_catalog.rscrates/ironclaw_reborn_composition/src/llm_admin/llm_config_service.rscrates/ironclaw_reborn_composition/src/llm_admin/llm_key_store.rscrates/ironclaw_reborn_composition/src/llm_admin/llm_reload.rscrates/ironclaw_reborn_composition/src/llm_admin/nearai_login_serve.rscrates/ironclaw_reborn_composition/src/llm_admin/nearai_mcp.rscrates/ironclaw_reborn_composition/src/llm_admin/provider_admin.rscrates/ironclaw_reborn_composition/src/llm_admin/provider_admin_product_command.rscrates/ironclaw_reborn_composition/src/llm_admin/provider_repo.rscrates/ironclaw_reborn_composition/src/observability/operator_logs.rscrates/ironclaw_reborn_composition/src/observability/operator_service_lifecycle.rscrates/ironclaw_reborn_composition/src/projection.rscrates/ironclaw_reborn_composition/src/projection/display_preview.rscrates/ironclaw_reborn_composition/src/projection/live_progress.rscrates/ironclaw_reborn_composition/src/projection/runtime_replay.rscrates/ironclaw_reborn_composition/src/projection/tests/turn_stream_auth.rscrates/ironclaw_reborn_composition/src/projection/turn_events.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/tests/core.rscrates/ironclaw_reborn_composition/src/runtime_input.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6615
July 24, 2026 06:53
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 2
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_cli/src/commands/models.rs (1)
69-86: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftKeep model commands behind the composition provider-admin facade.
These commands now instantiate
ironclaw_operator::RebornProviderAdmindirectly, creating a second CLI-specific path that can bypass composition-owned profile, policy, and compatibility checks. Route model UX through the Reborn composition provider-admin facade and retain architecture coverage preventing direct operator construction in CLI code.As per path instructions, provider registry, authentication, and model UX must enter through the Reborn composition provider-admin facade.
Also applies to: 90-101, 105-111, 114-121
🤖 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_cli/src/commands/models.rs` around lines 69 - 86, Update the model command methods, including execute and the additional provider/model handlers, to obtain and invoke the Reborn composition provider-admin facade instead of constructing ironclaw_operator::RebornProviderAdmin directly. Preserve the existing listing, detail, JSON, and verbosity behavior while routing all provider registry, authentication, and model UX through the composition-owned facade, and retain or add architecture coverage preventing direct operator construction in CLI code.Source: Path instructions
crates/ironclaw_reborn_composition/src/llm_admin/nearai_mcp.rs (1)
11-20: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftRemove the internal operator re-export shim.
pub(crate) use ironclaw_operator::...preserves the old composition module as a path-preservation layer. Import these symbols directly fromironclaw_operatorat their consumers and keep this module limited to composition-owned adapters.As per path instructions, extracted-crate types must be imported directly and re-exports are reserved for downstream API facades.
🤖 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/llm_admin/nearai_mcp.rs` around lines 11 - 20, Remove the pub(crate) operator re-export block from the nearai_mcp composition module, including NearAiMcpBootstrapOutcome, NearAiMcpEndpoint, durable_product_auth_storage_enabled, and the nearai_mcp helper functions. Update each consumer to import these symbols directly from ironclaw_operator, leaving this module only with composition-owned adapters and its direct NearAiMcpBootstrapConfig import.Source: 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_cli/src/commands/models.rs`:
- Line 124: Update the CLI’s provider DTO references in print_provider_list and
the corresponding code at the indicated additional locations to import
RebornProviderList, RebornProviderStatus, and RebornProviderWriteOutcome from
ironclaw_host_api::operator_llm. Retain ironclaw_operator only for
administration implementation APIs, removing any reliance on its DTO re-exports.
In `@crates/ironclaw_reborn_composition/src/observability/mod.rs`:
- Line 8: Remove the OperatorServiceLifecycle re-export from observability and
update all callers to import it directly from ironclaw_operator. In
available_extensions.rs, import the complete NEAR AI MCP endpoint/helper set
from ironclaw_operator, or rename any intentional composition adapter to a
distinct contract name; do not preserve the old composition paths.
---
Outside diff comments:
In `@crates/ironclaw_reborn_cli/src/commands/models.rs`:
- Around line 69-86: Update the model command methods, including execute and the
additional provider/model handlers, to obtain and invoke the Reborn composition
provider-admin facade instead of constructing
ironclaw_operator::RebornProviderAdmin directly. Preserve the existing listing,
detail, JSON, and verbosity behavior while routing all provider registry,
authentication, and model UX through the composition-owned facade, and retain or
add architecture coverage preventing direct operator construction in CLI code.
In `@crates/ironclaw_reborn_composition/src/llm_admin/nearai_mcp.rs`:
- Around line 11-20: Remove the pub(crate) operator re-export block from the
nearai_mcp composition module, including NearAiMcpBootstrapOutcome,
NearAiMcpEndpoint, durable_product_auth_storage_enabled, and the nearai_mcp
helper functions. Update each consumer to import these symbols directly from
ironclaw_operator, leaving this module only with composition-owned adapters and
its direct NearAiMcpBootstrapConfig import.
🪄 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: ad49718a-3aa3-4dd9-8227-0d7d36d45af4
📒 Files selected for processing (32)
crates/ironclaw_architecture/tests/reborn_extension_specificity.rscrates/ironclaw_reborn_cli/src/commands/config/init.rscrates/ironclaw_reborn_cli/src/commands/doctor.rscrates/ironclaw_reborn_cli/src/commands/models.rscrates/ironclaw_reborn_cli/src/commands/onboard/llm_credentials.rscrates/ironclaw_reborn_cli/src/commands/onboard/prompts.rscrates/ironclaw_reborn_cli/src/commands/service/launchd.rscrates/ironclaw_reborn_cli/src/commands/service/mod.rscrates/ironclaw_reborn_cli/src/runtime/mod.rscrates/ironclaw_reborn_composition/src/deployment.rscrates/ironclaw_reborn_composition/src/extension_host/available_extensions.rscrates/ironclaw_reborn_composition/src/extension_host/run_delivery_ports.rscrates/ironclaw_reborn_composition/src/extension_host/skill_learning.rscrates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/factory/tests.rscrates/ironclaw_reborn_composition/src/google_oauth_secret_store.rscrates/ironclaw_reborn_composition/src/input.rscrates/ironclaw_reborn_composition/src/lib.rscrates/ironclaw_reborn_composition/src/llm_admin/mod.rscrates/ironclaw_reborn_composition/src/llm_admin/nearai_login_serve.rscrates/ironclaw_reborn_composition/src/llm_admin/nearai_mcp.rscrates/ironclaw_reborn_composition/src/observability/mod.rscrates/ironclaw_reborn_composition/src/root/product_live_adapters.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rscrates/ironclaw_reborn_composition/src/runtime/tests/core.rscrates/ironclaw_reborn_composition/src/test_support/projection.rscrates/ironclaw_reborn_composition/src/webui/facade.rscrates/ironclaw_reborn_composition/tests/provider_admin.rscrates/ironclaw_reborn_composition/tests/provider_admin_probe.rscrates/ironclaw_reborn_composition/tests/provider_admin_product_command.rsdocs/plans/composition-pubuse.snapshot
💤 Files with no reviewable changes (4)
- crates/ironclaw_reborn_composition/src/llm_admin/mod.rs
- crates/ironclaw_architecture/tests/reborn_extension_specificity.rs
- docs/plans/composition-pubuse.snapshot
- crates/ironclaw_reborn_composition/src/lib.rs
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/extension_host/extension_lifecycle_capabilities.rs`:
- Around line 308-318: Remove the WebGeneratedCode exception that preserves
extension.summary.channel_connection in the model-visible projection, keeping
the redaction contract enforced by the existing model-facing path. If product/UI
consumers need this data, preserve it through a separate product projection
rather than the redacted copy, and add an ExtensionSearch regression test
confirming WebGeneratedCode connection instructions, placeholders, labels, and
errors are absent from model-visible results.
🪄 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: a7009524-ed53-4a57-b2f4-195860f6aaa0
📒 Files selected for processing (1)
crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 86.23% — 314395 / 364600 lines Per-crate breakdown (61 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
|
@ironloopai review --agent reviewer |
Contributor
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | 5614decdd77a |
Head: 5614decdd77a510ab55696fa147922ce938ee919
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
The ownership refactor drops structural tenant/user isolation for the projection EventStreamManager’s outbound state store.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [HIGH] Restore scope-derived outbound state mounts
Location: crates/ironclaw_product/src/projection.rs:256
This resolver ignores ResourceScope and maps every caller to /engine/outbound. Before the move, build_reborn_projection_services used composition's wrap_scoped, which rewrites /outbound under /tenants/<tenant>/users/<user>. The OutboundStateStore backing this shared EventStreamManager relies on that mount boundary for structural isolation, including list/query operations. Restore a tenant/user-derived mount view here (or inject the scoped filesystem from composition) so one user's projection/delivery state cannot share another user's subtree.
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.
| Arc::new(ScopedFilesystem::new(root, outbound_mount_view)) | ||
| } | ||
|
|
||
| fn outbound_mount_view(_scope: &ResourceScope) -> Result<MountView, HostApiError> { |
Contributor
There was a problem hiding this comment.
This resolver ignores ResourceScope and maps every caller to /engine/outbound. The previous composition wrap_scoped rewrote this alias under /tenants/<tenant>/users/<user>, which OutboundStateStore relies on for structural isolation. Please restore scope-derived mounting (or inject it) rather than sharing this subtree.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6615
July 24, 2026 07:57
Destroyed
Member
Author
|
@ironloopai review --agent reviewer |
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_product/src/projection/tests.rs`:
- Around line 79-98: Extend the isolation test around scope_a and scope_b to
validate each scope dimension independently: add a same-tenant/different-user
scope and a different-tenant/same-user scope, then assert both cannot read the
data written through scope_a. Keep the existing both-dimensions-different
assertion and use the same shared outbound path and resource setup.
🪄 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: 7f9fd6cd-92f9-44ff-9cac-c3a7d1fa3420
📒 Files selected for processing (2)
crates/ironclaw_product/src/projection.rscrates/ironclaw_product/src/projection/tests.rs
Contributor
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | d412db62a107 |
Head: d412db62a10744a68d95a549e57f16c31ce5071b
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
The extraction is largely mechanical, but the generated-code exception exposes untrusted extension-manifest text in model-visible lifecycle output.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [HIGH] Keep imported channel copy out of model-visible search output
Location: crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs:314
WebGeneratedCode now retains the entire manifest-derived channel_connection in the value serialized for model-visible extension_search. The catalog accepts imported InstalledLocal bundles, while these fields (including instructions, labels, and error text) are only validated as non-empty. A third-party bundle can therefore inject arbitrary directives into model context. Strip this connection data as before, or replace it with host-authored/enveloped guidance, and add a regression test using hostile imported-manifest text.
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.
| .as_ref() | ||
| .is_some_and(|connection| { | ||
| connection.strategy | ||
| == ironclaw_product::RebornChannelConnectStrategy::WebGeneratedCode |
Contributor
There was a problem hiding this comment.
Leaving WebGeneratedCode summaries here makes model-visible extension_search include manifest-provided instructions, labels, and error text. Imported third-party bundles can control these non-empty-only-validated fields, so this creates a prompt-injection path. Keep channel_connection out of the model output (or use host-authored/enveloped guidance) and add a regression test.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6615
July 24, 2026 08:13
Destroyed
…r-and-projections # Conflicts: # crates/ironclaw_product/src/projection/tests/nested_dispatch_stream.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6615
July 24, 2026 15:34
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6615
July 24, 2026 16:12
Destroyed
…r-and-projections
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6615
July 24, 2026 16:28
Destroyed
…r-and-projections # Conflicts: # Cargo.toml
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6615
July 24, 2026 16:47
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 (4)
crates/ironclaw_reborn_composition/src/factory.rs (1)
2598-2606: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse a stable namespace for local-dev mem0 memory.
The local-dev
app_idis the mem0 partition key, butDefaultHasheris not stable across Rust releases/platforms. A toolchain/platform upgrade using the same storage root can scope memories to a different namespace and make existing local-dev memory appear missing. Store a workspace ID or use a stable, versioned digest with migration/compatibility handling.Rule cited: persisted state must remain reconstructible after interruption (
.claude/rules/database.md:67).🤖 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 2598 - 2606, Replace the DefaultHasher-based app_id generation in the memory_service_resolver block with a stable, versioned namespace derived from the local-dev storage root, or reuse a persisted workspace ID. Add compatibility handling or migration for namespaces previously generated by the old hash so existing mem0 memories remain accessible across Rust toolchain and platform changes.Source: Coding guidelines
crates/ironclaw_reborn_cli/Cargo.toml (1)
26-33: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the forwarding-only CLI feature.
memory-mem0incrates/ironclaw_reborn_cli/Cargo.toml:32is onlyironclaw_reborn_composition/memory-mem0, which violates the workspace Cargo-feature rule: forwarding features belong on the dependency declaration, and this alias has no direct CLI consumers. Selectironclaw_reborn_composition/memory-mem0from the build invocation instead, or make this a CLI-owned build shape with another consumer.🤖 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_cli/Cargo.toml` around lines 26 - 33, Remove the forwarding-only memory-mem0 feature from the CLI crate’s [features] section. Update the build invocation or dependency configuration to select ironclaw_reborn_composition/memory-mem0 directly, unless the CLI gains a direct consumer that justifies retaining a CLI-owned feature.Source: Coding guidelines
crates/ironclaw_reborn_composition/src/lib.rs (1)
173-177: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep concrete memory factories out of the composition facade.
Mem0ConnectionConfig,MemoryProviderDeps, and provider-construction functions expose filesystem/transport wiring rather than facade-shaped composition handles.
crates/ironclaw_reborn_composition/src/lib.rs#L173-L177: make these crate-private or move the public factory contract to its owning memory/configuration crate.docs/plans/composition-pubuse.snapshot#L84-L88: remove the corresponding public snapshot entries after narrowing the facade.As per coding guidelines, “Own only top-level Reborn composition … [and] expose only facade-shaped handles.”
🤖 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` around lines 173 - 177, Narrow the composition facade exports in memory_provider_factory: make Mem0ConnectionConfig, MemoryProviderDeps, build_memory_service_resolver, and create_document_store_provider crate-private or relocate their public contract to the owning memory/configuration crate. Remove the corresponding entries from docs/plans/composition-pubuse.snapshot at lines 84-88; no other facade-shaped exports should change.Source: Coding guidelines
crates/ironclaw_reborn_composition/src/runtime.rs (1)
3971-3979: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPass the native-memory audit sink to
resolve_document_store.
NativeMemoryService::from_filesystempasses the sink intoRepositoryMemoryBackend;threads/<turn_run_id>.mdwrites use that backend path and emitchecked/warned/bypasssafety events. WithNone, native-after-turn writes on protected paths can fail to emit required events. Use the composition-local audit sink during resolver construction and keep the sameNonebehavior only where the production tool path is already covered.🤖 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 3971 - 3979, Update the local runtime document-store resolution around memory_service_resolver.resolve_document_store to pass the composition-local native-memory audit sink instead of None. Preserve the existing filesystem argument and ensure the sink used by NativeMemoryService::from_filesystem and RepositoryMemoryBackend reaches this resolver, retaining None only for the already-covered production tool path.Source: Coding guidelines
🤖 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_cli/Cargo.toml`:
- Around line 26-33: Remove the forwarding-only memory-mem0 feature from the CLI
crate’s [features] section. Update the build invocation or dependency
configuration to select ironclaw_reborn_composition/memory-mem0 directly, unless
the CLI gains a direct consumer that justifies retaining a CLI-owned feature.
In `@crates/ironclaw_reborn_composition/src/factory.rs`:
- Around line 2598-2606: Replace the DefaultHasher-based app_id generation in
the memory_service_resolver block with a stable, versioned namespace derived
from the local-dev storage root, or reuse a persisted workspace ID. Add
compatibility handling or migration for namespaces previously generated by the
old hash so existing mem0 memories remain accessible across Rust toolchain and
platform changes.
In `@crates/ironclaw_reborn_composition/src/lib.rs`:
- Around line 173-177: Narrow the composition facade exports in
memory_provider_factory: make Mem0ConnectionConfig, MemoryProviderDeps,
build_memory_service_resolver, and create_document_store_provider crate-private
or relocate their public contract to the owning memory/configuration crate.
Remove the corresponding entries from docs/plans/composition-pubuse.snapshot at
lines 84-88; no other facade-shaped exports should change.
In `@crates/ironclaw_reborn_composition/src/runtime.rs`:
- Around line 3971-3979: Update the local runtime document-store resolution
around memory_service_resolver.resolve_document_store to pass the
composition-local native-memory audit sink instead of None. Preserve the
existing filesystem argument and ensure the sink used by
NativeMemoryService::from_filesystem and RepositoryMemoryBackend reaches this
resolver, retaining None only for the already-covered production tool path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7f82a905-c9d0-4442-a143-8681966261f6
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (15)
Cargo.tomlcrates/ironclaw_architecture/tests/reborn_dependency_boundaries.rscrates/ironclaw_host_api/src/lib.rscrates/ironclaw_product/src/projection/display_preview.rscrates/ironclaw_product/src/projection/tests/display_preview.rscrates/ironclaw_reborn_cli/Cargo.tomlcrates/ironclaw_reborn_cli/src/runtime/mod.rscrates/ironclaw_reborn_composition/Cargo.tomlcrates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rscrates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/input.rscrates/ironclaw_reborn_composition/src/lib.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rsdocs/plans/composition-pubuse.snapshot
💤 Files with no reviewable changes (1)
- crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6615
July 24, 2026 17:06
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 (1)
crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs (1)
295-309: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestore full redaction for model-visible search results.
Clearing only
error_messageleaves manifest-controlleddisplay_name,strategy,instructions,input_placeholder, andsubmit_labelin model-visible output. Imported bundles can control these fields, reintroducing the prompt-injection path rejected by the existing model-visible redaction invariant and prior review.Keep the full connection requirement on the product/display-preview path, but set
extension.summary.channel_connection = NoneforExtensionSearch; update the Telegram assertion at Line 745 through Line 752 accordingly.Proposed fix
Some(LifecycleProductPayload::ExtensionSearch { extensions, .. }) => { for extension in extensions { - if let Some(connection) = extension.summary.channel_connection.as_mut() { - connection.error_message.clear(); - } + extension.summary.channel_connection = None; } }🤖 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/extension_host/extension_lifecycle_capabilities.rs` around lines 295 - 309, Update without_model_visible_connection_chrome so ExtensionSearch entries set extension.summary.channel_connection to None instead of only clearing error_message, fully redacting manifest-controlled connection guidance from model-visible search results. Preserve the full connection requirement for the product/display-preview path, and update the Telegram assertion in the relevant lifecycle test to expect no channel_connection.
🤖 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/extension_host/extension_lifecycle_capabilities.rs`:
- Around line 295-309: Update without_model_visible_connection_chrome so
ExtensionSearch entries set extension.summary.channel_connection to None instead
of only clearing error_message, fully redacting manifest-controlled connection
guidance from model-visible search results. Preserve the full connection
requirement for the product/display-preview path, and update the Telegram
assertion in the relevant lifecycle test to expect no channel_connection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 03e43583-9954-4b39-9bab-1e7d75ba66f1
⛔ Files ignored due to path filters (3)
tests/snapshots/golden_payload__context_surfacing.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__parallel_tool_calls.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__tool_call.snapis excluded by!**/*.snap,!tests/snapshots/**
📒 Files selected for processing (1)
crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle_capabilities.rs
This was referenced Jul 24, 2026
25 of 29 tasks
BenKurrek
added a commit
that referenced
this pull request
Jul 27, 2026
… composition Continues the direction of #6615/#6616/#6619: composition is assembly, and these files were not assembly. Backend selection already happened upstream — composition hands them a `ScopedFilesystem`. What is left is contract policy owned by the port they implement: alias confinement with the explicit sibling-prefix guard, sensitive-filename omission from listings, the TOCTOU-hardened two-stage size guard, extension→MIME derivation that must match the download `Content-Type`, and the substrate→port error sanitization table (including the deliberate MountNotFound→503 vs Contract→400 split). `ProjectScopedFilesystemReader` and the attachment lander/reader therefore move to `ironclaw_product::scoped_fs`, beside the `ProjectFilesystemReader` / `InboundAttachmentLander` traits they implement. Every import they need was already a production dependency of that crate, and the in-crate precedent is `filesystem_ledger.rs`, which likewise hosts a generic `ScopedFilesystem` implementation of a product-owned port. `mount_filesystem_reader.rs` deliberately stays in composition: its `alias_for(FsMount)` table is the "which mounts does this deployment serve" decision, which is composition's job. It now consumes the shared scoped-path helpers from the owner crate by name. Composition src: 66,998 → 66,208 LOC (10.39% → 10.27% of production). Two gates caught this change and were fixed rather than silenced: the struct ratchet entry follows the file to its new path, and the extension-specificity gate rejected a comment naming a concrete vendor in generic code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
BenKurrek
added a commit
that referenced
this pull request
Jul 31, 2026
* feat(channels): add attachment transfer vocabulary to egress descriptors
Re-applied from codex/telegram-slack-attachments: ironclaw_attachments
materialized-file/budget/workspace-ref types + ChannelEgressDescriptor
paths/path_prefixes/body-limit bounds with fail-closed validation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(channels): transfer inbound/outbound channel attachments through restricted egress
Re-applied from codex/telegram-slack-attachments onto the restructured
tree: ChannelAdapter::fetch_attachment seam + AttachmentTransfer error in
ironclaw_host_api's product_adapter contract; envelope-transient
channel_attachment_refs; ChannelInboundProductSurface transfer door with
fail-closed default; post-policy fetch/validate/land orchestration in
ironclaw_product's inbound turn service; workspace-file materialization in
the delivery coordinator; Telegram getFile/download + sendDocument via the
manifest's path-constrained egress; Slack fails closed both directions;
composition wiring for the per-request policy-enforced channel egress.
InboundAttachment/MaterializedFile moved down into ironclaw_host_api (the
trait contract owner) because ironclaw_attachments depends on host_api.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(reborn): prove channel attachment journeys on the production mount
Relocates the composition-resident attachment journey coverage to the
extension-delivery integration lane per the tests/integration-first rule:
the telegram delivery scenario now drives a document update through the
production ingress mount — transient getFile failure releases the ledger
attempt (503), the vendor redelivery refetches through the manifest's
path-constrained egress with the token injected host-side, bytes land at
the canonical /workspace/attachments ref exactly once, duplicate replay
does no vendor I/O, and a follow-up conversation's final reply
referencing the landed file is materialized through the real
project-scoped reader and delivered natively via sendDocument. The
envelope's transient-refs serde(skip) contract is pinned beside the type
in ironclaw_host_api; sink-level door routing and inherited fail-closed
transfer stay as local contract tests beside the moved code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore(reborn): quality-gate fixes for the attachment re-application
Bundle the delivery coordinator's materialization inputs (clippy arg
budget), reuse the VendorResponseRouter alias, drop a dead test accessor,
update the ingress contract fixture to the current manifest schema
(admin_configuration-declared verification handle), and annotate
large-file growth per the arch-sprawl gate.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore(telegram): move channel test modules under src/tests/
The no-panics production gate exempts src/**/tests/*.rs; the flat
channel_*_tests.rs siblings were test-only (cfg(test) #[path] mounts)
but not recognizable as such by the path heuristic.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(channels): correct attachment transfer bugs and retire the large-file exempts
Audit follow-ups on the attachment transfer path.
Functional fixes, each with a regression test that fails without it:
- The fetched/declared MIME check normalized only the fetched side, so a
descriptor carrying a parameter (`text/plain; charset=utf-8` — what
Telegram clients routinely report for text documents) never matched and
rejected the whole message, caption included. Compare canonical forms on
both sides so the check catches a real provider mismatch instead.
- The descriptor filename overwrote the fetched one unconditionally,
discarding the name the adapter recovers from the `getFile` path for
payloads that carry none (photos, voice notes, stickers). Keep it when
the descriptor has no filename.
- The multipart boundary was derived from `bytes.len()` and searched in a
`0..=u32::MAX` loop with a full payload scan per iteration. The payload is
attacker-authored (an inbound attachment can be landed and later referenced
by a reply), so a sender could pad a file with collisions and force
unbounded rescans. Use a v4 UUID and scan once.
- The manifest declared 5 MiB transfer caps while the code enforced the
10 MiB host budget, so a file in between passed every code check and was
then refused at egress — inbound reported as "denied" rather than "too
large", outbound after the coordinator had committed to the send. Derive
one bound (TELEGRAM_MAX_TRANSFER_BYTES), pre-flight the assembled
multipart body against the declared request cap, map ResponseTooLarge to
a size error, and pin constants to the manifest with a test.
- The Bot API target's 64 KiB response cap also covered sendMessage, whose
response echoes `reply_to_message` now that replies are threaded; an
oversize echo failed a send the user had already received. Keep the host
default there and document why.
- The two fail-closed defaults for "this layer cannot transfer attachments"
disagreed: the product surface said retryable, the inbound turn service
said permanent. Retryable left the vendor redelivering forever while the
user got nothing at all. Both are permanent now; a missing deployment
egress transport stays retryable as an operator-fixable condition, and a
permanent Invalid outcome is logged rather than settling silently.
Security and hygiene:
- `path_prefixes` matched by raw byte prefix, so a declared
`/file/bot{token}` also authorized `/file/bot{token}Evil/…`. Require a
trailing `/` at descriptor validation and match on a segment boundary.
- `InboundAttachment` (new in this PR) derived `Debug` over raw bytes while
its sibling in the same file hand-writes a redacting one with a leak test.
Both now redact, and `ProjectFsFile` — the wire type — does too.
- `MaterializedFile<P>` had exactly one instantiation; collapsed to
`WorkspaceFile`.
- Removed 39 stale committed frontend build artifacts (3.5 MB) under
`crates/ironclaw_webui_v2/`, a crate folded into `ironclaw_webui`; nothing
reads the path. Closed the `.gitignore` gap that admitted them.
- Dropped all four `arch-exempt: large_file` annotations by shrinking the
files instead: two were spurious (one on a 1,242-line file the 1,500-line
gate never fires on, one licensing a semantically no-op edit), and the
exempt grep scans the whole file body, so each would have disabled the
gate for that file permanently. Test modules carved into their own files
per the composition budget's own carve-out guidance.
- Deleted a sink test whose distinguishing assertion read the test double's
own payload construction; the telegram journey covers door selection.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* refactor(attachments): close the egress prefix bypass and collapse duplicated vocabulary
Security:
- `path_prefixes` matched by raw byte prefix, so a declared
`/file/bot{token}` also authorized `/file/bot{token}Evil/…` — a sibling
path on the same pinned host and credential that the manifest author never
allowed. Descriptor validation now requires the prefix to end on a segment
boundary, and the matcher enforces the boundary independently so a prefix
reaching policy by another route still cannot authorize a sibling.
Duplicated vocabulary, per .claude/rules/type-placement.md:
- `mime_hint` was written in two adapters as a verbatim copy of
`descriptor.mime_type` and read in zero production locations. Deleted with
its writers; the one new production consumer already reads the descriptor.
- `AttachmentRef` named two different concepts — the durable byte-free
transcript reference and this transient vendor fetch reference — which
forced an `as ChannelAttachmentRef` import alias where both appeared. The
channel one is now `ChannelAttachmentRef` at its definition and the alias
is gone.
- `ProductAttachmentCapabilities` re-declared `AttachmentBudgets`' three
fields and hand-copied each. Embedded with `#[serde(flatten)]`; the JSON
shape is unchanged (the wire assertions still read the same keys) and a new
budget field now reaches the browser with no intermediate edit.
- The `/workspace` prefix was defined independently in `ironclaw_attachments`
(deciding which model-text refs become egress attachments) and in
composition (deciding which paths are readable at all). Divergence would be
a silently undelivered file or an extraction/confinement mismatch, so
`ironclaw_attachments` owns it, composition imports it, and a test pins the
prefix to the alias.
- `NoProjectFilesystem` was defined verbatim in three crates. One inert double
now lives beside the trait it implements, under `test-support`.
Also corrects two comments that asserted guarantees the code no longer made:
the reader's "same 25 MiB limit" (the delivery instance is deliberately
tighter) and `ProjectFsEntryKind`'s "without depending on that crate" (the
dependency exists; it is a wire projection, which is the real reason).
Retains the cause of a provider parse failure server-side via `tracing::debug`
while keeping the user-facing reason a fixed literal, and constructs the
declared bot-token handle in one place.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* refactor(product): move the scoped project-filesystem adapters out of composition
Continues the direction of #6615/#6616/#6619: composition is assembly, and
these files were not assembly. Backend selection already happened upstream —
composition hands them a `ScopedFilesystem`. What is left is contract policy
owned by the port they implement: alias confinement with the explicit
sibling-prefix guard, sensitive-filename omission from listings, the
TOCTOU-hardened two-stage size guard, extension→MIME derivation that must
match the download `Content-Type`, and the substrate→port error sanitization
table (including the deliberate MountNotFound→503 vs Contract→400 split).
`ProjectScopedFilesystemReader` and the attachment lander/reader therefore
move to `ironclaw_product::scoped_fs`, beside the `ProjectFilesystemReader` /
`InboundAttachmentLander` traits they implement. Every import they need was
already a production dependency of that crate, and the in-crate precedent is
`filesystem_ledger.rs`, which likewise hosts a generic `ScopedFilesystem`
implementation of a product-owned port.
`mount_filesystem_reader.rs` deliberately stays in composition: its
`alias_for(FsMount)` table is the "which mounts does this deployment serve"
decision, which is composition's job. It now consumes the shared scoped-path
helpers from the owner crate by name.
Composition src: 66,998 → 66,208 LOC (10.39% → 10.27% of production).
Two gates caught this change and were fixed rather than silenced: the struct
ratchet entry follows the file to its new path, and the extension-specificity
gate rejected a comment naming a concrete vendor in generic code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* refactor(channels): invert the pairing-outcome observer to a host-owned trait
The sink is generic channel machinery, but its pairing-outcome observer was
an enum naming a concrete composition type (`RunDeliveryPostAdmissionObserver`)
plus a `#[cfg(test)]` `Recording` variant compiled into the production type.
That is the coupling that keeps the generic ingress sink pinned to composition.
It is now a trait: the delivery observer implements it, and tests supply an
ordinary double instead of a variant. This is also the seam that has to invert
before the sink itself can move to `ironclaw_extension_host`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* refactor(extension-host): move the generic channel ingress sink out of composition
`ironclaw_extension_host` already owned the ingress *port* (`InboundSink`,
`InboundAdmission`, the router and verifier); composition owned the
*implementation*. The boundary test's own inventory shows the neighbourhood was
already evacuated — `reply_contexts`, `channel_delivery`, `channel_dm_targets`
and `channel_lifecycle` are all listed as externalized generic modules, and
`extension_ingress` appeared on neither that list nor the internal one. It was
the holdout.
Moved to `ironclaw_extension_host::ingress::sink`: the registration table
behind the router's ports, the trusted-evidence mint, the pairing
pre-admission gate, `GenericChannelInboundSink`, `StaticIngressSecrets`, and
the `build_extension_ingress` factory — module-owned initialization, as the
composition guide requires. The pairing outcome vocabulary moves with it to
`ingress::pairing`; the pairing *service* (CAS claim, identity bind,
completion fan-out) stays in composition and implements the host trait.
Composition keeps `mod serve_mount`: `ingress/mod.rs` states the crate is
deliberately transport-neutral, so the axum `PublicRouteMount` stays on the
composition side. Its public re-export is preserved, so downstream binaries
and tests are unaffected — only the source crate changed, which is what the
pub-use snapshot update records.
`host-auth-mint` is enabled on the host crate's `ironclaw_product` dependency
with the rationale the feature rule requires: it is a privilege boundary, and
this crate is one of the host runtimes entitled to mint verified evidence
after the router has executed the manifest's verification recipe.
Composition src: 66,205 → 65,560 LOC (10.27% → 10.17%). Across this branch:
66,998 → 65,560, with the file itself going 1,242 → 186 lines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* fix(llm): point the fault-injection doc example at its own crate
The example imported `ironclaw::testing::fault_injection`, a path that died
with the v1 monolith, so the doc-test failed to compile. It only runs under
workspace-wide feature unification — the root dev-dependency enables
`ironclaw_llm/test-support`, which compiles the `testing` module — so
`cargo test -p ironclaw_llm --doc` alone reports zero tests and never
surfaced it. `cargo test --workspace` has been failing on it.
The doc-test is its own regression test: it now compiles, where before it
could not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* fix(cli): pin channel authority in native ingress test
* fix(egress): enforce body limits after secret injection
* fix(product): preserve delivery failure semantics
* fix(telegram): allow attachments without size hints
* docs: design generic cross-channel attachments
* docs: plan generic cross-channel attachments
* feat(filesystem): add atomic subtree creation
* fix(attachments): land inbound batches atomically
* test(attachments): isolate channel lander seam
* feat(outbound): persist reply attachment intents
* feat(attachments): complete cross-channel reply delivery
* feat(attachments): durably assemble provider batches
* fix(slack): verify batched attachment delivery
* docs: record cross-channel attachment verification
* fix(attachments): harden replay and provider boundaries
* fix(attachments): repair merge-blocking CI coverage
* fix(attachments): retain test reply intent store
* fix(attachments): reuse outbound test store seam
* fix(attachments): render durable file references cleanly
* docs(attachments): define structured multimodal replies
* feat(attachments): complete multimodal kind vocabulary
* feat(attachments): add opaque reply attachment handles
* feat(attachments): return opaque handles to the model
* refactor(attachments): make structured replies canonical
* fix(ci): close attachment coverage gates
* fix mixed attachment cleanup snapshots
* fix(attachments): harden cross-channel reply delivery
* fix(ci): qualify attachment integration type
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
BenKurrek
added a commit
that referenced
this pull request
Jul 31, 2026
…nst HEAD (#6944) * docs(guidance): WS11.3 drift hotfixes — make agent guidance true against HEAD Fix developer guidance that is wrong today against origin/main: references to crates, symbols, paths, cargo features and tests that do not exist. Guidance and docs only — no code, test, or manifest changes. Classes fixed: - Seven nonexistent crates presented as current architecture (ironclaw_engine, _tui, _gateway, _oauth, _skill_learning, _webui_v2, _product_context), plus the deleted root `src/` monolith and the phantom ironclaw_hooks_{postgres, libsql,parity} / ironclaw_product_adapters / _product_adapter_registry. - `build_reborn_services` (crates/Architecture.md, tests/integration/CLAUDE.md) — no such function exists; the entry point is `build_reborn_runtime`. - `NetworkPolicyDecider` and six other phantom trait names in root CLAUDE.md's "key traits for extensibility" line. - The composition guide's phantom `src/webui/` and `src/projection.rs` paths (moved to ironclaw_webui / ironclaw_product by #6618 / #6615) and its `llm_admin::llm_catalog` ownership claim (that module is ironclaw_operator's). - Stale feature-gating claims: ironclaw_product's `storage`/`libsql`/`postgres`, openai_compat's and event_store's feature gates (neither crate declares any features), webui's `openai-compat-beta`/`dev-in-memory-session`, and ironclaw_llm's `testing` feature (the real gate is `test-support`). - Stale measured numbers in .claude/rules (fan-in figures, type/trait counts, duplicate-scan candidates, too_many_arguments allows) and two phantom cross-references to headings that do not exist. Every edit was re-verified against origin/main @ cd5d3fe; per-item evidence is in the PR body, including items recorded as already-resolved or verified-correct with no change made. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: record the WS11.3 drift-hotfix tranche in the checklist Annotates the two WS11 guidance rows this PR partially discharges. Both boxes stay unchecked: only the drift-hotfix half landed (claims wrong against HEAD today); the rewrite half — thin family index, family model, family map, new crate guides — waits on the restructure. Edit is confined to those two rows to stay clear of the concurrent WS0 deletion PRs (#6942, #6943), which touch other rows in this file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(guidance): track the hooks coverage gap and harden the dead-command banner Closes the two gaps the WS11.3 review flagged, both surfaced by this PR's own verification rather than by the checklist. - `ironclaw_hooks/CLAUDE.md`: the cross-run hook-isolation semantic this PR found unpinned now carries its tracking pointer (#6945), plus two facts a reader needs to size the risk — production wires the isolating `with_hook_dispatcher_builder_factory`, so the property holds today, and the existing `poisoned_during_dispatch_skips_subsequent_invocations` covers poisoning within one dispatcher, not across host builds. - `.claude/commands/add-sse-event.md`: the warning is now a banner that names every dead step (1-3 `src/channels/`, 4-5 `crates/ironclaw_gateway/static/`, 6 `src/agent/`+`src/worker/`; only step 7 survives) and states plainly that the command needs rewriting onto the `ironclaw_webui` streaming path. It deliberately does not invent that procedure — replacing a scaffold is new guidance, not a drift fix. Every path and crate named in the banner was verified to exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(guidance): address CodeRabbit review on the WS11.3 drift hotfixes Eight of eleven comments were valid; three were residual drift I left behind in the first pass, and two were self-inflicted by it. Residual drift I missed: - `ironclaw_hooks/CLAUDE.md` still named `BuiltinHookSink`/`TrustedHookSink`/ `InstalledHookSink` at lines 33/36/39; I had only fixed the module bullet. All three are phantom types. Replaced with the real privileged/restricted families, with the tier mapping read out of the `install_*` signatures (`install_builtin_*`/`install_trusted_*` take `Privileged*Hook`; `install_installed_*` takes `Restricted*Hook`, whose gate sink has no `allow`). `SelfAuthoredHookSink` is real and was left alone. - `reborn-extension-surfaces` still said `[channel.config]` twice; no manifest declares that section. - `ironclaw_llm/CLAUDE.md` still claimed the binary supplies adapters in `src/llm_host.rs` and plugs impls into `SessionManager`. Self-inflicted by the first pass: - `triage-prs.md`: I put `crates/ironclaw_agent_loop/` in Agent Core while it was already in the Reborn-stack row. - `composition/AGENTS.md`: appending the llm_catalog correction left the sentence contradicting itself mid-clause. Also: repo-qualified the parity paths in the testing skill, merged the duplicate `ironclaw_webui` row in `crates/README.md`, made the worked-example re-verify command execute the equivalence proof instead of `ls`, and noted the `ironclaw_silk_decoder` workspace exclusion in the de-slop loop. Two claims I wrote in this round were wrong on first draft and corrected after running them: the parity command needs `--features integration` (the targets do not build without it), and `build_reborn_runtime` delegates to `build_runtime`, not the reverse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 1, 2026
Merged
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
* feat(channels): add attachment transfer vocabulary to egress descriptors
Re-applied from codex/telegram-slack-attachments: ironclaw_attachments
materialized-file/budget/workspace-ref types + ChannelEgressDescriptor
paths/path_prefixes/body-limit bounds with fail-closed validation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(channels): transfer inbound/outbound channel attachments through restricted egress
Re-applied from codex/telegram-slack-attachments onto the restructured
tree: ChannelAdapter::fetch_attachment seam + AttachmentTransfer error in
ironclaw_host_api's product_adapter contract; envelope-transient
channel_attachment_refs; ChannelInboundProductSurface transfer door with
fail-closed default; post-policy fetch/validate/land orchestration in
ironclaw_product's inbound turn service; workspace-file materialization in
the delivery coordinator; Telegram getFile/download + sendDocument via the
manifest's path-constrained egress; Slack fails closed both directions;
composition wiring for the per-request policy-enforced channel egress.
InboundAttachment/MaterializedFile moved down into ironclaw_host_api (the
trait contract owner) because ironclaw_attachments depends on host_api.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(reborn): prove channel attachment journeys on the production mount
Relocates the composition-resident attachment journey coverage to the
extension-delivery integration lane per the tests/integration-first rule:
the telegram delivery scenario now drives a document update through the
production ingress mount — transient getFile failure releases the ledger
attempt (503), the vendor redelivery refetches through the manifest's
path-constrained egress with the token injected host-side, bytes land at
the canonical /workspace/attachments ref exactly once, duplicate replay
does no vendor I/O, and a follow-up conversation's final reply
referencing the landed file is materialized through the real
project-scoped reader and delivered natively via sendDocument. The
envelope's transient-refs serde(skip) contract is pinned beside the type
in ironclaw_host_api; sink-level door routing and inherited fail-closed
transfer stay as local contract tests beside the moved code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore(reborn): quality-gate fixes for the attachment re-application
Bundle the delivery coordinator's materialization inputs (clippy arg
budget), reuse the VendorResponseRouter alias, drop a dead test accessor,
update the ingress contract fixture to the current manifest schema
(admin_configuration-declared verification handle), and annotate
large-file growth per the arch-sprawl gate.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore(telegram): move channel test modules under src/tests/
The no-panics production gate exempts src/**/tests/*.rs; the flat
channel_*_tests.rs siblings were test-only (cfg(test) #[path] mounts)
but not recognizable as such by the path heuristic.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(channels): correct attachment transfer bugs and retire the large-file exempts
Audit follow-ups on the attachment transfer path.
Functional fixes, each with a regression test that fails without it:
- The fetched/declared MIME check normalized only the fetched side, so a
descriptor carrying a parameter (`text/plain; charset=utf-8` — what
Telegram clients routinely report for text documents) never matched and
rejected the whole message, caption included. Compare canonical forms on
both sides so the check catches a real provider mismatch instead.
- The descriptor filename overwrote the fetched one unconditionally,
discarding the name the adapter recovers from the `getFile` path for
payloads that carry none (photos, voice notes, stickers). Keep it when
the descriptor has no filename.
- The multipart boundary was derived from `bytes.len()` and searched in a
`0..=u32::MAX` loop with a full payload scan per iteration. The payload is
attacker-authored (an inbound attachment can be landed and later referenced
by a reply), so a sender could pad a file with collisions and force
unbounded rescans. Use a v4 UUID and scan once.
- The manifest declared 5 MiB transfer caps while the code enforced the
10 MiB host budget, so a file in between passed every code check and was
then refused at egress — inbound reported as "denied" rather than "too
large", outbound after the coordinator had committed to the send. Derive
one bound (TELEGRAM_MAX_TRANSFER_BYTES), pre-flight the assembled
multipart body against the declared request cap, map ResponseTooLarge to
a size error, and pin constants to the manifest with a test.
- The Bot API target's 64 KiB response cap also covered sendMessage, whose
response echoes `reply_to_message` now that replies are threaded; an
oversize echo failed a send the user had already received. Keep the host
default there and document why.
- The two fail-closed defaults for "this layer cannot transfer attachments"
disagreed: the product surface said retryable, the inbound turn service
said permanent. Retryable left the vendor redelivering forever while the
user got nothing at all. Both are permanent now; a missing deployment
egress transport stays retryable as an operator-fixable condition, and a
permanent Invalid outcome is logged rather than settling silently.
Security and hygiene:
- `path_prefixes` matched by raw byte prefix, so a declared
`/file/bot{token}` also authorized `/file/bot{token}Evil/…`. Require a
trailing `/` at descriptor validation and match on a segment boundary.
- `InboundAttachment` (new in this PR) derived `Debug` over raw bytes while
its sibling in the same file hand-writes a redacting one with a leak test.
Both now redact, and `ProjectFsFile` — the wire type — does too.
- `MaterializedFile<P>` had exactly one instantiation; collapsed to
`WorkspaceFile`.
- Removed 39 stale committed frontend build artifacts (3.5 MB) under
`crates/ironclaw_webui_v2/`, a crate folded into `ironclaw_webui`; nothing
reads the path. Closed the `.gitignore` gap that admitted them.
- Dropped all four `arch-exempt: large_file` annotations by shrinking the
files instead: two were spurious (one on a 1,242-line file the 1,500-line
gate never fires on, one licensing a semantically no-op edit), and the
exempt grep scans the whole file body, so each would have disabled the
gate for that file permanently. Test modules carved into their own files
per the composition budget's own carve-out guidance.
- Deleted a sink test whose distinguishing assertion read the test double's
own payload construction; the telegram journey covers door selection.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* refactor(attachments): close the egress prefix bypass and collapse duplicated vocabulary
Security:
- `path_prefixes` matched by raw byte prefix, so a declared
`/file/bot{token}` also authorized `/file/bot{token}Evil/…` — a sibling
path on the same pinned host and credential that the manifest author never
allowed. Descriptor validation now requires the prefix to end on a segment
boundary, and the matcher enforces the boundary independently so a prefix
reaching policy by another route still cannot authorize a sibling.
Duplicated vocabulary, per .claude/rules/type-placement.md:
- `mime_hint` was written in two adapters as a verbatim copy of
`descriptor.mime_type` and read in zero production locations. Deleted with
its writers; the one new production consumer already reads the descriptor.
- `AttachmentRef` named two different concepts — the durable byte-free
transcript reference and this transient vendor fetch reference — which
forced an `as ChannelAttachmentRef` import alias where both appeared. The
channel one is now `ChannelAttachmentRef` at its definition and the alias
is gone.
- `ProductAttachmentCapabilities` re-declared `AttachmentBudgets`' three
fields and hand-copied each. Embedded with `#[serde(flatten)]`; the JSON
shape is unchanged (the wire assertions still read the same keys) and a new
budget field now reaches the browser with no intermediate edit.
- The `/workspace` prefix was defined independently in `ironclaw_attachments`
(deciding which model-text refs become egress attachments) and in
composition (deciding which paths are readable at all). Divergence would be
a silently undelivered file or an extraction/confinement mismatch, so
`ironclaw_attachments` owns it, composition imports it, and a test pins the
prefix to the alias.
- `NoProjectFilesystem` was defined verbatim in three crates. One inert double
now lives beside the trait it implements, under `test-support`.
Also corrects two comments that asserted guarantees the code no longer made:
the reader's "same 25 MiB limit" (the delivery instance is deliberately
tighter) and `ProjectFsEntryKind`'s "without depending on that crate" (the
dependency exists; it is a wire projection, which is the real reason).
Retains the cause of a provider parse failure server-side via `tracing::debug`
while keeping the user-facing reason a fixed literal, and constructs the
declared bot-token handle in one place.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* refactor(product): move the scoped project-filesystem adapters out of composition
Continues the direction of nearai#6615/nearai#6616/nearai#6619: composition is assembly, and
these files were not assembly. Backend selection already happened upstream —
composition hands them a `ScopedFilesystem`. What is left is contract policy
owned by the port they implement: alias confinement with the explicit
sibling-prefix guard, sensitive-filename omission from listings, the
TOCTOU-hardened two-stage size guard, extension→MIME derivation that must
match the download `Content-Type`, and the substrate→port error sanitization
table (including the deliberate MountNotFound→503 vs Contract→400 split).
`ProjectScopedFilesystemReader` and the attachment lander/reader therefore
move to `ironclaw_product::scoped_fs`, beside the `ProjectFilesystemReader` /
`InboundAttachmentLander` traits they implement. Every import they need was
already a production dependency of that crate, and the in-crate precedent is
`filesystem_ledger.rs`, which likewise hosts a generic `ScopedFilesystem`
implementation of a product-owned port.
`mount_filesystem_reader.rs` deliberately stays in composition: its
`alias_for(FsMount)` table is the "which mounts does this deployment serve"
decision, which is composition's job. It now consumes the shared scoped-path
helpers from the owner crate by name.
Composition src: 66,998 → 66,208 LOC (10.39% → 10.27% of production).
Two gates caught this change and were fixed rather than silenced: the struct
ratchet entry follows the file to its new path, and the extension-specificity
gate rejected a comment naming a concrete vendor in generic code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* refactor(channels): invert the pairing-outcome observer to a host-owned trait
The sink is generic channel machinery, but its pairing-outcome observer was
an enum naming a concrete composition type (`RunDeliveryPostAdmissionObserver`)
plus a `#[cfg(test)]` `Recording` variant compiled into the production type.
That is the coupling that keeps the generic ingress sink pinned to composition.
It is now a trait: the delivery observer implements it, and tests supply an
ordinary double instead of a variant. This is also the seam that has to invert
before the sink itself can move to `ironclaw_extension_host`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* refactor(extension-host): move the generic channel ingress sink out of composition
`ironclaw_extension_host` already owned the ingress *port* (`InboundSink`,
`InboundAdmission`, the router and verifier); composition owned the
*implementation*. The boundary test's own inventory shows the neighbourhood was
already evacuated — `reply_contexts`, `channel_delivery`, `channel_dm_targets`
and `channel_lifecycle` are all listed as externalized generic modules, and
`extension_ingress` appeared on neither that list nor the internal one. It was
the holdout.
Moved to `ironclaw_extension_host::ingress::sink`: the registration table
behind the router's ports, the trusted-evidence mint, the pairing
pre-admission gate, `GenericChannelInboundSink`, `StaticIngressSecrets`, and
the `build_extension_ingress` factory — module-owned initialization, as the
composition guide requires. The pairing outcome vocabulary moves with it to
`ingress::pairing`; the pairing *service* (CAS claim, identity bind,
completion fan-out) stays in composition and implements the host trait.
Composition keeps `mod serve_mount`: `ingress/mod.rs` states the crate is
deliberately transport-neutral, so the axum `PublicRouteMount` stays on the
composition side. Its public re-export is preserved, so downstream binaries
and tests are unaffected — only the source crate changed, which is what the
pub-use snapshot update records.
`host-auth-mint` is enabled on the host crate's `ironclaw_product` dependency
with the rationale the feature rule requires: it is a privilege boundary, and
this crate is one of the host runtimes entitled to mint verified evidence
after the router has executed the manifest's verification recipe.
Composition src: 66,205 → 65,560 LOC (10.27% → 10.17%). Across this branch:
66,998 → 65,560, with the file itself going 1,242 → 186 lines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* fix(llm): point the fault-injection doc example at its own crate
The example imported `ironclaw::testing::fault_injection`, a path that died
with the v1 monolith, so the doc-test failed to compile. It only runs under
workspace-wide feature unification — the root dev-dependency enables
`ironclaw_llm/test-support`, which compiles the `testing` module — so
`cargo test -p ironclaw_llm --doc` alone reports zero tests and never
surfaced it. `cargo test --workspace` has been failing on it.
The doc-test is its own regression test: it now compiles, where before it
could not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5yb6tF3rwMq8KSvuMDhyK
* fix(cli): pin channel authority in native ingress test
* fix(egress): enforce body limits after secret injection
* fix(product): preserve delivery failure semantics
* fix(telegram): allow attachments without size hints
* docs: design generic cross-channel attachments
* docs: plan generic cross-channel attachments
* feat(filesystem): add atomic subtree creation
* fix(attachments): land inbound batches atomically
* test(attachments): isolate channel lander seam
* feat(outbound): persist reply attachment intents
* feat(attachments): complete cross-channel reply delivery
* feat(attachments): durably assemble provider batches
* fix(slack): verify batched attachment delivery
* docs: record cross-channel attachment verification
* fix(attachments): harden replay and provider boundaries
* fix(attachments): repair merge-blocking CI coverage
* fix(attachments): retain test reply intent store
* fix(attachments): reuse outbound test store seam
* fix(attachments): render durable file references cleanly
* docs(attachments): define structured multimodal replies
* feat(attachments): complete multimodal kind vocabulary
* feat(attachments): add opaque reply attachment handles
* feat(attachments): return opaque handles to the model
* refactor(attachments): make structured replies canonical
* fix(ci): close attachment coverage gates
* fix mixed attachment cleanup snapshots
* fix(attachments): harden cross-channel reply delivery
* fix(ci): qualify attachment integration type
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
…nst HEAD (nearai#6944) * docs(guidance): WS11.3 drift hotfixes — make agent guidance true against HEAD Fix developer guidance that is wrong today against origin/main: references to crates, symbols, paths, cargo features and tests that do not exist. Guidance and docs only — no code, test, or manifest changes. Classes fixed: - Seven nonexistent crates presented as current architecture (ironclaw_engine, _tui, _gateway, _oauth, _skill_learning, _webui_v2, _product_context), plus the deleted root `src/` monolith and the phantom ironclaw_hooks_{postgres, libsql,parity} / ironclaw_product_adapters / _product_adapter_registry. - `build_reborn_services` (crates/Architecture.md, tests/integration/CLAUDE.md) — no such function exists; the entry point is `build_reborn_runtime`. - `NetworkPolicyDecider` and six other phantom trait names in root CLAUDE.md's "key traits for extensibility" line. - The composition guide's phantom `src/webui/` and `src/projection.rs` paths (moved to ironclaw_webui / ironclaw_product by nearai#6618 / nearai#6615) and its `llm_admin::llm_catalog` ownership claim (that module is ironclaw_operator's). - Stale feature-gating claims: ironclaw_product's `storage`/`libsql`/`postgres`, openai_compat's and event_store's feature gates (neither crate declares any features), webui's `openai-compat-beta`/`dev-in-memory-session`, and ironclaw_llm's `testing` feature (the real gate is `test-support`). - Stale measured numbers in .claude/rules (fan-in figures, type/trait counts, duplicate-scan candidates, too_many_arguments allows) and two phantom cross-references to headings that do not exist. Every edit was re-verified against origin/main @ cd5d3fe; per-item evidence is in the PR body, including items recorded as already-resolved or verified-correct with no change made. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: record the WS11.3 drift-hotfix tranche in the checklist Annotates the two WS11 guidance rows this PR partially discharges. Both boxes stay unchecked: only the drift-hotfix half landed (claims wrong against HEAD today); the rewrite half — thin family index, family model, family map, new crate guides — waits on the restructure. Edit is confined to those two rows to stay clear of the concurrent WS0 deletion PRs (nearai#6942, nearai#6943), which touch other rows in this file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(guidance): track the hooks coverage gap and harden the dead-command banner Closes the two gaps the WS11.3 review flagged, both surfaced by this PR's own verification rather than by the checklist. - `ironclaw_hooks/CLAUDE.md`: the cross-run hook-isolation semantic this PR found unpinned now carries its tracking pointer (nearai#6945), plus two facts a reader needs to size the risk — production wires the isolating `with_hook_dispatcher_builder_factory`, so the property holds today, and the existing `poisoned_during_dispatch_skips_subsequent_invocations` covers poisoning within one dispatcher, not across host builds. - `.claude/commands/add-sse-event.md`: the warning is now a banner that names every dead step (1-3 `src/channels/`, 4-5 `crates/ironclaw_gateway/static/`, 6 `src/agent/`+`src/worker/`; only step 7 survives) and states plainly that the command needs rewriting onto the `ironclaw_webui` streaming path. It deliberately does not invent that procedure — replacing a scaffold is new guidance, not a drift fix. Every path and crate named in the banner was verified to exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(guidance): address CodeRabbit review on the WS11.3 drift hotfixes Eight of eleven comments were valid; three were residual drift I left behind in the first pass, and two were self-inflicted by it. Residual drift I missed: - `ironclaw_hooks/CLAUDE.md` still named `BuiltinHookSink`/`TrustedHookSink`/ `InstalledHookSink` at lines 33/36/39; I had only fixed the module bullet. All three are phantom types. Replaced with the real privileged/restricted families, with the tier mapping read out of the `install_*` signatures (`install_builtin_*`/`install_trusted_*` take `Privileged*Hook`; `install_installed_*` takes `Restricted*Hook`, whose gate sink has no `allow`). `SelfAuthoredHookSink` is real and was left alone. - `reborn-extension-surfaces` still said `[channel.config]` twice; no manifest declares that section. - `ironclaw_llm/CLAUDE.md` still claimed the binary supplies adapters in `src/llm_host.rs` and plugs impls into `SessionManager`. Self-inflicted by the first pass: - `triage-prs.md`: I put `crates/ironclaw_agent_loop/` in Agent Core while it was already in the Reborn-stack row. - `composition/AGENTS.md`: appending the llm_catalog correction left the sentence contradicting itself mid-clause. Also: repo-qualified the parity paths in the testing skill, merged the duplicate `ironclaw_webui` row in `crates/README.md`, made the worked-example re-verify command execute the equivalence proof instead of `ls`, and noted the `ironclaw_silk_decoder` workspace exclusion in the de-slop loop. Two claims I wrote in this round were wrong on first draft and corrected after running them: the parity command needs `--features integration` (the targets do not build without it), and `build_reborn_runtime` delegates to `build_runtime`, not the reverse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Summary
ironclaw_operatorcrate, including LLM admin, provider admin, operator logs, service lifecycle, NEAR AI login route logic, and NEAR AI MCP config language.ironclaw_product::projection, with composition reduced to route/runtime adapters only.ironclaw_host_api::operator_llmso CLI/product/composition do not mirror the same admin wire types.ironclaw_operator/ironclaw_productowner crates.Composition LOC reduction:
crates/ironclaw_reborn_compositionoverall: 123 added / 24,493 deleted = 24,370 net LOC removed.94,597 LOC of 647,030=14.62%, well below the current24.28%effective ceiling.RebornRuntime, and NEAR AI MCP bootstrap still adapts product-auth/extension-lifecycle composition ports. The composition re-export shim files for projections, operator logs/lifecycle, and LLM/provider admin have been deleted.Change Type
Linked Issue
None.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildcargo test --features integrationif database-backed or integration behavior changed: Not applicable: no new database-backed/integration behavior.review-prorpr-shepherd --fixwas run before requesting review: Not applicable: no such local review tool is installed in this workspace.Test Strategy
User behavior:
No intended behavior change. This is an ownership/boundary simplification: composition remains the assembly root, while operator/admin code and product projection code move to their owning crates.
Risk areas:
Tests added or updated:
ironclaw_product; moved NEAR AI login/MCP tests intoironclaw_operator; updated architecture ratchets for new crate/path ownership and the reduced composition public facade.cargo test -p ironclaw_reborn_composition --lib, provider-admin integration tests,cargo test -p ironclaw_product,cargo test -p ironclaw_product --lib,cargo test -p ironclaw_operator,cargo test -p ironclaw_architecture.What the tests prove:
The moved operator/admin services still compile and pass their original unit coverage in
ironclaw_operator; product projection behavior still passes the projection stream tests from its new owning crate, including tenant/user-scoped outbound state isolation; composition runtime/bootstrap tests continue passing through direct owner-crate imports; architecture ratchets accept the new dependency, public facade, and specificity boundaries. The channel-connection projection regression also proves model-visible lifecycle search keeps the typedweb_generated_codestrategy while omitting manifest-authored presentation copy. Restored auth-challenge stream tests now live underironclaw_product::projection, covering OAuth URL enrichment, pairing context projection, manual-token fallback, retired pairing fallback, OAuth-without-URL fallback, and provider lookup failure.Commands run:
cargo check -p ironclaw_operatorcargo test -p ironclaw_operatorcargo clippy -p ironclaw_operator --all-targets --all-features -- -D warningscargo check -p ironclaw_productcargo test -p ironclaw_productcargo test -p ironclaw_product projection_outbound_store_mount_is_tenant_user_scoped --libcargo test -p ironclaw_product projection::tests::turn_stream_auth --libcargo test -p ironclaw_product --libcargo clippy -p ironclaw_product --all-targets --all-features -- -D warningscargo clippy -p ironclaw_product -p ironclaw_reborn_composition --all-targets --all-features -- -D warningscargo check -p ironclaw_reborn_compositioncargo test -p ironclaw_reborn_composition --libcargo check -p ironclawcargo test -p ironclaw_architecturecargo fmt --all -- --checkgit diff --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo clippy -p ironclaw_reborn_composition -p ironclaw --all-targets --all-features -- -D warningscargo test -p ironclaw_reborn_composition --test provider_admin --test provider_admin_probe --test provider_admin_product_commandcargo test -p ironclaw_reborn_integration_tests --test reborn_integration_channel_connection_projection extension_search_projects_descriptor_declared_web_generated_code_guidance -- --nocapturecargo test --test reborn_integration_channel_connection_projection extension_search_projects_descriptor_declared_web_generated_code_guidance -- --nocapture(latest focused re-run after stripping manifest-authored copy from model-visible WebGeneratedCode lifecycle output)cargo test -p ironclaw_reborn_composition extension_host::extension_lifecycle_capabilities::tests::model_visible_extension_search_omits_channel_connection_failure_copy --libcargo buildscripts/pre-commit-safety.shSecurity Impact
No policy weakening intended. Operator lifecycle/log/admin code moved crates but keeps the same authorization checks and product-command boundaries. NEAR AI login descriptor construction now returns typed errors instead of using production
expect.Reborn Trust-Boundary Checklist
serde(default)fields fail closed or have migration tests. No schema/default additions.Transient,Permanent,Misconfigured,PolicyDeniedor equivalent). Existing operator-visible error semantics preserved; NEAR AI route descriptor errors now propagate throughRebornBuildError.ironclaw_operatorfor host/operator control-plane ownership.Database Impact
None. No migrations or schema changes.
Blast Radius
Composition assembly, CLI admin/model/onboard paths, operator logs/service lifecycle, LLM admin/provider setup, NEAR AI login/MCP bootstrap, product projection streams, and architecture boundary tests. Main risk is missing a path during crate ownership rewiring; mitigated by product/operator/composition tests, CLI check, architecture tests, provider-admin integration tests, and clippy.
Rollback Plan
Revert this PR. The change is a crate ownership refactor with no migration or persisted schema changes, so rollback restores the previous composition-owned implementation layout.
Review Follow-Through
Requested
ironloopreview through GitHub, but GitHub rejectedironloop/IronLoopas non-collaborator. Added a PR comment tagging@IronLoopwith that blocker. Known follow-up: OpenAI-compatible serving and the NEAR AI MCP lifecycle bootstrap still contain real composition adapters because they depend directly onRebornRuntime, product-auth, and extension lifecycle ports.Review track: B (feature/maintainer-requested refactor)