1fbc2cbd79
Follow-up to #1503. When an extension registered a flag whose name collides with a value-taking built-in — e.g. plan-mode's boolean `--plan` vs the built-in `--plan <plan-model>` selector — the extension-aware reparse still took the built-in branch. `omp --extension plan-mode --plan "review the diff"` consumed "review the diff" as the plan-model value, leaving parsed.messages empty and overwriting result.plan with the prompt text. recoverFlagValue only patched the extension flag value, not the corrupted parsed object that applyExtensionFlags returns as initialArgs. Fix at the source: parseArgs now checks the registered extension-flag set BEFORE the built-in branches, so a registered flag is parsed with the extension's semantics (boolean toggle / string value) and surfaces in unknownFlags without consuming the following token or touching the built-in field. This makes recoverFlagValue dead, so applyExtensionFlags is simplified to read resolved values straight from unknownFlags. Tests: parseArgs-level shadowing guard (boolean --plan keeps the message and leaves result.plan unset); applyExtensionFlags message/built-in-field preservation for colliding boolean (--plan) and string (--model) flags; non-colliding flag-looking-value rule retained. Verified the new guards fail without the shadowing fix.
49 lines
2.2 KiB
TypeScript
49 lines
2.2 KiB
TypeScript
import { type Args, parseArgs } from "./args";
|
|
|
|
/**
|
|
* Minimal extension-runner surface needed to resolve CLI flag values. The real
|
|
* `ExtensionRunner` satisfies this structurally; depending only on the surface
|
|
* keeps this module free of the heavier runner/session imports and unit-testable
|
|
* with a fake.
|
|
*/
|
|
export interface ExtensionFlagSink {
|
|
getFlags(): Map<string, { type: "boolean" | "string" }>;
|
|
setFlagValue(name: string, value: boolean | string): void;
|
|
}
|
|
|
|
/**
|
|
* Resolve extension-registered CLI flags from `rawArgs` once the flag set is
|
|
* known, push the resolved values onto the sink, and return the parsed
|
|
* {@link Args} (whose `messages` and `fileArgs` now reflect those flags).
|
|
*
|
|
* The startup parse runs before extensions load, so it cannot recognise their
|
|
* flags: a string flag's value (`--spawn-peer reviewer` or `--spawn-peer=reviewer`)
|
|
* is otherwise left in `messages` and leaks into the initial prompt. Re-parsing
|
|
* here — through the *same* {@link parseArgs} the startup pass uses, now seeded
|
|
* with the registered flags — consumes every flag form (`--flag`, `--flag value`,
|
|
* `--flag=value`).
|
|
*
|
|
* {@link parseArgs} lets a registered flag shadow a same-named built-in, so even
|
|
* a built-in-colliding flag (e.g. plan-mode's boolean `--plan`, which would
|
|
* otherwise hit the built-in plan-model branch) is parsed with the extension's
|
|
* semantics and surfaces in `unknownFlags` — without consuming the following
|
|
* message or overwriting the built-in field. No built-in name list to maintain.
|
|
*
|
|
* Returns `null` when there is no sink or no registered extension flags, in
|
|
* which case the caller keeps its original startup parse (an extension-aware
|
|
* re-parse would be identical anyway).
|
|
*/
|
|
export function applyExtensionFlags(runner: ExtensionFlagSink | undefined, rawArgs: string[]): Args | null {
|
|
const extensionFlags = runner?.getFlags();
|
|
if (!runner || !extensionFlags || extensionFlags.size === 0) {
|
|
return null;
|
|
}
|
|
const parsed = parseArgs(rawArgs, extensionFlags);
|
|
// `parseArgs` only records registered extension flags in `unknownFlags`, so
|
|
// every entry here is a flag this runner owns that was actually passed.
|
|
for (const [name, value] of parsed.unknownFlags) {
|
|
runner.setFlagValue(name, value);
|
|
}
|
|
return parsed;
|
|
}
|