test(reborn): switch integration harness to the production capability-port factory (seam PR-B) - #6026
Merged
Merged
Conversation
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6026
July 13, 2026 01:26
Destroyed
github-actions
Bot
added
scope: docs
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR centralizes local-dev test capability-port assembly, adds optional filtering, synthetic grants, extension management, mount overrides, and shared I/O, removes redundant wrapper seams, refactors integration harness wiring, and adds surface-disclosure coverage. ChangesCapability surface and integration harness
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant IntegrationTest
participant RebornIntegrationHarness
participant RefreshingLocalDevCapabilityPort
participant TraceLlm
IntegrationTest->>RebornIntegrationHarness: configure confirmed /host mount
RebornIntegrationHarness->>RefreshingLocalDevCapabilityPort: build recording capability port
RefreshingLocalDevCapabilityPort->>TraceLlm: expose refreshed tool definitions
IntegrationTest->>TraceLlm: assert conditional tool descriptions
IntegrationTest->>RefreshingLocalDevCapabilityPort: dispatch builtin.time
RefreshingLocalDevCapabilityPort-->>IntegrationTest: return correlated result
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 integration-test capability harness to align with the production RefreshingLocalDevCapabilityPort factory, replacing hand-rebuilt test-only wrappers with direct usage of the production factory. Key changes include extending RefreshingLocalDevCapabilityPortTestParts with extension_management and additional_capability_grants, transitioning capability_id_filter to an Option<HashSet>, and implementing a new integration test suite for surface_disclosure to verify schema annotations under confirmed host mounts. I have no feedback to provide as there were no review comments to assess.
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.
henrypark133
commented
Jul 13, 2026
henrypark133
left a comment
henrypark133
left a comment
Collaborator
Author
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Switch the Reborn integration harness to production's capability-port factory while preserving production behavior and covering all production port layers in tests.
Stats: 3 raw findings, 2 after same-line dedup across 2 files. Reviewers with results: security, tests, conventions. Reviewer dispatch was limited by the collaboration service thread cap; bugs, performance, local-patterns, maintainability, and approach did not complete. Body-only: 0.
Tests
- Medium Shell disclosure annotation is not integration-tested (
tests/integration/surface_disclosure.rs:53-66, confidence 90) — anchor:crates/ironclaw_reborn_composition/src/runtime/local_dev/surface_disclosure.rs:126
The new positive test checks onlyread_file's scoped-roots annotation. The separatebuiltin.shellbranch adds a local-host shell note but has no adjacent integration assertion. Add a test covering the shell description disclosure. Also flagged by the visible-descriptor coverage concern at the same location. - Medium Visible descriptor disclosure path is untested (
tests/integration/surface_disclosure.rs:53-66, confidence 85) — anchor:crates/ironclaw_reborn_composition/src/runtime/local_dev/surface_disclosure.rs:62
The integration test inspects provider tool definitions, but the wrapper also mutatesVisibleCapabilitySurfacedescriptors invisible_capabilities. Add a test verifying the scoped-roots note reachessafe_descriptionandparameters_schemathrough that path.
Conventions
- Medium Document the new accessor as test-only (
crates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rs:139-141, confidence 100) — anchor:crates/ironclaw_reborn_composition/CLAUDE.md:6
The new test-support accessor names the production call site but does not explicitly state that the accessor and returned handle are for tests only. Add that sentence to the doc comment.
Verification: cargo check -p ironclaw_reborn_composition --features test-support --tests passed at head 9eaf63de55317edb766de95bb00c127c0c2d56a7.
| //! invisible to every integration test before this seam PR. | ||
| //! | ||
| //! Ground truth (verified against | ||
| //! `crates/ironclaw_reborn_composition/src/runtime/local_dev/surface_disclosure.rs`, |
Collaborator
Author
There was a problem hiding this comment.
Medium — Shell disclosure annotation is not integration-tested.
The positive integration test checks only read_file's scoped-roots annotation; the separate builtin.shell branch adds a local-host shell note without an integration assertion.
Fix: Add a test covering builtin.shell description disclosure. Also flagged by the visible-descriptor coverage concern at this location.
Collaborator
Author
There was a problem hiding this comment.
Fixed in eb40617 — added confirmed_host_mount_adds_local_host_shell_note_to_shell + workspace_only_mount_excludes_local_host_shell_note, mirroring the read_file pair. Learned condition: the shell note appends unconditionally inside the wrapper, but the wrapper itself gates on a confirmed /host mount — the negative control pins that gate. 14/14 green.
| /// `RefreshingLocalDevCapabilityPortConfig` minus the no-op-by-default parts | ||
| /// (`extension_surface_source`, `external_tool_catalog`, `policy`). | ||
| /// (`external_tool_catalog`, `policy`). `extension_surface_source` itself | ||
| /// stays no-op-by-default too — the harness supplies the raw |
Collaborator
Author
There was a problem hiding this comment.
Medium — Document the new accessor as test-only.
The new test-support accessor names the production call site but does not explicitly state that the accessor and returned handle are for tests only.
Fix: Add that sentence to the doc comment.
Collaborator
Author
There was a problem hiding this comment.
Fixed in eb40617 — appended the standard test-only sentence to the accessor doc-comment, matching sibling phrasing.
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.37% — 296947 / 347833 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
|
🚅 Deployed to the ironclaw-pr-6026 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6026
July 13, 2026 04:25
Destroyed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rs (1)
129-151: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the documentation with the implementation.
This function returns
Somewheneverlocal_runtime.extension_managementexists; it does not check whether an extension is active. Remove the “no extension is currently active” condition from the documentation, or add that check ifNoneis intended for inactive extensions.🤖 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/test_support/refreshing_capability_port.rs` around lines 129 - 151, Update the documentation for build_local_dev_extension_management_for_test to state that it returns Some whenever local_runtime.extension_management exists, regardless of whether an extension is currently active. Remove the claims that inactive extensions produce None and that callers may omit the call for that reason; leave the implementation unchanged.
🤖 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/test_support/refreshing_capability_port.rs`:
- Around line 129-151: Update the documentation for
build_local_dev_extension_management_for_test to state that it returns Some
whenever local_runtime.extension_management exists, regardless of whether an
extension is currently active. Remove the claims that inactive extensions
produce None and that callers may omit the call for that reason; leave the
implementation unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ebb744c2-43e1-4bbb-bab3-8e63c32c38c0
📒 Files selected for processing (2)
crates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rstests/integration/surface_disclosure.rs
henrypark133
force-pushed
the
harness-port-switch
branch
from
July 13, 2026 04:36
eb40617 to
c5055b1
Compare
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6026
July 13, 2026 04:36
Destroyed
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 `@tests/integration/support/harness/mod.rs`:
- Around line 1323-1349: The additional provider trust setup must not overwrite
production decisions supplied by extension_management. Update the insertion
logic around provider_id and self.additional_provider_trust to add synthetic
user_trusted decisions only when the provider ID is absent from the existing
production trust map, preserving active extensions’ trust and resource ceilings.
🪄 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: 67162e67-cb2c-49dc-a0dd-2360b098e844
⛔ Files ignored due to path filters (5)
tests/snapshots/golden_payload__context_surfacing.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__gated_turn_approve.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__image_attachment.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 (26)
Cargo.tomlcrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/project_create.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/skill_activation.rscrates/ironclaw_reborn_composition/src/test_support/mod.rscrates/ironclaw_reborn_composition/src/test_support/outbound_delivery.rscrates/ironclaw_reborn_composition/src/test_support/project_create.rscrates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rscrates/ironclaw_reborn_composition/src/test_support/skill_activation.rscrates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rstests/integration/CLAUDE.mdtests/integration/group_triggers/scenario_webui_automations_list.rstests/integration/group_triggers/scenario_webui_automations_rename.rstests/integration/support/assertions.rstests/integration/support/builder.rstests/integration/support/capability_backend.rstests/integration/support/doubles/mod.rstests/integration/support/doubles/recording_capability_result_writer.rstests/integration/support/doubles/unavailable_project_service.rstests/integration/support/harness/mod.rstests/integration/support/harness/options.rstests/integration/support/harness/profiles/core_builtin.rstests/integration/surface_disclosure.rs
💤 Files with no reviewable changes (5)
- crates/ironclaw_reborn_composition/src/runtime/local_dev/skill_activation.rs
- crates/ironclaw_reborn_composition/src/runtime/local_dev/project_create.rs
- crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs
- crates/ironclaw_reborn_composition/src/test_support/project_create.rs
- crates/ironclaw_reborn_composition/src/test_support/outbound_delivery.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6026
July 13, 2026 05:10
Destroyed
henrypark133
added a commit
that referenced
this pull request
Jul 13, 2026
…cked ceilings CodeRabbit flagged (PR #6026) that the harness's additional_provider_trust blanket entries (e.g. bundled_extension_provider_trust()'s user_trusted, unbounded ceiling) overwrite a real production trust decision once a provider is activated through extension_management, since the config field's overwrite semantics apply last. Gate the synthetic-entry merge on `reborn_services.is_none()` (the genuinely ad-hoc mock-mcp/standalone github/web-access harnesses) so any harness with a wired extension_management leaves activation-backed providers (gmail, github, visprobe, ...) to production's extension_surface.provider_trust() alone. Extracted the merge into build_additional_provider_trust so the invariant is unit-testable without a full async harness/tokio runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6026
July 13, 2026 05:39
Destroyed
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 `@tests/integration/support/harness/mod.rs`:
- Around line 1695-1747: Add a caller-level integration regression test in the
harness that activates a non-builtin provider, drives
create_recording_capability_port through the real Reborn gateway/decorator
chain, and fakes only the vendor SDK boundary. Assert at the
visible-capability/dispatch seam that the activated provider’s production trust
ceiling remains effective after refresh, rather than invoking
build_additional_provider_trust directly; keep the existing helper tests
unchanged.
- Around line 1557-1571: The result-building logic must not synthesize
primary-provider trust when has_reborn_services is true. Update the provider_id
insertion condition to exclude activation-backed harnesses, while preserving the
existing builtin-provider exclusion and the additional_provider_trust handling
so production extension-surface trust remains authoritative.
🪄 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: b2f47015-119b-48e2-9977-fbb7cfbf6590
📒 Files selected for processing (1)
tests/integration/support/harness/mod.rs
Comment on lines
+1695
to
+1747
| fn activation_backed_provider_gets_no_synthetic_trust_entry() { | ||
| let builtin_provider = | ||
| ExtensionId::new(ironclaw_host_runtime::BUILTIN_FIRST_PARTY_PROVIDER) | ||
| .expect("builtin provider id"); | ||
| let gmail_provider = ExtensionId::new("gmail").expect("gmail provider id"); | ||
| let blanket_additional = vec![(gmail_provider.clone(), local_dev_all_effects())]; | ||
|
|
||
| let result = HostRuntimeCapabilityHarness::build_additional_provider_trust( | ||
| &builtin_provider, | ||
| &local_dev_all_effects(), | ||
| &blanket_additional, | ||
| /* has_reborn_services */ true, | ||
| ); | ||
|
|
||
| assert!( | ||
| !result.contains_key(&gmail_provider), | ||
| "an activation-backed harness (reborn_services wired) must never mint a \ | ||
| synthetic trust entry for a bundled-extension provider like `gmail` -- \ | ||
| production's `extension_surface.provider_trust()` must be the sole \ | ||
| source of `gmail`'s ceiling once it is activated, and this entry would \ | ||
| silently overwrite it: {result:?}" | ||
| ); | ||
| } | ||
|
|
||
| /// The genuinely ad-hoc case (mock-mcp / standalone github / standalone | ||
| /// web-access harnesses): `reborn_services` is never wired for these, so | ||
| /// there is no production decision to ever clobber, and the entry IS | ||
| /// needed (no other path trusts these providers at all). | ||
| #[test] | ||
| fn ad_hoc_provider_without_reborn_services_still_gets_trust_entry() { | ||
| let builtin_provider = | ||
| ExtensionId::new(ironclaw_host_runtime::BUILTIN_FIRST_PARTY_PROVIDER) | ||
| .expect("builtin provider id"); | ||
| let mock_mcp_provider = ExtensionId::new("mock-mcp").expect("mock-mcp provider id"); | ||
| let effects = vec![EffectKind::DispatchCapability, EffectKind::Network]; | ||
| let additional = vec![(mock_mcp_provider.clone(), effects.clone())]; | ||
|
|
||
| let result = HostRuntimeCapabilityHarness::build_additional_provider_trust( | ||
| &builtin_provider, | ||
| &effects, | ||
| &additional, | ||
| /* has_reborn_services */ false, | ||
| ); | ||
|
|
||
| assert_eq!( | ||
| result | ||
| .get(&mock_mcp_provider) | ||
| .map(|decision| &decision.authority_ceiling.allowed_effects), | ||
| Some(&effects), | ||
| "a harness with no wired extension_management must still mint its ad-hoc \ | ||
| provider's trust entry, or that provider's capabilities become invisible: {result:?}" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Add a caller-level refresh regression test.
These tests invoke build_additional_provider_trust directly, so they cannot prove that create_recording_capability_port preserves an activated provider’s actual trust ceiling after refresh. Add a tests/integration/ harness test that activates a non-builtin provider and asserts the production trust decision remains effective at the visible-capability/dispatch seam.
As per coding guidelines, “the test must drive the caller-level surface ... rather than testing the helper in isolation”; as per path instructions, “Test through the caller.” Based on learnings, retain the real Reborn gateway/decorator chain and fake only the vendor SDK seam.
🤖 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 `@tests/integration/support/harness/mod.rs` around lines 1695 - 1747, Add a
caller-level integration regression test in the harness that activates a
non-builtin provider, drives create_recording_capability_port through the real
Reborn gateway/decorator chain, and fakes only the vendor SDK boundary. Assert
at the visible-capability/dispatch seam that the activated provider’s production
trust ceiling remains effective after refresh, rather than invoking
build_additional_provider_trust directly; keep the existing helper tests
unchanged.
Sources: Coding guidelines, Path instructions, Learnings
henrypark133
added a commit
that referenced
this pull request
Jul 13, 2026
…cked ceilings CodeRabbit flagged (PR #6026) that the harness's additional_provider_trust blanket entries (e.g. bundled_extension_provider_trust()'s user_trusted, unbounded ceiling) overwrite a real production trust decision once a provider is activated through extension_management, since the config field's overwrite semantics apply last. Gate the synthetic-entry merge on `reborn_services.is_none()` (the genuinely ad-hoc mock-mcp/standalone github/web-access harnesses) so any harness with a wired extension_management leaves activation-backed providers (gmail, github, visprobe, ...) to production's extension_surface.provider_trust() alone. Extracted the merge into build_additional_provider_trust so the invariant is unit-testable without a full async harness/tokio runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrypark133
force-pushed
the
harness-port-switch
branch
from
July 13, 2026 07:01
b4df85d to
882fb45
Compare
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6026
July 13, 2026 07:02
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/support/doubles/recording_capability_result_writer.rs (1)
46-106: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winForward the missing capability hooks
RecordingCapabilityResultWriteris wired in as both halves of the harness I/O, but it only forwards the two input-resolver methods and the two result-write methods.record_provider_tool_call_display_input,record_running_invocation,stage_capability_failure_preview, anddelete_capability_resultstill fall through to the trait defaults, so the durable display-preview / delete path thatLocalDevCapabilityIoandProductLiveCapabilityIoimplement gets dropped. Forward those hooks to the inner resolver/writer to keep the harness aligned with production.🤖 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 `@tests/integration/support/doubles/recording_capability_result_writer.rs` around lines 46 - 106, Update RecordingCapabilityResultWriter to forward record_provider_tool_call_display_input and record_running_invocation through its inner input_resolver, and stage_capability_failure_preview and delete_capability_result through its inner result_writer. Match the corresponding trait method signatures and propagate their results so the harness preserves the production I/O 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.
Inline comments:
In `@crates/ironclaw_reborn_composition/src/runtime/local_dev.rs`:
- Around line 420-424: Change the internal lock-failure diagnostics in the
local-dev capability paths, including the branches near the result staging lock
and the propagated persistence failure, from warn! to debug!. Preserve the
existing best-effort staging and persistence-error behavior unchanged.
In
`@crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs`:
- Around line 179-206: Add a port-level test in
refreshing_capability_port_test_support.rs that configures test_parts() through
the real factory path with additional_capability_grants containing both a grant
matching an existing capability and a grant with a missing capability ID. Assert
the resulting request overlays network and secrets for the matching grant and
inserts the missing synthetic grant, covering both branches in the
additional_capability_grants handling.
In
`@crates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rs`:
- Around line 137-143: Update the helper documentation around the local-dev
extension port to distinguish missing local_runtime or extension_management,
which return None, from having no active extension, which still returns
Some(handle) with an initially empty snapshot. State that harnesses should
retain the handle to preserve refresh behavior when an extension is activated
later.
In `@tests/integration/support/harness/profiles/file.rs`:
- Around line 39-61: Update file_tools_requiring_approval_profile_for_user to
accept a typed UserId reference or value instead of &str, and pass it directly
to with_user_id without reconstructing it. Adjust
file_tools_requiring_approval_profile and the binary_e2e.rs caller near the
referenced usage to retain and pass UserId values throughout, removing any
as_str() round trips.
---
Outside diff comments:
In `@tests/integration/support/doubles/recording_capability_result_writer.rs`:
- Around line 46-106: Update RecordingCapabilityResultWriter to forward
record_provider_tool_call_display_input and record_running_invocation through
its inner input_resolver, and stage_capability_failure_preview and
delete_capability_result through its inner result_writer. Match the
corresponding trait method signatures and propagate their results so the harness
preserves the production I/O behavior.
🪄 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: 67372f02-c53b-4623-9228-2e439ab4323e
⛔ Files ignored due to path filters (5)
tests/snapshots/golden_payload__context_surfacing.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__gated_turn_approve.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__image_attachment.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 (28)
Cargo.tomlcrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/project_create.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/skill_activation.rscrates/ironclaw_reborn_composition/src/test_support/mod.rscrates/ironclaw_reborn_composition/src/test_support/outbound_delivery.rscrates/ironclaw_reborn_composition/src/test_support/project_create.rscrates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rscrates/ironclaw_reborn_composition/src/test_support/skill_activation.rscrates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rstests/integration/CLAUDE.mdtests/integration/group_triggers/scenario_webui_automations_list.rstests/integration/group_triggers/scenario_webui_automations_rename.rstests/integration/support/assertions.rstests/integration/support/builder.rstests/integration/support/capability_backend.rstests/integration/support/doubles/mod.rstests/integration/support/doubles/recording_capability_result_writer.rstests/integration/support/doubles/unavailable_project_service.rstests/integration/support/harness/mod.rstests/integration/support/harness/options.rstests/integration/support/harness/profiles/core_builtin.rstests/integration/support/harness/profiles/file.rstests/integration/surface_disclosure.rstests/support/reborn_parity_qa/binary_e2e.rs
💤 Files with no reviewable changes (2)
- crates/ironclaw_reborn_composition/src/test_support/outbound_delivery.rs
- crates/ironclaw_reborn_composition/src/test_support/project_create.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/support/doubles/recording_capability_result_writer.rs (1)
46-106: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winForward the missing capability hooks
RecordingCapabilityResultWriteris wired in as both halves of the harness I/O, but it only forwards the two input-resolver methods and the two result-write methods.record_provider_tool_call_display_input,record_running_invocation,stage_capability_failure_preview, anddelete_capability_resultstill fall through to the trait defaults, so the durable display-preview / delete path thatLocalDevCapabilityIoandProductLiveCapabilityIoimplement gets dropped. Forward those hooks to the inner resolver/writer to keep the harness aligned with production.🤖 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 `@tests/integration/support/doubles/recording_capability_result_writer.rs` around lines 46 - 106, Update RecordingCapabilityResultWriter to forward record_provider_tool_call_display_input and record_running_invocation through its inner input_resolver, and stage_capability_failure_preview and delete_capability_result through its inner result_writer. Match the corresponding trait method signatures and propagate their results so the harness preserves the production I/O 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.
Inline comments:
In `@crates/ironclaw_reborn_composition/src/runtime/local_dev.rs`:
- Around line 420-424: Change the internal lock-failure diagnostics in the
local-dev capability paths, including the branches near the result staging lock
and the propagated persistence failure, from warn! to debug!. Preserve the
existing best-effort staging and persistence-error behavior unchanged.
In
`@crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs`:
- Around line 179-206: Add a port-level test in
refreshing_capability_port_test_support.rs that configures test_parts() through
the real factory path with additional_capability_grants containing both a grant
matching an existing capability and a grant with a missing capability ID. Assert
the resulting request overlays network and secrets for the matching grant and
inserts the missing synthetic grant, covering both branches in the
additional_capability_grants handling.
In
`@crates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rs`:
- Around line 137-143: Update the helper documentation around the local-dev
extension port to distinguish missing local_runtime or extension_management,
which return None, from having no active extension, which still returns
Some(handle) with an initially empty snapshot. State that harnesses should
retain the handle to preserve refresh behavior when an extension is activated
later.
In `@tests/integration/support/harness/profiles/file.rs`:
- Around line 39-61: Update file_tools_requiring_approval_profile_for_user to
accept a typed UserId reference or value instead of &str, and pass it directly
to with_user_id without reconstructing it. Adjust
file_tools_requiring_approval_profile and the binary_e2e.rs caller near the
referenced usage to retain and pass UserId values throughout, removing any
as_str() round trips.
---
Outside diff comments:
In `@tests/integration/support/doubles/recording_capability_result_writer.rs`:
- Around line 46-106: Update RecordingCapabilityResultWriter to forward
record_provider_tool_call_display_input and record_running_invocation through
its inner input_resolver, and stage_capability_failure_preview and
delete_capability_result through its inner result_writer. Match the
corresponding trait method signatures and propagate their results so the harness
preserves the production I/O behavior.
🪄 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: 67372f02-c53b-4623-9228-2e439ab4323e
⛔ Files ignored due to path filters (5)
tests/snapshots/golden_payload__context_surfacing.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__gated_turn_approve.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__image_attachment.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 (28)
Cargo.tomlcrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/project_create.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/skill_activation.rscrates/ironclaw_reborn_composition/src/test_support/mod.rscrates/ironclaw_reborn_composition/src/test_support/outbound_delivery.rscrates/ironclaw_reborn_composition/src/test_support/project_create.rscrates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rscrates/ironclaw_reborn_composition/src/test_support/skill_activation.rscrates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rstests/integration/CLAUDE.mdtests/integration/group_triggers/scenario_webui_automations_list.rstests/integration/group_triggers/scenario_webui_automations_rename.rstests/integration/support/assertions.rstests/integration/support/builder.rstests/integration/support/capability_backend.rstests/integration/support/doubles/mod.rstests/integration/support/doubles/recording_capability_result_writer.rstests/integration/support/doubles/unavailable_project_service.rstests/integration/support/harness/mod.rstests/integration/support/harness/options.rstests/integration/support/harness/profiles/core_builtin.rstests/integration/support/harness/profiles/file.rstests/integration/surface_disclosure.rstests/support/reborn_parity_qa/binary_e2e.rs
💤 Files with no reviewable changes (2)
- crates/ironclaw_reborn_composition/src/test_support/outbound_delivery.rs
- crates/ironclaw_reborn_composition/src/test_support/project_create.rs
🛑 Comments failed to post (4)
crates/ironclaw_reborn_composition/src/runtime/local_dev.rs (1)
420-424: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use
debug!for these internal diagnostics.Lines 421 and 991 run on the local-dev capability path;
warn!can corrupt the REPL/TUI. Both paths already retain their intended behavior (best-effort staging or propagated persistence failure), so downgrade the diagnostics todebug!.Proposed fix
- tracing::warn!( + tracing::debug!( "local-dev capability result staging lock failed; using durable result only" ); - tracing::warn!(error = %error, "durable capability result persistence failed"); + tracing::debug!(error = %error, "durable capability result persistence failed");As per path instructions, “REPL/TUI logging: info!/warn! corrupt the terminal UI — internal diagnostics use debug!”.
Also applies to: 990-995
🤖 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/local_dev.rs` around lines 420 - 424, Change the internal lock-failure diagnostics in the local-dev capability paths, including the branches near the result staging lock and the propagated persistence failure, from warn! to debug!. Preserve the existing best-effort staging and persistence-error behavior unchanged.Sources: Coding guidelines, Path instructions
crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs (1)
179-206: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash ast-grep outline crates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rs --items all rg -n -C3 'additional_capability_grants|CapabilityGrant|synthetic.*grant' \ crates/ironclaw_reborn_composition/tests \ crates/ironclaw_reborn_composition/src/runtime/local_devRepository: nearai/ironclaw
Length of output: 21755
🏁 Script executed:
#!/bin/bash set -euo pipefail wc -l crates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rs \ crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs \ crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs echo '--- support outline ---' ast-grep outline crates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rs --view expanded | sed -n '1,220p' echo '--- tests around synthetic grant-related cases ---' sed -n '400,820p' crates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rs sed -n '420,780p' crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rsRepository: nearai/ironclaw
Length of output: 34784
🏁 Script executed:
#!/bin/bash set -euo pipefail # Focus on the local-dev refresh tests that build via the real caller path. rg -n -C4 'additional_capability_grants|synthetic|grant|capability_id_filter|visible_capabilities|build.*port|refreshing_local_dev_capability_port' \ crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs \ crates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rsRepository: nearai/ironclaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- synthetic grant plumbing ---' rg -n -C3 'additional_capability_grants|with_additional_capability_grants|synthetic grant|same-id builtin grant|overlays network\+secrets' \ crates/ironclaw_reborn_composition/src/runtime/local_dev \ crates/ironclaw_reborn_composition/tests echo '--- support file tail around the multi-entry test and any later tests ---' sed -n '756,888p' crates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rs echo '--- source file around the overlay logic ---' sed -n '170,210p' crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rsRepository: nearai/ironclaw
Length of output: 15536
Add caller-level tests for both synthetic-grant branches
crates/ironclaw_reborn_composition/tests/refreshing_capability_port_test_support.rscoversproject_create, filtering, mounts, and provider trust, buttest_parts()still hardcodesadditional_capability_grants: Vec::new(). That leaves both paths incrates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs:179-206untested: same-id overlay and missing-id insertion. Add one port-level test that seeds each mode through the real factory path.🤖 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/local_dev/refreshing_capability_port.rs` around lines 179 - 206, Add a port-level test in refreshing_capability_port_test_support.rs that configures test_parts() through the real factory path with additional_capability_grants containing both a grant matching an existing capability and a grant with a missing capability ID. Assert the resulting request overlays network and secrets for the matching grant and inserts the missing synthetic grant, covering both branches in the additional_capability_grants handling.Source: Path instructions
crates/ironclaw_reborn_composition/src/test_support/refreshing_capability_port.rs (1)
137-143: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the helper’s
Nonecontract.The implementation returns
Noneonly whenlocal_runtimeorextension_managementis absent. With no active extension, it returnsSome(handle)and the later snapshot is empty. Document that distinction so harnesses retain refresh behavior after activation.Proposed fix
-/// `None` when the services were built without a local-dev runtime (mirrors -/// `local_dev_active_extension_authority_for_test`'s `None`-propagation -/// shape), OR when no extension is currently active (matches production: -/// `LocalDevExtensionSurfaceSource::new` accepts the port either way and -/// `snapshot()` just returns an empty surface); tests that never +/// Returns `None` when the services were built without a local-dev runtime or +/// without extension management. When no extension is active, this still +/// returns `Some(..)`; the production surface snapshot is simply empty. Tests that never📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./// Returns `None` when the services were built without a local-dev runtime or /// without extension management. When no extension is active, this still /// returns `Some(..)`; the production surface snapshot is simply empty. Tests that never /// install/activate an extension can also just omit this call and leave the /// field `None` for the same no-op 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/test_support/refreshing_capability_port.rs` around lines 137 - 143, Update the helper documentation around the local-dev extension port to distinguish missing local_runtime or extension_management, which return None, from having no active extension, which still returns Some(handle) with an initially empty snapshot. State that harnesses should retain the handle to preserve refresh behavior when an extension is activated later.tests/integration/support/harness/profiles/file.rs (1)
39-61: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Keep the approval-profile user typed.
This new internal helper accepts
user_id: &str, while callers already holdUserIdand immediately round-trip throughas_str()before reconstructing it. Use&UserIdorUserIdthroughout, includingbinary_e2e.rsLine [327].Proposed fix
pub(crate) fn file_tools_requiring_approval_profile() -> HarnessResult<ToolsProfile> { - file_tools_requiring_approval_profile_for_user("reborn-e2e-builtin-user") + file_tools_requiring_approval_profile_for_user( + &UserId::new("reborn-e2e-builtin-user")?, + ) } pub(crate) fn file_tools_requiring_approval_profile_for_user( - user_id: &str, + user_id: &UserId, ) -> HarnessResult<ToolsProfile> { Ok(file_tools_with_runtime_policy(None)? - .with_user_id(UserId::new(user_id)?) + .with_user_id(user_id.clone()) .with_auto_approve_default(false)) }As per path instructions, internal identifiers must use specialized types rather than raw strings.
🤖 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 `@tests/integration/support/harness/profiles/file.rs` around lines 39 - 61, Update file_tools_requiring_approval_profile_for_user to accept a typed UserId reference or value instead of &str, and pass it directly to with_user_id without reconstructing it. Adjust file_tools_requiring_approval_profile and the binary_e2e.rs caller near the referenced usage to retain and pass UserId values throughout, removing any as_str() round trips.Source: Path instructions
…rt factory Replace the hand-rebuilt capability-port assembly in HostRuntimeCapabilityHarness with the real production factory (create_refreshing_local_dev_capability_port) via a test-support seam, so every production wrap layer (synthetic capabilities, surface disclosure, external tools, StaleSurface refresh, shared capability io) is exercised by the integration fleet. Extension-lane regression fixes (switched harness dropped github/mcp/ web-access capabilities): - expose production's extension_surface_source on RefreshingLocalDevCapabilityPortTestParts via an opaque ExtensionManagementTestHandle + build_local_dev_extension_management_for_test, wrapping the SAME LocalDevExtensionSurfaceSource::new(...) production's capability_wiring calls (runtime/local_dev.rs:132) - additional_capability_grants seam: hand-minted Extension-principal grants for ad-hoc test-only HostRuntime backends (mock MCP, GitHub/web-access WASM) that never go through a real extension activation; overlays network/secrets onto same-id builtin grants, inserts missing ones, never touches extension-surface grants - grant-constraint mount overrides: capability_execution_mount_overrides now also applies to grant constraints (memory capability family) - RecordingApprovalRequestStore reinstated around the parts' approval store so port-level synthetic gates (outbound_delivery_target_set) reach approve/deny_local_dev_gate bookkeeping - primary-provider trust entry restored for non-builtin providers only (overwrite semantics are pinned; a narrow profile effect list must not clobber the builtin baseline ceiling) - golden payload snapshots re-approved: production factory always surfaces the synthetic builtin.project_create capability - webui automations scenarios: caller now the binding subject (the production port resolves the execution user owner-first; the old fixed-constructor-user override was a pre-port harness quirk) Full reborn integration fleet green: 48/48 binaries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… trim comments Review fixes for PR-B (both reviewers, consolidated): - capability_id_filter is now Option<HashSet<CapabilityId>>: None = no filtering (production), Some(set) retains exactly set, including Some(empty) = zero grants. The attachment profile's empty allowlist no longer silently reverses into the full 34-capability surface; the image_attachment golden snapshot is re-approved to the honest minimal surface (capability_info + builtin.project_create). New crate test pins Some(empty) -> zero granted capabilities. - Delete the three orphaned test-support wrap chains (wrap_project_create/skill_activation/outbound_delivery _capabilit(y|ies)_for_test) end to end, plus the now-unused OutboundDeliveryCapabilityTestParts; kept capability-id constants that still have callers. - Trim oversized block comments to the 2-3 sentence standard; the config field doc-comments are the single canonical home per invariant. - Re-add RecordingApprovalRequestStore to tests/integration/CLAUDE.md's doubles list (dropped accidentally). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extends surface_disclosure.rs with positive/negative pins for the SHELL_CAPABILITY_ID branch in apply_to_surface_fields, which appends its note unconditionally once the port is enabled (confirmed /host mount), unlike the scoped-path capabilities' gated note. Also completes the standard test-support doc sentence on ExtensionManagementTestHandle::extension_management. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ject
RebornBinaryE2EHarness::with_host_runtime_file_capabilities_requiring_approval
submits turns as a fixed "alice" actor, but disabled global auto-approve
only at the harness's own fixed user_id ("reborn-e2e-builtin-user"). Since
the production capability port (now wired in via the harness-port switch)
resolves the dispatch/approval scope owner-first from the turn's real
binding subject, the disable never reached the scope the run actually
checked, and default-ON global auto-approve silently completed the turn
instead of raising BlockedApproval.
Adds file_tools_requiring_approval_profile_for_user (mirrors
extension_lifecycle_tools_profile_for_user) and has the binary-e2e
constructor resolve+build under the real subject user, fixing
reborn_qa_connect_flows (8 tests) and
reborn_trace_file_write_local_dev_approval_gate_bubbles.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cked ceilings CodeRabbit flagged (PR #6026) that the harness's additional_provider_trust blanket entries (e.g. bundled_extension_provider_trust()'s user_trusted, unbounded ceiling) overwrite a real production trust decision once a provider is activated through extension_management, since the config field's overwrite semantics apply last. Gate the synthetic-entry merge on `reborn_services.is_none()` (the genuinely ad-hoc mock-mcp/standalone github/web-access harnesses) so any harness with a wired extension_management leaves activation-backed providers (gmail, github, visprobe, ...) to production's extension_surface.provider_trust() alone. Extracted the merge into build_additional_provider_trust so the invariant is unit-testable without a full async harness/tokio runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Post-rebase alignment of the harness-port seam onto main's #5902 (durable tool-result projection + builtin.result_read synthetic): - RefreshingLocalDevCapabilityPortTestParts carries thread_service (required by result_read); the harness feeds its durable-io thread service when opted in, else an inert InMemorySessionThreadService. - RecordingCapabilityResultWriter wraps trait-object input_resolver + result_writer views of one shared io (was a concrete ProductLiveCapabilityIo), so the durable-io swap stays recordable. - build_additional_provider_trust gate is now per-provider (activation_backed_providers from local_dev_active_extension_authority_for_test) instead of a blanket reborn_services.is_some(): a publish-shortcut-only provider (github fixture, visprobe) has no production trust decision and must still get its synthetic entry; new regression test pins that shape. - result_read excluded from PR-A capability-filter assertions and pinned as filter-bypassing in the Some(empty) crate test. - Golden payload snapshots re-approved: the only delta in all five is builtin.result_read joining the capability surface + tools list, the documented #5902 semantic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… ref Three golden_payload snaps pinned a stale surface sha256 (2b9db23e) left from a mid-commit snapshot pass; model-visible payload is byte-identical to the current run (verified token-by-token), only the version digest over non-rendered surface inputs moved. Also repoints a result_read.rs doc comment at the removed wrap_project_create_capability_for_test to the surviving RefreshingLocalDevCapabilityPortTestParts same-io rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrypark133
force-pushed
the
harness-port-switch
branch
from
July 13, 2026 16:29
882fb45 to
c73857f
Compare
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6026
July 13, 2026 16:29
Destroyed
github-actions
Bot
added
size: XL
This was referenced Jul 15, 2026
29 tasks
27 tasks
29 tasks
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
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
PR-B of the harness-port-seam plan (PR-A: #5950). The integration harness's
create_recording_capability_portno longer hand-assembles the capability-port wrapper stack — it feeds the harness's parts into production'screate_refreshing_local_dev_capability_portand wraps the result in the recording decorator. One composition path: every production port layer (synthetics, surface disclosure, external tools, StaleSurface refresh, trajectory hook) now executes in every harness test, and future port-layer additions arrive in the harness automatically. This closes the gap class that forced #5902'sresult_readdown to crate-tier coverage.31 files, +880/−735. Deleted outright:
apply_synthetic_capability_wrappers,build_recording_capability_port_snapshot,RefreshingHostRuntimeHarnessCapabilityPort,capability_grants, and three now-orphanedwrap_*_for_testchains end-to-end (grep-verified zero call sites).Test-support seam extensions (all empty/None-by-default at the sole production call site — zero production behavior change)
Beyond PR-A's three fields, implementation surfaced two more injection points the old harness's hand-built authority provided implicitly:
additional_capability_grants(4th documented knob, architecture.md smell feat: adding Web UI #2 exception): ad-hoc testHostRuntimebackends (mock MCP, GitHub WASM, web-access) register capabilities without real extension activation, so production's factory grants them nothing; grants must be minted forPrincipal::Extension(loop_driver_execution_extension_id(..))to match production's authorization check. No other seam exists — grants are baked into the visible request at construction.extension_managementhandle (opaqueExtensionManagementTestHandle, mirrors theSkillActivationTestSourcepattern) so activated-extension grants/trust fold in via the realLocalDevExtensionSurfaceSource.capability_id_filteris now tri-state (Option<HashSet<_>>):None= production (no filtering),Some(set)= exhaustive allowlist, includingSome(empty)= zero grants — so a profile documented as "no first-party dispatch" (attachments) keeps a minimal surface instead of silently inheriting the full builtin catalog. Pinned bycapability_id_filter_some_empty_grants_zero_capabilities.New regression pin
tests/integration/surface_disclosure.rs(reborn_integration_surface_disclosure, 12 tests): pinswrap_local_dev_surface_disclosure(scoped-roots description annotation — the test corrects the plan's earlier "deny-layer" framing), positive/negative controls, plus a shared-LocalDevCapabilityIoinput-ref/result-ref round trip. Verified RED on the old harness for the right reason before the switch.Intentional behavior changes (production-true; enumerated per the plan's migration rule)
builtin.project_createis now always in the model surface (production wraps it unconditionally) — 4 golden snapshots re-approved for exactly this one-capability diff.golden_payload__image_attachment.snap: re-approved tocapability_info+project_createonly (the tri-state fix keeps the attachment profile's zero-dispatch design; an earlier draft wrongly inflated it to the full catalog — caught in review).RefreshingLocalDevCapabilityPort(StaleSurface refresh now real in tests).Verification
reborn_integration_*/reborn_group_*binaries green, run individually (13 were red mid-migration; every fix addressed a real seam gap — no assertion weakened;wiring_paritygreen).cargo test -p ironclaw_reborn_composition --features test-supportgreen;cargo clippy --all --benches --tests --examples --all-featureszero warnings;cargo build --tests --all-featuresgreen.🤖 Generated with Claude Code