Configure: defer channel status until selection#68007
Conversation
openclaw-barnacle
Bot
added
commands
There was a problem hiding this comment.
Pull request overview
This PR improves the openclaw config channel configuration UX by avoiding expensive channel probing/plugin loading before the channel picker is shown, and by making channel removal operate purely off configured openclaw.json keys.
Changes:
- Defer channel status collection and setup-plugin preloading until after a channel is selected.
- Alphabetize channel picker entries by their displayed label.
- In remove mode, list removable channels directly from
openclaw.json(cfg.channels) rather than the loaded plugin registry, including unknown/legacy keys.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/flows/channel-setup.ts | Adds deferStatusUntilSelection behavior to avoid eager status/plugin preloads before prompting. |
| src/flows/channel-setup.test.ts | Adds a regression test asserting deferred mode avoids status checks and plugin loading before selection. |
| src/flows/channel-setup.status.ts | Sorts picker contributions alphabetically and avoids repeated listChatChannels() scans. |
| src/flows/channel-setup.status.test.ts | Adds tests for alphabetical sorting and hint behavior when status is absent/present. |
| src/commands/configure.wizard.ts | Switches the wizard to use deferred channel setup (no pre-picker status note). |
| src/commands/configure.wizard.test.ts | Verifies the wizard passes deferStatusUntilSelection: true into setupChannels. |
| src/commands/configure.channels.ts | Removes channel configs by enumerating cfg.channels keys instead of loaded plugins. |
| src/commands/configure.channels.test.ts | Adds tests ensuring remove mode lists configured keys and deletes selected blocks. |
| src/channels/plugins/setup-wizard-types.ts | Extends SetupChannelsOptions with deferStatusUntilSelection. |
Greptile SummaryThis PR defers channel status collection and plugin loading in Confidence Score: 5/5Safe to merge — no P0/P1 issues; all remaining findings are P2 suggestions. The core deferral logic is correct and well-tested for the fast-exit path. The remove-wizard rewrite directly solves the empty-channel-list bug. Both sorting implementations are correct and match each other. The two P2 notes (bundled-channel config mutation in deferred mode and missing post-selection test) are worth addressing in a follow-up but do not block this change. src/flows/channel-setup.ts (post-selection deferred path), src/flows/channel-setup.test.ts (missing selection-path coverage) Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/flows/channel-setup.ts
Line: 514
Comment:
**Bundled catalog entries silently re-routed to `enableBundledPluginForSetup`**
The `origin !== "bundled"` guard changes the non-deferred path too: a bundled `installedCatalogEntry` that previously went through `loadScopedChannelPlugin(channel, installedCatalogEntry.pluginId)` now falls through to `enableBundledPluginForSetup`. In non-deferred mode this is functionally equivalent (the plugin is already visible via `getChannelSetupPlugin`, so `enableBundledPluginForSetup` exits early after `refreshStatus`), but in deferred mode the `enablePluginInConfig(next, channel)` call runs before the plugin is loaded, which may write an explicit `enabled: true` entry to the in-progress config for an already-enabled bundled plugin. Worth adding a test that selects a bundled channel in deferred mode and asserts the resulting config shape.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/flows/channel-setup.test.ts
Line: 193-228
Comment:
**Test only covers the fast-exit (no-selection) path**
The new deferred test verifies that status/plugins are not loaded when the user exits immediately with `__done__`. The complementary path — user actually selects a channel in deferred mode and runs through setup — is not covered. That path exercises `enableBundledPluginForSetup` + `loadScopedChannelPlugin` + `refreshStatus` under the deferred constraint and is the riskiest code branch introduced here. A follow-up test that selects a channel (returning `"telegram"` from `select`) and then checks that `loadChannelSetupPluginRegistrySnapshotForChannel` and `getChannelSetupPlugin` are called only for that channel would lock in the intended lazy-load behaviour.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Configure: defer channel status until se..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75d153b16c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
gumadeiras
force-pushed
the
codex/configure-lazy-channel-status
branch
3 times, most recently
from
670ee66 to
9cf0545
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dfa0e79ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟡 Terminal escape/control-sequence injection via unsanitized channel ID in setup notes
DescriptionUser-controlled/untrusted
Vulnerable code: await prompter.note(
`${channel} cannot be configured while ${disabledHint}. Enable it before setup.`,
"Channel setup",
);RecommendationSanitize any untrusted text before rendering to terminal, and prefer displaying trusted labels over raw IDs. Example fix using existing helper: import { sanitizeTerminalText } from "../terminal/safe-text.js";
const safeChannel = sanitizeTerminalText(String(channel));
const safeHint = sanitizeTerminalText(String(disabledHint));
await prompter.note(
`${safeChannel} cannot be configured while ${safeHint}. Enable it before setup.`,
"Channel setup",
);Apply the same treatment to other Analyzed PR: #68007 at commit Last updated on: 2026-04-17T17:35:54Z |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b0f73b0d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f645baddba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48e5d51a26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
gumadeiras
force-pushed
the
codex/configure-lazy-channel-status
branch
from
c31b7cd to
24cafcd
Compare
|
Merged via squash.
Thanks @gumadeiras! |
Merged via squash. Prepared head SHA: 24cafcd Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 24cafcd Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 24cafcd Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 24cafcd Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 24cafcd Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 24cafcd Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 24cafcd Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Summary
openclaw configeagerly probed every channel before the channel picker, causing slow startup and duplicate/stale status output; remove mode could also show no channels when plugins were not loaded.openclaw.json.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
Regression Test Plan (if applicable)
src/flows/channel-setup.test.ts,src/flows/channel-setup.status.test.ts,src/commands/configure.channels.test.ts,src/commands/configure.wizard.test.tsUser-visible / Behavior Changes
openclaw configreaches the channel picker without probing every channel first.openclaw.json, including unknown/legacy keys.Diagram (if applicable)
Security Impact (required)
No)No)No)No)No)Yes, explain risk + mitigation: N/ARepro + Verification
Environment
openclaw.jsonwith channel config blocksSteps
openclaw config.Expected
Actual
Evidence
Human Verification (required)
pnpm check.Review Conversations
Compatibility / Migration
Yes)No)No)Risks and Mitigations