fix(create-sdk): keep --json stdout pure NDJSON
The SKILL.md contract says every stdout line is one JSON event, but createProject wrote the Created/Next-steps templates to stdout and the default install/build path inherited the launcher's stdio, so package- manager child output interleaved with the event stream. Under --json, route human progress to stderr and run install/build through a NodeCommandRunner that redirects child stdout+stderr to stderr.
This commit is contained in:
4 files changed
+65
-7
No files matched your search
@@ -9,6 +9,7 @@ import {
|
||||
ClackPromptPort,
|
||||
HeadlessPromptError,
|
||||
HeadlessPromptPort,
|
||||
NodeCommandRunner,
|
||||
PromptCancelledError,
|
||||
type PackageManagerVersionProbe,
|
||||
type PromptPort,
|
||||
@@ -45,6 +46,9 @@ export async function createProject(
|
||||
context: CreateCommandContext,
|
||||
): Promise<ScaffoldResult | undefined> {
|
||||
const args = parseCreateArgs(argv)
|
||||
// Under --json, stdout carries only NDJSON events: human-readable progress
|
||||
// and package-manager child output move to stderr.
|
||||
const progress = args.json === true ? context.stderr : context.stdout
|
||||
if (args.help) {
|
||||
context.stdout.write(CREATE_TEMPLATES.usage.render({}))
|
||||
return undefined
|
||||
@@ -64,7 +68,7 @@ export async function createProject(
|
||||
})
|
||||
const resolved = await wizard.run()
|
||||
const result = await scaffoldProject(resolved.directory, resolved.request)
|
||||
context.stdout.write(CREATE_TEMPLATES.created.render({
|
||||
progress.write(CREATE_TEMPLATES.created.render({
|
||||
name: resolved.request.name,
|
||||
directory: resolved.directory,
|
||||
}))
|
||||
@@ -72,8 +76,9 @@ export async function createProject(
|
||||
try {
|
||||
if (context.setup) await context.setup(resolved)
|
||||
else {
|
||||
await resolved.request.packageManager.install(resolved.directory)
|
||||
await resolved.request.packageManager.build(resolved.directory)
|
||||
const runner = args.json === true ? new NodeCommandRunner(context.stderr) : new NodeCommandRunner()
|
||||
await resolved.request.packageManager.install(resolved.directory, runner)
|
||||
await resolved.request.packageManager.build(resolved.directory, runner)
|
||||
}
|
||||
} catch (error) {
|
||||
context.stderr.write(CREATE_TEMPLATES.setupFailure.render({
|
||||
@@ -84,7 +89,7 @@ export async function createProject(
|
||||
throw error
|
||||
}
|
||||
}
|
||||
context.stdout.write(CREATE_TEMPLATES.nextSteps.render({
|
||||
progress.write(CREATE_TEMPLATES.nextSteps.render({
|
||||
directory: resolved.directory,
|
||||
setupRequired: !resolved.install,
|
||||
...packageManagerTemplateModel(resolved.request.packageManager),
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
HeadlessPromptPort,
|
||||
LocalPluginBlueprint,
|
||||
featureId,
|
||||
NodeCommandRunner,
|
||||
NpmPackageManager,
|
||||
type FeatureSelection,
|
||||
type NestedMultiSelectValue,
|
||||
@@ -511,6 +512,12 @@ describe('create command composition', () => {
|
||||
const okSpec = JSON.stringify({ ...base, directory: 'done-agent', provider: 'deepseek', apiKey: 'key', features: [] })
|
||||
await expect(runCreateCommand(['--config-json', okSpec, '--json'], ok)).resolves.toBe(0)
|
||||
expect(ok.readStdout()).toContain('{"type":"done"}')
|
||||
// stdout stays pure NDJSON: every line parses, human progress goes to stderr
|
||||
for (const line of ok.readStdout().split('\n').filter(line => line.length > 0)) {
|
||||
expect(() => { JSON.parse(line) }).not.toThrow()
|
||||
}
|
||||
expect(ok.readStderr()).toContain('Created done-agent')
|
||||
expect(ok.readStderr()).toContain('Next: cd')
|
||||
|
||||
const missing = commandContext(root)
|
||||
missing.stdin.isTTY = false
|
||||
@@ -564,6 +571,17 @@ describe('create command composition', () => {
|
||||
await createProject(argv('agent', true), context)
|
||||
expect(install).toHaveBeenCalledOnce()
|
||||
expect(build).toHaveBeenCalledOnce()
|
||||
const spec = JSON.stringify({
|
||||
directory: 'json-agent', description: 'test', provider: 'deepseek', apiKey: 'key',
|
||||
model: 'deepseek-v4-flash', interface: 'embed', pm: 'npm', install: true, features: [],
|
||||
})
|
||||
const json = commandContext(root)
|
||||
json.stdin.isTTY = false
|
||||
json.stdout.isTTY = false
|
||||
await createProject(['--config-json', spec, '--json'], json)
|
||||
// json mode hands install/build a runner that redirects child output to stderr
|
||||
expect(install).toHaveBeenCalledTimes(2)
|
||||
expect(install.mock.calls[1]?.[1]).toBeInstanceOf(NodeCommandRunner)
|
||||
install.mockRestore()
|
||||
build.mockRestore()
|
||||
})
|
||||
|
||||
@@ -58,17 +58,38 @@ export function scrubEnvironment(environment: NodeJS.ProcessEnv = process.env):
|
||||
|
||||
/** Node child-process command runner with inherited stdio and quiescent completion. */
|
||||
export class NodeCommandRunner implements CommandRunner {
|
||||
/** Spawn one child and settle only after its exit. */
|
||||
private readonly output: NodeJS.WritableStream | undefined
|
||||
|
||||
/**
|
||||
* @param output - redirect target for child stdout+stderr; the child inherits
|
||||
* this process's stdio when absent. Callers whose own stdout carries a machine
|
||||
* protocol (create-sdk --json NDJSON) redirect child output to keep the
|
||||
* protocol stream pure.
|
||||
*/
|
||||
constructor(output?: NodeJS.WritableStream) {
|
||||
this.output = output
|
||||
}
|
||||
|
||||
/** Spawn one child and settle only after exit, with redirected stdio drained. */
|
||||
run(command: string, args: readonly string[], cwd: string): Promise<CommandResult> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const output = this.output
|
||||
if (output === undefined) {
|
||||
const child = spawn(command, [...args], { cwd, env: scrubEnvironment(), stdio: 'inherit', shell: false })
|
||||
child.once('error', reject)
|
||||
child.once('exit', (exitCode, signal) => { resolve({ exitCode, signal }) })
|
||||
return
|
||||
}
|
||||
const child = spawn(command, [...args], {
|
||||
cwd,
|
||||
env: scrubEnvironment(),
|
||||
stdio: 'inherit',
|
||||
stdio: ['inherit', 'pipe', 'pipe'],
|
||||
shell: false,
|
||||
})
|
||||
child.stdout.pipe(output, { end: false })
|
||||
child.stderr.pipe(output, { end: false })
|
||||
child.once('error', reject)
|
||||
child.once('exit', (exitCode, signal) => { resolve({ exitCode, signal }) })
|
||||
child.once('close', (exitCode, signal) => { resolve({ exitCode, signal }) })
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { chmod, mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { Writable } from 'node:stream'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { CordisYamlFile, JsExpression } from '../src/documents/cordis-yaml-file.ts'
|
||||
import { EnvFile } from '../src/documents/env-file.ts'
|
||||
@@ -325,6 +326,19 @@ describe('package manager strategies', () => {
|
||||
const runner = new NodeCommandRunner()
|
||||
await expect(runner.run(process.execPath, ['-e', ''], root)).resolves.toEqual({ exitCode: 0, signal: null })
|
||||
await expect(runner.run('missing-dsh-command', [], root)).rejects.toThrow()
|
||||
let redirected = ''
|
||||
const output = new Writable({
|
||||
write(chunk, _encoding, callback) { redirected += String(chunk); callback() },
|
||||
})
|
||||
const redirecting = new NodeCommandRunner(output)
|
||||
await expect(redirecting.run(
|
||||
process.execPath,
|
||||
['-e', 'process.stdout.write("child-out"); process.stderr.write("child-err")'],
|
||||
root,
|
||||
)).resolves.toEqual({ exitCode: 0, signal: null })
|
||||
expect(redirected).toContain('child-out')
|
||||
expect(redirected).toContain('child-err')
|
||||
await expect(redirecting.run('missing-dsh-command', [], root)).rejects.toThrow()
|
||||
})
|
||||
|
||||
it('discovers and rewrites a repository-local NPM dependency closure', async () => {
|
||||
|
||||
Reference in New Issue
Block a user