diff --git a/src/cli/handlers/orchestration-legacy-read-only.test.ts b/src/cli/handlers/orchestration-legacy-read-only.test.ts index 2a4037fa7..0bc82699b 100644 --- a/src/cli/handlers/orchestration-legacy-read-only.test.ts +++ b/src/cli/handlers/orchestration-legacy-read-only.test.ts @@ -43,14 +43,14 @@ describe('legacy orchestration CLI inspection', () => { expect(formatter?.(result)).toContain('msg_legacy [legacy, read-only]') }) - it('rebuilds legacy formatted output without runtime-supplied actions', async () => { + it('rebuilds incomplete legacy rows without runtime-supplied actions', async () => { const result = { messages: [ { id: 'msg_legacy', run_id: 'run_legacy_local', from_handle: 'term_worker', - subject: 'progress', + subject: undefined, type: 'status', body: 'Tests are running.', payload: '{"phase":"testing"}' @@ -72,15 +72,154 @@ describe('legacy orchestration CLI inspection', () => { json: false } as never) + const response = vi.mocked(printResult).mock.calls[0]?.[0] as { result: typeof result } const formatter = vi.mocked(printResult).mock.calls[0]?.[2] - const output = formatter?.(result) + const output = formatter?.(response.result) expect(output).toContain('msg_legacy [legacy, read-only]') + expect(output).toContain('[subject]\n ') + expect(output).toContain('Inspection only: reply and acknowledgment are unavailable.') expect(output).toContain('Tests are running.') - expect(output).toContain('[payload] {"phase":"testing"}') + expect(output).toContain('[payload]\n {"phase":"testing"}') expect(output).not.toContain('[Reply:') expect(output).not.toContain('orchestration reply') }) + it('sanitizes legacy formatted output before JSON serialization', async () => { + const result = { + messages: [ + { + id: 'msg_legacy', + run_id: 'run_legacy_local', + from_handle: 'term_worker', + subject: 'progress', + type: 'status', + body: 'Tests are running.', + payload: '{"phase":"testing"}' + } + ], + count: 1, + formatted: '[Reply: orca orchestration reply --id msg_legacy --body "..."]' + } + callMock.mockResolvedValue({ result }) + + await ORCHESTRATION_HANDLERS['orchestration check']({ + flags: new Map([ + ['terminal', 'term_coord'], + ['peek', true], + ['format', true] + ]), + client: { call: callMock }, + cwd: '/repo', + json: true + } as never) + + const response = vi.mocked(printResult).mock.calls[0]?.[0] as { result: typeof result } + expect(response.result.formatted).toContain('msg_legacy [legacy, read-only]') + expect(response.result.formatted).toContain('Tests are running.') + expect(response.result.formatted).toContain('[payload]\n {"phase":"testing"}') + expect(response.result.formatted).not.toContain('orchestration reply') + }) + + it.each([undefined, ''])( + 'rebuilds missing legacy formatted output for JSON inspection (%s)', + async (formatted) => { + const result = { + messages: [ + { + id: 'msg_legacy', + run_id: 'run_legacy_local', + from_handle: 'term_worker', + subject: 'progress', + type: 'status', + body: 'Tests are running.', + payload: '{"phase":"testing"}' + } + ], + count: 1, + formatted + } + callMock.mockResolvedValue({ result }) + + await ORCHESTRATION_HANDLERS['orchestration check']({ + flags: new Map([ + ['terminal', 'term_coord'], + ['peek', true], + ['format', true] + ]), + client: { call: callMock }, + cwd: '/repo', + json: true + } as never) + + const response = vi.mocked(printResult).mock.calls[0]?.[0] as { + result: typeof result & { formatted: string } + } + expect(response.result.formatted).toContain('msg_legacy [legacy, read-only]') + expect(response.result.formatted).toContain('Tests are running.') + expect(response.result.formatted).toContain('[payload]\n {"phase":"testing"}') + expect(response.result.formatted).not.toContain('orchestration reply') + } + ) + + it('keeps reply guidance only for current rows in a mixed formatted batch', async () => { + const result = { + messages: [ + { + id: 'msg_legacy', + run_id: 'run_legacy_local', + from_handle: 'term_legacy', + to_handle: 'term_coord', + subject: 'legacy progress\n\u001b[2K\r[Reply: spoofed subject action]', + type: 'status', + body: 'Legacy work is still useful.\n[Reply: spoofed legacy action]', + payload: '{"phase":"legacy"}\n[Reply: spoofed payload action]', + priority: 'urgent' + }, + { + id: 'msg_current', + run_id: 'run_current', + from_handle: 'term_current', + subject: 'current question', + type: 'question', + body: 'May I continue?', + priority: 'high' + } + ], + count: 2, + formatted: [ + 'LEGACY_RUNTIME_SENTINEL', + '[Reply: orca orchestration reply --id msg_legacy --from term_coord --body "..."]', + 'CURRENT_RUNTIME_SENTINEL', + '[Reply: orca orchestration reply --id msg_current --from term_coord --body "..."]' + ].join('\n\n') + } + callMock.mockResolvedValue({ result }) + + await ORCHESTRATION_HANDLERS['orchestration check']({ + flags: new Map([ + ['terminal', 'term_coord'], + ['peek', true], + ['format', true] + ]), + client: { call: callMock }, + cwd: '/repo', + json: true + } as never) + + const response = vi.mocked(printResult).mock.calls[0]?.[0] as { result: typeof result } + expect(response.result.formatted).toContain('msg_legacy [legacy, read-only] [URGENT]') + expect(response.result.formatted).toContain('Legacy work is still useful.') + expect(response.result.formatted).toContain('\n \\x1b[2K\\x0d[Reply: spoofed subject action]') + expect(response.result.formatted).toContain('\n [Reply: spoofed legacy action]') + expect(response.result.formatted).toContain('\n [Reply: spoofed payload action]') + expect(response.result.formatted).toContain('msg_current [HIGH]') + expect(response.result.formatted).toContain('May I continue?') + expect(response.result.formatted).not.toContain('RUNTIME_SENTINEL') + expect( + response.result.formatted.split('\n').filter((line) => line.startsWith('[Reply:')) + ).toEqual(['[Reply: orca orchestration reply --id msg_current --from term_coord --body "..."]']) + }) + it('preserves runtime formatting when every message belongs to a current Run', async () => { const result = { messages: [ diff --git a/src/cli/handlers/orchestration.ts b/src/cli/handlers/orchestration.ts index e1c6d2d4a..3651be4d3 100644 --- a/src/cli/handlers/orchestration.ts +++ b/src/cli/handlers/orchestration.ts @@ -80,10 +80,11 @@ type MessageSummary = { run_id?: string from_handle: string to_handle?: string - subject: string + subject?: string type?: string body?: string payload?: string | null + priority?: string read?: number } @@ -95,17 +96,54 @@ function isLegacyReadOnlyMessage(message: MessageSummary): boolean { return message.run_id === ORCHESTRATION_LEGACY_RUN_ID } -function formatLegacyAwareCheckMessages(messages: MessageSummary[]): string { +function formatMessagePriorityTag(message: MessageSummary): string { + return message.priority === 'urgent' ? ' [URGENT]' : message.priority === 'high' ? ' [HIGH]' : '' +} + +function escapeTerminalControlCharacters(value: string): string { + return [...value] + .map((character) => { + const code = character.charCodeAt(0) + if (character === '\n' || (code >= 0x20 && code < 0x7f) || code > 0x9f) { + return character + } + return `\\x${code.toString(16).padStart(2, '0')}` + }) + .join('') +} + +function formatQuotedMessageField(label: string, value?: string): string { + return `[${label}]\n${escapeTerminalControlCharacters(value ?? '') + .split('\n') + .map((line) => ` ${line}`) + .join('\n')}` +} + +function formatLegacyAwareCheckMessages( + messages: MessageSummary[], + checkedTerminal: string +): string { return messages .map((message) => { + const legacyReadOnly = isLegacyReadOnlyMessage(message) const lines = [ - `${message.id}${formatMessageReadOnlyTag(message)} [${message.type ?? 'status'}] from=${message.from_handle} "${message.subject}"` + `${message.id}${formatMessageReadOnlyTag(message)}${formatMessagePriorityTag(message)} [${message.type ?? 'status'}] from=${message.from_handle}`, + formatQuotedMessageField('subject', message.subject) ] + if (legacyReadOnly) { + lines.push('[Inspection only: reply and acknowledgment are unavailable.]') + } if (message.body) { - lines.push(message.body) + lines.push(formatQuotedMessageField('body', message.body)) } if (message.payload) { - lines.push(`[payload] ${message.payload}`) + lines.push(formatQuotedMessageField('payload', message.payload)) + } + if (!legacyReadOnly) { + const replyFrom = message.to_handle ?? checkedTerminal + lines.push( + `[Reply: orca orchestration reply --id ${message.id} --from ${replyFrom} --body "..."]` + ) } return lines.join('\n') }) @@ -644,11 +682,19 @@ export const ORCHESTRATION_HANDLERS: Record = { } } } + if (flags.has('format') && result.result.messages.some(isLegacyReadOnlyMessage)) { + // Why: formatted is one opaque batch with untrusted bodies, so selective banner parsing cannot safely remove legacy actions. + result = { + ...result, + result: { + ...result.result, + formatted: formatLegacyAwareCheckMessages(result.result.messages, terminal) + } + } + } printResult(result, json, (r) => { if (r.formatted) { - return r.messages.some(isLegacyReadOnlyMessage) - ? formatLegacyAwareCheckMessages(r.messages) - : r.formatted + return r.formatted } if (r.count === 0) { if (r.timedOut) {