feat: add provider override CLI flags#314
Conversation
umputun
left a comment
There was a problem hiding this comment.
couple of things before this can merge. The core feature is reasonable: the empty-args fix is solving a real documented gap in pkg/config/values.go:435 (mergeFrom skips empty values, so claude_args = in config falls back to the embedded default), and the boot reorder is needed for --claude-command to actually work against checkClaudeDep.
underscore aliases have no precedent
--claude_command, --claude_args, --external_review_tool, --custom_review_script are the only underscore-aliased flags in the CLI. About 35 long flags in cmd/ralphex/main.go, none of the others ship aliases despite the same hyphen-vs-underscore mismatch with their config keys (--task-model / task_model, --review-patience / review_patience, etc.). The aliases cost 4 alias fields + 4 *Set bools + validateAliasConflict + cliStringOverride + a test matrix nothing else needs. I'd prefer dropping them. If consistent typo tolerance is something you want across the whole CLI, that's a separate scope.
--external-review-tool=custom silently flips CodexEnabled=true
cmd/ralphex/main.go:1322-1327. This is real back-compat work for users with codex_enabled = false legacy config, which is fine, but it lives in the wrong layer. A CLI flag silently mutating an unrelated config field is surprising. Cleaner: have Runner.externalReviewTool() defer to an explicit ExternalReviewTool regardless of CodexEnabled, then drop the implicit flip from applyCLIOverrides. At minimum the flag description should mention the side-effect.
Removes hidden --claude_command, --claude_args, --external_review_tool, and --custom_review_script aliases. They were the only underscore-spelled CLI flags in ralphex and didn't justify the alias fields, *Set bools, conflict validation, and test matrix needed to support them. The hyphen flags and the underscored config keys are unchanged.
… override A CLI flag mutating an unrelated config field is surprising. Push the explicitness signal down instead: processor.Config gains an ExternalReviewToolSet flag, Runner.externalReviewTool() honors an explicit choice over the legacy codex_enabled=false back-compat path, and applyCLIOverrides only writes the field the user actually set. Legacy behavior unchanged: a config with only codex_enabled=false (no explicit external_review_tool from CLI) still resolves to "none".
|
FIxed |
umputun
left a comment
There was a problem hiding this comment.
both points addressed cleanly:
- aliases dropped in 8996ab0, only the canonical hyphen forms remain
codex_enabledflip moved out ofapplyCLIOverridesintoRunner.externalReviewTool()in 7c3e82a, which now defers to an explicitExternalReviewToolSetover the legacycodex_enabled=falseback-compat. Regression test locks the behavior.
tests pass, lint clean. Lgtm, thx
This PR adds per-run CLI flags that override configured provider settings for Claude-compatible task/review execution and external review tooling.
Why do we do these changes?
RALPHEX_CONFIG_DIRprofiles for simple variations in provider setup. One-off tests or temporary switches can now be handled directly via flags instead of maintaining redundant configuration state.--claude-commandand--claude-argsto override config settings before dependency checks, developers can easily pointralphexto experimental wrapper scripts even if the default Claude installation is missing or broken.--task-modelor--session-timeout.What changed?
--claude-command,--claude-args,--external-review-tool, and--custom-review-script.ArgsSetlogic in the executor to allow explicit empty overrides (e.g.,--claude-args=) to clear default Claude flags.main.goto apply CLI overrides immediately after config loading but before verifying the Claude dependency.--claude_command) to match configuration file field names.README.mdwith a new "Options" table and usage examples.docs/custom-providers.mdwith details on per-run provider switching.llms.txtfor AI-assisted context.main_test.gocovering flag parsing, conflict resolution, and the dependency injection regression.Example Usage