fix(orchestration): sanitize legacy formatted JSON (#11263)
* fix(orchestration): sanitize legacy formatted JSON * fix(orchestration): harden legacy message formatting --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
OrcaWin
GitHub
parent
d9fec8fd61
commit
76b6c137c6
|
|
@ -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<string, string | boolean>([
|
||||
['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<string, string | boolean>([
|
||||
['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<string, string | boolean>([
|
||||
['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: [
|
||||
|
|
|
|||
|
|
@ -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<string, CommandHandler> = {
|
|||
}
|
||||
}
|
||||
}
|
||||
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) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue