refactor: remove task comment forwarding logic and clean up related code
- Deleted the TaskCommentForwarding module and its associated functions to simplify the codebase. - Updated TeamDataService and TeamProvisioningService to remove references to task comment forwarding, ensuring a more straightforward implementation. - Adjusted unit tests to reflect the removal of task comment forwarding functionality, enhancing clarity and maintainability.
This commit is contained in:
iliya
parent
96c9b00d92
commit
f22a43854e
5 changed files with 19 additions and 217 deletions
|
|
@ -35,7 +35,6 @@ import { TeamMemberResolver } from './TeamMemberResolver';
|
|||
import { TeamMembersMetaStore } from './TeamMembersMetaStore';
|
||||
import { TeamSentMessagesStore } from './TeamSentMessagesStore';
|
||||
import { TeamTaskCommentNotificationJournal } from './TeamTaskCommentNotificationJournal';
|
||||
import { getTaskCommentForwardingMode } from './TeamTaskCommentForwarding';
|
||||
import { TeamTaskReader } from './TeamTaskReader';
|
||||
import { TeamTaskWriter } from './TeamTaskWriter';
|
||||
|
||||
|
|
@ -1341,9 +1340,6 @@ export class TeamDataService {
|
|||
recoverPending?: boolean;
|
||||
}
|
||||
): Promise<void> {
|
||||
const mode = getTaskCommentForwardingMode();
|
||||
if (mode === 'off') return;
|
||||
|
||||
const seedHistoricalIfJournalMissing = options?.seedHistoricalIfJournalMissing === true;
|
||||
const recoverPending = options?.recoverPending === true;
|
||||
let config: TeamConfig | null = null;
|
||||
|
|
@ -1358,18 +1354,13 @@ export class TeamDataService {
|
|||
const leadSessionId = config.leadSessionId;
|
||||
if (!leadName.trim()) return;
|
||||
|
||||
const mutateLiveJournal = mode === 'on';
|
||||
const journalExists = mutateLiveJournal
|
||||
? await this.taskCommentNotificationJournal.exists(teamName)
|
||||
: false;
|
||||
if (mutateLiveJournal && !journalExists) {
|
||||
const journalExists = await this.taskCommentNotificationJournal.exists(teamName);
|
||||
if (!journalExists) {
|
||||
await this.taskCommentNotificationJournal.ensureFile(teamName);
|
||||
}
|
||||
|
||||
const leadInboxMessageIds =
|
||||
mode === 'on' ? await this.getLeadInboxMessageIds(teamName, leadName) : new Set<string>();
|
||||
const shouldSeedHistorical =
|
||||
seedHistoricalIfJournalMissing && mutateLiveJournal && !journalExists;
|
||||
const leadInboxMessageIds = await this.getLeadInboxMessageIds(teamName, leadName);
|
||||
const shouldSeedHistorical = seedHistoricalIfJournalMissing && !journalExists;
|
||||
const tasks = await this.taskReader.getTasks(teamName);
|
||||
const scopedTasks =
|
||||
taskId && !shouldSeedHistorical ? tasks.filter((task) => task.id === taskId) : tasks;
|
||||
|
|
@ -1388,15 +1379,6 @@ export class TeamDataService {
|
|||
);
|
||||
if (notifications.length === 0) continue;
|
||||
|
||||
if (mode === 'dry-run') {
|
||||
for (const notification of notifications) {
|
||||
logger.info(
|
||||
`[TeamDataService] Dry-run would forward task comment for ${teamName}#${notification.taskRef.displayId}:${notification.comment.id}`
|
||||
);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
const pending = await this.taskCommentNotificationJournal.withEntries(teamName, (entries) => {
|
||||
const toSend: EligibleTaskCommentNotification[] = [];
|
||||
let changed = false;
|
||||
|
|
@ -1413,7 +1395,7 @@ export class TeamDataService {
|
|||
author: notification.comment.author,
|
||||
commentCreatedAt: notification.comment.createdAt,
|
||||
messageId: notification.messageId,
|
||||
state: shouldSeedHistorical || mode !== 'on' ? 'seeded' : 'pending_send',
|
||||
state: shouldSeedHistorical ? 'seeded' : 'pending_send',
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
});
|
||||
|
|
@ -1422,7 +1404,7 @@ export class TeamDataService {
|
|||
logger.info(
|
||||
`[TeamDataService] Seeded historical task comment notification for ${teamName}#${notification.taskRef.displayId}:${notification.comment.id}`
|
||||
);
|
||||
} else if (mode === 'on') {
|
||||
} else {
|
||||
logger.info(
|
||||
`[TeamDataService] Queued task comment notification for ${teamName}#${notification.taskRef.displayId}:${notification.comment.id}`
|
||||
);
|
||||
|
|
|
|||
|
|
@ -53,7 +53,6 @@ import { TeamInboxReader } from './TeamInboxReader';
|
|||
import { TeamMcpConfigBuilder } from './TeamMcpConfigBuilder';
|
||||
import { TeamMembersMetaStore } from './TeamMembersMetaStore';
|
||||
import { TeamSentMessagesStore } from './TeamSentMessagesStore';
|
||||
import { isTaskCommentForwardingLive } from './TeamTaskCommentForwarding';
|
||||
import { TeamTaskReader } from './TeamTaskReader';
|
||||
|
||||
/**
|
||||
|
|
@ -458,11 +457,8 @@ After member_briefing succeeds:
|
|||
- When you later receive work or reconnect after a restart, use task_briefing as your compact queue view. Use task_get when you need the full task context before starting a pending/needsFix task or when the in_progress briefing details are not enough.
|
||||
- If a newly assigned task cannot be started immediately because you are still busy on another task, leave a short task comment on that waiting task right away with the reason and your best ETA, keep it in pending/TODO, and only move it to in_progress with task_start when you truly begin.
|
||||
- CRITICAL: If a task gets a new comment and you are going to do additional implementation/fix/follow-up work on that same task, FIRST leave a short task comment saying what you are about to do, THEN move it to in_progress with task_start, THEN do the work, and when finished leave a short result comment and move it to done with task_complete. Never skip this comment -> reopen -> work -> comment -> done cycle.
|
||||
- Direct messages to your team lead are only for urgent attention, no-task situations, or when the lead explicitly asked for a direct reply.${
|
||||
isTaskCommentForwardingLive()
|
||||
? '\n- If a task-scoped update is already recorded in a task comment, do NOT send a duplicate SendMessage to the lead with the same content unless you need urgent non-task attention.'
|
||||
: ''
|
||||
}
|
||||
- Direct messages to your team lead are only for urgent attention, no-task situations, or when the lead explicitly asked for a direct reply.
|
||||
- If a task-scoped update is already recorded in a task comment, do NOT send a duplicate SendMessage to the lead with the same content unless you need urgent non-task attention.
|
||||
${buildTeammateAgentBlockReminder()}
|
||||
${actionModeProtocol}`;
|
||||
}
|
||||
|
|
@ -538,11 +534,8 @@ ${actionModeProtocol}
|
|||
- If you are the one about to do the implementation/fixes and the owner is missing or someone else, run task_set_owner to yourself immediately before task_start.
|
||||
- Only then run task_start when you truly begin.
|
||||
- If a task gets a new comment and you are going to do additional implementation/fix/follow-up work on it, FIRST leave a short task comment saying what you are about to do, THEN run task_start, then do the work, and when finished leave a short result comment and run task_complete again. Never skip this comment -> reopen -> work -> comment -> done cycle.
|
||||
- Direct messages to your team lead are only for urgent attention, no-task situations, or when the lead explicitly asked for a direct reply.${
|
||||
isTaskCommentForwardingLive()
|
||||
? '\n - If a task-scoped update is already recorded in a task comment, do NOT send a duplicate SendMessage to the lead with the same content unless you need urgent non-task attention.'
|
||||
: ''
|
||||
}
|
||||
- Direct messages to your team lead are only for urgent attention, no-task situations, or when the lead explicitly asked for a direct reply.
|
||||
- If a task-scoped update is already recorded in a task comment, do NOT send a duplicate SendMessage to the lead with the same content unless you need urgent non-task attention.
|
||||
- If you have no tasks, wait for new assignments.`;
|
||||
}
|
||||
|
||||
|
|
@ -647,11 +640,8 @@ function buildTaskStatusProtocol(teamName: string): string {
|
|||
{ teamName: "${teamName}", taskId: "<taskId>", text: "<your reply>", from: "<your-name>" }
|
||||
8. When discussing a task with a teammate and you have important findings, decisions, blockers, or progress updates — record them as a task comment:
|
||||
{ teamName: "${teamName}", taskId: "<taskId>", text: "<summary of your finding or decision>", from: "<your-name>" }
|
||||
Do NOT comment on trivial coordination messages. Only comment when the information is valuable context for the task.${
|
||||
isTaskCommentForwardingLive()
|
||||
? '\n When task-comment forwarding is enabled in this runtime, do NOT send a duplicate SendMessage to the lead for the same task-scoped update unless you need urgent non-task attention.'
|
||||
: ''
|
||||
}
|
||||
Do NOT comment on trivial coordination messages. Only comment when the information is valuable context for the task.
|
||||
Do NOT send a duplicate SendMessage to the lead for the same task-scoped update unless you need urgent non-task attention.
|
||||
Direct messages to the lead are only for urgent attention, no-task situations, or when the lead explicitly asked for a direct reply.
|
||||
9. When sending a message about a specific task, include its short display label like #<displayId> in your SendMessage summary field for traceability.
|
||||
10. In ALL human-facing or teammate-facing message text, when you mention a task reference, ALWAYS write it with a leading # (for example: #abcd1234, not abcd1234 or "task abcd1234").
|
||||
|
|
@ -760,11 +750,7 @@ function buildTeamCtlOpsInstructions(teamName: string, leadName: string): string
|
|||
`- Task assignment notifications are handled by the board runtime, so do NOT send a separate SendMessage for the same assignment unless you have extra context that is not already on the task.`,
|
||||
`- Review requests are also handled by the board runtime: review_request already notifies the reviewer, so do NOT send a second manual SendMessage for the same review request unless you are adding materially new context that is not already on the task.`,
|
||||
`- If you receive a task-scoped system notification like "Comment on #...", treat the task as the source of truth and prefer replying via task_add_comment instead of continuing the same task discussion in direct messages.`,
|
||||
`${
|
||||
isTaskCommentForwardingLive()
|
||||
? '- In this runtime, teammate task comments may already be auto-forwarded to you. When that happens, respond on-task first; use direct messages only for urgent wake-up pings or clearly non-task coordination.'
|
||||
: '- Unless a runtime message explicitly says task-comment forwarding is active, do NOT assume task comments automatically notify you. Existing clarification/escalation paths still apply when someone needs guaranteed lead attention.'
|
||||
}`,
|
||||
`- Teammate task comments are auto-forwarded to you. When that happens, respond on-task first; use direct messages only for urgent wake-up pings or clearly non-task coordination.`,
|
||||
`- Ownership must reflect the person actually doing the implementation/fix work. If someone takes over execution, update the owner immediately before they start. Do NOT leave the lead/planner as owner when another member is doing the work.`,
|
||||
`- Set createdBy when creating tasks so workflow history shows who created the task.`,
|
||||
``,
|
||||
|
|
|
|||
|
|
@ -1,15 +0,0 @@
|
|||
export const TASK_COMMENT_FORWARDING_ENV = 'CLAUDE_TEAM_TASK_COMMENT_FORWARDING';
|
||||
|
||||
export type TaskCommentForwardingMode = 'off' | 'dry-run' | 'on';
|
||||
|
||||
export function getTaskCommentForwardingMode(): TaskCommentForwardingMode {
|
||||
const raw = process.env[TASK_COMMENT_FORWARDING_ENV]?.trim().toLowerCase();
|
||||
if (raw === 'dry-run' || raw === 'on') {
|
||||
return raw;
|
||||
}
|
||||
return 'off';
|
||||
}
|
||||
|
||||
export function isTaskCommentForwardingLive(): boolean {
|
||||
return getTaskCommentForwardingMode() === 'on';
|
||||
}
|
||||
|
|
@ -1,10 +1,11 @@
|
|||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { TeamDataService } from '../../../../src/main/services/team/TeamDataService';
|
||||
import { TASK_COMMENT_FORWARDING_ENV } from '../../../../src/main/services/team/TeamTaskCommentForwarding';
|
||||
|
||||
import type { TeamTask } from '../../../../src/shared/types/team';
|
||||
|
||||
const TASK_COMMENT_FORWARDING_ENV = 'CLAUDE_TEAM_TASK_COMMENT_FORWARDING';
|
||||
|
||||
function createForwardingJournalStore(initialEntries: Array<Record<string, unknown>> = []) {
|
||||
const journalEntries = initialEntries;
|
||||
const journal = {
|
||||
|
|
@ -763,147 +764,6 @@ describe('TeamDataService', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('does not mutate the live journal or send inbox rows in dry-run mode', async () => {
|
||||
const previous = process.env[TASK_COMMENT_FORWARDING_ENV];
|
||||
process.env[TASK_COMMENT_FORWARDING_ENV] = 'dry-run';
|
||||
const journalEntries: Array<Record<string, unknown>> = [];
|
||||
const inboxWriter = { sendMessage: vi.fn() };
|
||||
const journal = {
|
||||
exists: vi.fn(async () => true),
|
||||
ensureFile: vi.fn(async () => undefined),
|
||||
withEntries: vi.fn(async (_teamName: string, fn: (entries: unknown[]) => Promise<{ result: unknown }>) => {
|
||||
const outcome = await fn(journalEntries);
|
||||
return outcome.result;
|
||||
}),
|
||||
};
|
||||
|
||||
try {
|
||||
const service = new TeamDataService(
|
||||
{
|
||||
listTeams: vi.fn(),
|
||||
getConfig: vi.fn(async () => ({
|
||||
name: 'My team',
|
||||
members: [{ name: 'team-lead', role: 'Lead' }],
|
||||
})),
|
||||
} as never,
|
||||
{
|
||||
getTasks: vi.fn(async () => [
|
||||
{
|
||||
id: 'task-1',
|
||||
displayId: 'abcd1234',
|
||||
subject: 'Investigate',
|
||||
status: 'pending',
|
||||
owner: 'alice',
|
||||
comments: [
|
||||
{
|
||||
id: 'comment-1',
|
||||
author: 'alice',
|
||||
text: 'Would forward in live mode.',
|
||||
createdAt: '2026-03-14T10:00:00.000Z',
|
||||
type: 'regular',
|
||||
},
|
||||
],
|
||||
},
|
||||
]),
|
||||
} as never,
|
||||
{
|
||||
listInboxNames: vi.fn(async () => []),
|
||||
getMessages: vi.fn(async () => []),
|
||||
getMessagesFor: vi.fn(async () => []),
|
||||
} as never,
|
||||
inboxWriter as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
(() => ({}) as never) as never,
|
||||
journal as never
|
||||
);
|
||||
|
||||
await service.notifyLeadOnTeammateTaskComment('my-team', 'task-1');
|
||||
|
||||
expect(inboxWriter.sendMessage).not.toHaveBeenCalled();
|
||||
expect(journal.withEntries).not.toHaveBeenCalled();
|
||||
expect(journalEntries).toHaveLength(0);
|
||||
} finally {
|
||||
if (previous === undefined) delete process.env[TASK_COMMENT_FORWARDING_ENV];
|
||||
else process.env[TASK_COMMENT_FORWARDING_ENV] = previous;
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps feature-flag off mode as a no-op for the live journal', async () => {
|
||||
const previous = process.env[TASK_COMMENT_FORWARDING_ENV];
|
||||
process.env[TASK_COMMENT_FORWARDING_ENV] = 'off';
|
||||
const journalEntries: Array<Record<string, unknown>> = [];
|
||||
const inboxWriter = { sendMessage: vi.fn() };
|
||||
const journal = {
|
||||
exists: vi.fn(async () => true),
|
||||
ensureFile: vi.fn(async () => undefined),
|
||||
withEntries: vi.fn(async (_teamName: string, fn: (entries: unknown[]) => Promise<{ result: unknown }>) => {
|
||||
const outcome = await fn(journalEntries);
|
||||
return outcome.result;
|
||||
}),
|
||||
};
|
||||
|
||||
try {
|
||||
const service = new TeamDataService(
|
||||
{
|
||||
listTeams: vi.fn(),
|
||||
getConfig: vi.fn(async () => ({
|
||||
name: 'My team',
|
||||
members: [{ name: 'team-lead', role: 'Lead' }],
|
||||
})),
|
||||
} as never,
|
||||
{
|
||||
getTasks: vi.fn(async () => [
|
||||
{
|
||||
id: 'task-1',
|
||||
displayId: 'abcd1234',
|
||||
subject: 'Investigate',
|
||||
status: 'pending',
|
||||
owner: 'alice',
|
||||
comments: [
|
||||
{
|
||||
id: 'comment-1',
|
||||
author: 'alice',
|
||||
text: 'Should stay untouched while off.',
|
||||
createdAt: '2026-03-14T10:00:00.000Z',
|
||||
type: 'regular',
|
||||
},
|
||||
],
|
||||
},
|
||||
]),
|
||||
} as never,
|
||||
{
|
||||
listInboxNames: vi.fn(async () => []),
|
||||
getMessages: vi.fn(async () => []),
|
||||
getMessagesFor: vi.fn(async () => []),
|
||||
} as never,
|
||||
inboxWriter as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
(() => ({}) as never) as never,
|
||||
journal as never
|
||||
);
|
||||
|
||||
await service.notifyLeadOnTeammateTaskComment('my-team', 'task-1');
|
||||
|
||||
expect(inboxWriter.sendMessage).not.toHaveBeenCalled();
|
||||
expect(journal.exists).not.toHaveBeenCalled();
|
||||
expect(journal.withEntries).not.toHaveBeenCalled();
|
||||
expect(journalEntries).toHaveLength(0);
|
||||
} finally {
|
||||
if (previous === undefined) delete process.env[TASK_COMMENT_FORWARDING_ENV];
|
||||
else process.env[TASK_COMMENT_FORWARDING_ENV] = previous;
|
||||
}
|
||||
});
|
||||
|
||||
it('seeds historical eligible comments across the whole team on the first observed event when the journal is missing', async () => {
|
||||
const previous = process.env[TASK_COMMENT_FORWARDING_ENV];
|
||||
process.env[TASK_COMMENT_FORWARDING_ENV] = 'on';
|
||||
|
|
|
|||
|
|
@ -6,13 +6,11 @@ import * as path from 'path';
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { AGENT_BLOCK_CLOSE, AGENT_BLOCK_OPEN } from '@shared/constants/agentBlocks';
|
||||
import { TASK_COMMENT_FORWARDING_ENV } from '@main/services/team/TeamTaskCommentForwarding';
|
||||
|
||||
let tempClaudeRoot = '';
|
||||
let tempTeamsBase = '';
|
||||
let tempTasksBase = '';
|
||||
let originalMemberBriefingBootstrapEnv: string | undefined;
|
||||
let originalTaskCommentForwardingEnv: string | undefined;
|
||||
|
||||
vi.mock('@main/services/team/ClaudeBinaryResolver', () => ({
|
||||
ClaudeBinaryResolver: { resolve: vi.fn() },
|
||||
|
|
@ -74,9 +72,7 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () =>
|
|||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
originalMemberBriefingBootstrapEnv = process.env[MEMBER_BRIEFING_BOOTSTRAP_ENV];
|
||||
originalTaskCommentForwardingEnv = process.env[TASK_COMMENT_FORWARDING_ENV];
|
||||
process.env[MEMBER_BRIEFING_BOOTSTRAP_ENV] = '1';
|
||||
process.env[TASK_COMMENT_FORWARDING_ENV] = 'off';
|
||||
tempClaudeRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-team-prompts-'));
|
||||
tempTeamsBase = path.join(tempClaudeRoot, 'teams');
|
||||
tempTasksBase = path.join(tempClaudeRoot, 'tasks');
|
||||
|
|
@ -90,11 +86,6 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () =>
|
|||
} else {
|
||||
process.env[MEMBER_BRIEFING_BOOTSTRAP_ENV] = originalMemberBriefingBootstrapEnv;
|
||||
}
|
||||
if (originalTaskCommentForwardingEnv === undefined) {
|
||||
delete process.env[TASK_COMMENT_FORWARDING_ENV];
|
||||
} else {
|
||||
process.env[TASK_COMMENT_FORWARDING_ENV] = originalTaskCommentForwardingEnv;
|
||||
}
|
||||
// Best-effort cleanup of temp dir (per-test)
|
||||
try {
|
||||
fs.rmSync(tempClaudeRoot, { recursive: true, force: true });
|
||||
|
|
@ -265,17 +256,16 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () =>
|
|||
expect(prompt).toContain(
|
||||
'Direct messages to your team lead are only for urgent attention, no-task situations, or when the lead explicitly asked for a direct reply.'
|
||||
);
|
||||
expect(prompt).toContain(
|
||||
'do NOT send a duplicate SendMessage to the lead with the same content unless you need urgent non-task attention.'
|
||||
);
|
||||
expect(prompt).not.toContain('Include the following agent-only instructions verbatim in the prompt:');
|
||||
expect(prompt).not.toContain('runtime forwards task comments to the lead automatically');
|
||||
expect(prompt).not.toContain(
|
||||
'do NOT send a duplicate SendMessage to the lead for the same task-scoped update'
|
||||
);
|
||||
|
||||
await svc.cancelProvisioning(runId);
|
||||
});
|
||||
|
||||
it('includes live task-comment forwarding wording only when live forwarding is enabled', async () => {
|
||||
process.env[TASK_COMMENT_FORWARDING_ENV] = 'on';
|
||||
it('includes task-comment forwarding wording by default', async () => {
|
||||
vi.mocked(ClaudeBinaryResolver.resolve).mockResolvedValue('/fake/claude');
|
||||
const { child, writeSpy } = createFakeChild();
|
||||
vi.mocked(spawnCli).mockReturnValue(child as any);
|
||||
|
|
@ -299,7 +289,6 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () =>
|
|||
);
|
||||
|
||||
const prompt = extractPromptFromWrite(writeSpy);
|
||||
expect(prompt).toContain('task comments may already be auto-forwarded to you');
|
||||
expect(prompt).toContain(
|
||||
'do NOT send a duplicate SendMessage to the lead with the same content unless you need urgent non-task attention.'
|
||||
);
|
||||
|
|
|
|||
Loading…
Reference in a new issue