fix(cli): wait for valid serve recipe JSON (#8361)
* fix(cli): wait for valid serve recipe JSON * fix(cli): harden recipe output diagnostics Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Siddharth Ahire <siddharth@Siddharths-MacBook-Air.local> Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Sidd
GitHub
parent
deb8152c9a
commit
a5faf19631
|
|
@ -1,6 +1,7 @@
|
|||
import { EventEmitter } from 'node:events'
|
||||
import { resolve } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { encodePairingOffer, PAIRING_OFFER_VERSION } from '../../shared/pairing'
|
||||
|
||||
const { spawnMock } = vi.hoisted(() => ({
|
||||
spawnMock: vi.fn()
|
||||
|
|
@ -18,6 +19,57 @@ class FakeChildProcess extends EventEmitter {
|
|||
unref = vi.fn()
|
||||
}
|
||||
|
||||
const RECIPE_JSON = JSON.stringify({
|
||||
schemaVersion: 1,
|
||||
pairingCode: encodePairingOffer({
|
||||
v: PAIRING_OFFER_VERSION,
|
||||
endpoint: 'wss://sandbox.example.com',
|
||||
deviceToken: 'token',
|
||||
publicKeyB64: 'public-key'
|
||||
}),
|
||||
projectRoot: '/workspace/repo'
|
||||
})
|
||||
const SERVE_INSTALL_STATUS = '[serve] orca CLI install: installed'
|
||||
const SSH_PRIVATE_KEY = 'TOP-SECRET-PRIVATE-KEY'
|
||||
const SSH_AUTHORIZATION = 'Bearer TOP-SECRET-AUTHORIZATION'
|
||||
const SSH_PASSPHRASE = 'TOP-SECRET-PASSPHRASE'
|
||||
const SSH_COOKIE = 'session=TOP-SECRET-COOKIE'
|
||||
const SSH_RECIPE_JSON = JSON.stringify({
|
||||
schemaVersion: 1,
|
||||
connection: {
|
||||
type: 'ssh',
|
||||
target: {
|
||||
label: 'Sandbox',
|
||||
host: 'sandbox.example.com',
|
||||
port: 22,
|
||||
username: 'root'
|
||||
},
|
||||
projectRoot: '/workspace/repo'
|
||||
},
|
||||
userData: {
|
||||
credentials: {
|
||||
privateKey: SSH_PRIVATE_KEY,
|
||||
authorization: SSH_AUTHORIZATION,
|
||||
passphrase: SSH_PASSPHRASE,
|
||||
cookie: SSH_COOKIE
|
||||
}
|
||||
}
|
||||
})
|
||||
const INVALID_SSH_RECIPE_JSON = SSH_RECIPE_JSON.replace('/workspace/repo', 'relative/repo')
|
||||
const IGNORED_NON_RECIPE_STDOUT = '[serve] ignored non-recipe stdout'
|
||||
|
||||
function startRecipeJsonServer() {
|
||||
const child = new FakeChildProcess()
|
||||
spawnMock.mockReturnValue(child)
|
||||
const stdoutSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true)
|
||||
const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true)
|
||||
const result = serveOrcaApp({
|
||||
recipeJson: true,
|
||||
projectRoot: '/workspace/repo'
|
||||
})
|
||||
return { child, result, stdoutSpy, stderrSpy }
|
||||
}
|
||||
|
||||
describe('serveOrcaApp', () => {
|
||||
beforeEach(() => {
|
||||
spawnMock.mockReset()
|
||||
|
|
@ -133,10 +185,7 @@ describe('serveOrcaApp', () => {
|
|||
projectRoot: '/workspace/repo'
|
||||
})
|
||||
queueMicrotask(() => {
|
||||
child.stdout.emit(
|
||||
'data',
|
||||
'{"schemaVersion":1,"pairingCode":"orca://pair?code=abc","projectRoot":"/workspace/repo"}\n'
|
||||
)
|
||||
child.stdout.emit('data', `${RECIPE_JSON}\n`)
|
||||
})
|
||||
|
||||
await expect(result).resolves.toBe(0)
|
||||
|
|
@ -157,12 +206,92 @@ describe('serveOrcaApp', () => {
|
|||
stdio: ['ignore', 'pipe', 'inherit']
|
||||
})
|
||||
)
|
||||
expect(writeSpy).toHaveBeenCalledWith(
|
||||
'{"schemaVersion":1,"pairingCode":"orca://pair?code=abc","projectRoot":"/workspace/repo"}\n'
|
||||
)
|
||||
expect(writeSpy).toHaveBeenCalledWith(`${RECIPE_JSON}\n`)
|
||||
expect(child.unref).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('waits past startup status lines for valid recipe JSON', async () => {
|
||||
const { child, result, stdoutSpy, stderrSpy } = startRecipeJsonServer()
|
||||
queueMicrotask(() => {
|
||||
child.stdout.emit(
|
||||
'data',
|
||||
`${SERVE_INSTALL_STATUS}\n${INVALID_SSH_RECIPE_JSON}\n${SSH_RECIPE_JSON}\n${RECIPE_JSON.slice(0, 40)}`
|
||||
)
|
||||
child.stdout.emit('data', `${RECIPE_JSON.slice(40)}\n`)
|
||||
})
|
||||
|
||||
await expect(result).resolves.toBe(0)
|
||||
|
||||
expect(stderrSpy).toHaveBeenNthCalledWith(1, `${IGNORED_NON_RECIPE_STDOUT}\n`)
|
||||
expect(stderrSpy).toHaveBeenNthCalledWith(2, `${IGNORED_NON_RECIPE_STDOUT}\n`)
|
||||
expect(stderrSpy).toHaveBeenNthCalledWith(3, `${IGNORED_NON_RECIPE_STDOUT}\n`)
|
||||
for (const secret of [SSH_PRIVATE_KEY, SSH_AUTHORIZATION, SSH_PASSPHRASE, SSH_COOKIE]) {
|
||||
expect(stderrSpy).not.toHaveBeenCalledWith(expect.stringContaining(secret))
|
||||
}
|
||||
expect(stdoutSpy).toHaveBeenCalledTimes(1)
|
||||
expect(stdoutSpy).toHaveBeenCalledWith(`${RECIPE_JSON}\n`)
|
||||
expect(child.unref).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('preserves UTF-8 recipe JSON split across Buffer chunks', async () => {
|
||||
const { child, result, stdoutSpy } = startRecipeJsonServer()
|
||||
const unicodeRecipeJson = RECIPE_JSON.replace('/workspace/repo', '/workspace/café')
|
||||
const recipeBuffer = Buffer.from(`${unicodeRecipeJson}\n`)
|
||||
const splitIndex = recipeBuffer.indexOf(Buffer.from('é')) + 1
|
||||
queueMicrotask(() => {
|
||||
child.stdout.emit('data', recipeBuffer.subarray(0, splitIndex))
|
||||
child.stdout.emit('data', recipeBuffer.subarray(splitIndex))
|
||||
})
|
||||
|
||||
await expect(result).resolves.toBe(0)
|
||||
|
||||
expect(stdoutSpy).toHaveBeenCalledWith(`${unicodeRecipeJson}\n`)
|
||||
expect(child.unref).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('rejects when the server exits without valid recipe JSON', async () => {
|
||||
const { child, result, stdoutSpy, stderrSpy } = startRecipeJsonServer()
|
||||
const secrets = ['UPPER-SECRET', 'SLASH-SECRET', 'LEGACY-SECRET', 'PRIVATE-SECRET']
|
||||
const untrustedLines = [
|
||||
'ORCA://pair?code=UPPER-SECRET',
|
||||
'orca://pair/?code=SLASH-SECRET',
|
||||
'orca://pair#LEGACY-SECRET',
|
||||
'"embedded privateKey PRIVATE-SECRET"',
|
||||
'{privateKey:"PRIVATE-SECRET"}'
|
||||
].join('\n')
|
||||
queueMicrotask(() => {
|
||||
child.stdout.emit('data', `${untrustedLines}\n`)
|
||||
child.emit('exit', 0, null)
|
||||
child.emit('close', 0, null)
|
||||
})
|
||||
|
||||
await expect(result).rejects.toMatchObject({
|
||||
code: 'runtime_serve_failed',
|
||||
message: 'Orca serve exited before printing valid recipe JSON with code 0.'
|
||||
})
|
||||
expect(stdoutSpy).not.toHaveBeenCalled()
|
||||
expect(stderrSpy).toHaveBeenCalledTimes(5)
|
||||
expect(stderrSpy).toHaveBeenCalledWith(`${IGNORED_NON_RECIPE_STDOUT}\n`)
|
||||
for (const secret of secrets) {
|
||||
expect(stderrSpy).not.toHaveBeenCalledWith(expect.stringContaining(secret))
|
||||
}
|
||||
expect(child.unref).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('accepts valid recipe JSON at exit without a trailing newline', async () => {
|
||||
const { child, result, stdoutSpy } = startRecipeJsonServer()
|
||||
queueMicrotask(() => {
|
||||
child.emit('exit', 0, null)
|
||||
child.stdout.emit('data', RECIPE_JSON)
|
||||
child.emit('close', 0, null)
|
||||
})
|
||||
|
||||
await expect(result).resolves.toBe(0)
|
||||
|
||||
expect(stdoutSpy).toHaveBeenCalledWith(`${RECIPE_JSON}\n`)
|
||||
expect(child.unref).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('uses a shell when a Windows npm command shim is the Electron executable', async () => {
|
||||
const platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { value: 'win32' })
|
||||
|
|
|
|||
|
|
@ -1,7 +1,14 @@
|
|||
import { spawn as spawnProcess, type SpawnOptions } from 'node:child_process'
|
||||
import { dirname, resolve } from 'node:path'
|
||||
import { StringDecoder } from 'node:string_decoder'
|
||||
import {
|
||||
getEphemeralVmRecipeResultConnection,
|
||||
parseEphemeralVmRecipeResult
|
||||
} from '../../shared/ephemeral-vm-recipes'
|
||||
import { RuntimeClientError } from './types'
|
||||
|
||||
const IGNORED_NON_RECIPE_STDOUT = '[serve] ignored non-recipe stdout'
|
||||
|
||||
export function launchOrcaApp(): void {
|
||||
const overrideCommand = process.env.ORCA_OPEN_COMMAND
|
||||
if (typeof overrideCommand === 'string' && overrideCommand.trim().length > 0) {
|
||||
|
|
@ -155,7 +162,7 @@ function waitForRecipeJson(child: ReturnType<typeof spawnProcess>): Promise<numb
|
|||
clearTimeout(timeout)
|
||||
child.stdout?.off('data', onData)
|
||||
child.off('error', onError)
|
||||
child.off('exit', onExit)
|
||||
child.off('close', onClose)
|
||||
if (error) {
|
||||
reject(error)
|
||||
return
|
||||
|
|
@ -164,42 +171,69 @@ function waitForRecipeJson(child: ReturnType<typeof spawnProcess>): Promise<numb
|
|||
child.unref()
|
||||
resolve(0)
|
||||
}
|
||||
const emitLine = (line: string): void => {
|
||||
process.stdout.write(`${line}\n`)
|
||||
finish()
|
||||
const writeIgnoredRecipeStdout = (): void => {
|
||||
// Why: non-readiness child stdout is untrusted and cannot be safely
|
||||
// redacted, including schema-valid results with arbitrary user data.
|
||||
process.stderr.write(`${IGNORED_NON_RECIPE_STDOUT}\n`)
|
||||
}
|
||||
const onData = (chunk: Buffer | string): void => {
|
||||
output += chunk.toString()
|
||||
const newlineIndex = output.indexOf('\n')
|
||||
if (newlineIndex === -1) {
|
||||
const processRecipeOutputLine = (line: string): void => {
|
||||
const normalizedLine = line.endsWith('\r') ? line.slice(0, -1) : line
|
||||
if (!normalizedLine.trim()) {
|
||||
return
|
||||
}
|
||||
emitLine(output.slice(0, newlineIndex))
|
||||
const parsed = parseEphemeralVmRecipeResult(normalizedLine)
|
||||
if (!parsed.ok) {
|
||||
writeIgnoredRecipeStdout()
|
||||
return
|
||||
}
|
||||
if (getEphemeralVmRecipeResultConnection(parsed.result).type !== 'orca-server') {
|
||||
writeIgnoredRecipeStdout()
|
||||
return
|
||||
}
|
||||
process.stdout.write(`${normalizedLine.trim()}\n`)
|
||||
finish()
|
||||
}
|
||||
const stdoutDecoder = new StringDecoder('utf8')
|
||||
const onData = (chunk: Buffer | string): void => {
|
||||
output += typeof chunk === 'string' ? chunk : stdoutDecoder.write(chunk)
|
||||
while (!settled) {
|
||||
const newlineIndex = output.indexOf('\n')
|
||||
if (newlineIndex === -1) {
|
||||
return
|
||||
}
|
||||
const line = output.slice(0, newlineIndex)
|
||||
output = output.slice(newlineIndex + 1)
|
||||
processRecipeOutputLine(line)
|
||||
}
|
||||
}
|
||||
const onError = (error: Error): void => {
|
||||
finish(error)
|
||||
}
|
||||
const onExit = (code: number | null, signal: NodeJS.Signals | null): void => {
|
||||
const onClose = (code: number | null, signal: NodeJS.Signals | null): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
const trimmed = output.trim()
|
||||
if (trimmed) {
|
||||
emitLine(trimmed)
|
||||
output += stdoutDecoder.end()
|
||||
if (output.trim()) {
|
||||
processRecipeOutputLine(output)
|
||||
}
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
finish(
|
||||
new RuntimeClientError(
|
||||
'runtime_serve_failed',
|
||||
typeof code === 'number'
|
||||
? `Orca serve exited before printing recipe JSON with code ${code}.`
|
||||
: `Orca serve exited before printing recipe JSON via ${signal}.`
|
||||
? `Orca serve exited before printing valid recipe JSON with code ${code}.`
|
||||
: `Orca serve exited before printing valid recipe JSON via ${signal}.`
|
||||
)
|
||||
)
|
||||
}
|
||||
child.stdout?.on('data', onData)
|
||||
child.once('error', onError)
|
||||
child.once('exit', onExit)
|
||||
// Why: `exit` can precede the final piped stdout data. `close` waits until
|
||||
// stdio closes so a last recipe chunk is not mistaken for missing output.
|
||||
child.once('close', onClose)
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue