Skip to content

Comments

fix(routing): resolve channel default account instead of hardcoded 'default'#15727

Open
FuseWeb wants to merge 1 commit intoopenclaw:mainfrom
FuseWeb:fix/binding-routing-default-account
Open

fix(routing): resolve channel default account instead of hardcoded 'default'#15727
FuseWeb wants to merge 1 commit intoopenclaw:mainfrom
FuseWeb:fix/binding-routing-default-account

Conversation

@FuseWeb
Copy link

@FuseWeb FuseWeb commented Feb 13, 2026

Summary

Fixes #9198 — binding routing fails for multi-account channels where the default account id is not literally "default".

Root Cause

matchesAccountId() in src/routing/resolve-route.ts compared the inbound account against the hardcoded DEFAULT_ACCOUNT_ID ("default") when a binding omitted accountId. This meant bindings like:

bindings:
  - agentId: bizPeer
    match:
      channel: whatsapp
      peer: { kind: direct, id: "+1000" }

…would never match on a channel whose sole (and thus default) account is named "biz".

Fix

  • Use resolveChannelDefaultAccountId from src/channels/plugins/helpers.ts to determine the channel's actual default account id.
  • Normalize both sides of the comparison via normalizeAccountId from session-key.ts.
  • Added regression test for the non-literal default account scenario.

Testing

  • All 30 existing + 1 new test pass (pnpm test src/routing/resolve-route.test.ts)
  • AI-assisted (Claude, fully tested)
  • I understand what this code does

Greptile Overview

Greptile Summary

This PR fixes account matching for bindings that omit accountId by resolving the channel’s actual default account id (instead of comparing against the hardcoded "default"), and adds a regression test for channels whose default account id is non-literal.

The change is implemented in src/routing/resolve-route.ts by introducing a helper that looks up the channel plugin and asks it (via resolveChannelDefaultAccountId) for the default account id, then uses that value when deciding whether a binding with no accountId matches. The test suite is extended in src/routing/resolve-route.test.ts to cover a single-account channel named "biz".

Confidence Score: 2/5

  • This PR has a real risk of breaking routing in config-only / early-init contexts due to relying on the runtime plugin registry during route resolution.
  • The functional intent looks correct and a regression test was added, but resolveAgentRoute() now imports/calls getChannelPlugin() from the runtime plugin registry module, which can throw if the registry isn’t initialized and also pulls heavy runtime dependencies into a shared routing path. That’s a behavior change that can surface at runtime outside the test case.
  • src/routing/resolve-route.ts

Last reviewed commit: ee90398

(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines 5 to 7
import { resolveChannelDefaultAccountId } from "../channels/plugins/helpers.js";
import { getChannelPlugin } from "../channels/plugins/index.js";
import { listBindings } from "./bindings.js";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heavy plugin registry import

resolve-route.ts now imports getChannelPlugin from src/channels/plugins/index.ts, which (per its own header comment) depends on the active plugin registry and is intentionally “heavy”. resolveAgentRoute() is used in tests/config-only contexts and can run before plugin registry initialization, so calling getChannelPlugin() here can throw (via requireActivePluginRegistry()) or pull in heavy runtime code during routing. Consider switching to a config-only default-account resolution path that doesn’t require plugin registry, or ensure the plugin registry is guaranteed initialized before any routing resolution runs.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/routing/resolve-route.ts
Line: 5:7

Comment:
**Heavy plugin registry import**

`resolve-route.ts` now imports `getChannelPlugin` from `src/channels/plugins/index.ts`, which (per its own header comment) depends on the *active plugin registry* and is intentionally “heavy”. `resolveAgentRoute()` is used in tests/config-only contexts and can run before plugin registry initialization, so calling `getChannelPlugin()` here can throw (via `requireActivePluginRegistry()`) or pull in heavy runtime code during routing. Consider switching to a config-only default-account resolution path that doesn’t require plugin registry, or ensure the plugin registry is guaranteed initialized before any routing resolution runs.

How can I resolve this? If you propose a fix, please make it concise.

@FuseWeb
Copy link
Author

FuseWeb commented Feb 14, 2026

Thanks for the feedback @greptile-apps!

The try-catch fallback in the latest commit handles the runtime case - if the plugin registry isn't initialized, we gracefully fall back to DEFAULT_ACCOUNT_ID, which matches the pre-PR behavior.

You're right that this adds a "heavy" import to resolve-route.ts. A cleaner architecture would be a config-only account resolution path, but that would require:

  1. Duplicating account-listing logic for each channel type
  2. Or extracting a shared lightweight config parser

For this bugfix PR, the try-catch approach seems like a reasonable tradeoff - it fixes the issue without major refactoring. Happy to discuss if maintainers prefer a different approach.

The key point: in early-init/config-only contexts, falling back to DEFAULT_ACCOUNT_ID is actually correct behavior (same as before this PR).

@FuseWeb
Copy link
Author

FuseWeb commented Feb 14, 2026

Can you suggest a fix for this @greptileai?

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 14, 2026

I see the issue now. Your reasoning about the try-catch is correct, but there's a subtle problem with the fallback behavior that undermines your fix.

When the plugin registry isn't initialized and you fall back to DEFAULT_ACCOUNT_ID ("default"), bindings with omitted accountId will again fail to match channels whose default account is non-literal (like "biz"). Here's why:

In matchesAccountId():

  • When match (binding's accountId) is empty/omitted → you check actual === channelDefaultAccountId
  • But in early-init contexts, channelDefaultAccountId falls back to "default"
  • So a binding with no accountId will only match if actual (the inbound accountId) is literally "default"

This means the bug you're fixing reappears in config-only contexts.

The real tradeoff here:

  1. Runtime contexts (plugin registry available): ✅ Fix works
  2. Early-init contexts (plugin registry unavailable): ❌ Reverts to broken behavior

A more robust fallback would inspect cfg.channels[channel].accounts directly to find the default account without needing the runtime plugin:

function resolveChannelDefaultAccountIdForRoute(cfg: OpenClawConfig, channel: string): string {
  try {
    const plugin = getChannelPlugin(channel);
    if (!plugin) {
      return DEFAULT_ACCOUNT_ID;
    }
    return normalizeAccountIdFromSessionKey(resolveChannelDefaultAccountId({ plugin, cfg }));
  } catch {
    // Plugin registry not initialized - fall back to config introspection
    const channelCfg = cfg.channels?.[channel];
    if (channelCfg?.accounts && typeof channelCfg.accounts === 'object') {
      const accountIds = Object.keys(channelCfg.accounts);
      if (accountIds.length === 1) {
        return normalizeAccountIdFromSessionKey(accountIds[0]);
      }
    }
    return DEFAULT_ACCOUNT_ID;
  }
}

This preserves your fix even when the plugin registry isn't ready yet.

@FuseWeb
Copy link
Author

FuseWeb commented Feb 14, 2026

Thanks @greptileai! Implemented your suggestion - now falls back to config introspection when plugin registry isn't available. If there's exactly one account in cfg.channels[channel].accounts, we use that as the default.

This preserves the fix even in early-init contexts. 🙏

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 14, 2026

Perfect! The implementation looks solid. You've correctly added the config fallback in resolveChannelDefaultAccountIdForRoute:

  • First attempts to use the plugin registry (getChannelPlugin + resolveChannelDefaultAccountId)
  • On failure (registry not initialized), inspects cfg.channels[channel].accounts directly
  • If exactly one account exists, uses it as the default
  • Otherwise falls back to DEFAULT_ACCOUNT_ID

This preserves the fix for the non-literal default account issue (#9198) even during early initialization when the plugin registry isn't available yet. The regression test also confirms the behavior works correctly.

Nice work on the defensive implementation! 🚀

@FuseWeb FuseWeb force-pushed the fix/binding-routing-default-account branch 2 times, most recently from c81cfc6 to ae58c7e Compare February 17, 2026 15:14
When a channel has a single account that isn't literally named 'default',
bindings with no accountId match should still route to that account.

This change:
- Adds resolveChannelDefaultAccountIdForRoute to dynamically determine the
  channel's default account
- Updates matchesAccountId to compare against the resolved default instead
  of hardcoded 'default'
- Includes channelDefaultAccountId in the binding cache key for correctness

Fixes openclaw#9198
@FuseWeb FuseWeb force-pushed the fix/binding-routing-default-account branch from ae58c7e to 18cfffd Compare February 17, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Google Chat DM routing ignores bindings — always routes to default agent main session

1 participant