fix(mosaic): stop yolo runtime from leaking runtime name as first user message #455
108
docs/scratchpads/yolo-runtime-initial-arg-20260411.md
Normal file
108
docs/scratchpads/yolo-runtime-initial-arg-20260411.md
Normal 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)
|
||||
111
packages/mosaic/src/commands/launch.spec.ts
Normal file
111
packages/mosaic/src/commands/launch.spec.ts
Normal 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);
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user