[PR #42] [MERGED] fix: correct agent validation in _apply_codex_shortcut #108

New issue

Closed

opened 2026-02-27 10:22:15 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 10:22:15 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/mikeyobrien/ralph-orchestrator/pull/42
Author: @CoderMageFox
Created: 1/9/2026
Status: ✅ Merged
Merged: 1/10/2026
Merged by: @mikeyobrien

Base: main ← Head: fix/codex-shortcut-agent-validation

📝 Commits (1)

024990a fix: correct agent validation in _apply_codex_shortcut

📊 Changes

1 file changed (+5 additions, -1 deletions)

View changed files

📝 src/ralph_orchestrator/__main__.py (+5 -1)

📄 Description

Summary

This PR fixes a bug in the _apply_codex_shortcut function where the --codex flag fails to work correctly when used with the run subcommand.

Problem

When running ralph run --codex, users encounter the following error:

ralph: error: --codex requires --agent auto or acp (or omit -a/--agent)

This happens even though no explicit --agent argument was provided.

Root Cause

The issue is in line 45 of __main__.py:

if getattr(args, "agent", "auto") not in ("auto", "acp"):

The problem is that getattr(obj, name, default) only returns the default value when the attribute does not exist. However, when the attribute exists but has a value of None, getattr returns None, not the default value.

Since the --agent argument is defined with default=None in argparse:

p.add_argument(
    "-a", "--agent",
    choices=["claude", "q", "gemini", "kiro", "acp", "auto"],
    default=None,  # <-- This is the issue
    help="AI agent to use (default: auto)"
)

When a user runs ralph run --codex without specifying --agent:

args.agent exists and equals None
getattr(args, "agent", "auto") returns None (not "auto")
None not in ("auto", "acp") evaluates to True
The error is triggered incorrectly

Solution

Replace the problematic getattr check with an explicit None check:

# Before (buggy)
if getattr(args, "agent", "auto") not in ("auto", "acp"):
    parser.error(...)

# After (fixed)
agent_value = getattr(args, "agent", None)
if agent_value is not None and agent_value not in ("auto", "acp"):
    parser.error(...)

This correctly treats None (no explicit agent specified) as equivalent to "auto".

Test Cases

Command	Expected	Result
`ralph run --codex`	Use ACP with codex-acp	✅ Works
`ralph run --codex -a auto`	Use ACP with codex-acp	✅ Works
`ralph run --codex -a acp`	Use ACP with codex-acp	✅ Works
`ralph run --codex -a claude`	Error (conflicting)	✅ Error

Additional Notes

There is also a related UX issue: the --codex flag must be placed after the run subcommand:

# Works:
ralph run --codex

# Does NOT work:
ralph --codex run

This is due to argparse subparser behavior and may warrant documentation updates in a future PR.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/mikeyobrien/ralph-orchestrator/pull/42 **Author:** [@CoderMageFox](https://github.com/CoderMageFox) **Created:** 1/9/2026 **Status:** ✅ Merged **Merged:** 1/10/2026 **Merged by:** [@mikeyobrien](https://github.com/mikeyobrien) **Base:** `main` ← **Head:** `fix/codex-shortcut-agent-validation` --- ### 📝 Commits (1) - [`024990a`](https://github.com/mikeyobrien/ralph-orchestrator/commit/024990a5135aec17156f4a9d911402f8431a4f10) fix: correct agent validation in _apply_codex_shortcut ### 📊 Changes **1 file changed** (+5 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/ralph_orchestrator/__main__.py` (+5 -1) </details> ### 📄 Description ## Summary This PR fixes a bug in the `_apply_codex_shortcut` function where the `--codex` flag fails to work correctly when used with the `run` subcommand. ## Problem When running `ralph run --codex`, users encounter the following error: ``` ralph: error: --codex requires --agent auto or acp (or omit -a/--agent) ``` This happens even though no explicit `--agent` argument was provided. ## Root Cause The issue is in line 45 of `__main__.py`: ```python if getattr(args, "agent", "auto") not in ("auto", "acp"): ``` The problem is that `getattr(obj, name, default)` only returns the `default` value when the attribute **does not exist**. However, when the attribute exists but has a value of `None`, `getattr` returns `None`, not the default value. Since the `--agent` argument is defined with `default=None` in argparse: ```python p.add_argument( "-a", "--agent", choices=["claude", "q", "gemini", "kiro", "acp", "auto"], default=None, # <-- This is the issue help="AI agent to use (default: auto)" ) ``` When a user runs `ralph run --codex` without specifying `--agent`: - `args.agent` exists and equals `None` - `getattr(args, "agent", "auto")` returns `None` (not `"auto"`) - `None not in ("auto", "acp")` evaluates to `True` - The error is triggered incorrectly ## Solution Replace the problematic `getattr` check with an explicit `None` check: ```python # Before (buggy) if getattr(args, "agent", "auto") not in ("auto", "acp"): parser.error(...) # After (fixed) agent_value = getattr(args, "agent", None) if agent_value is not None and agent_value not in ("auto", "acp"): parser.error(...) ``` This correctly treats `None` (no explicit agent specified) as equivalent to `"auto"`. ## Test Cases | Command | Expected | Result | |---------|----------|--------| | `ralph run --codex` | Use ACP with codex-acp | ✅ Works | | `ralph run --codex -a auto` | Use ACP with codex-acp | ✅ Works | | `ralph run --codex -a acp` | Use ACP with codex-acp | ✅ Works | | `ralph run --codex -a claude` | Error (conflicting) | ✅ Error | ## Additional Notes There is also a related UX issue: the `--codex` flag must be placed **after** the `run` subcommand: ```bash # Works: ralph run --codex # Does NOT work: ralph --codex run ``` This is due to argparse subparser behavior and may warrant documentation updates in a future PR. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>