From 19e038bc729af36669299bb3b0eca317d721c28f Mon Sep 17 00:00:00 2001 From: Tackbear Date: Thu, 4 Jun 2026 21:16:38 +0800 Subject: [PATCH] Restore Windows CLI runtimes in local studio The runtime assumed Unix command lookup and Unix executable semantics, so Windows npm shims for Codex and Claude were invisible to the Studio even though they were installed on PATH. The CLI package also used Unix chmod during build, which blocked a clean Windows install. This teaches detection to use where.exe on Windows, resolves the actual command path before spawning agents, wraps Windows .cmd/.bat shims through cmd.exe, and keeps package build/test scripts portable for fresh Windows checkouts. Constraint: Windows npm global CLIs are exposed as .cmd/.ps1 shims rather than executable Unix-style binaries. Rejected: Require users to launch Studio from a special shell | the app should use the same PATH-visible CLIs users already have. Confidence: high Scope-risk: moderate Directive: Do not reintroduce Unix-only which/chmod/test assumptions into CLI install or runtime detection. Tested: pnpm -r build; pnpm -r typecheck; pnpm -r test; pnpm smoke; /api/agents?force=1 shows claude and codex available; /api/agents/claude/test and /api/agents/codex/test returned hello --- packages/adapter-hyperframes/package.json | 2 +- packages/cli/package.json | 4 +- packages/cli/scripts/chmod-bin.mjs | 7 + packages/cli/src/commands/doctor.ts | 52 +++++--- packages/cli/src/studio-server.ts | 6 + packages/core/package.json | 2 +- packages/runtime/src/detect.ts | 40 +++++- packages/runtime/src/spawn.ts | 149 +++++++++++++--------- scripts/run-tests-if-present.mjs | 13 ++ 9 files changed, 187 insertions(+), 88 deletions(-) create mode 100644 packages/cli/scripts/chmod-bin.mjs create mode 100644 scripts/run-tests-if-present.mjs diff --git a/packages/adapter-hyperframes/package.json b/packages/adapter-hyperframes/package.json index 488d97c..a57071a 100644 --- a/packages/adapter-hyperframes/package.json +++ b/packages/adapter-hyperframes/package.json @@ -16,7 +16,7 @@ "scripts": { "build": "tsc -p tsconfig.json", "typecheck": "tsc -p tsconfig.json --noEmit", - "test": "node --test test/" + "test": "node ../../scripts/run-tests-if-present.mjs" }, "dependencies": { "@html-video/core": "workspace:*", diff --git a/packages/cli/package.json b/packages/cli/package.json index d2863ca..a6dbd4d 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -11,10 +11,10 @@ "types": "./dist/index.d.ts", "files": ["dist", "README.md"], "scripts": { - "build": "tsc -p tsconfig.json && chmod +x dist/bin.js", + "build": "tsc -p tsconfig.json && node scripts/chmod-bin.mjs", "typecheck": "tsc -p tsconfig.json --noEmit", "smoke": "node dist/smoke.js", - "test": "node --test test/" + "test": "node ../../scripts/run-tests-if-present.mjs" }, "dependencies": { "@html-video/content-graph": "workspace:*", diff --git a/packages/cli/scripts/chmod-bin.mjs b/packages/cli/scripts/chmod-bin.mjs new file mode 100644 index 0000000..dbcacc1 --- /dev/null +++ b/packages/cli/scripts/chmod-bin.mjs @@ -0,0 +1,7 @@ +import { chmodSync, existsSync } from "node:fs"; + +const binPath = new URL("../dist/bin.js", import.meta.url); + +if (existsSync(binPath)) { + chmodSync(binPath, 0o755); +} diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 76ed57b..1fb0cc2 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -1,4 +1,5 @@ -import { execSync } from 'node:child_process'; +import { execFileSync } from 'node:child_process'; +import { accessSync } from 'node:fs'; import type { CliContext } from '../context.js'; import { ok } from '../output.js'; @@ -12,9 +13,10 @@ interface Check { function which(cmd: string): string | null { try { - return execSync(`which ${cmd}`, { stdio: ['ignore', 'pipe', 'ignore'] }) - .toString() - .trim() || null; + const lookup = process.platform === 'win32' ? 'where.exe' : 'which'; + const output = execFileSync(lookup, [cmd], { stdio: ['ignore', 'pipe', 'ignore'] }).toString(); + const firstMatch = output.split(/\r?\n/)[0] ?? ''; + return firstMatch.trim() || null; } catch { return null; } @@ -22,16 +24,25 @@ function which(cmd: string): string | null { function version(cmd: string, args = '--version'): string | null { try { - return execSync(`${cmd} ${args}`, { stdio: ['ignore', 'pipe', 'ignore'] }) + const output = execFileSync(cmd, args.split(' '), { stdio: ['ignore', 'pipe', 'ignore'] }) .toString() .trim() - .split('\n')[0] - ?? null; + .split('\n')[0]; + return output ?? null; } catch { return null; } } +function existsOnDisk(path: string): boolean { + try { + accessSync(path); + return true; + } catch { + return false; + } +} + export async function runDoctor(ctx: CliContext): Promise { const checks: Check[] = []; @@ -55,24 +66,29 @@ export async function runDoctor(ctx: CliContext): Promise { }); } - // chromium / chrome (for HF puppeteer) + // chromium / chrome (for HF playwright) const chromePaths = [ '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome', '/usr/bin/chromium', '/usr/bin/google-chrome', + 'C:\\Program Files\\Google\\Chrome\\Application\\chrome.exe', + 'C:\\Program Files (x86)\\Google\\Chrome\\Application\\chrome.exe', + 'C:\\Program Files\\Microsoft\\Edge\\Application\\msedge.exe', + 'C:\\Program Files (x86)\\Microsoft\\Edge\\Application\\msedge.exe', ]; - const chromiumOk = chromePaths.some((p) => { - try { - execSync(`test -x "${p}"`); - return true; - } catch { - return false; - } - }); + const chromiumPath = + which('chromium') || + which('chromium-browser') || + which('google-chrome') || + which('chrome') || + which('msedge') || + chromePaths.find(existsOnDisk) || + null; checks.push({ name: 'chromium', - status: chromiumOk ? 'ok' : 'warning', - detail: chromiumOk ? 'Chrome found in standard location' : 'Chrome/Chromium not detected; HF render will need a browser', + status: chromiumPath ? 'ok' : 'warning', + value: chromiumPath ?? undefined, + detail: chromiumPath ? 'Chrome/Chromium-compatible browser detected' : 'Chrome/Chromium not detected; HF render will need a browser', }); // Engines diff --git a/packages/cli/src/studio-server.ts b/packages/cli/src/studio-server.ts index ad1a0d0..4cc5861 100644 --- a/packages/cli/src/studio-server.ts +++ b/packages/cli/src/studio-server.ts @@ -60,6 +60,12 @@ export async function startStudioServer(ctx: CliContext, port: number): Promise< const url = new URL(req.url, 'http://x'); const m = req.method ?? 'GET'; + if (url.pathname === '/favicon.ico') { + res.writeHead(204); + res.end(); + return; + } + // ============== API ============== // List projects diff --git a/packages/core/package.json b/packages/core/package.json index 5bac310..dae2f7d 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -20,7 +20,7 @@ "scripts": { "build": "tsc -p tsconfig.json", "typecheck": "tsc -p tsconfig.json --noEmit", - "test": "node --test test/" + "test": "node ../../scripts/run-tests-if-present.mjs" }, "dependencies": { "@html-video/content-graph": "workspace:*", diff --git a/packages/runtime/src/detect.ts b/packages/runtime/src/detect.ts index 576978b..c31163e 100644 --- a/packages/runtime/src/detect.ts +++ b/packages/runtime/src/detect.ts @@ -1,15 +1,39 @@ import { execFile } from 'node:child_process'; import { accessSync, constants } from 'node:fs'; +import { extname } from 'node:path'; import { promisify } from 'node:util'; import { AGENT_DEFS } from './registry.js'; import type { AgentDef, DetectedAgent } from './types.js'; const exec = promisify(execFile); +function pickPathMatch(stdout: string): string | null { + const matches = stdout + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean); + if (matches.length === 0) return null; + if (process.platform !== 'win32') return matches[0] ?? null; + + return matches.find((p) => ['.exe', '.cmd', '.bat'].includes(extname(p).toLowerCase())) + ?? matches[0] + ?? null; +} + +function quoteCmdArg(arg: string): string { + if (/^[A-Za-z0-9_./:\\-]+$/.test(arg)) return arg; + return `"${arg.replace(/"/g, '\\"')}"`; +} + +function isWindowsCommandShim(bin: string): boolean { + return process.platform === 'win32' && ['.cmd', '.bat'].includes(extname(bin).toLowerCase()); +} + async function which(bin: string): Promise { try { - const { stdout } = await exec('which', [bin], { timeout: 2000 }); - return stdout.trim() || null; + const lookup = process.platform === 'win32' ? 'where.exe' : 'which'; + const { stdout } = await exec(lookup, [bin], { timeout: 2000 }); + return pickPathMatch(stdout); } catch { return null; } @@ -21,7 +45,7 @@ export async function resolveBin(def: AgentDef): Promise { if (onPath) return onPath; for (const candidate of def.binFallbacks ?? []) { try { - accessSync(candidate, constants.X_OK); + accessSync(candidate, constants.F_OK); return candidate; } catch { /* not there / not executable — try next */ @@ -31,7 +55,7 @@ export async function resolveBin(def: AgentDef): Promise { try { const resolved = await def.resolveBinFallback(); if (resolved) { - accessSync(resolved, constants.X_OK); + accessSync(resolved, constants.F_OK); return resolved; } } catch { @@ -43,7 +67,13 @@ export async function resolveBin(def: AgentDef): Promise { async function probeVersion(bin: string, args: string[]): Promise { try { - const { stdout } = await exec(bin, args, { timeout: 5000 }); + const command = isWindowsCommandShim(bin) + ? { + file: 'cmd.exe', + args: ['/d', '/s', '/c', [bin, ...args].map(quoteCmdArg).join(' ')], + } + : { file: bin, args }; + const { stdout } = await exec(command.file, command.args, { timeout: 5000 }); return stdout.trim().split('\n')[0] ?? null; } catch { return null; diff --git a/packages/runtime/src/spawn.ts b/packages/runtime/src/spawn.ts index 1f1b513..cb922c5 100644 --- a/packages/runtime/src/spawn.ts +++ b/packages/runtime/src/spawn.ts @@ -1,6 +1,23 @@ import { spawn as cpSpawn } from 'node:child_process'; +import { extname } from 'node:path'; import type { AgentDef, AgentEvent, AgentInvokeContext, SpawnHandle } from './types.js'; +function quoteCmdArg(arg: string): string { + if (/^[A-Za-z0-9_./:\\-]+$/.test(arg)) return arg; + return `"${arg.replace(/"/g, '\\"')}"`; +} + +function spawnTarget(bin: string, args: string[]): { bin: string; args: string[] } { + const ext = extname(bin).toLowerCase(); + if (process.platform === 'win32' && ['.cmd', '.bat'].includes(ext)) { + return { + bin: 'cmd.exe', + args: ['/d', '/s', '/c', [bin, ...args].map(quoteCmdArg).join(' ')], + }; + } + return { bin, args }; +} + /** * Spawn an agent CLI and stream events to the listener. * v0.1: only supports streamFormat='plain' fully (chunks emitted as text events). @@ -68,82 +85,92 @@ export function spawnAgent(opts: SpawnOptions): SpawnHandle { return { pid: 0, stop: () => ac.abort(), done }; } - const args = def.buildArgs(prompt, context); - const env = { ...process.env, ...(def.env ?? {}) }; + let child: ReturnType | null = null; + const done = (async () => { + const { resolveBin } = await import('./detect.js'); + const bin = await resolveBin(def); + if (!bin) { + onEvent?.({ type: 'error', message: `${def.name}: binary "${def.bin}" not found` }); + onEvent?.({ type: 'message_end', reason: 'error' }); + return { exitCode: -1, signal: null as NodeJS.Signals | null }; + } - const child = cpSpawn(def.bin, args, { - cwd: context.cwd, - env, - stdio: ['pipe', 'pipe', 'pipe'], - }); + const args = def.buildArgs(prompt, context); + const env = { ...process.env, ...(def.env ?? {}) }; + const target = spawnTarget(bin, args); - if (def.promptViaStdin && child.stdin) { - child.stdin.write(prompt); - child.stdin.end(); - } + child = cpSpawn(target.bin, target.args, { + cwd: context.cwd, + env, + stdio: ['pipe', 'pipe', 'pipe'], + }); - let stdoutBuf = ''; - let stderrBuf = ''; - - child.stdout?.on('data', (chunk: Buffer) => { - const text = chunk.toString('utf8'); - stdoutBuf += text; - if (def.streamFormat === 'plain') { - onEvent?.({ type: 'text', chunk: text }); - } else if (def.streamFormat === 'claude-stream' || def.streamFormat === 'json-event-stream') { - // v0.2 hook: parse NDJSON and emit structured events - const lines = text.split('\n'); - for (const line of lines) { - if (!line.trim()) continue; - try { - const obj = JSON.parse(line); - if (typeof obj === 'object' && obj && 'type' in obj) { - // claude stream-json events have richer shape; treat unknown as text - onEvent?.({ type: 'text', chunk: JSON.stringify(obj) + '\n' }); - } - } catch { - onEvent?.({ type: 'text', chunk: line + '\n' }); - } - } + if (def.promptViaStdin && child.stdin) { + child.stdin.write(prompt); + child.stdin.end(); } - }); - child.stderr?.on('data', (chunk: Buffer) => { - stderrBuf += chunk.toString('utf8'); - }); + let stderrBuf = ''; - if (opts.signal) { - opts.signal.addEventListener('abort', () => { - try { - child.kill('SIGTERM'); - } catch { - // ignore + child.stdout?.on('data', (chunk: Buffer) => { + const text = chunk.toString('utf8'); + if (def.streamFormat === 'plain') { + onEvent?.({ type: 'text', chunk: text }); + } else if (def.streamFormat === 'claude-stream' || def.streamFormat === 'json-event-stream') { + // v0.2 hook: parse NDJSON and emit structured events + const lines = text.split('\n'); + for (const line of lines) { + if (!line.trim()) continue; + try { + const obj = JSON.parse(line); + if (typeof obj === 'object' && obj && 'type' in obj) { + // claude stream-json events have richer shape; treat unknown as text + onEvent?.({ type: 'text', chunk: JSON.stringify(obj) + '\n' }); + } + } catch { + onEvent?.({ type: 'text', chunk: line + '\n' }); + } + } } }); - } - const done = new Promise<{ exitCode: number; signal: NodeJS.Signals | null }>((resolve) => { - child.on('close', (code, signal) => { - if (code !== 0) { - onEvent?.({ - type: 'error', - message: `agent exit code ${code}${stderrBuf ? `: ${stderrBuf.slice(0, 500)}` : ''}`, - }); - } - onEvent?.({ type: 'message_end', reason: code === 0 ? 'ok' : 'error' }); - resolve({ exitCode: code ?? 0, signal }); + child.stderr?.on('data', (chunk: Buffer) => { + stderrBuf += chunk.toString('utf8'); }); - child.on('error', (err) => { - onEvent?.({ type: 'error', message: err.message }); - resolve({ exitCode: -1, signal: null }); + + if (opts.signal) { + opts.signal.addEventListener('abort', () => { + try { + child?.kill('SIGTERM'); + } catch { + // ignore + } + }); + } + + return await new Promise<{ exitCode: number; signal: NodeJS.Signals | null }>((resolve) => { + child?.on('close', (code, signal) => { + if (code !== 0) { + onEvent?.({ + type: 'error', + message: `agent exit code ${code}${stderrBuf ? `: ${stderrBuf.slice(0, 500)}` : ''}`, + }); + } + onEvent?.({ type: 'message_end', reason: code === 0 ? 'ok' : 'error' }); + resolve({ exitCode: code ?? 0, signal }); + }); + child?.on('error', (err) => { + onEvent?.({ type: 'error', message: err.message }); + resolve({ exitCode: -1, signal: null }); + }); }); - }); + })(); return { - pid: child.pid ?? 0, + pid: 0, stop: () => { try { - child.kill('SIGTERM'); + child?.kill('SIGTERM'); } catch { // ignore } diff --git a/scripts/run-tests-if-present.mjs b/scripts/run-tests-if-present.mjs new file mode 100644 index 0000000..3ded52f --- /dev/null +++ b/scripts/run-tests-if-present.mjs @@ -0,0 +1,13 @@ +import { existsSync } from "node:fs"; +import { spawnSync } from "node:child_process"; + +if (!existsSync("test")) { + console.log("No test/ directory; skipping package tests."); + process.exit(0); +} + +const result = spawnSync(process.execPath, ["--test", "test/"], { + stdio: "inherit", +}); + +process.exit(result.status ?? 1);