feat: support Gemini and custom agent runtimes#37
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR adds Gemini and configurable custom child-agent support: docs updated, tmux command paths made agent-aware and fail-fast on unknown agents, core runtime now derives/normalizes agents and builds per-agent CLI (including Gemini), session detection generalized, process-ancestry checks added, and tests extended. ChangesMulti-Agent Support: Gemini and Custom LLM Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/bmad-story-automator/data/tmux-commands.md`:
- Line 193: Update the CLI table row in tmux-commands.md so the Codex entry
reflects the actual runtime command built by the automator: replace the stale
`codex exec --full-auto` string with the full command template that includes
explicit flags (e.g. `codex exec -s workspace-write --allow-policy
--allow-reasoning` or the project’s exact policy/reasoning flags), and ensure
the corresponding environment variable `STORY_AUTOMATOR_AGENT_<NAME>_COMMAND`
example matches that same full command template.
In `@skills/bmad-story-automator/src/story_automator/core/tmux_runtime.py`:
- Around line 914-917: The current liveness check uses pgrep -P <pane_pid> which
only finds immediate children and misses wrapped agents; update the logic that
sets agent_running in tmux_runtime.py to: run pgrep -f
agent_process_pattern(agent) (using the existing run_cmd) to get candidate PIDs,
then for each candidate call a new helper (e.g., _is_descendant(pid, pane_pid))
that walks parent PIDs (via /proc/<pid>/stat or ps -o ppid) to determine if
pane_pid is an ancestor; set agent_running true if any candidate is a
descendant. Keep references to detect_agent_session, tmux_display,
agent_process_pattern, run_cmd and replace the existing pgrep -P usage with this
descendant-aware check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5fd794dc-a27c-4f57-a763-199b971e073f
📒 Files selected for processing (8)
README.mddocs/agents-and-monitoring.mdskills/bmad-story-automator/data/agent-fallback.mdskills/bmad-story-automator/data/tmux-commands.mdskills/bmad-story-automator/src/story_automator/adapters/tmux.pyskills/bmad-story-automator/src/story_automator/commands/tmux.pyskills/bmad-story-automator/src/story_automator/core/tmux_runtime.pytests/test_agent_config_model.py
|
Follow-up after CodeRabbit's two actionable comments:
Validation after the follow-up commit
CodeRabbit appears rate-limited on the latest pass, but the current PR check is green and both original inline review threads are addressed by the follow-up commit. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
skills/bmad-story-automator/src/story_automator/core/tmux_runtime.py (1)
1013-1015:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLegacy heartbeat still only looks for Claude/Codex processes.
_legacy_heartbeat_check()is the path behindcmd_heartbeat_check()in legacy mode, but Lines 1013-1014 still hard-code"codex"vs"claude". Gemini/custom sessions will be reported as dead/completed even whilesession_status()can still see them viaagent_process_pattern()and the new descendant-aware checks, so the two monitors can disagree on the same live session.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/bmad-story-automator/src/story_automator/core/tmux_runtime.py` around lines 1013 - 1015, The legacy heartbeat path (_legacy_heartbeat_check called by cmd_heartbeat_check) currently hard-codes "codex"/"claude" when building the `pattern` used by _find_agent_pid, which causes Gemini/custom sessions to be missed; update the logic to derive the search pattern via the existing agent_process_pattern(selected_agent) (or equivalent helper) instead of the ternary, and pass that pattern into _find_agent_pid so legacy checks use the same descendant-aware process pattern resolution as session_status()/agent_process_pattern; ensure the code references selected_agent, _find_agent_pid, and agent_process_pattern so the monitors remain consistent.skills/bmad-story-automator/src/story_automator/commands/tmux.py (1)
206-214:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't bypass the built-in Gemini launcher when only
AI_COMMANDis set.
_raw_agent_selection()now infers"gemini"fromAI_COMMAND, but this branch still emits the rawAI_COMMANDwheneverAI_AGENTis unset. WithAI_COMMAND=gemini, the generated command becomesgemini <prompt>and drops both--approval-mode yolo -pand any task-level--model, so the legacy env path still mis-dispatches Gemini instead of using the new built-in CLI shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/bmad-story-automator/src/story_automator/commands/tmux.py` around lines 206 - 214, The branch that sets cli = ai_command when AI_COMMAND is present bypasses the built-in Gemini launcher; change it so that when AI_COMMAND is set but the inferred agent is "gemini" (or agent == "gemini") you call agent_cli(agent, model) instead of using the raw AI_COMMAND, otherwise fall back to using ai_command; update the block handling ai_command/AI_AGENT to prefer agent_cli for known agents (at minimum check agent == "gemini") and only emit the raw ai_command for truly custom/unknown agents.
🧹 Nitpick comments (1)
tests/test_agent_config_model.py (1)
718-739: ⚡ Quick winAdd the
AI_COMMAND-only Gemini regression case.These tests cover
AI_AGENT=gemini, but the broken branch is whenAI_AGENTis empty andAI_COMMANDis the only signal. A case likeAI_COMMAND=geminiplus--model gemini-2.5-prowould catch the current fallback to rawAI_COMMANDinstead ofagent_cli("gemini", model).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_agent_config_model.py` around lines 718 - 739, Add a test that simulates the regression where AI_AGENT is empty but AI_COMMAND is set: in tests referencing _build_cmd, patch the environment to set AI_COMMAND="gemini" (and ensure AI_AGENT is not set or is empty) and call _build_cmd(["review","9.1","--model","gemini-2.5-pro"]) capturing stdout; assert the rendered command contains the expected "gemini --approval-mode yolo -p" invocation and does not contain "claude --dangerously-skip-permissions" to ensure the _raw_agent_selection path does not incorrectly bypass agent_cli("gemini", model) and fall back to raw AI_COMMAND handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@skills/bmad-story-automator/src/story_automator/commands/tmux.py`:
- Around line 206-214: The branch that sets cli = ai_command when AI_COMMAND is
present bypasses the built-in Gemini launcher; change it so that when AI_COMMAND
is set but the inferred agent is "gemini" (or agent == "gemini") you call
agent_cli(agent, model) instead of using the raw AI_COMMAND, otherwise fall back
to using ai_command; update the block handling ai_command/AI_AGENT to prefer
agent_cli for known agents (at minimum check agent == "gemini") and only emit
the raw ai_command for truly custom/unknown agents.
In `@skills/bmad-story-automator/src/story_automator/core/tmux_runtime.py`:
- Around line 1013-1015: The legacy heartbeat path (_legacy_heartbeat_check
called by cmd_heartbeat_check) currently hard-codes "codex"/"claude" when
building the `pattern` used by _find_agent_pid, which causes Gemini/custom
sessions to be missed; update the logic to derive the search pattern via the
existing agent_process_pattern(selected_agent) (or equivalent helper) instead of
the ternary, and pass that pattern into _find_agent_pid so legacy checks use the
same descendant-aware process pattern resolution as
session_status()/agent_process_pattern; ensure the code references
selected_agent, _find_agent_pid, and agent_process_pattern so the monitors
remain consistent.
---
Nitpick comments:
In `@tests/test_agent_config_model.py`:
- Around line 718-739: Add a test that simulates the regression where AI_AGENT
is empty but AI_COMMAND is set: in tests referencing _build_cmd, patch the
environment to set AI_COMMAND="gemini" (and ensure AI_AGENT is not set or is
empty) and call _build_cmd(["review","9.1","--model","gemini-2.5-pro"])
capturing stdout; assert the rendered command contains the expected "gemini
--approval-mode yolo -p" invocation and does not contain "claude
--dangerously-skip-permissions" to ensure the _raw_agent_selection path does not
incorrectly bypass agent_cli("gemini", model) and fall back to raw AI_COMMAND
handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9dcf2ab6-e5f0-4dd6-a4b4-afd8b4ac3456
📒 Files selected for processing (7)
docs/agents-and-monitoring.mdskills/bmad-story-automator/data/agent-fallback.mdskills/bmad-story-automator/data/tmux-commands.mdskills/bmad-story-automator/src/story_automator/commands/tmux.pyskills/bmad-story-automator/src/story_automator/core/tmux_runtime.pytests/test_agent_config_model.pytests/test_tmux_runtime.py
✅ Files skipped from review due to trivial changes (1)
- docs/agents-and-monitoring.md
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/bmad-story-automator/data/tmux-commands.md
- skills/bmad-story-automator/data/agent-fallback.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/bmad-story-automator/src/story_automator/commands/tmux.py (1)
501-515:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse exact executable matching in agent inference to avoid silent misrouting.
On Line 509/Line 511/Line 513, substring checks can misclassify custom wrappers (e.g.,
my-gemini-wrapper) as built-in agents. That can make_build_cmdignore the rawAI_COMMANDpath and dispatch the wrong CLI.Proposed fix
def _infer_agent_from_command(command: str) -> str: value = command.strip() if not value: return "" try: - executable = Path(shlex.split(value)[0]).name.lower() + executable = Path(shlex.split(value)[0]).name.lower() + token = Path(executable).stem.lower() # handles .exe and similar suffixes except ValueError: return "" - if "codex" in executable: + if token in {"codex", "codex-cli"}: return "codex" - if "claude" in executable: + if token in {"claude"}: return "claude" - if "gemini" in executable: + if token in {"gemini", "gemini-cli"}: return "gemini" return ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/bmad-story-automator/src/story_automator/commands/tmux.py` around lines 501 - 515, _infer_agent_from_command currently uses substring checks on the resolved executable name (variable executable), which can misclassify wrappers like "my-gemini-wrapper"; change the logic to perform exact matching against the known agent binary names (e.g., compare executable == "codex" or executable == "codex.exe" and likewise for "claude" and "gemini") rather than "in" substring checks so only true agent executables are detected; update the checks in _infer_agent_from_command to normalize the name (lowercase) and compare equality against the allowed set of agent names (and common platform extensions like .exe) and otherwise return "".
🧹 Nitpick comments (1)
tests/test_agent_config_model.py (1)
677-689: ⚡ Quick winAdd a regression test for wrapper-name collisions in
_infer_agent_from_command.Current cases don’t guard against false positives like
my-gemini-wrapper run, which should remain unknown ("") rather than infer built-in Gemini.Suggested test addition
class InferAgentFromCommandTests(unittest.TestCase): @@ def test_empty_and_unknown_return_empty(self) -> None: self.assertEqual(_infer_agent_from_command(""), "") self.assertEqual(_infer_agent_from_command("some-other-tool run"), "") + self.assertEqual(_infer_agent_from_command("my-gemini-wrapper run"), "") + self.assertEqual(_infer_agent_from_command("codextra run"), "")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_agent_config_model.py` around lines 677 - 689, Add a regression test to ensure _infer_agent_from_command does not falsely match wrapper names that contain agent names as substrings (e.g., "my-gemini-wrapper run" should return ""), by adding assertions to InferAgentFromCommandTests that verify commands like "my-gemini-wrapper run" and similarly prefixed/suffixed variants (e.g., "prefix-gemini", "gemini-wrapper") return an empty string; update or add test cases in the class referencing the _infer_agent_from_command function to cover these collision scenarios.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@skills/bmad-story-automator/src/story_automator/commands/tmux.py`:
- Around line 501-515: _infer_agent_from_command currently uses substring checks
on the resolved executable name (variable executable), which can misclassify
wrappers like "my-gemini-wrapper"; change the logic to perform exact matching
against the known agent binary names (e.g., compare executable == "codex" or
executable == "codex.exe" and likewise for "claude" and "gemini") rather than
"in" substring checks so only true agent executables are detected; update the
checks in _infer_agent_from_command to normalize the name (lowercase) and
compare equality against the allowed set of agent names (and common platform
extensions like .exe) and otherwise return "".
---
Nitpick comments:
In `@tests/test_agent_config_model.py`:
- Around line 677-689: Add a regression test to ensure _infer_agent_from_command
does not falsely match wrapper names that contain agent names as substrings
(e.g., "my-gemini-wrapper run" should return ""), by adding assertions to
InferAgentFromCommandTests that verify commands like "my-gemini-wrapper run" and
similarly prefixed/suffixed variants (e.g., "prefix-gemini", "gemini-wrapper")
return an empty string; update or add test cases in the class referencing the
_infer_agent_from_command function to cover these collision scenarios.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df656f2a-7632-4fa1-9a7b-0d77ed2f62fe
📒 Files selected for processing (4)
skills/bmad-story-automator/src/story_automator/commands/tmux.pyskills/bmad-story-automator/src/story_automator/core/tmux_runtime.pytests/test_agent_config_model.pytests/test_tmux_runtime.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_tmux_runtime.py
- skills/bmad-story-automator/src/story_automator/core/tmux_runtime.py
|
@augmentcode review |
🤖 Augment PR SummarySummary: This PR makes Story Automator tmux child-session dispatch explicit and safer by adding first-class Gemini support and enabling configurable custom non-Codex agent runtimes. Changes:
Technical Notes: The liveness monitor now uses a process-pattern lookup per agent and validates candidate PIDs by walking parent PIDs up to the tmux pane PID to avoid missing wrapped child processes. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| def agent_type() -> str: | ||
| value = os.environ.get("AI_AGENT", "").strip().lower() | ||
| if value in {"claude", "codex"}: | ||
| value = normalize_agent_name(os.environ.get("AI_AGENT", "")) |
There was a problem hiding this comment.
agent_type() now returns any non-empty AI_AGENT value verbatim, so AI_AGENT=auto/runtime (which _raw_agent_selection() preserves) will flow into tmux-wrapper agent-cli and fail because agent_cli() rejects those names. Consider resolving auto/runtime via runtime_provider() here to keep env-based selection behavior consistent across entry points.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Addressed in de80f2a.
agent_type() now treats auto and runtime as meta-selectors and resolves them through runtime_provider() instead of returning them as concrete agent names. Explicit built-in/custom agent names still pass through unchanged. Added regression coverage in AgentTypeResolutionTests for auto, runtime, empty, built-in, and custom agent values.
Validation:
env -u AI_AGENT PYTHONPATH=skills/bmad-story-automator/src python3 -m pytest tests/test_agent_config_model.py tests/test_tmux_runtime.py -q→ 94 passed, 14 subtests passedenv -u AI_AGENT PYTHONPATH=skills/bmad-story-automator/src python3 -m pytest -q→ 453 passed, 20 subtests passednpm run test:smoke→ smoke ok
| f"unsupported agent {agent!r}; supported agents are claude, codex, gemini, " | ||
| f"or set STORY_AUTOMATOR_AGENT_{env_agent_name(agent)}_COMMAND" | ||
| ) | ||
| if model: |
There was a problem hiding this comment.
When custom_agent_command() is used, the later if model: appends --model ... to the end of the custom command, which can break CLIs where the prompt flag must be last (e.g. a custom Gemini command ending in -p). This can result in the model flag being swallowed/ignored and the session running with an unintended model.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Addressed in de80f2a.
For custom commands ending in a trailing -p, agent_cli() now injects --model <quoted model> before -p, preventing prompt flags from swallowing the model argument. Custom commands without trailing -p keep the previous append-at-end behavior, and Claude/Codex behavior is unchanged. Added regression tests for trailing -p, quoted model values, and the non--p append path.
Validation:
env -u AI_AGENT PYTHONPATH=skills/bmad-story-automator/src python3 -m pytest tests/test_agent_config_model.py tests/test_tmux_runtime.py -q→ 94 passed, 14 subtests passedenv -u AI_AGENT PYTHONPATH=skills/bmad-story-automator/src python3 -m pytest -q→ 453 passed, 20 subtests passednpm run test:smoke→ smoke ok
2 participants
Summary
Adds runtime dispatch support for Gemini and custom non-Codex child agents in the Python Story Automator tmux flow.
This PR makes per-task
agentConfigsafer and more explicit:gemininow launches Gemini CLI instead of silently falling through to Claude.STORY_AUTOMATOR_AGENT_<NAME>_COMMANDAI_COMMAND_<NAME>STORY_AUTOMATOR_AGENT_<NAME>_PROCESSfor liveness monitoring when the executable name differs from the agent alias.Fixes #33.
Motivation
agentConfigcan express per-task agent choices, but the tmux runtime previously treated unknown/non-Codex agents like Claude in important paths. That made configurations such asreview: geminimisleading and unsafe, because they could silently run under Claude.The goal of this PR is to make child-agent dispatch explicit:
Implementation notes
Built-in command mapping now includes:
claude --dangerously-skip-permissionsgemini --approval-mode yolo -pgemini --approval-mode yolo --model <id> -pGemini uses
--approval-mode yolobecause BMAD child workflows need autonomous file edits and shell/test execution. The docs call out the safety boundary: this is only appropriate in trusted workspaces controlled by the operator.Custom agents use
STORY_AUTOMATOR_AGENT_<NAME>_COMMAND; liveness monitoring defaults to the configured command executable basename unless an explicitSTORY_AUTOMATOR_AGENT_<NAME>_PROCESSoverride is provided.Relationship to #34
This does not duplicate #34's native OpenCode dispatch work. This PR stays in the existing tmux child-session runtime and adds Gemini/custom non-Codex CLI selection there.
Validation
Final validation on the current branch:
Additional manual validation:
create,dev,auto,review, andretroroles;gemini -pwas verified to be insufficient for autonomous workflows, so built-in Gemini dispatch now uses--approval-mode yolo;APPROVE.CodeRabbit is green on the latest commit.
Summary by CodeRabbit
AI_AGENTor command inference.1.