Skip to content

Commit 831a9b3

Browse files
arvinxxarvinxx
authored
🐛 fix: fix group broadcast trigger tool use (#11646)
* fix broadcast issue * fix broadcast * fix broadcast * fix group slug
1 parent ad32a61 commit 831a9b3

File tree

8 files changed

+417
-70
lines changed

8 files changed

+417
-70
lines changed

‎packages/agent-runtime/src/groupOrchestration/GroupOrchestrationSupervisor.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ export class GroupOrchestrationSupervisor implements IGroupOrchestrationSupervis
7676
return {
7777
payload: {
7878
agentIds: params.agentIds as string[],
79+
// Broadcast agents should not call tools by default
80+
disableTools: true,
7981
instruction: params.instruction as string | undefined,
8082
toolMessageId: params.toolMessageId as string,
8183
},

‎packages/agent-runtime/src/groupOrchestration/__tests__/GroupOrchestrationSupervisor.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ describe('GroupOrchestrationSupervisor', () => {
9696
});
9797
});
9898

99-
it('should return parallel_call_agents instruction for broadcast decision', async () => {
99+
it('should return parallel_call_agents instruction for broadcast decision with disableTools: true', async () => {
100100
const supervisor = new GroupOrchestrationSupervisor(defaultConfig);
101101
const state = createMockState();
102102

@@ -119,6 +119,8 @@ describe('GroupOrchestrationSupervisor', () => {
119119
type: 'parallel_call_agents',
120120
payload: {
121121
agentIds: ['agent-1', 'agent-2'],
122+
// Broadcast agents should have tools disabled by default
123+
disableTools: true,
122124
instruction: 'Discuss',
123125
toolMessageId: 'tool-msg-1',
124126
},

‎packages/agent-runtime/src/groupOrchestration/types.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ export interface SupervisorInstructionCallAgent {
3232
export interface SupervisorInstructionParallelCallAgents {
3333
payload: {
3434
agentIds: string[];
35+
/**
36+
* Whether to disable tools for broadcast agents
37+
* When true, agents will respond without calling any tools
38+
*/
39+
disableTools?: boolean;
3540
instruction?: string;
3641
/**
3742
* The tool message ID that triggered the broadcast

‎src/services/chat/mecha/agentConfigResolver.test.ts‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,96 @@ describe('resolveAgentConfig', () => {
651651
);
652652
});
653653

654+
describe('supervisor with own slug (priority check)', () => {
655+
// This tests the fix for LOBE-4127: When supervisor agent has its own slug,
656+
// it should still use 'group-supervisor' slug when in group scope
657+
658+
it('should use group-supervisor slug even when agent has its own slug in group scope', () => {
659+
// Supervisor agent has its own slug (e.g., from being a builtin agent)
660+
vi.spyOn(agentSelectors.agentSelectors, 'getAgentSlugById').mockReturnValue(
661+
() => 'some-agent-slug',
662+
);
663+
664+
// Mock: groupById returns the group
665+
vi.spyOn(agentGroupSelectors.agentGroupByIdSelectors, 'groupById').mockReturnValue(
666+
() => mockGroupWithSupervisor as any,
667+
);
668+
669+
vi.spyOn(agentGroupSelectors.agentGroupSelectors, 'getGroupMembers').mockReturnValue(
670+
() =>
671+
[
672+
{ id: 'member-agent-1', title: 'Agent 1' },
673+
{ id: 'member-agent-2', title: 'Agent 2' },
674+
] as any,
675+
);
676+
677+
vi.spyOn(builtinAgents, 'getAgentRuntimeConfig').mockReturnValue({
678+
chatConfig: { enableHistoryCount: false },
679+
plugins: [GroupManagementIdentifier, GTDIdentifier],
680+
systemRole: 'You are a group supervisor...',
681+
});
682+
683+
const result = resolveAgentConfig({
684+
agentId: 'supervisor-agent-id',
685+
groupId: 'group-123',
686+
scope: 'group', // Key: must be 'group' scope
687+
});
688+
689+
// Should use group-supervisor, NOT the agent's own slug
690+
expect(result.isBuiltinAgent).toBe(true);
691+
expect(result.slug).toBe('group-supervisor');
692+
expect(result.plugins).toContain(GroupManagementIdentifier);
693+
});
694+
695+
it('should use agent own slug when scope is not group', () => {
696+
// Supervisor agent has its own slug
697+
vi.spyOn(agentSelectors.agentSelectors, 'getAgentSlugById').mockReturnValue(
698+
() => 'some-agent-slug',
699+
);
700+
701+
// Mock: groupById returns the group
702+
vi.spyOn(agentGroupSelectors.agentGroupByIdSelectors, 'groupById').mockReturnValue(
703+
() => mockGroupWithSupervisor as any,
704+
);
705+
706+
vi.spyOn(builtinAgents, 'getAgentRuntimeConfig').mockReturnValue({
707+
plugins: ['agent-specific-plugin'],
708+
systemRole: 'Agent specific system role',
709+
});
710+
711+
const result = resolveAgentConfig({
712+
agentId: 'supervisor-agent-id',
713+
groupId: 'group-123',
714+
scope: 'main', // Not 'group' scope
715+
});
716+
717+
// Should use agent's own slug since scope is not 'group'
718+
expect(result.isBuiltinAgent).toBe(true);
719+
expect(result.slug).toBe('some-agent-slug');
720+
});
721+
722+
it('should use agent own slug when groupId is not provided', () => {
723+
// Supervisor agent has its own slug
724+
vi.spyOn(agentSelectors.agentSelectors, 'getAgentSlugById').mockReturnValue(
725+
() => 'some-agent-slug',
726+
);
727+
728+
vi.spyOn(builtinAgents, 'getAgentRuntimeConfig').mockReturnValue({
729+
plugins: ['agent-specific-plugin'],
730+
systemRole: 'Agent specific system role',
731+
});
732+
733+
const result = resolveAgentConfig({
734+
agentId: 'supervisor-agent-id',
735+
scope: 'group', // Even with group scope, no groupId
736+
});
737+
738+
// Should use agent's own slug since no groupId
739+
expect(result.isBuiltinAgent).toBe(true);
740+
expect(result.slug).toBe('some-agent-slug');
741+
});
742+
});
743+
654744
it('should detect supervisor agent using groupId for direct lookup', () => {
655745
// Mock: groupById returns the group
656746
vi.spyOn(agentGroupSelectors.agentGroupByIdSelectors, 'groupById').mockReturnValue(
@@ -676,6 +766,7 @@ describe('resolveAgentConfig', () => {
676766
const result = resolveAgentConfig({
677767
agentId: 'supervisor-agent-id',
678768
groupId: 'group-123',
769+
scope: 'group', // Required: supervisor detection only works in group scope
679770
});
680771

681772
expect(result.isBuiltinAgent).toBe(true);
@@ -710,6 +801,7 @@ describe('resolveAgentConfig', () => {
710801
resolveAgentConfig({
711802
agentId: 'supervisor-agent-id',
712803
groupId: 'group-123',
804+
scope: 'group', // Required: supervisor detection only works in group scope
713805
});
714806

715807
expect(getAgentRuntimeConfigSpy).toHaveBeenCalledWith(
@@ -789,6 +881,7 @@ describe('resolveAgentConfig', () => {
789881
const result = resolveAgentConfig({
790882
agentId: 'supervisor-agent-id',
791883
groupId: 'group-123',
884+
scope: 'group', // Required: supervisor detection only works in group scope
792885
});
793886

794887
// Should correctly identify as builtin supervisor agent
@@ -913,4 +1006,108 @@ describe('resolveAgentConfig', () => {
9131006
expect(result.plugins).toContain('lobe-gtd');
9141007
});
9151008
});
1009+
1010+
describe('disableTools (broadcast scenario)', () => {
1011+
beforeEach(() => {
1012+
vi.spyOn(agentSelectors.agentSelectors, 'getAgentSlugById').mockReturnValue(() => undefined);
1013+
});
1014+
1015+
it('should return empty plugins when disableTools is true for regular agent', () => {
1016+
vi.spyOn(agentSelectors.agentSelectors, 'getAgentConfigById').mockReturnValue(
1017+
() =>
1018+
({
1019+
...mockAgentConfig,
1020+
plugins: ['plugin-a', 'plugin-b', 'lobe-gtd'],
1021+
}) as any,
1022+
);
1023+
1024+
const result = resolveAgentConfig({
1025+
agentId: 'test-agent',
1026+
disableTools: true,
1027+
});
1028+
1029+
expect(result.plugins).toEqual([]);
1030+
});
1031+
1032+
it('should keep plugins when disableTools is false', () => {
1033+
const result = resolveAgentConfig({
1034+
agentId: 'test-agent',
1035+
disableTools: false,
1036+
});
1037+
1038+
expect(result.plugins).toEqual(['plugin-a', 'plugin-b']);
1039+
});
1040+
1041+
it('should keep plugins when disableTools is undefined', () => {
1042+
const result = resolveAgentConfig({ agentId: 'test-agent' });
1043+
1044+
expect(result.plugins).toEqual(['plugin-a', 'plugin-b']);
1045+
});
1046+
1047+
it('should return empty plugins for builtin agent when disableTools is true', () => {
1048+
vi.spyOn(agentSelectors.agentSelectors, 'getAgentSlugById').mockReturnValue(
1049+
() => 'some-builtin-slug',
1050+
);
1051+
vi.spyOn(builtinAgents, 'getAgentRuntimeConfig').mockReturnValue({
1052+
plugins: ['runtime-plugin-1', 'runtime-plugin-2'],
1053+
systemRole: 'Runtime system role',
1054+
});
1055+
1056+
const result = resolveAgentConfig({
1057+
agentId: 'builtin-agent',
1058+
disableTools: true,
1059+
});
1060+
1061+
expect(result.plugins).toEqual([]);
1062+
expect(result.isBuiltinAgent).toBe(true);
1063+
});
1064+
1065+
it('should return empty plugins in page scope when disableTools is true', () => {
1066+
vi.spyOn(builtinAgents, 'getAgentRuntimeConfig').mockReturnValue({
1067+
plugins: [PageAgentIdentifier],
1068+
systemRole: 'Page agent system role',
1069+
});
1070+
1071+
const result = resolveAgentConfig({
1072+
agentId: 'test-agent',
1073+
disableTools: true,
1074+
scope: 'page',
1075+
});
1076+
1077+
// disableTools should override page scope injection
1078+
expect(result.plugins).toEqual([]);
1079+
});
1080+
1081+
it('should take precedence over isSubTask filtering', () => {
1082+
vi.spyOn(agentSelectors.agentSelectors, 'getAgentConfigById').mockReturnValue(
1083+
() =>
1084+
({
1085+
...mockAgentConfig,
1086+
plugins: ['lobe-gtd', 'plugin-a'],
1087+
}) as any,
1088+
);
1089+
1090+
const result = resolveAgentConfig({
1091+
agentId: 'test-agent',
1092+
disableTools: true,
1093+
isSubTask: true,
1094+
});
1095+
1096+
// disableTools should result in empty plugins regardless of isSubTask
1097+
expect(result.plugins).toEqual([]);
1098+
});
1099+
1100+
it('should preserve agentConfig and chatConfig when disableTools is true', () => {
1101+
const result = resolveAgentConfig({
1102+
agentId: 'test-agent',
1103+
disableTools: true,
1104+
});
1105+
1106+
// Only plugins should be empty, other config should be preserved
1107+
expect(result.plugins).toEqual([]);
1108+
expect(result.agentConfig).toEqual(mockAgentConfig);
1109+
expect(result.chatConfig).toEqual(mockChatConfig);
1110+
expect(result.isBuiltinAgent).toBe(false);
1111+
});
1112+
});
9161113
});

‎src/services/chat/mecha/agentConfigResolver.ts‎

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ export interface AgentConfigResolverContext {
5151
/** Agent ID to resolve config for */
5252
agentId: string;
5353

54+
/**
55+
* Whether to disable all tools for this agent execution.
56+
* When true, returns empty plugins array (used for broadcast scenarios).
57+
*/
58+
disableTools?: boolean;
59+
5460
// Builtin agent specific context
5561
/** Document content for page-agent */
5662
documentContent?: string;
@@ -112,13 +118,27 @@ export interface ResolvedAgentConfig {
112118
* For regular agents, this simply returns the config from the store.
113119
*/
114120
export const resolveAgentConfig = (ctx: AgentConfigResolverContext): ResolvedAgentConfig => {
115-
const { agentId, model, documentContent, plugins, targetAgentConfig, isSubTask } = ctx;
116-
117-
log('resolveAgentConfig called with agentId: %s, scope: %s, isSubTask: %s', agentId, ctx.scope, isSubTask);
118-
119-
// Helper to filter out lobe-gtd in sub-task context to prevent nested sub-task creation
120-
const applySubTaskFilter = (pluginIds: string[]) =>
121-
isSubTask ? pluginIds.filter((id) => id !== 'lobe-gtd') : pluginIds;
121+
const { agentId, model, documentContent, plugins, targetAgentConfig, isSubTask, disableTools } =
122+
ctx;
123+
124+
log(
125+
'resolveAgentConfig called with agentId: %s, scope: %s, isSubTask: %s, disableTools: %s',
126+
agentId,
127+
ctx.scope,
128+
isSubTask,
129+
disableTools,
130+
);
131+
132+
// Helper to apply plugin filters:
133+
// 1. If disableTools is true, return empty array (for broadcast scenarios)
134+
// 2. If isSubTask is true, filter out lobe-gtd to prevent nested sub-task creation
135+
const applyPluginFilters = (pluginIds: string[]) => {
136+
if (disableTools) {
137+
log('disableTools is true, returning empty plugins');
138+
return [];
139+
}
140+
return isSubTask ? pluginIds.filter((id) => id !== 'lobe-gtd') : pluginIds;
141+
};
122142

123143
const agentStoreState = getAgentStoreState();
124144

@@ -130,18 +150,18 @@ export const resolveAgentConfig = (ctx: AgentConfigResolverContext): ResolvedAge
130150
const basePlugins = agentConfig.plugins ?? [];
131151

132152
// Check if this is a builtin agent
133-
// First check agent store, then check if this is a supervisor agent via groupId
134-
let slug = agentSelectors.getAgentSlugById(agentId)(agentStoreState);
135-
log('slug from agentStore: %s (agentId: %s)', slug, agentId);
153+
// Priority: supervisor check (when in group scope) > agent store slug
154+
let slug: string | undefined;
136155

137-
// If not found in agent store, check if this is a supervisor agent using groupId
138-
// This is more reliable than iterating all groups to find a match
139-
if (!slug && ctx.groupId) {
156+
// IMPORTANT: When in group scope with groupId, check if this agent is the group's supervisor FIRST
157+
// This takes priority because supervisor needs special group-supervisor behavior,
158+
// even if the agent has its own slug
159+
if (ctx.groupId && ctx.scope === 'group') {
140160
const groupStoreState = getChatGroupStoreState();
141161
const group = agentGroupByIdSelectors.groupById(ctx.groupId)(groupStoreState);
142162

143163
log(
144-
'checking supervisor via groupId %s: group=%o',
164+
'checking supervisor FIRST (scope=group): groupId=%s, group=%O, agentId=%s',
145165
ctx.groupId,
146166
group
147167
? {
@@ -150,6 +170,7 @@ export const resolveAgentConfig = (ctx: AgentConfigResolverContext): ResolvedAge
150170
title: group.title,
151171
}
152172
: null,
173+
agentId,
153174
);
154175

155176
// Check if this agent is the supervisor of the specified group
@@ -164,6 +185,12 @@ export const resolveAgentConfig = (ctx: AgentConfigResolverContext): ResolvedAge
164185
}
165186
}
166187

188+
// If not identified as supervisor, check agent store for slug
189+
if (!slug) {
190+
slug = agentSelectors.getAgentSlugById(agentId)(agentStoreState) ?? undefined;
191+
log('slug from agentStore: %s (agentId: %s)', slug, agentId);
192+
}
193+
167194
if (!slug) {
168195
log('agentId %s is not a builtin agent (no slug found)', agentId);
169196
// Regular agent - use provided plugins if available, fallback to agent's plugins
@@ -209,7 +236,7 @@ export const resolveAgentConfig = (ctx: AgentConfigResolverContext): ResolvedAge
209236
agentConfig: finalAgentConfig,
210237
chatConfig: finalChatConfig,
211238
isBuiltinAgent: false,
212-
plugins: applySubTaskFilter(pageAgentPlugins),
239+
plugins: applyPluginFilters(pageAgentPlugins),
213240
};
214241
}
215242

@@ -218,7 +245,7 @@ export const resolveAgentConfig = (ctx: AgentConfigResolverContext): ResolvedAge
218245
agentConfig: finalAgentConfig,
219246
chatConfig: finalChatConfig,
220247
isBuiltinAgent: false,
221-
plugins: applySubTaskFilter(finalPlugins),
248+
plugins: applyPluginFilters(finalPlugins),
222249
};
223250
}
224251

@@ -339,7 +366,7 @@ export const resolveAgentConfig = (ctx: AgentConfigResolverContext): ResolvedAge
339366
agentConfig: finalAgentConfig,
340367
chatConfig: resolvedChatConfig,
341368
isBuiltinAgent: true,
342-
plugins: applySubTaskFilter(finalPlugins),
369+
plugins: applyPluginFilters(finalPlugins),
343370
slug,
344371
};
345372
};

0 commit comments

Comments
 (0)