From 1dd4f5996ffd57cb968103aaf1ecb63e54910796 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Sat, 11 Apr 2026 11:53:22 -0500 Subject: [PATCH] fix(mosaic): stop yolo runtime from leaking runtime name as first user message `mosaic yolo ` forwarded `cmd.args` (which Commander populates with the declared `` positional) to the underlying CLI. For the claude runtime that meant argv included the literal string "claude" as an initial positional argument, which Claude Code interpreted as the first user message. As a secondary consequence, the mission-auto-prompt path was also bypassed because `hasMissionNoArgs` saw a non-empty args list. Refactor the runtime subcommand wiring into an exported `registerRuntimeLaunchers(program, handler)` factory with a `RuntimeLaunchHandler` type so the commander integration is unit-testable without spawning subprocesses. Slice the declared positional off `cmd.args` in the yolo action before forwarding. Adds `packages/mosaic/src/commands/launch.spec.ts` with 11 regression tests covering all four runtimes (claude, codex, opencode, pi) for both `` and `yolo ` paths, excess-args forwarding, and invalid-runtime rejection. The critical assertion blocks the regression: yolo parses must forward `extraArgs = []`, not `[runtime]`. Fixes mosaicstack/stack#454 Co-Authored-By: Claude Opus 4.6 --- .../yolo-runtime-initial-arg-20260411.md | 108 +++++++++++++++++ packages/mosaic/src/commands/launch.spec.ts | 111 ++++++++++++++++++ packages/mosaic/src/commands/launch.ts | 37 +++++- 3 files changed, 251 insertions(+), 5 deletions(-) create mode 100644 docs/scratchpads/yolo-runtime-initial-arg-20260411.md create mode 100644 packages/mosaic/src/commands/launch.spec.ts diff --git a/docs/scratchpads/yolo-runtime-initial-arg-20260411.md b/docs/scratchpads/yolo-runtime-initial-arg-20260411.md new file mode 100644 index 0000000..9270dc3 --- /dev/null +++ b/docs/scratchpads/yolo-runtime-initial-arg-20260411.md @@ -0,0 +1,108 @@ +# Hotfix Scratchpad — `mosaic yolo ` passes runtime name as initial user message + +- **Issue:** mosaicstack/stack#454 +- **Branch:** `fix/yolo-runtime-initial-arg` +- **Type:** Out-of-mission hotfix (not part of Install UX v2 mission) +- **Started:** 2026-04-11 + +## Objective + +Stop `mosaic yolo ` from passing the runtime name (`claude`, `codex`, etc.) as the initial user message to the underlying CLI. Restore the mission-auto-prompt path for yolo launches. + +## Root cause (confirmed) + +`packages/mosaic/src/commands/launch.ts:779` — the `yolo ` action handler: + +```ts +.action((runtime: string, _opts: unknown, cmd: Command) => { + // ... validate runtime ... + launchRuntime(runtime as RuntimeName, cmd.args, true); +}); +``` + +Commander.js includes declared positional arguments in `cmd.args`. For `mosaic yolo claude`: + +- `runtime` (destructured) = `"claude"` +- `cmd.args` = `["claude"]` — the same value + +`launchRuntime` treats `["claude"]` as excess positional args, and for the `claude` case that becomes the initial user message. As a secondary consequence, `hasMissionNoArgs` evaluates false, so the mission-auto-prompt path is bypassed too. + +## Live reproduction (intercepted claude binary) + +``` +$ PATH=/tmp/fake-claude-bin:$PATH mosaic yolo claude +[mosaic] Launching Claude Code in YOLO mode... +argv[1]: --dangerously-skip-permissions +argv[2]: --append-system-prompt +argv[3] (len=25601): # ACTIVE MISSION — HARD GATE ... +argv[4]: claude ← the bug +``` + +Non-yolo variant `mosaic claude` is clean: + +``` +argv[1]: --append-system-prompt +argv[2]: +argv[3]: Active mission detected: MVP. Read the mission state files and report status. +``` + +## Plan + +1. Refactor `launch.ts`: extract `registerRuntimeLaunchers(program, handler)` with an injectable handler so commander wiring is testable without spawning subprocesses. `registerLaunchCommands` delegates to it with `launchRuntime` as the handler. +2. Fix: in the `yolo ` action, pass `cmd.args.slice(1)` instead of `cmd.args`. +3. Add `packages/mosaic/src/commands/launch.spec.ts`: + - Failing-first reproducer: parse `['node','x','yolo','claude']` and assert handler receives `extraArgs=[]` and `yolo=true`. + - Regression test: parse `['node','x','claude']` asserts handler receives `extraArgs=[]` and `yolo=false`. + - Excess args: parse `['node','x','yolo','claude','--print','hi']` asserts handler receives `extraArgs=['--print','hi']` (with `--print` kept because `allowUnknownOption` is true). + - Excess args non-yolo: parse `['node','x','claude','--print','hi']` asserts `extraArgs=['--print','hi']`. + - Reject unknown runtime under yolo. +4. Run typecheck, lint, format:check, vitest for `@mosaicstack/mosaic`. +5. Independent code review (feature-dev:code-reviewer subagent, sonnet tier). +6. Commit → push → PR via wrappers → merge → CI green → close issue #454. +7. Release decision (`mosaic-v0.0.30`) deferred to Jason after merge. + +## Framework compliance sub-findings (out-of-scope; to capture in OpenBrain after) + +- `~/.config/mosaic/tools/git/issue-create.sh` uses `eval` on `$BODY`; arbitrary bodies with backticks, `$`, or parens break catastrophically. +- `gitea_issue_create_api` fallback uses `curl -fsS` without `-L`; after the `mosaicstack/mosaic-stack → mosaicstack/stack` rename, the API redirect is not followed and the fallback silently fails. +- Local repo `origin` remote still points at old `mosaic/mosaic-stack.git` slug. Not touched here per git-config safety rule. +- `~/.config/mosaic/TOOLS.md` referenced by the global load order but does not exist on disk. + +These will be captured to OpenBrain after the hotfix merges so they don't get lost, and filed as separate tracking items. + +## Progress checkpoints + +- [x] Branch created (`fix/yolo-runtime-initial-arg`) +- [x] Issue #454 opened +- [x] Scratchpad scaffolded +- [x] Failing test added (red) +- [x] Refactor + fix applied +- [x] Tests green (launch.spec.ts 11/11) +- [x] Baselines green (typecheck, lint, format:check, vitest — pre-existing `uninstall.spec.ts:138` failure on branch main acknowledged, not caused by this change) +- [x] Code review pass (feature-dev:code-reviewer, sonnet — no blockers) +- [ ] Commit + push +- [ ] PR opened +- [ ] CI queue guard cleared +- [ ] PR merged (squash) +- [ ] CI green on main +- [ ] Issue #454 closed +- [ ] Scratchpad final evidence entry + +## Tests run + +- `pnpm --filter @mosaicstack/mosaic run typecheck` → green +- `pnpm --filter @mosaicstack/mosaic run lint` → green +- `pnpm --filter @mosaicstack/mosaic exec prettier --check "src/**/*.ts"` → green +- `pnpm --filter @mosaicstack/mosaic exec vitest run src/commands/launch.spec.ts` → 11/11 pass +- `pnpm --filter @mosaicstack/mosaic exec vitest run` → 270/271 pass (1 pre-existing `uninstall.spec.ts:138` EACCES failure, confirmed on the branch before this change) +- `pnpm typecheck` (repo) → green +- `pnpm lint` (repo) → green +- `pnpm format:check` (repo) → green (after prettier-writing the scratchpad) + +## Risks / blockers + +None expected. Refactor is small and the Commander API is stable. Test needs `exitOverride()` to prevent `process.exit` on invalid runtime. + +## Final verification evidence + +(to be filled at completion) diff --git a/packages/mosaic/src/commands/launch.spec.ts b/packages/mosaic/src/commands/launch.spec.ts new file mode 100644 index 0000000..e1846d2 --- /dev/null +++ b/packages/mosaic/src/commands/launch.spec.ts @@ -0,0 +1,111 @@ +import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from 'vitest'; +import { Command } from 'commander'; +import { registerRuntimeLaunchers, type RuntimeLaunchHandler } from './launch.js'; + +/** + * Tests for the commander wiring between `mosaic ` / `mosaic yolo ` + * subcommands and the internal `launchRuntime` dispatcher. + * + * Regression target: see mosaicstack/stack#454 — before the fix, `mosaic yolo claude` + * passed the literal string "claude" as an excess positional argument to the + * underlying CLI, which Claude Code then interpreted as the first user message. + * + * The bug existed because Commander.js includes declared positional arguments + * (here ``) in `cmd.args` alongside any true excess args. The action + * handler must slice them off before forwarding. + */ + +function buildProgram(handler: RuntimeLaunchHandler): Command { + const program = new Command(); + program.exitOverride(); // prevent process.exit on parse errors + registerRuntimeLaunchers(program, handler); + return program; +} + +// `process.exit` returns `never`, so vi.spyOn demands a replacement with the +// same signature. We throw from the mock to short-circuit into test-land. +const exitThrows = (): never => { + throw new Error('process.exit called'); +}; + +describe('registerRuntimeLaunchers — non-yolo subcommands', () => { + let mockExit: MockInstance; + + beforeEach(() => { + // process.exit is called when the yolo action rejects an invalid runtime. + // Stub it so the assertion catches the rejection instead of terminating + // the test runner. + mockExit = vi.spyOn(process, 'exit').mockImplementation(exitThrows); + }); + + afterEach(() => { + mockExit.mockRestore(); + }); + + it.each(['claude', 'codex', 'opencode', 'pi'] as const)( + 'forwards %s with empty extraArgs and yolo=false', + (runtime) => { + const handler = vi.fn(); + const program = buildProgram(handler); + program.parse(['node', 'mosaic', runtime]); + + expect(handler).toHaveBeenCalledTimes(1); + expect(handler).toHaveBeenCalledWith(runtime, [], false); + }, + ); + + it('forwards excess args after a non-yolo runtime subcommand', () => { + const handler = vi.fn(); + const program = buildProgram(handler); + program.parse(['node', 'mosaic', 'claude', '--print', 'hello']); + + expect(handler).toHaveBeenCalledWith('claude', ['--print', 'hello'], false); + }); +}); + +describe('registerRuntimeLaunchers — yolo ', () => { + let mockExit: MockInstance; + let mockError: MockInstance; + + beforeEach(() => { + mockExit = vi.spyOn(process, 'exit').mockImplementation(exitThrows); + mockError = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + mockExit.mockRestore(); + mockError.mockRestore(); + }); + + it.each(['claude', 'codex', 'opencode', 'pi'] as const)( + 'does NOT pass the runtime name as an extra arg (regression #454) for yolo %s', + (runtime) => { + const handler = vi.fn(); + const program = buildProgram(handler); + program.parse(['node', 'mosaic', 'yolo', runtime]); + + expect(handler).toHaveBeenCalledTimes(1); + // The critical assertion: extraArgs must be empty, not [runtime]. + // Before the fix, cmd.args was [runtime] and the runtime name leaked + // through to the underlying CLI as an initial positional argument. + expect(handler).toHaveBeenCalledWith(runtime, [], true); + }, + ); + + it('forwards true excess args after a yolo runtime', () => { + const handler = vi.fn(); + const program = buildProgram(handler); + program.parse(['node', 'mosaic', 'yolo', 'claude', '--print', 'hi']); + + expect(handler).toHaveBeenCalledWith('claude', ['--print', 'hi'], true); + }); + + it('rejects an unknown runtime under yolo without invoking the handler', () => { + const handler = vi.fn(); + const program = buildProgram(handler); + + expect(() => program.parse(['node', 'mosaic', 'yolo', 'bogus'])).toThrow('process.exit called'); + expect(handler).not.toHaveBeenCalled(); + expect(mockExit).toHaveBeenCalledWith(1); + }); +}); diff --git a/packages/mosaic/src/commands/launch.ts b/packages/mosaic/src/commands/launch.ts index fe7c57d..76c1702 100644 --- a/packages/mosaic/src/commands/launch.ts +++ b/packages/mosaic/src/commands/launch.ts @@ -757,8 +757,23 @@ function runUpgrade(args: string[]): never { // ─── Commander registration ───────────────────────────────────────────────── -export function registerLaunchCommands(program: Command): void { - // Runtime launchers +/** + * Handler invoked when a runtime subcommand (`` or `yolo `) + * is parsed. Exposed so tests can exercise the commander wiring without + * spawning subprocesses. + */ +export type RuntimeLaunchHandler = ( + runtime: RuntimeName, + extraArgs: string[], + yolo: boolean, +) => void; + +/** + * Wire `` and `yolo ` subcommands onto `program` using a + * pluggable launch handler. Separated from `registerLaunchCommands` so tests + * can inject a spy and verify argument forwarding. + */ +export function registerRuntimeLaunchers(program: Command, handler: RuntimeLaunchHandler): void { for (const runtime of ['claude', 'codex', 'opencode', 'pi'] as const) { program .command(runtime) @@ -766,11 +781,10 @@ export function registerLaunchCommands(program: Command): void { .allowUnknownOption(true) .allowExcessArguments(true) .action((_opts: unknown, cmd: Command) => { - launchRuntime(runtime, cmd.args, false); + handler(runtime, cmd.args, false); }); } - // Yolo mode program .command('yolo ') .description('Launch a runtime in dangerous-permissions mode (claude|codex|opencode|pi)') @@ -784,8 +798,21 @@ export function registerLaunchCommands(program: Command): void { ); process.exit(1); } - launchRuntime(runtime as RuntimeName, cmd.args, true); + // Commander includes declared positional arguments (``) in + // `cmd.args` alongside any trailing excess args. Slice off the first + // element so we forward only true excess args — otherwise the runtime + // name leaks into the underlying CLI as an initial positional arg, + // which Claude Code interprets as the first user message. + // Regression test: launch.spec.ts, issue mosaicstack/stack#454. + handler(runtime as RuntimeName, cmd.args.slice(1), true); }); +} + +export function registerLaunchCommands(program: Command): void { + // Runtime launchers + yolo mode wired to the real process-replacing launcher. + registerRuntimeLaunchers(program, (runtime, extraArgs, yolo) => { + launchRuntime(runtime, extraArgs, yolo); + }); // Coord (mission orchestrator) program -- 2.49.1