feat: enhance team member resolution and activity components
- Introduced a new method to avoid duplicate member names in TeamMemberResolver, ensuring case-insensitive uniqueness. - Updated ActivityItem and ActivityTimeline components to utilize local member names for improved recipient qualification checks. - Added tests to validate the handling of dotted names and deduplication in cross-team messaging scenarios.
This commit is contained in:
parent
0e7e34ab8a
commit
4e82102ceb
7 changed files with 113 additions and 10 deletions
|
|
@ -29,13 +29,22 @@ export class TeamMemberResolver {
|
||||||
): ResolvedTeamMember[] {
|
): ResolvedTeamMember[] {
|
||||||
const names = new Set<string>();
|
const names = new Set<string>();
|
||||||
const explicitNames = new Set<string>();
|
const explicitNames = new Set<string>();
|
||||||
|
const seenNames = new Set<string>();
|
||||||
|
const addName = (name: string): void => {
|
||||||
|
const normalized = name.toLowerCase();
|
||||||
|
if (seenNames.has(normalized)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
seenNames.add(normalized);
|
||||||
|
names.add(name);
|
||||||
|
};
|
||||||
|
|
||||||
if (Array.isArray(config.members)) {
|
if (Array.isArray(config.members)) {
|
||||||
for (const member of config.members) {
|
for (const member of config.members) {
|
||||||
if (typeof member?.name === 'string' && member.name.trim() !== '') {
|
if (typeof member?.name === 'string' && member.name.trim() !== '') {
|
||||||
const trimmed = member.name.trim();
|
const trimmed = member.name.trim();
|
||||||
names.add(trimmed);
|
addName(trimmed);
|
||||||
explicitNames.add(trimmed);
|
explicitNames.add(trimmed.toLowerCase());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -44,8 +53,8 @@ export class TeamMemberResolver {
|
||||||
for (const member of metaMembers) {
|
for (const member of metaMembers) {
|
||||||
if (typeof member?.name === 'string' && member.name.trim() !== '') {
|
if (typeof member?.name === 'string' && member.name.trim() !== '') {
|
||||||
const trimmed = member.name.trim();
|
const trimmed = member.name.trim();
|
||||||
names.add(trimmed);
|
addName(trimmed);
|
||||||
explicitNames.add(trimmed);
|
explicitNames.add(trimmed.toLowerCase());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -53,10 +62,13 @@ export class TeamMemberResolver {
|
||||||
for (const inboxName of inboxNames) {
|
for (const inboxName of inboxNames) {
|
||||||
if (typeof inboxName === 'string' && inboxName.trim() !== '') {
|
if (typeof inboxName === 'string' && inboxName.trim() !== '') {
|
||||||
const trimmed = inboxName.trim();
|
const trimmed = inboxName.trim();
|
||||||
if (!explicitNames.has(trimmed) && looksLikeQualifiedExternalRecipient(trimmed)) {
|
if (
|
||||||
|
!explicitNames.has(trimmed.toLowerCase()) &&
|
||||||
|
looksLikeQualifiedExternalRecipient(trimmed)
|
||||||
|
) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
names.add(trimmed);
|
addName(trimmed);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -3117,7 +3117,10 @@ export class TeamProvisioningService {
|
||||||
crossTeamMeta?.conversationId ??
|
crossTeamMeta?.conversationId ??
|
||||||
replyMeta?.conversationId,
|
replyMeta?.conversationId,
|
||||||
})
|
})
|
||||||
.then(() => {
|
.then((result) => {
|
||||||
|
if (result.deduplicated) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
const msg: InboxMessage = {
|
const msg: InboxMessage = {
|
||||||
from: 'user',
|
from: 'user',
|
||||||
to: `${crossTeamRecipient.teamName}.${crossTeamRecipient.memberName}`,
|
to: `${crossTeamRecipient.teamName}.${crossTeamRecipient.memberName}`,
|
||||||
|
|
@ -3128,7 +3131,7 @@ export class TeamProvisioningService {
|
||||||
(summary || strippedCrossTeamContent).length > 60
|
(summary || strippedCrossTeamContent).length > 60
|
||||||
? (summary || strippedCrossTeamContent).slice(0, 57) + '...'
|
? (summary || strippedCrossTeamContent).slice(0, 57) + '...'
|
||||||
: summary || strippedCrossTeamContent,
|
: summary || strippedCrossTeamContent,
|
||||||
messageId,
|
messageId: result.messageId,
|
||||||
source: 'cross_team_sent',
|
source: 'cross_team_sent',
|
||||||
conversationId:
|
conversationId:
|
||||||
explicitReplyMeta?.conversationId ??
|
explicitReplyMeta?.conversationId ??
|
||||||
|
|
|
||||||
|
|
@ -58,9 +58,21 @@ function parseQualifiedRecipient(
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function isQualifiedExternalRecipient(
|
||||||
|
value: string | undefined,
|
||||||
|
teamName: string,
|
||||||
|
localMemberNames?: Set<string>
|
||||||
|
): boolean {
|
||||||
|
const recipient = parseQualifiedRecipient(value);
|
||||||
|
if (!recipient) return false;
|
||||||
|
if (recipient.teamName === teamName) return false;
|
||||||
|
return !localMemberNames?.has(value?.trim() ?? '');
|
||||||
|
}
|
||||||
|
|
||||||
interface ActivityItemProps {
|
interface ActivityItemProps {
|
||||||
message: InboxMessage;
|
message: InboxMessage;
|
||||||
teamName: string;
|
teamName: string;
|
||||||
|
localMemberNames?: Set<string>;
|
||||||
memberRole?: string;
|
memberRole?: string;
|
||||||
memberColor?: string;
|
memberColor?: string;
|
||||||
recipientColor?: string;
|
recipientColor?: string;
|
||||||
|
|
@ -239,6 +251,7 @@ function linkifyTaskIds(text: string, onClick: (taskId: string) => void): React.
|
||||||
export const ActivityItem = ({
|
export const ActivityItem = ({
|
||||||
message,
|
message,
|
||||||
teamName,
|
teamName,
|
||||||
|
localMemberNames,
|
||||||
memberRole,
|
memberRole,
|
||||||
memberColor,
|
memberColor,
|
||||||
recipientColor,
|
recipientColor,
|
||||||
|
|
@ -284,7 +297,7 @@ export const ActivityItem = ({
|
||||||
const isCrossTeamSent =
|
const isCrossTeamSent =
|
||||||
message.source === CROSS_TEAM_SENT_SOURCE ||
|
message.source === CROSS_TEAM_SENT_SOURCE ||
|
||||||
parsedCrossTeamReplyPrefix !== null ||
|
parsedCrossTeamReplyPrefix !== null ||
|
||||||
(qualifiedRecipient?.teamName !== undefined && qualifiedRecipient.teamName !== teamName);
|
isQualifiedExternalRecipient(message.to, teamName, localMemberNames);
|
||||||
const isCrossTeamAny = isCrossTeam || isCrossTeamSent;
|
const isCrossTeamAny = isCrossTeam || isCrossTeamSent;
|
||||||
const crossTeamOrigin = useMemo(() => {
|
const crossTeamOrigin = useMemo(() => {
|
||||||
if (!isCrossTeam) return null;
|
if (!isCrossTeam) return null;
|
||||||
|
|
|
||||||
|
|
@ -65,6 +65,7 @@ const MessageRowWithObserver = ({
|
||||||
isNew,
|
isNew,
|
||||||
zebraShade,
|
zebraShade,
|
||||||
memberColorMap,
|
memberColorMap,
|
||||||
|
localMemberNames,
|
||||||
onMemberNameClick,
|
onMemberNameClick,
|
||||||
onCreateTask,
|
onCreateTask,
|
||||||
onReply,
|
onReply,
|
||||||
|
|
@ -82,6 +83,7 @@ const MessageRowWithObserver = ({
|
||||||
isNew?: boolean;
|
isNew?: boolean;
|
||||||
zebraShade?: boolean;
|
zebraShade?: boolean;
|
||||||
memberColorMap?: Map<string, string>;
|
memberColorMap?: Map<string, string>;
|
||||||
|
localMemberNames?: Set<string>;
|
||||||
onMemberNameClick?: (name: string) => void;
|
onMemberNameClick?: (name: string) => void;
|
||||||
onCreateTask?: (subject: string, description: string) => void;
|
onCreateTask?: (subject: string, description: string) => void;
|
||||||
onReply?: (message: InboxMessage) => void;
|
onReply?: (message: InboxMessage) => void;
|
||||||
|
|
@ -131,6 +133,7 @@ const MessageRowWithObserver = ({
|
||||||
isUnread={isUnread}
|
isUnread={isUnread}
|
||||||
zebraShade={zebraShade}
|
zebraShade={zebraShade}
|
||||||
memberColorMap={memberColorMap}
|
memberColorMap={memberColorMap}
|
||||||
|
localMemberNames={localMemberNames}
|
||||||
onMemberNameClick={onMemberNameClick}
|
onMemberNameClick={onMemberNameClick}
|
||||||
onCreateTask={onCreateTask}
|
onCreateTask={onCreateTask}
|
||||||
onReply={onReply}
|
onReply={onReply}
|
||||||
|
|
@ -162,6 +165,7 @@ export const ActivityTimeline = ({
|
||||||
const [visibleCount, setVisibleCount] = useState(MESSAGES_PAGE_SIZE);
|
const [visibleCount, setVisibleCount] = useState(MESSAGES_PAGE_SIZE);
|
||||||
|
|
||||||
const colorMap = members ? buildMemberColorMap(members) : new Map<string, string>();
|
const colorMap = members ? buildMemberColorMap(members) : new Map<string, string>();
|
||||||
|
const localMemberNames = new Set((members ?? []).map((member) => member.name.trim()));
|
||||||
const memberInfo = new Map<string, { role?: string; color?: string }>();
|
const memberInfo = new Map<string, { role?: string; color?: string }>();
|
||||||
if (members) {
|
if (members) {
|
||||||
for (const m of members) {
|
for (const m of members) {
|
||||||
|
|
@ -439,6 +443,7 @@ export const ActivityTimeline = ({
|
||||||
isNew={newItemKeys.has(messageKey)}
|
isNew={newItemKeys.has(messageKey)}
|
||||||
zebraShade={zebraShadeSet.has(realIndex)}
|
zebraShade={zebraShadeSet.has(realIndex)}
|
||||||
memberColorMap={colorMap}
|
memberColorMap={colorMap}
|
||||||
|
localMemberNames={localMemberNames}
|
||||||
onMemberNameClick={onMemberClick ? handleMemberNameClick : undefined}
|
onMemberNameClick={onMemberClick ? handleMemberNameClick : undefined}
|
||||||
onCreateTask={onCreateTaskFromMessage}
|
onCreateTask={onCreateTaskFromMessage}
|
||||||
onReply={onReplyToMessage}
|
onReply={onReplyToMessage}
|
||||||
|
|
|
||||||
|
|
@ -107,6 +107,23 @@ describe('TeamMemberResolver', () => {
|
||||||
expect(names).toContain('ops.bot');
|
expect(names).toContain('ops.bot');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('keeps dotted names when config casing differs from inbox casing', () => {
|
||||||
|
const resolver = new TeamMemberResolver();
|
||||||
|
const config: TeamConfig = {
|
||||||
|
name: 'Team',
|
||||||
|
members: [
|
||||||
|
{ name: 'team-lead', agentType: 'team-lead', role: 'lead' },
|
||||||
|
{ name: 'Ops.Bot', agentType: 'general-purpose' },
|
||||||
|
],
|
||||||
|
};
|
||||||
|
|
||||||
|
const members = resolver.resolveMembers(config, [], ['ops.bot'], [], []);
|
||||||
|
const names = members.map((m) => m.name);
|
||||||
|
|
||||||
|
expect(names).toContain('Ops.Bot');
|
||||||
|
expect(names).not.toContain('ops.bot');
|
||||||
|
});
|
||||||
|
|
||||||
it('sets currentTaskId for in_progress task', () => {
|
it('sets currentTaskId for in_progress task', () => {
|
||||||
const resolver = new TeamMemberResolver();
|
const resolver = new TeamMemberResolver();
|
||||||
const config: TeamConfig = {
|
const config: TeamConfig = {
|
||||||
|
|
|
||||||
|
|
@ -472,6 +472,49 @@ describe('TeamProvisioningService pre-ready live messages', () => {
|
||||||
expect(hoisted.appendSentMessage).not.toHaveBeenCalled();
|
expect(hoisted.appendSentMessage).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does not push a duplicate live row when cross-team fallback deduplicates', async () => {
|
||||||
|
const service = new TeamProvisioningService();
|
||||||
|
seedConfig('my-team');
|
||||||
|
const emitter = vi.fn<(event: TeamChangeEvent) => void>();
|
||||||
|
const crossTeamSender = vi.fn(async () => ({
|
||||||
|
deliveredToInbox: true,
|
||||||
|
messageId: 'existing-cross-1',
|
||||||
|
deduplicated: true,
|
||||||
|
}));
|
||||||
|
service.setTeamChangeEmitter(emitter);
|
||||||
|
service.setCrossTeamSender(crossTeamSender);
|
||||||
|
const run = attachRun(service, 'my-team', { provisioningComplete: true });
|
||||||
|
|
||||||
|
callHandleStreamJsonMessage(service, run, {
|
||||||
|
type: 'assistant',
|
||||||
|
content: [
|
||||||
|
{
|
||||||
|
type: 'tool_use',
|
||||||
|
name: 'SendMessage',
|
||||||
|
input: {
|
||||||
|
type: 'message',
|
||||||
|
recipient: 'team-best.user',
|
||||||
|
content: 'Повтор без нового live row',
|
||||||
|
summary: 'Повтор',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
|
||||||
|
await vi.waitFor(() => {
|
||||||
|
expect(crossTeamSender).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(service.getLiveLeadProcessMessages('my-team')).toHaveLength(0);
|
||||||
|
expect(emitter).not.toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
type: 'lead-message',
|
||||||
|
teamName: 'my-team',
|
||||||
|
detail: 'cross-team-send',
|
||||||
|
})
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it('does not upgrade dotted local teammate names into cross-team sends', async () => {
|
it('does not upgrade dotted local teammate names into cross-team sends', async () => {
|
||||||
const service = new TeamProvisioningService();
|
const service = new TeamProvisioningService();
|
||||||
seedConfig('my-team');
|
seedConfig('my-team');
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,9 @@
|
||||||
import { describe, expect, it } from 'vitest';
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
import { getSystemMessageLabel } from '@renderer/components/team/activity/ActivityItem';
|
import {
|
||||||
|
getSystemMessageLabel,
|
||||||
|
isQualifiedExternalRecipient,
|
||||||
|
} from '@renderer/components/team/activity/ActivityItem';
|
||||||
|
|
||||||
describe('ActivityItem legacy system message fallback', () => {
|
describe('ActivityItem legacy system message fallback', () => {
|
||||||
it('recognizes historical assignment and review message wording', () => {
|
it('recognizes historical assignment and review message wording', () => {
|
||||||
|
|
@ -18,4 +21,11 @@ describe('ActivityItem legacy system message fallback', () => {
|
||||||
expect(getSystemMessageLabel('Approved abcd1234')).toBeNull();
|
expect(getSystemMessageLabel('Approved abcd1234')).toBeNull();
|
||||||
expect(getSystemMessageLabel('Fix request for abcd1234')).toBeNull();
|
expect(getSystemMessageLabel('Fix request for abcd1234')).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does not classify dotted local teammates as external recipients', () => {
|
||||||
|
expect(isQualifiedExternalRecipient('ops.bot', 'my-team', new Set(['ops.bot']))).toBe(false);
|
||||||
|
expect(isQualifiedExternalRecipient('team-best.user', 'my-team', new Set(['ops.bot']))).toBe(
|
||||||
|
true
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue