fix(mosaic): stop yolo runtime from leaking runtime name as first user message (#455)
Fixes mosaicstack/stack#454 Co-authored-by: Jason Woltje <jason@diversecanvas.com> Co-committed-by: Jason Woltje <jason@diversecanvas.com>
This commit was merged in pull request #455.
This commit is contained in:
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 ─────────────────────────────────────────────────
|
// ─── 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) {
|
for (const runtime of ['claude', 'codex', 'opencode', 'pi'] as const) {
|
||||||
program
|
program
|
||||||
.command(runtime)
|
.command(runtime)
|
||||||
@@ -766,11 +781,10 @@ export function registerLaunchCommands(program: Command): void {
|
|||||||
.allowUnknownOption(true)
|
.allowUnknownOption(true)
|
||||||
.allowExcessArguments(true)
|
.allowExcessArguments(true)
|
||||||
.action((_opts: unknown, cmd: Command) => {
|
.action((_opts: unknown, cmd: Command) => {
|
||||||
launchRuntime(runtime, cmd.args, false);
|
handler(runtime, cmd.args, false);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Yolo mode
|
|
||||||
program
|
program
|
||||||
.command('yolo <runtime>')
|
.command('yolo <runtime>')
|
||||||
.description('Launch a runtime in dangerous-permissions mode (claude|codex|opencode|pi)')
|
.description('Launch a runtime in dangerous-permissions mode (claude|codex|opencode|pi)')
|
||||||
@@ -784,7 +798,20 @@ export function registerLaunchCommands(program: Command): void {
|
|||||||
);
|
);
|
||||||
process.exit(1);
|
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)
|
// Coord (mission orchestrator)
|
||||||
|
|||||||
Reference in New Issue
Block a user