runtime: replace owner progress with final delivery
This commit is contained in:
@@ -109,6 +109,7 @@ interface CreateExecuteTurnDeps {
|
||||
forcedAgentType?: AgentType;
|
||||
deliveryRole: PairedRoomRole | null;
|
||||
deliveryServiceId: string | null;
|
||||
replaceMessageId?: string | null;
|
||||
}) => Promise<boolean>;
|
||||
afterDeliverySuccess?: (args: {
|
||||
chatJid: string;
|
||||
@@ -197,7 +198,7 @@ export function createExecuteTurn(deps: CreateExecuteTurnDeps): ExecuteTurnFn {
|
||||
});
|
||||
return ownership.state !== 'inactive';
|
||||
},
|
||||
deliverFinalText: async (text) => {
|
||||
deliverFinalText: async (text, options) => {
|
||||
try {
|
||||
return await deps.deliverFinalText({
|
||||
text,
|
||||
@@ -210,6 +211,7 @@ export function createExecuteTurn(deps: CreateExecuteTurnDeps): ExecuteTurnFn {
|
||||
forcedAgentType: args.forcedAgentType,
|
||||
deliveryRole: resolvedDeliveryRole,
|
||||
deliveryServiceId: resolvedDeliveryServiceId,
|
||||
replaceMessageId: options?.replaceMessageId ?? null,
|
||||
});
|
||||
} catch (err) {
|
||||
logger.warn(
|
||||
|
||||
@@ -3556,10 +3556,11 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
'progress-1',
|
||||
P('CI 상태 확인 중입니다.\n\n10초'),
|
||||
);
|
||||
// finish() promotes the last flushed progress text to a final message
|
||||
expect(channel.sendMessage).toHaveBeenCalledTimes(1);
|
||||
expect(channel.sendMessage).toHaveBeenCalledWith(
|
||||
// finish() replaces the tracked progress message with the final text
|
||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'progress-1',
|
||||
'CI 상태 확인 중입니다.',
|
||||
);
|
||||
expect(notifyIdle).not.toHaveBeenCalled();
|
||||
@@ -3850,9 +3851,10 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
'progress-1',
|
||||
expect.stringContaining('늦게 도착한 진행상황입니다.'),
|
||||
);
|
||||
expect(channel.sendMessage).toHaveBeenCalledTimes(1);
|
||||
expect(channel.sendMessage).toHaveBeenCalledWith(
|
||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'progress-1',
|
||||
'최종 답변입니다.',
|
||||
);
|
||||
} finally {
|
||||
@@ -3955,9 +3957,11 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
'progress-1',
|
||||
P('오래 걸리는 작업입니다.\n\n1시간 10초'),
|
||||
);
|
||||
// finish() promotes the last flushed progress text to a final message
|
||||
expect(channel.sendMessage).toHaveBeenCalledWith(
|
||||
// finish() replaces the tracked progress message with the final text
|
||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'progress-1',
|
||||
'오래 걸리는 작업입니다.',
|
||||
);
|
||||
} finally {
|
||||
@@ -3965,7 +3969,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps progress separate from the final Codex answer', async () => {
|
||||
it('promotes the tracked progress message into the final Codex answer', async () => {
|
||||
vi.useFakeTimers();
|
||||
const chatJid = 'group@test';
|
||||
const group = makeGroup('codex');
|
||||
@@ -4054,10 +4058,11 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
'progress-1',
|
||||
P('테스트를 돌리는 중입니다.\n\n10초'),
|
||||
);
|
||||
// Final delivered as separate message
|
||||
expect(channel.sendMessage).toHaveBeenCalledTimes(1);
|
||||
expect(channel.sendMessage).toHaveBeenCalledWith(
|
||||
// Final replaces the tracked progress message instead of creating a new one
|
||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'progress-1',
|
||||
'테스트가 끝났습니다.',
|
||||
);
|
||||
expect(notifyIdle).not.toHaveBeenCalled();
|
||||
@@ -4270,9 +4275,10 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(channel.sendAndTrack).toHaveBeenCalledTimes(1);
|
||||
expect(channel.sendMessage).toHaveBeenCalledTimes(1);
|
||||
expect(channel.sendMessage).toHaveBeenCalledWith(
|
||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'progress-1',
|
||||
'첫 번째 진행상황입니다.',
|
||||
);
|
||||
} finally {
|
||||
@@ -4473,10 +4479,11 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
'progress-2',
|
||||
P('두 번째 진행상황입니다.\n\n10초'),
|
||||
);
|
||||
// finish() promotes the last flushed progress text to a final message
|
||||
expect(channel.sendMessage).toHaveBeenCalledTimes(1);
|
||||
expect(channel.sendMessage).toHaveBeenCalledWith(
|
||||
// finish() replaces the latest tracked progress message with the final text
|
||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'progress-2',
|
||||
'두 번째 진행상황입니다.',
|
||||
);
|
||||
} finally {
|
||||
@@ -4581,10 +4588,11 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
'progress-1',
|
||||
P('테스트도 통과했습니다.\n\n5초'),
|
||||
);
|
||||
// finish() promotes the last flushed progress text to a final message
|
||||
expect(channel.sendMessage).toHaveBeenCalledTimes(1);
|
||||
expect(channel.sendMessage).toHaveBeenCalledWith(
|
||||
// finish() replaces the tracked progress message with the replayed final text
|
||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'progress-1',
|
||||
'검증 중입니다.',
|
||||
);
|
||||
} finally {
|
||||
@@ -4692,10 +4700,11 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
'progress-1',
|
||||
expect.any(String),
|
||||
);
|
||||
// finish() promotes the last flushed progress text to a final message
|
||||
expect(channel.sendMessage).toHaveBeenCalledTimes(1);
|
||||
expect(channel.sendMessage).toHaveBeenCalledWith(
|
||||
// finish() replaces the tracked progress message with the replayed final text
|
||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'progress-1',
|
||||
'진행 중입니다.',
|
||||
);
|
||||
} finally {
|
||||
@@ -4924,10 +4933,11 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
chatJid,
|
||||
P('중간 진행상황입니다.\n\n0초'),
|
||||
);
|
||||
// Error causes failure final to be published
|
||||
expect(channel.sendMessage).toHaveBeenCalledTimes(1);
|
||||
expect(channel.sendMessage).toHaveBeenCalledWith(
|
||||
// Error replaces the tracked progress message with the failure final
|
||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'progress-1',
|
||||
'요청을 완료하지 못했습니다. 다시 시도해 주세요.',
|
||||
);
|
||||
expect(lastAgentTimestamps[chatJid]).toBe('1');
|
||||
|
||||
@@ -167,6 +167,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
||||
forcedAgentType,
|
||||
deliveryRole,
|
||||
deliveryServiceId,
|
||||
replaceMessageId,
|
||||
}) => {
|
||||
if (
|
||||
(deliveryRole === 'reviewer' || deliveryRole === 'arbiter') &&
|
||||
@@ -200,6 +201,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
||||
channel,
|
||||
item: workItem,
|
||||
log: logger,
|
||||
replaceMessageId,
|
||||
isDuplicateOfLastBotFinal: checkDuplicateOfLastBotFinal,
|
||||
openContinuation: (targetChatJid) =>
|
||||
continuationTracker.open(targetChatJid),
|
||||
|
||||
@@ -121,12 +121,10 @@ describe('MessageTurnController outbound audit logging', () => {
|
||||
'dc:test-room',
|
||||
expect.stringContaining('첫 진행 상황'),
|
||||
);
|
||||
expect(channel.editMessage).toHaveBeenCalledWith(
|
||||
'dc:test-room',
|
||||
'progress-1',
|
||||
expect.stringContaining('첫 진행 상황'),
|
||||
);
|
||||
expect(deliverFinalText).toHaveBeenCalledWith('최종 답변');
|
||||
expect(channel.editMessage).not.toHaveBeenCalled();
|
||||
expect(deliverFinalText).toHaveBeenCalledWith('최종 답변', {
|
||||
replaceMessageId: 'progress-1',
|
||||
});
|
||||
|
||||
expect(getAuditEntries()).toEqual(
|
||||
expect.arrayContaining([
|
||||
@@ -143,24 +141,21 @@ describe('MessageTurnController outbound audit logging', () => {
|
||||
botUsername: 'reviewer-bot',
|
||||
messageId: 'progress-1',
|
||||
}),
|
||||
expect.objectContaining({
|
||||
auditEvent: 'progress-edit',
|
||||
deliveryRole: 'reviewer',
|
||||
serviceId: 'codex-review',
|
||||
turnId: 'task-1:2026-04-10T14:22:00.000Z:reviewer-turn',
|
||||
messageId: 'progress-1',
|
||||
}),
|
||||
expect.objectContaining({
|
||||
auditEvent: 'final-delivery-attempt',
|
||||
deliveryRole: 'reviewer',
|
||||
serviceId: 'codex-review',
|
||||
turnId: 'task-1:2026-04-10T14:22:00.000Z:reviewer-turn',
|
||||
messageId: 'progress-1',
|
||||
deliveryMode: 'edit',
|
||||
}),
|
||||
expect.objectContaining({
|
||||
auditEvent: 'final-delivery-result',
|
||||
deliveryRole: 'reviewer',
|
||||
serviceId: 'codex-review',
|
||||
turnId: 'task-1:2026-04-10T14:22:00.000Z:reviewer-turn',
|
||||
messageId: 'progress-1',
|
||||
deliveryMode: 'edit',
|
||||
delivered: true,
|
||||
}),
|
||||
]),
|
||||
@@ -316,9 +311,112 @@ describe('MessageTurnController outbound audit logging', () => {
|
||||
expect(deliverFinalText).toHaveBeenCalledTimes(1);
|
||||
expect(deliverFinalText).toHaveBeenCalledWith(
|
||||
'PROCEED 근거를 확인했습니다.',
|
||||
{
|
||||
replaceMessageId: null,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('replaces the tracked progress message when an owner final arrives', async () => {
|
||||
const channel = {
|
||||
...makeChannel(),
|
||||
name: 'discord',
|
||||
} satisfies Channel;
|
||||
const deliverFinalText = vi.fn().mockResolvedValue(true);
|
||||
const controller = new MessageTurnController({
|
||||
chatJid: 'dc:test-room',
|
||||
group: makeGroup(),
|
||||
runId: 'run-owner-final-replace',
|
||||
channel,
|
||||
idleTimeout: 1_000,
|
||||
failureFinalText: '실패',
|
||||
isClaudeCodeAgent: true,
|
||||
clearSession: vi.fn(),
|
||||
requestClose: vi.fn(),
|
||||
deliverFinalText,
|
||||
deliveryRole: 'owner',
|
||||
pairedTurnIdentity: {
|
||||
turnId: 'task-1:2026-04-10T14:22:00.000Z:owner-turn',
|
||||
taskId: 'task-1',
|
||||
taskUpdatedAt: '2026-04-10T14:22:00.000Z',
|
||||
intentKind: 'finalize-owner-turn',
|
||||
role: 'owner',
|
||||
},
|
||||
});
|
||||
|
||||
await controller.start();
|
||||
await controller.handleOutput({
|
||||
status: 'success',
|
||||
phase: 'progress',
|
||||
result: '첫 진행 상황',
|
||||
} as any);
|
||||
await controller.handleOutput({
|
||||
status: 'success',
|
||||
phase: 'progress',
|
||||
result: '둘째 진행 상황',
|
||||
} as any);
|
||||
await flushAsync();
|
||||
await controller.handleOutput({
|
||||
status: 'success',
|
||||
phase: 'final',
|
||||
result: 'DONE 최종 답변',
|
||||
} as any);
|
||||
await controller.finish('success');
|
||||
|
||||
expect(channel.sendAndTrack).toHaveBeenCalledTimes(1);
|
||||
expect(deliverFinalText).toHaveBeenCalledWith('DONE 최종 답변', {
|
||||
replaceMessageId: 'progress-1',
|
||||
});
|
||||
});
|
||||
|
||||
it('replaces the tracked progress message when finish() replays the last owner progress as final', async () => {
|
||||
const channel = {
|
||||
...makeChannel(),
|
||||
name: 'discord',
|
||||
} satisfies Channel;
|
||||
const deliverFinalText = vi.fn().mockResolvedValue(true);
|
||||
const controller = new MessageTurnController({
|
||||
chatJid: 'dc:test-room',
|
||||
group: makeGroup(),
|
||||
runId: 'run-owner-progress-replay',
|
||||
channel,
|
||||
idleTimeout: 1_000,
|
||||
failureFinalText: '실패',
|
||||
isClaudeCodeAgent: true,
|
||||
clearSession: vi.fn(),
|
||||
requestClose: vi.fn(),
|
||||
deliverFinalText,
|
||||
deliveryRole: 'owner',
|
||||
pairedTurnIdentity: {
|
||||
turnId: 'task-1:2026-04-10T14:22:00.000Z:owner-turn',
|
||||
taskId: 'task-1',
|
||||
taskUpdatedAt: '2026-04-10T14:22:00.000Z',
|
||||
intentKind: 'finalize-owner-turn',
|
||||
role: 'owner',
|
||||
},
|
||||
});
|
||||
|
||||
await controller.start();
|
||||
await controller.handleOutput({
|
||||
status: 'success',
|
||||
phase: 'progress',
|
||||
result: '첫 진행 상황',
|
||||
} as any);
|
||||
await controller.handleOutput({
|
||||
status: 'success',
|
||||
phase: 'progress',
|
||||
result: '둘째 진행 상황',
|
||||
} as any);
|
||||
await flushAsync();
|
||||
|
||||
await controller.finish('success');
|
||||
|
||||
expect(channel.sendAndTrack).toHaveBeenCalledTimes(1);
|
||||
expect(deliverFinalText).toHaveBeenCalledWith('첫 진행 상황', {
|
||||
replaceMessageId: 'progress-1',
|
||||
});
|
||||
});
|
||||
|
||||
it('suppresses replaying the last progress update as final when final delivery is disallowed', async () => {
|
||||
const channel = makeChannel();
|
||||
const deliverFinalText = vi.fn().mockResolvedValue(true);
|
||||
|
||||
@@ -33,7 +33,10 @@ interface MessageTurnControllerOptions {
|
||||
isClaudeCodeAgent: boolean;
|
||||
clearSession: () => void;
|
||||
requestClose: (reason: string) => void;
|
||||
deliverFinalText: (text: string) => Promise<boolean>;
|
||||
deliverFinalText: (
|
||||
text: string,
|
||||
options?: { replaceMessageId?: string | null },
|
||||
) => Promise<boolean>;
|
||||
canDeliverFinalText?: () => boolean;
|
||||
allowProgressReplayWithoutFinal?: boolean;
|
||||
deliveryRole?: PairedRoomRole | null;
|
||||
@@ -294,8 +297,8 @@ export class MessageTurnController {
|
||||
// then discard the pending buffer so it never shows up.
|
||||
if (text) {
|
||||
await this.flushPendingProgress(text);
|
||||
await this.finalizeProgressMessage();
|
||||
await this.deliverFinalText(text);
|
||||
const replaceMessageId = this.consumeProgressForFinalDelivery();
|
||||
await this.deliverFinalText(text, { replaceMessageId });
|
||||
} else if (raw) {
|
||||
this.log.info(
|
||||
{
|
||||
@@ -308,7 +311,15 @@ export class MessageTurnController {
|
||||
await this.finalizeProgressMessage();
|
||||
this.latestProgressTextForFinal = null;
|
||||
} else {
|
||||
await this.finalizeProgressMessage();
|
||||
this.log.info(
|
||||
{
|
||||
resultStatus: result.status,
|
||||
resultPhase: result.phase,
|
||||
progressMessageId: this.progressMessageId,
|
||||
latestProgressTextForFinal: this.latestProgressTextForFinal,
|
||||
},
|
||||
'Received a final output with no visible text; deferring any progress replay to finish()',
|
||||
);
|
||||
}
|
||||
|
||||
await this.deactivateTyping('turn:handle-output', {
|
||||
@@ -340,13 +351,16 @@ export class MessageTurnController {
|
||||
!this.hadError &&
|
||||
this.latestProgressTextForFinal
|
||||
) {
|
||||
await this.finalizeProgressMessage();
|
||||
if (this.options.allowProgressReplayWithoutFinal !== false) {
|
||||
const replaceMessageId = this.consumeProgressForFinalDelivery();
|
||||
this.log.info(
|
||||
'Sending a separate final message from the last progress output after agent completion',
|
||||
);
|
||||
await this.deliverFinalText(this.latestProgressTextForFinal);
|
||||
await this.deliverFinalText(this.latestProgressTextForFinal, {
|
||||
replaceMessageId,
|
||||
});
|
||||
} else {
|
||||
await this.finalizeProgressMessage();
|
||||
this.log.info(
|
||||
'Skipped replaying the last progress output as a final message for this turn',
|
||||
);
|
||||
@@ -599,9 +613,28 @@ export class MessageTurnController {
|
||||
this.resetProgressState();
|
||||
}
|
||||
|
||||
private async deliverFinalText(text: string): Promise<void> {
|
||||
private consumeProgressForFinalDelivery(): string | null {
|
||||
const replaceMessageId = this.progressMessageId;
|
||||
this.log.info(
|
||||
{
|
||||
progressMessageId: replaceMessageId,
|
||||
latestProgressText: this.latestProgressText,
|
||||
},
|
||||
replaceMessageId
|
||||
? 'Promoting tracked progress message to final delivery'
|
||||
: 'Delivering final output without a tracked progress message to replace',
|
||||
);
|
||||
this.resetProgressState();
|
||||
return replaceMessageId;
|
||||
}
|
||||
|
||||
private async deliverFinalText(
|
||||
text: string,
|
||||
options?: { replaceMessageId?: string | null },
|
||||
): Promise<void> {
|
||||
await this.activateTyping('turn:deliver-final');
|
||||
this.visiblePhase = toVisiblePhase('final');
|
||||
const replaceMessageId = options?.replaceMessageId ?? null;
|
||||
if (
|
||||
this.options.canDeliverFinalText &&
|
||||
!this.options.canDeliverFinalText()
|
||||
@@ -619,13 +652,17 @@ export class MessageTurnController {
|
||||
return;
|
||||
}
|
||||
this.logOutboundAudit('final-delivery-attempt', {
|
||||
messageId: this.progressMessageId,
|
||||
messageId: replaceMessageId,
|
||||
textLength: text.length,
|
||||
deliveryMode: replaceMessageId ? 'edit' : 'send',
|
||||
});
|
||||
const delivered = await this.options.deliverFinalText(text, {
|
||||
replaceMessageId,
|
||||
});
|
||||
const delivered = await this.options.deliverFinalText(text);
|
||||
this.logOutboundAudit('final-delivery-result', {
|
||||
messageId: this.progressMessageId,
|
||||
messageId: replaceMessageId,
|
||||
textLength: text.length,
|
||||
deliveryMode: replaceMessageId ? 'edit' : 'send',
|
||||
delivered,
|
||||
});
|
||||
if (!delivered) {
|
||||
@@ -638,8 +675,10 @@ export class MessageTurnController {
|
||||
if (this.terminalObserved()) {
|
||||
return;
|
||||
}
|
||||
await this.finalizeProgressMessage();
|
||||
await this.deliverFinalText(this.options.failureFinalText);
|
||||
const replaceMessageId = this.consumeProgressForFinalDelivery();
|
||||
await this.deliverFinalText(this.options.failureFinalText, {
|
||||
replaceMessageId,
|
||||
});
|
||||
}
|
||||
|
||||
private requestAgentClose(reason: string): void {
|
||||
|
||||
Reference in New Issue
Block a user