Repository navigation
feat(cli): setup-crush — OmniRoute openai-compat provider in crush.json - #4298
Conversation
CLI #9 of the series. `omniroute setup-crush` writes Crush's file-based ~/.config/crush/crush.json with an `openai-compat` provider for OmniRoute: base_url WITH /v1, api_key referenced as $OMNIROUTE_API_KEY (secret off disk), curated catalog models with context_window. Merges (preserves existing config). Remote-aware (--remote/--api-key → active context → localhost); --only filter. Researched against charmbracelet/crush: openai-compat provider type, base_url needs /v1, $VAR api_key references. Crush's wire (/v1/chat/completions) already validated → "OK". Tests: resolveCrushTarget (/v1, key), buildCrushProvider (openai-compat + env-ref + curated models + skip-unknown), mergeCrushConfig (preserve). 4 unit tests; cli-i18n green.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request introduces the setup-crush command to configure the Crush terminal AI agent for OmniRoute, including its implementation, CLI registration, documentation, and unit tests. The review feedback highlights critical improvement opportunities, including preventing silent data loss when parsing corrupt configuration files, handling potential filesystem write and parsing exceptions gracefully, and registering the missing --context command-line option.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function readJson(path) { | ||
| try { | ||
| if (existsSync(path)) return JSON.parse(readFileSync(path, "utf8")); | ||
| } catch { | ||
| /* corrupt/missing */ | ||
| } | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
If the config file exists but contains invalid JSON, JSON.parse will throw an error. Currently, this error is silently caught and an empty object {} is returned, which will cause the entire existing configuration to be silently overwritten and lost when the merged config is written back. Instead, we should check if the file exists, and if parsing fails, throw an error so we can handle it safely without data loss.
| function readJson(path) { | |
| try { | |
| if (existsSync(path)) return JSON.parse(readFileSync(path, "utf8")); | |
| } catch { | |
| /* corrupt/missing */ | |
| } | |
| return {}; | |
| } | |
| function readJson(path) { | |
| if (!existsSync(path)) return {}; | |
| try { | |
| return JSON.parse(readFileSync(path, "utf8")); | |
| } catch (e) { | |
| throw new Error("Invalid JSON in " + path + ": " + e.message); | |
| } | |
| } |
| printError("No matching curated models (try --only or check the server)."); | ||
| return 1; | ||
| } | ||
| const merged = mergeCrushConfig(readJson(configPath), provider); |
There was a problem hiding this comment.
Handle potential JSON parsing errors from readJson to prevent the command from crashing with an unhandled exception, and instead print a clean error message and exit gracefully.
| const merged = mergeCrushConfig(readJson(configPath), provider); | |
| let existing; | |
| try { | |
| existing = readJson(configPath); | |
| } catch (e) { | |
| printError(e.message); | |
| return 1; | |
| } | |
| const merged = mergeCrushConfig(existing, provider); |
| mkdirSync(join(configPath, ".."), { recursive: true }); | ||
| writeFileSync(configPath, out, "utf8"); |
There was a problem hiding this comment.
Wrap the filesystem write operations in a try-catch block to handle potential write/permission errors gracefully, printing a user-friendly error message instead of letting the CLI crash with an unhandled exception.
try {
mkdirSync(join(configPath, ".."), { recursive: true });
writeFileSync(configPath, out, "utf8");
} catch (e) {
printError("Failed to write config to " + configPath + ": " + e.message);
return 1;
}| export function registerSetupCrush(program) { | ||
| program | ||
| .command("setup-crush") | ||
| .description("Generate the OmniRoute openai-compat provider in ~/.config/crush/crush.json") | ||
| .option("--port <port>", "Local OmniRoute port (ignored when --remote is set)", "20128") | ||
| .option("--remote <url>", "Remote OmniRoute URL, e.g. http://192.168.0.15:20128") | ||
| .option("--api-key <key>", "OmniRoute API key (defaults to OMNIROUTE_API_KEY env var)") | ||
| .option("--only <patterns>", "Comma-separated substrings — keep only matching model IDs") | ||
| .option("--config-path <path>", "crush.json path (default: ~/.config/crush/crush.json)") | ||
| .option("--dry-run", "Print what would be written without touching the filesystem") | ||
| .action(async (opts) => { | ||
| const code = await runSetupCrushCommand(opts); | ||
| if (code !== 0) process.exit(code); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The resolveCrushTarget function attempts to resolve the active context using opts.context. However, the --context option is not registered in the setup-crush command definition. Registering --context <name> will allow users to specify a context directly from the command line, matching the behavior of other setup commands.
| export function registerSetupCrush(program) { | |
| program | |
| .command("setup-crush") | |
| .description("Generate the OmniRoute openai-compat provider in ~/.config/crush/crush.json") | |
| .option("--port <port>", "Local OmniRoute port (ignored when --remote is set)", "20128") | |
| .option("--remote <url>", "Remote OmniRoute URL, e.g. http://192.168.0.15:20128") | |
| .option("--api-key <key>", "OmniRoute API key (defaults to OMNIROUTE_API_KEY env var)") | |
| .option("--only <patterns>", "Comma-separated substrings — keep only matching model IDs") | |
| .option("--config-path <path>", "crush.json path (default: ~/.config/crush/crush.json)") | |
| .option("--dry-run", "Print what would be written without touching the filesystem") | |
| .action(async (opts) => { | |
| const code = await runSetupCrushCommand(opts); | |
| if (code !== 0) process.exit(code); | |
| }); | |
| } | |
| export function registerSetupCrush(program) { | |
| program | |
| .command("setup-crush") | |
| .description("Generate the OmniRoute openai-compat provider in ~/.config/crush/crush.json") | |
| .option("--port <port>", "Local OmniRoute port (ignored when --remote is set)", "20128") | |
| .option("--remote <url>", "Remote OmniRoute URL, e.g. http://192.168.0.15:20128") | |
| .option("--context <name>", "OmniRoute context name") | |
| .option("--api-key <key>", "OmniRoute API key (defaults to OMNIROUTE_API_KEY env var)") | |
| .option("--only <patterns>", "Comma-separated substrings — keep only matching model IDs") | |
| .option("--config-path <path>", "crush.json path (default: ~/.config/crush/crush.json)") | |
| .option("--dry-run", "Print what would be written without touching the filesystem") | |
| .action(async (opts) => { | |
| const code = await runSetupCrushCommand(opts); | |
| if (code !== 0) process.exit(code); | |
| }); | |
| } |
…on (cheaper-inference#4298) CLI cheaper-inference#9 of the series. `omniroute setup-crush` writes Crush's file-based ~/.config/crush/crush.json with an `openai-compat` provider for OmniRoute: base_url WITH /v1, api_key referenced as $OMNIROUTE_API_KEY (secret off disk), curated catalog models with context_window. Merges (preserves existing config). Remote-aware (--remote/--api-key → active context → localhost); --only filter. Researched against charmbracelet/crush: openai-compat provider type, base_url needs /v1, $VAR api_key references. Crush's wire (/v1/chat/completions) already validated → "OK". Tests: resolveCrushTarget (/v1, key), buildCrushProvider (openai-compat + env-ref + curated models + skip-unknown), mergeCrushConfig (preserve). 4 unit tests; cli-i18n green.
What
CLI #9.
omniroute setup-crushwrites Crush's file-based~/.config/crush/crush.jsonwith anopenai-compatprovider for OmniRoute:base_urlWITH/v1,api_key=$OMNIROUTE_API_KEY(secret off disk), curated catalog models withcontext_window. Merges (preserves existing). Remote-aware;--only.Validation
setup-crush --remote --only glm --dry-run→ correct provider JSON./v1/chat/completions) already validated → "OK". 4 unit tests;check:cli-i18n✓.Series: …Cursor #4291 · Roo #4292 · Crush (this). Next: Goose, Qwen Code, Gemini CLI, Aider.
🤖 Generated with Claude Code