Repository navigation
fix active channel extension search guidance - #6653
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
🔎 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. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughSearch guidance now distinguishes setup-needed external channels from active ready results. Predicate logic, routing, and related tests are updated in ChangesExternal channel guidance
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size: S
![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 | bd25dc0bb3c8 |
Head: bd25dc0bb3c85967f4c23f0e3e913cc169a29682
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 active-channel regression reaches the lifecycle facade, but the retained setup-needed path is only helper-tested and does not verify the emitted guidance branch.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Exercise setup-needed channel guidance through the facade
Location: crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rs:3387
This test only invokes the classifier with a fabricated payload. It does not execute ExtensionManagementPort::search, which is where the classifier selects and attaches the model-visible setup/install message. Add a caller-level regression with an external channel that remains SetupNeeded, then assert its search response retains the incomplete-setup and builtin.extension_install guidance.
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.
|
|
||
| #[test] | ||
| fn installed_external_channel_search_result_gets_activation_guidance() { | ||
| fn setup_needed_external_channel_search_result_gets_activation_guidance() { |
There was a problem hiding this comment.
This only tests the classifier, not the search branch that emits the model-visible setup/install guidance. Please add a facade-level SetupNeeded external-channel search regression asserting the incomplete-setup and builtin.extension_install message.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rs`:
- Around line 3070-3083: Update
extension_search_has_setup_needed_external_channel_result so it only matches
SetupNeeded channel extensions whose credential_requirements are empty; preserve
the existing false result for non-ExtensionSearch payloads and leave
credential-gated extensions out of the channel-specific guidance.
🪄 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: d7751aa2-ab3c-4337-90e4-30a9b77367fb
📒 Files selected for processing (1)
crates/ironclaw_reborn_composition/src/extension_host/extension_lifecycle.rs
| fn extension_search_has_setup_needed_external_channel_result( | ||
| payload: Option<&LifecycleProductPayload>, | ||
| ) -> bool { | ||
| let Some(LifecycleProductPayload::ExtensionSearch { extensions, .. }) = payload else { | ||
| return false; | ||
| }; | ||
| extensions.iter().any(|extension| { | ||
| matches!( | ||
| extension.installation_phase, | ||
| Some(LifecyclePublicState::SetupNeeded | LifecyclePublicState::Active) | ||
| ) && extension | ||
| .summary | ||
| .surface_kinds | ||
| .contains(&CapabilitySurfaceKind::Channel) | ||
| extension.installation_phase == Some(LifecyclePublicState::SetupNeeded) | ||
| && extension | ||
| .summary | ||
| .surface_kinds | ||
| .contains(&CapabilitySurfaceKind::Channel) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the credential-requirements guard.
This predicate now classifies every SetupNeeded channel as personal connection setup, including results with unresolved credential_requirements. Restore the empty-requirements check so credential-gated extensions do not receive the channel-specific builtin.extension_install guidance.
Proposed fix
extensions.iter().any(|extension| {
extension.installation_phase == Some(LifecyclePublicState::SetupNeeded)
+ && extension.summary.credential_requirements.is_empty()
&& extension
.summary
.surface_kindsBased on the supplied lifecycle guidance contract, this branch must remain limited to setup-needed channels without package credential requirements.
📝 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.
| fn extension_search_has_setup_needed_external_channel_result( | |
| payload: Option<&LifecycleProductPayload>, | |
| ) -> bool { | |
| let Some(LifecycleProductPayload::ExtensionSearch { extensions, .. }) = payload else { | |
| return false; | |
| }; | |
| extensions.iter().any(|extension| { | |
| matches!( | |
| extension.installation_phase, | |
| Some(LifecyclePublicState::SetupNeeded | LifecyclePublicState::Active) | |
| ) && extension | |
| .summary | |
| .surface_kinds | |
| .contains(&CapabilitySurfaceKind::Channel) | |
| extension.installation_phase == Some(LifecyclePublicState::SetupNeeded) | |
| && extension | |
| .summary | |
| .surface_kinds | |
| .contains(&CapabilitySurfaceKind::Channel) | |
| }) | |
| } | |
| fn extension_search_has_setup_needed_external_channel_result( | |
| payload: Option<&LifecycleProductPayload>, | |
| ) -> bool { | |
| let Some(LifecycleProductPayload::ExtensionSearch { extensions, .. }) = payload else { | |
| return false; | |
| }; | |
| extensions.iter().any(|extension| { | |
| extension.installation_phase == Some(LifecyclePublicState::SetupNeeded) | |
| && extension.summary.credential_requirements.is_empty() | |
| && extension | |
| .summary | |
| .surface_kinds | |
| .contains(&CapabilitySurfaceKind::Channel) | |
| }) | |
| } |
🤖 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.rs`
around lines 3070 - 3083, Update
extension_search_has_setup_needed_external_channel_result so it only matches
SetupNeeded channel extensions whose credential_requirements are empty; preserve
the existing false result for non-ExtensionSearch payloads and leave
credential-gated extensions out of the channel-specific guidance.
|
🚅 Deployed to the ironclaw-pr-6653 environment in ironclaw-ci-preview
|
This branch was successfully deployed
1 participant
Summary
setup_neededexternal-channel search results as requiring connection setupactiveexternal-channel results through the existing ready guidancebuiltin.extension_installChange Type
Linked Issue
None.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warnings— owning-crate all-targets/all-features clippy passed; the broader workspace run was stopped after the user requested publicationcargo build— covered by the owning-crate test/clippy buildscargo test --features integrationif database-backed or integration behavior changed — not applicable: no database or integration behavior changedreview-prorpr-shepherd --fixwas run before requesting review — not run before draft publicationAdditional evidence:
active_external_channel_search_result_gets_ready_guidancefailed against the old classifier with the contradictory “personal setup is incomplete” response.cargo test -p ironclaw_reborn_composition external_channel_search_result -- --nocapture— 2 passed.cargo clippy -p ironclaw_reborn_composition --all-targets --all-features -- -D warnings— passed.scripts/pre-commit-safety.sh— passed.multi_tool_call_response_survives_surface_change_mid_registerhit its unrelated 10-second timeout under suite load, then passed in isolation in 2.24 seconds. All integration targets passed.mainhas a pre-existing test-build mismatch inruntime.rs(channel_host_assemblyvs_channel_host_assembly). Test/clippy evidence above used a temporary local one-line correction that is not included in this PR.Test Strategy
User behavior:
An active external channel is reported as ready and never receives setup-needed or reinstall guidance.
Risk areas:
Tests added or updated:
What the tests prove:
The old classifier reproduces the contradiction, the new classifier preserves setup guidance for
setup_needed, and anactivechannel returns ready guidance withoutbuiltin.extension_install.Commands run:
cargo test -p ironclaw_reborn_composition active_external_channel_search_result_gets_ready_guidance -- --nocapturecargo test -p ironclaw_reborn_composition external_channel_search_result -- --nocapturecargo test -p ironclaw_reborn_composition --no-fail-fastcargo test -p ironclaw_reborn_composition multi_tool_call_response_survives_surface_change_mid_register -- --nocapturecargo clippy -p ironclaw_reborn_composition --all-targets --all-features -- -D warningscargo fmt --all -- --checkgit diff --checkscripts/pre-commit-safety.shSecurity Impact
None. This changes only model-visible lifecycle guidance selection.
Reborn Trust-Boundary Checklist
N/A: no authority, trust-bearing types, untrusted prompt content, serialization, queues, errors, or runtime boundaries changed.
Database Impact
None.
Blast Radius
Limited to the top-level message attached to extension-search responses containing active or setup-needed external channels. Installation state and lifecycle transitions are unchanged.
Rollback Plan
Revert commit
bd25dc0bbto restore the previous classifier and guidance behavior.Review Follow-Through
The PR is intentionally limited to the guidance bug. Telegram
/startinterception and message formatting are excluded.Review track: B (product behavior bug fix)