ReviewOS

craft-native/craft

fix(sdk): CraftApp.show() re-enters the CLI shim unmarked, so #26's guidance never reaches SDK callers

#236
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Summary

#26 fixed the CLI re-entering itself, and 85ff14b added CRAFT_CLI_SPAWN_MARKER so a shim that gets spawned can say what happened. The marker is only ever set in runCraftBinary() — the CLI path. CraftApp.show() spawns the same resolved binary and never sets it, so every SDK caller on a machine where craft on PATH is the CLI still gets the bare Unknown option --url that #26 set out to remove.

This is not version drift. Both halves below are 0.0.92 from one checkout.

Environment

  • Checkout v0.0.92-3-g4d4be0c, SDK = packages/typescript/src, so SDK and CLI are the same source tree
  • craft on PATH is ~/.bun/bin/craft → a bun-linked checkout registered under the package's former name ts-craft, whose bin was craft. Today's package.json publishes craft-sdk, but a stale global link still owns craft — exactly the scenario binary-resolver.ts describes
  • No native binary present (packages/zig/zig-out/bin empty, no pantry install), so PATH resolution has nothing else to find

Reproduction

import { createApp } from './packages/typescript/src/index.ts'

const app = createApp({
  url: 'http://localhost:3002',
  quiet: process.argv.includes('--quiet'),
  window: { title: 'Repro', width: 800, height: 600 },
})

try {
  await app.show()
  console.log('RESULT: window opened')
}
catch (error) {
  console.log(`RESULT: ${error instanceof Error ? error.message.split('\n')[0] : String(error)}`)
}

bun repro.ts:

ClappError: Unknown option `--url`
Run `craft [url] --help` to see available options.
   exitCode: 2,
 isUsageError: true,
      usage: "$ craft [url]",
      at checkUnknownOptions (@stacksjs/clapp/dist/index.js:766:17)
      at packages/typescript/bin/cli.ts:955:5

RESULT: ❌ Craft process exited with code 1
⚠️  Craft binary/SDK version mismatch: SDK=0.0.92, native=craft/0.0.92 darwin-arm64 bun-v1.4.1.

Setting the marker by hand produces the message the SDK path should have shown, which is the whole fix in one line:

$ CRAFT_CLI_SPAWNED_FROM=~/.bun/bin/craft craft http://localhost:3002
"~/.bun/bin/craft" on PATH is the Craft CLI, not the native binary.

The CLI spawned it expecting the native build, and re-entered itself —
which is why an option you never typed (--url) came back as unknown.

Three defects, in the order a user meets them

1. The SDK spawn is unmarked. CraftApp.show() (packages/typescript/src/index.ts:327) does:

this.process = spawn(craftPath, args, {
  stdio: this.config.quiet ? 'ignore' : 'inherit',
})

while runCraftBinary() (bin/cli.ts:41) passes env: { ...process.env, [CRAFT_CLI_SPAWN_MARKER]: craftPath }. The detection at bin/cli.ts:18 can therefore never fire for an SDK-driven spawn.

2. quiet: true destroys the explanation. stdio: 'ignore' discards the child's stderr, and the rejection then reads:

❌ Craft process exited with code 1

This may indicate:
  • Invalid window configuration
  • Malformed HTML content
  • System resource constraints

Check the console output above for more details.

There is no console output above — the SDK suppressed it. This is what a real caller sees: @stacksjs/actions' dashboard dev server passes quiet: !verbose, so ./buddy dev --dashboard reports only Craft process exited with code 1, with no way to reach the cause.

3. The version warning misattributes it. parseCraftVersionOutput strips a leading craft version or v, but the CLI prints craft/0.0.92 darwin-arm64 bun-v1.4.1. The platform suffix survives, so the comparison at index.ts:45 never matches and a mismatch is reported for two identical versions. Combined with 1 and 2, the only signal a user gets points at version drift that isn't there — which is exactly where the debugging session that found this went first.

Suggested fix

  • Mark the SDK spawn the same way the CLI does: env: { ...process.env, [CRAFT_CLI_SPAWN_MARKER]: craftPath } in CraftApp.show(). That alone turns this into the intended message.
  • When quiet, prefer stdio: ['ignore', 'ignore', 'pipe'] and attach the captured stderr tail to the rejection, rather than pointing at output that was thrown away.
  • Have parseCraftVersionOutput handle the craft/<version> <platform> bun-<version> form, so the warning fires only on real drift.

Impact

Any SDK consumer on a machine where craft resolves to the CLI — which the resolver's own comment calls out as a normal consequence of installing the package — hits an error naming a flag the user never typed, and with quiet set, no error text at all.

Sign in to comment on this issue.