fix(mosaic): stop yolo runtime from leaking runtime name as first user message #455

Merged
jason.woltje merged 1 commits from fix/yolo-runtime-initial-arg into main 2026-04-11 16:57:44 +00:00
3 changed files with 251 additions and 5 deletions

View File

@@ -0,0 +1,108 @@
# Hotfix Scratchpad — `mosaic yolo <runtime>` 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 <runtime>` 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 <runtime>` 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]: <prompt>
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 <runtime>` 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)

View File

@@ -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 <runtime>` / `mosaic yolo <runtime>`
* 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 `<runtime>`) 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<typeof process.exit>;
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 <runtime>', () => {
let mockExit: MockInstance<typeof process.exit>;
let mockError: MockInstance<typeof console.error>;
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);
});
});

View File

@@ -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 (`<runtime>` or `yolo <runtime>`)
* is parsed. Exposed so tests can exercise the commander wiring without
* spawning subprocesses.
*/
export type RuntimeLaunchHandler = (
runtime: RuntimeName,
extraArgs: string[],
yolo: boolean,
) => void;
/**
* Wire `<runtime>` and `yolo <runtime>` 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 <runtime>')
.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 (`<runtime>`) 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