Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
scope: docs
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | 10c8f362a447 |
Head: 10c8f362a447be5fd1034692b9c924e93ac96a56
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
Found one blocking issue: a WebUI v2 static-assets contract test still asserts the removed connectable-channel implementation strings, so the targeted crate test suite is left broken by this refactor.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Update stale connectable-channel static asset assertions
Location: crates/ironclaw_webui_v2/src/static_assets/assets.rs:311
The chat_omits_connect_action_while_extensions_render_slack_setup_ui test still asserts old connectable-channel strings such as showBuiltinSlackConnectActions, admin_managed_channels, findSlackConnectActions, slackConnectActions, and action=${action.action}. This PR removed that frontend path in favor of the extension-surface connection flow, so the current channels-tab.ts and slack-setup-panel.ts no longer contain these strings. As written, cargo test -p ironclaw_webui_v2 --features webui-v2-beta will fail in this test instead of validating the new surface-based behavior. Please update the assertions to match the new implementation, for example ChannelConnectSections, channelConnection, isInboundProofCodeConnection, and the no-prop SlackChannelPicker render.
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. - Use
@ironloopai statusto check queued/running/completed/stale/stalled state while reviewers run.
Inline review fallback
Inline comment projection fell back to a body-only PR Review because GitHub rejected the inline payload.
Reason: Unprocessable Entity: "Path could not be resolved" - https://docs.github.com/rest/pulls/reviews#create-a-review-for-a-pull-request
IronLoop preserved the inline review comment payloads below instead of dropping them.
Inline fallback 1: crates/ironclaw_webui_v2/src/static_assets/assets.rs:311
IronLoop reviewer: [MEDIUM] Update stale connectable-channel static asset assertions
The chat_omits_connect_action_while_extensions_render_slack_setup_ui test still asserts old connectable-channel strings such as showBuiltinSlackConnectActions, admin_managed_channels, findSlackConnectActions, slackConnectActions, and action=${action.action}. This PR removed that frontend path in favor of the extension-surface connection flow, so the current channels-tab.ts and slack-setup-panel.ts no longer contain these strings. As written, cargo test -p ironclaw_webui_v2 --features webui-v2-beta will fail in this test instead of validating the new surface-based behavior. Please update the assertions to match the new implementation, for example ChannelConnectSections, channelConnection, isInboundProofCodeConnection, and the no-prop SlackChannelPicker render.
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. - Use
@ironloopai statusto check queued/running/completed/stale/stalled state while reviewers run.
|
🚅 Deployed to the ironclaw-pr-5842 environment in ironclaw-ci-preview
|
10c8f36 to
6f820a6
Compare
2fdd730 to
6b2eb09
Compare
412810e to
9a6ef20
Compare
…le-channels rail Channel discovery is now extension-surface data, not a parallel registry. RebornExtensionInfo carries `surfaces` - a tagged enum where `channel` has typed direction (inbound = external messages arrive; outbound = the host delivers final replies/notifications, from the adapter section's InboundMessages/ExternalFinalReplyPush flags), the caller's connection state, and the connect affordance. Lifecycle summaries carry channel_directions + channel_connection, produced from the PR-1 manifest projection instead of a section re-parse. Deleted outright (no shims): ConnectableChannelsProductFacade and its DTOs, GET /api/webchat/v2/channels/connectable (route, descriptor, handler, contract rows), slack_connectable_channel.rs, SlackOperatorRouteVisibility, and the never-read channel_connection_facade_slot activation wire. The one-variant LifecycleExtensionSurfaceKind is deleted; every crate imports ironclaw_host_api::CapabilitySurfaceKind from its owner (no facade re-export). ChannelConnectionFacade survives as the caller-scoped binding seam (connection state + disconnect cleanup). External-identity binding is host-owned and product-blind: the new generic ProviderIdentityActorResolver (provider_identity.rs) is parameterized by provider/adapter-id/actor-kind data; slack_actor_identity.rs is deleted and Slack's wiring is a three-line parameterization. A new channel gets actor-to-user resolution by declaring surfaces, not by writing a resolver. Frontend: channels tab renders from installed extensions' channel surfaces; the Slack admin section self-gates on the operator-scoped setup endpoint; the vestigial action-prop chain and the connectable-channels query/invalidation are gone. NEA-25 stack PR 3. Caller-level pins: reborn_services_contract list_extensions_projects_channel_surface_with_directions_and_connection; provider_identity resolver tests; frontend channels-tab/setup-panel suites (602 tests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
9ba3bdf to
f542fc5
Compare
…#5850) Atomic roll-up of the 8-PR NEA-25 taxonomy stack onto current main. Extension is the only installable product object; tool/channel/auth are derived capability surfaces; runtime kind controls loading only; manifest projection (v2, host_api contracts) is the sole surface-discovery source of truth. The connectable-channels rail and the parallel `kind` taxonomy are removed and pinned by a zero-legacy gate. slack_bot and slack_personal are retired into one `slack` extension with bounded forward migrations. Extensions wire carries runtime + surfaces, not a conflated kind. Supersedes #5833, #5839, #5842, #5845, #5847, #5848, #5849, #5850. Conflicts with main since the train forked were reconciled preserving main's newer behavior (#5851 unified slack cleanup, #6054 get_conversation_info DM resolution, #5499 extension import, #6057 TS source conventions). provider_identity domain duplication removed; the residual is a legitimate up-layer port adapter. See the PR description for the per-PR crosswalk, resolutions, placement audit, and verification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
Stack PR 3/7 for NEA-25, on #5839. Channel discovery becomes extension-surface data; the parallel channel registry dies with no shims. Net −900 lines.
New surface model on the wire.
RebornExtensionInfo.surfacesis a tagged enum —tool/auth/channel { inbound, outbound, connected?, connection? }. Direction is typed off the product-adapter section's capability flags (inbound_messages→ inbound,external_final_reply_push→ outbound), which pins the team's channel semantics: inbound is where external messages arrive; outbound is where the host delivers final replies and notifications (never a model tool). Lifecycle summaries carrychannel_directions+channel_connection(connect strategy + copy), produced from the stack's manifest surface projection instead of a second section parse. Caller-level pin:list_extensions_projects_channel_surface_with_directions_and_connection.Deleted outright:
ConnectableChannelsProductFacade+ statics + DTOs,GET /api/webchat/v2/channels/connectable(route, descriptor, handler, contract rows, handler tests),slack_connectable_channel.rs,SlackOperatorRouteVisibility(+ serve-side computation), and thechannel_connection_facade_slotlate-binding wire — which was filled but never read (the intended activation gate was never implemented; activation attaches the connect requirement declaratively instead).LifecycleExtensionSurfaceKind(one variant) is deleted; every crate importsironclaw_host_api::CapabilitySurfaceKindfrom its owner — deliberately no facade re-export, sincewebui_v2already depends onhost_apiand the type-placement rule bans path-preservation re-exports.ChannelConnectionFacadesurvives as the real caller-scoped binding seam: per-caller connection state feedinglist_extensions, and disconnect cleanup (credential revoke → DM-target delete → identity unbind) onremove_extension.Identity binding is host-owned and product-blind (Henry's requirement): new
provider_identity.rshosts the genericProviderIdentityActorResolver, parameterized entirely by data (provider id, adapter id, actor kind) with the same installation-scoped composite key and 30s positive cache;slack_actor_identity.rsis deleted and Slack's wiring is a three-line parameterization (slack_provider_identity_actor_resolver). Adapters still extract protocol-shaped external refs; resolution/binding/scoping stay behind the conversation-binding contract, which already forbids core from parsing external IDs.Frontend: channels tab renders from installed extensions' channel surfaces (
channelSurface/channelConnectionhelpers); the Slack admin section self-gates on the operator-scoped setup endpoint (renders nothing for non-operators) instead of being toggled by a server-computed visibility enum;channel-connect.ts, the connectable-channels query, its invalidations, and the vestigialactioncopy-prop chain are gone (copy comes from i18n).Testing
cargo check --workspace --all-targets --all-features— clean;cargo clippy(full CI invocation) — zero warningsironclaw_product_workflow(incl. new surfaces pin),ironclaw_reborn_composition(1,510 lib tests incl. relocated glue + generic resolver suite),ironclaw_webui_v2(descriptor/handler contracts updated),ironclaw_extensions,ironclaw_host_runtime— green (3 pre-existing Docker-socket sandbox tests fail locally, no Docker on this machine)cargo test -p ironclaw_architecture— green after regeneratingdocs/plans/composition-pubuse.snapshot(intentional facade change: connectable exports removed,provider_identityexports added)pnpm test602/602 across 82 files (channels-tab suite rewritten to the surface model; new SlackAdminManagedSection self-gate test);tsc --noEmitcleanslack_host_betatests that drove the deleted route via oneshot were removed; operator gating of the admin surface is covered by the operator-scoped setup/channel-route tests🤖 Generated with Claude Code