diff --git a/skill/scripts/live/roots.mjs b/skill/scripts/live/roots.mjs index 3c0a32e9e..a34b68c1e 100644 --- a/skill/scripts/live/roots.mjs +++ b/skill/scripts/live/roots.mjs @@ -442,13 +442,22 @@ export function resolveLiveRoots(cwd = process.cwd(), { targetPath = null } = {} export function consumeTargetArg(argv = process.argv) { for (let i = 0; i < argv.length; i++) { const arg = argv[i]; - if (arg === '--target' && typeof argv[i + 1] === 'string') { + if (arg === '--target') { const value = argv[i + 1]; + // A --target with no usable value must not degrade into implicit root + // selection: these helpers mutate session state, and "the most recent + // app" is exactly what the caller was trying NOT to get. + if (typeof value !== 'string' || value === '' || value.startsWith('--')) { + throw new Error('--target requires a path value (use --target or --target=)'); + } argv.splice(i, 2); return value; } if (typeof arg === 'string' && arg.startsWith('--target=')) { const value = arg.slice('--target='.length); + if (value === '') { + throw new Error('--target requires a path value (use --target or --target=)'); + } argv.splice(i, 1); return value; } @@ -462,12 +471,19 @@ export function consumeTargetArg(argv = process.argv) { * with the boot. An explicit `--target ` on the helper's command line * overrides pointer resolution, which is what disambiguates a repo with * several live apps (the multi-app warning names this escape hatch, so it - * has to actually work on every helper). Returns the manifest. Never - * throws; on selection ambiguity it stays in the current directory (the - * boot flow handles prompting). + * has to actually work on every helper). Returns the manifest. On selection + * ambiguity it stays in the current directory (the boot flow handles + * prompting); a malformed --target exits with an error instead of silently + * falling back to implicit selection, which could mutate the wrong app. */ export function enterLiveRoot(cwd = process.cwd()) { - const targetPath = consumeTargetArg(process.argv); + let targetPath; + try { + targetPath = consumeTargetArg(process.argv); + } catch (err) { + console.error(`[impeccable live] ${err.message}`); + process.exit(1); + } const resolved = resolveLiveRoots(cwd, targetPath ? { targetPath } : {}); if (!resolved.manifest) return null; const appRoot = resolved.manifest.appRoot; diff --git a/tests/live-roots.test.mjs b/tests/live-roots.test.mjs index 3a5704215..2f5f8d127 100644 --- a/tests/live-roots.test.mjs +++ b/tests/live-roots.test.mjs @@ -6,6 +6,7 @@ import { join, dirname } from 'node:path'; import { tmpdir } from 'node:os'; import { fileURLToPath } from 'node:url'; import { + consumeTargetArg, discoverAppCandidates, findGitRoot, resolveLiveRoots, @@ -322,6 +323,45 @@ describe('review regressions: helper --target', () => { rmSync(repo, { recursive: true, force: true }); } }); + + it('rejects a --target with no usable value instead of falling back to implicit selection', () => { + for (const argv of [ + ['node', 'live-complete.mjs', '--target'], + ['node', 'live-complete.mjs', '--target='], + ['node', 'live-complete.mjs', '--target', '--id'], + ]) { + assert.throws(() => consumeTargetArg([...argv]), /--target requires a path value/); + } + // Well-formed values still parse and are consumed. + const argv = ['node', 'live-complete.mjs', '--target', 'appB', '--id', 'x']; + assert.equal(consumeTargetArg(argv), 'appB'); + assert.deepEqual(argv, ['node', 'live-complete.mjs', '--id', 'x']); + }); + + it('enterLiveRoot exits with an error on a valueless --target rather than picking an app', () => { + const repo = realpathSync(mkdtempSync(join(tmpdir(), 'impeccable-roots-target-bad-'))); + try { + mkdirSync(join(repo, '.git'), { recursive: true }); + write(repo, 'appA/vite.config.js', 'export default {};'); + const a = resolveRoots({ cwd: repo, targetPath: join(repo, 'appA/vite.config.js') }).manifest; + writeRootsManifest(a); + write(repo, 'appA/.impeccable/live/server.json', JSON.stringify({ pid: process.pid, port: 1, token: 't' })); + + const res = spawnSync(process.execPath, [ + '-e', + `import(${JSON.stringify(ROOTS_MODULE)}).then((m) => { + process.argv.push('--target'); + m.enterLiveRoot(); + console.log('reached:' + process.cwd()); + });`, + ], { cwd: repo, encoding: 'utf-8' }); + assert.notEqual(res.status, 0, 'malformed --target must not proceed'); + assert.match(res.stderr, /--target requires a path value/); + assert.doesNotMatch(res.stdout, /reached:/, 'helper body must not run'); + } finally { + rmSync(repo, { recursive: true, force: true }); + } + }); }); describe('review regressions: pid reuse', () => {