fix(sdk): mark the SDK spawn, keep a quiet child's stderr, read the CLI version form

packages/typescript/src/__tests__/binary-resolver.test.tsmodified+17-0

Changes to packages/typescript/src/__tests__/binary-resolver.test.ts

		⋯140 linesShow the 140 lines above this hunk@@ -141,6 +141,23 @@ describe('CLI shim re-entry', () => {
141	141	expect(cli.indexOf('craftBinaryIsCliShimMessage')).toBeLessThan(cli.indexOf('const cli = new CLI'))
142	142	})
143	143
	144	+ it('marks the SDK spawn too, so an SDK caller reaches the same message', async () => {
	145	+ // #236: the marker was set only by the CLI, so CraftApp.show() — the
	146	+ // path a dashboard or any other SDK consumer takes — still met the raw
	147	+ // `Unknown option --url` that this whole mechanism exists to replace.
	148	+ const idx = await Bun.file(new URL('../index.ts', import.meta.url)).text()
	149	+ expect(idx).toContain('CRAFT_CLI_SPAWN_MARKER')
	150	+ expect(idx).toContain('[CRAFT_CLI_SPAWN_MARKER]: craftPath')
	151	+ })
	152	+
	153	+ it('keeps a quiet spawn\'s stderr, which is where that message arrives', async () => {
	154	+ // The marker only helps if the child's stderr survives. `quiet` used to
	155	+ // be stdio: 'ignore', which discarded exactly the explanation above.
	156	+ const idx = await Bun.file(new URL('../index.ts', import.meta.url)).text()
	157	+ expect(idx).toContain("['ignore', 'ignore', 'pipe']")
	158	+ expect(idx).not.toContain("stdio: this.config.quiet ? 'ignore' : 'inherit'")
	159	+ })
	160	+
144	161	it('names the offending PATH entry, since nothing else points at PATH', () => {
145	162	const message = craftBinaryIsCliShimMessage('/Users/me/.bun/bin/craft')
146	163	expect(message).toContain('/Users/me/.bun/bin/craft')

packages/typescript/src/__tests__/index.test.tsmodified+41-1

Changes to packages/typescript/src/__tests__/index.test.ts

		@@ -1,5 +1,5 @@
1	1	import { describe, expect, it, beforeEach } from 'bun:test'
2		-import { CraftApp, createApp, parseCraftVersionOutput, type WindowOptions, type AppConfig } from '../index'
	2	+import { CraftApp, craftProcessFailedMessage, createApp, parseCraftVersionOutput, type WindowOptions, type AppConfig } from '../index'
3	3
4	4	describe('CraftApp', () => {
5	5	describe('constructor', () => {
		⋯89 linesShow the 89 lines above this hunk@@ -95,6 +95,46 @@ describe('Helper functions', () => {
95	95	it('accepts compact registry binaries that print only a version', () => {
96	96	expect(parseCraftVersionOutput('v0.0.64\n')).toBe('0.0.64')
97	97	})
	98	+
	99	+ // #236: this is the CLI's own form, and the platform suffix used to
	100	+ // survive the strip, so the whole line was compared against a semver.
	101	+ // Two halves of one checkout were reported as drift — and that warning
	102	+ // was the only signal a user got for a failure it had nothing to do with.
	103	+ it('reads the CLI form, whose version is followed by the platform', () => {
	104	+ expect(parseCraftVersionOutput('craft/0.0.92 darwin-arm64 bun-v1.4.1\n')).toBe('0.0.92')
	105	+ })
	106	+
	107	+ it('reads a bare name and version, and a bare version', () => {
	108	+ expect(parseCraftVersionOutput('craft 0.0.92\n')).toBe('0.0.92')
	109	+ expect(parseCraftVersionOutput('0.0.92\n')).toBe('0.0.92')
	110	+ })
	111	+
	112	+ it('answers nothing for output that carries no version at all', () => {
	113	+ expect(parseCraftVersionOutput('')).toBe('')
	114	+ })
	115	+ })
	116	+
	117	+ describe('craftProcessFailedMessage', () => {
	118	+ // #236: `quiet` mapped to stdio: 'ignore', so the child's explanation was
	119	+ // discarded and the error then told the caller to read output that no
	120	+ // longer existed. A dashboard passing `quiet: !verbose` saw only the code.
	121	+ it('leads with what the child said, when it said anything', () => {
	122	+ const message = craftProcessFailedMessage(1, '"/Users/me/.bun/bin/craft" on PATH is the Craft CLI\n', true)
	123	+ expect(message).toContain('exited with code 1')
	124	+ expect(message).toContain('on PATH is the Craft CLI')
	125	+ expect(message).not.toContain('console output above')
	126	+ })
	127	+
	128	+ it('does not point a quiet caller at output that was never printed', () => {
	129	+ const message = craftProcessFailedMessage(1, '', true)
	130	+ expect(message).not.toContain('console output above')
	131	+ expect(message).toContain('quiet')
	132	+ })
	133	+
	134	+ it('still points an inheriting caller at its own console', () => {
	135	+ const message = craftProcessFailedMessage(1, '', false)
	136	+ expect(message).toContain('console output above')
	137	+ })
98	138	})
99	139
100	140	describe('createApp', () => {

packages/typescript/src/binary-resolver.tsmodified+5-1

Changes to packages/typescript/src/binary-resolver.ts

		⋯65 linesShow the 65 lines above this hunk@@ -66,6 +66,10 @@ export function resolveCraftBinary(explicit?: string): string {
66	66	* path-matrix behaviour the pantry contract exists to remove. So instead the
67	67	* spawn is marked, and a CLI that starts up already marked knows it *is* the
68	68	* thing that was spawned, and says so.
	69	+ *
	70	+ * Both spawners set it: `runCraftBinary()` in the CLI, and `CraftApp.show()`
	71	+ * in the SDK. The SDK one was missing until #236, so every SDK caller on such
	72	+ * a machine still met the raw parser error this marker exists to replace.
69	73	*/
70	74	export const CRAFT_CLI_SPAWN_MARKER = 'CRAFT_CLI_SPAWNED_FROM'
71	75
		⋯5 linesShow the 5 lines above this hunk@@ -77,7 +81,7 @@ export function craftBinaryIsCliShimMessage(spawnedFrom: string): string {
77	81	return [
78	82	`"${spawnedFrom}" on PATH is the Craft CLI, not the native binary.`,
79	83	'',
80		- 'The CLI spawned it expecting the native build, and re-entered itself —',
	84	+ 'Craft spawned it expecting the native build and reached this CLI instead —',
81	85	'which is why an option you never typed (--url) came back as unknown.',
82	86	'',
83	87	'Fix it in one of two ways:',

packages/typescript/src/index.tsmodified+68-12

Changes to packages/typescript/src/index.ts

		⋯7 linesShow the 7 lines above this hunk@@ -8,7 +8,7 @@ import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs'
8	8	import { tmpdir } from 'node:os'
9	9	import { join } from 'node:path'
10	10	import { promisify } from 'node:util'
11		-import { craftBinaryNotFoundMessage, resolveCraftBinary } from './binary-resolver.js'
	11	+import { CRAFT_CLI_SPAWN_MARKER, craftBinaryNotFoundMessage, resolveCraftBinary } from './binary-resolver.js'
12	12	import type { AppConfig, WindowOptions } from './types.js'
13	13
14	14	const execFileAsync = promisify(execFile)
		⋯40 linesShow the 40 lines above this hunk@@ -55,9 +55,51 @@ async function probeBinaryVersion(craftPath: string): Promise<void> {
55	55	}
56	56	}
57	57
	58	+/**
	59	+ * The version a `craft --version` first line reports, whichever form it takes:
	60	+ * `craft version 0.0.92`, `craft/0.0.92 darwin-arm64 bun-v1.4.1`,
	61	+ * `craft 0.0.92`, `v0.0.92`, or a bare `0.0.92`.
	62	+ *
	63	+ * The slash form is the CLI's own, and used to survive the strip intact, so
	64	+ * the whole line was compared against a semver and never matched — two
	65	+ * identical versions reported as drift, on the one path where the real fault
	66	+ * was that PATH had resolved to the CLI at all (#236).
	67	+ */
58	68	export function parseCraftVersionOutput(output: string): string {
59	69	const firstLine = output.trim().split(/\r?\n/, 1)[0] ?? ''
60		- return firstLine.replace(/^craft version\s+/i, '').replace(/^v/, '').trim()
	70	+ const withoutName = firstLine.replace(/^craft(?:\s+version)?[\s/]+/i, '').trim()
	71	+ const firstToken = withoutName.split(/\s+/, 1)[0] ?? ''
	72	+ return firstToken.replace(/^v/, '').trim()
	73	+}
	74	+
	75	+/** How much of a quiet child's stderr is kept for its failure message. */
	76	+const STDERR_TAIL_LIMIT = 4000
	77	+
	78	+/**
	79	+ * The rejection for a child that exited non-zero.
	80	+ *
	81	+ * Whatever the child managed to say comes first, because it is the only part
	82	+ * that can name the actual fault. `quiet` used to map to `stdio: 'ignore'`,
	83	+ * which threw that away and then told the caller to read output that no
	84	+ * longer existed — leaving `exited with code 1` as the whole of the error
	85	+ * (#236).
	86	+ */
	87	+export function craftProcessFailedMessage(code: number | null, stderr: string, quiet: boolean): string {
	88	+ const said = stderr.trim()
	89	+ const header = `❌ Craft process exited with code ${code}`
	90	+ if (said) return `${header}\n\n${said}`
	91	+
	92	+ const where = quiet
	93	+ ? 'It wrote nothing to stderr. Re-run without `quiet` to see its full output.'
	94	+ : 'Check the console output above for more details.'
	95	+ return (
	96	+ `${header}\n\n`
	97	+ + 'This may indicate:\n'
	98	+ + ' • Invalid window configuration\n'
	99	+ + ' • Malformed HTML content\n'
	100	+ + ' • System resource constraints\n\n'
	101	+ + where
	102	+ )
61	103	}
62	104
63	105	// Export packaging API
		⋯260 linesShow the 260 lines above this hunk@@ -324,8 +366,25 @@ export class CraftApp {
324	366	// shouldn't gate startup.
325	367	void probeBinaryVersion(craftPath)
326	368
	369	+ // Two things beyond the plain spawn, both from #236:
	370	+ //
	371	+ // `quiet` keeps stderr rather than dropping it. The child's own message
	372	+ // is the only thing that can say why it failed, and the caller is handed
	373	+ // it below instead of being pointed at output that was discarded.
	374	+ //
	375	+ // The marker is what the CLI checks at startup. When `craft` on PATH is
	376	+ // this package's CLI rather than the native binary — which installing the
	377	+ // SDK can arrange — the child recognises itself and says so, instead of
	378	+ // rejecting `--url` as an option the user never typed.
	379	+ const quiet = this.config.quiet === true
327	380	this.process = spawn(craftPath, args, {
328		- stdio: this.config.quiet ? 'ignore' : 'inherit',
	381	+ stdio: quiet ? ['ignore', 'ignore', 'pipe'] : 'inherit',
	382	+ env: { ...process.env, [CRAFT_CLI_SPAWN_MARKER]: craftPath },
	383	+ })
	384	+
	385	+ let stderrTail = ''
	386	+ this.process.stderr?.on('data', (chunk: Buffer | string) => {
	387	+ stderrTail = (stderrTail + chunk.toString()).slice(-STDERR_TAIL_LIMIT)
329	388	})
330	389
331	390	return new Promise((resolve, reject) => {
		⋯21 linesShow the 21 lines above this hunk@@ -353,19 +412,16 @@ export class CraftApp {
353	412	))
354	413	})
355	414
356		- this.process?.on('exit', (code) => {
	415	+ // `close`, not `exit`: stderr can still hold buffered data when the
	416	+ // process ends, and the child's last words are usually the whole of
	417	+ // the diagnosis. With no pipe — the inherit path — the two fire
	418	+ // together.
	419	+ this.process?.on('close', (code) => {
357	420	if (code === 0 || code === null) {
358	421	resolve()
359	422	}
360	423	else {
361		- reject(new Error(
362		- `❌ Craft process exited with code ${code}\n\n` +
363		- 'This may indicate:\n' +
364		- ' • Invalid window configuration\n' +
365		- ' • Malformed HTML content\n' +
366		- ' • System resource constraints\n\n' +
367		- 'Check the console output above for more details.'
368		- ))
	424	+ reject(new Error(craftProcessFailedMessage(code, stderrTail, quiet)))
369	425	}
370	426	})
371	427	})