Native opencode support#34
Conversation
|
Warning Review limit reached
More reviews will be available in 45 minutes and 1 second. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR integrates OpenCode as a new native task dispatch agent, replacing tmux-based session spawning with direct JSON payload dispatch. The changes span documentation, CLI infrastructure, runtime provider resolution, and core dispatch plumbing across the story automator and installer. ChangesOpenCode Native Dispatch Integration
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 |
Overview of Analysis of this PRI don't see a test harness / test cases / test results that shows 'as is functionality results' vs the test results of using specific named LLM models used in OpenCode. The PR needs to have the same functionality as the current main branch version, or else it's a different product. Automator works as is in an OpenCode Project FolderIt's the OpenCode LLM models performance and not the Automator running in the OpenCode IDE itself that's the issue I think. I believe that the current Automator would work un-altered in an OpenCode project (as long as you have Claude Code and Codex installed / configured and you have TMUX installed) i.e. :
CoderabbitAlso you might want to look at the Coderabbit reviews in this PR, as it has identified some major issues that need to be fixed Standalone OpenCode AutomatorGiven the above, perhaps this might be better off as a standalone OpenCode Automator in: Or ? |
There was a problem hiding this comment.
Actionable comments posted: 7
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/core/tmux_runtime.py (1)
333-355:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCritical: Session name check for OpenCode will never match.
The check
session.startswith("opencode-")at line 341 will never trigger because session names are generated bygenerate_session_name()(line 72-78) using the patternsa-{project_slug()}-{stamp}-e{epic}-s{suffix}-{step}, which doesn't include the agent type.When
session_status()is called for an OpenCode session, it will fall through to the tmux-based status checking logic, which will fail or return incorrect results since no tmux session exists for OpenCode.Root cause: The function signature only accepts a
codexboolean parameter (line 337), making it impossible to properly identify OpenCode sessions without encoding the agent type in the session name or adding anagentparameter.Suggested fix: Add an
agentparameter tosession_status()and update call sites to pass it, then checkagent == "opencode"instead of checking the session name prefix.🤖 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 333 - 355, The session_status function currently misidentifies OpenCode sessions because it checks session.startswith("opencode-") but session names (from generate_session_name) don’t include agent type; update session_status signature to accept an agent: str | None (e.g., def session_status(session: str, *, full: bool, codex: bool, agent: str | None = None, project_root: str | None = None, mode: str | None = None) ), replace the prefix check with agent == "opencode" to short-circuit and return the native dispatch dict, update all callers of session_status to pass the agent where available, and adjust type hints and any tests; keep the legacy/runner flow (_status_mode, _legacy_session_status, _runner_session_status) unchanged.
🧹 Nitpick comments (1)
skills/bmad-story-automator/src/story_automator/core/tmux_runtime.py (1)
112-131: 💤 Low valueConsider removing unused function or documenting its purpose.
The
opencode_task_dispatch()function doesn't appear to be called anywhere in the provided code. Theopencode_dispatch.pycommand generates an identical payload structure directly (lines 81-110 in context snippets), suggesting this function may be dead code. If it's intended as a public API or for future use, please add a docstring note explaining why it's not currently used.🤖 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 112 - 131, opencode_task_dispatch appears to be dead code: either remove the unused function opencode_task_dispatch from tmux_runtime.py or explicitly document why it is kept (e.g., public API for future use or compatibility) by updating its docstring to state its intended role and reference the opencode_dispatch command that currently builds the same payload; choose one approach and apply it consistently so reviewers understand whether opencode_task_dispatch is intentionally retained or should be deleted.
🤖 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 `@docs/agents-and-monitoring.md`:
- Line 31: Update the docs paragraph to use the actual env var names used by the
runtime detection logic: replace the incorrect "BMM_AGENT" with
"BMAD_RUNTIME_PROVIDER" (and mention "STORY_AUTOMATOR_RUNTIME_PROVIDER" as an
alternative); reference that these env vars are read in runtime_layout.py (the
loop checking for "BMAD_RUNTIME_PROVIDER" and
"STORY_AUTOMATOR_RUNTIME_PROVIDER") and accept values "claude", "codex", or
"opencode" to override the harness priority.
- Around line 156-168: The docs show a configurable `subagent_type` in the
opencode config example but the code uses a hardcoded mapping in
opencode_dispatch.py (`subagent_map`) so the setting is ignored; either remove
`subagent_type` from the YAML example in docs/agents-and-monitoring.md or add a
clear inline note that it is not implemented, or implement reading that config
and mapping it into opencode_dispatch.py so `subagent_map` is derived from the
config file rather than hardcoded.
In
`@skills/bmad-story-automator/src/story_automator/commands/opencode_dispatch.py`:
- Around line 91-100: The hardcoded subagent_map and subagent_type assignment in
opencode_dispatch.py ignores the deployed configuration; modify the logic in the
dispatch path (where subagent_map and subagent_type are defined/used) to first
read the configured value opencode.subagent_type from the BMM config (e.g., the
loaded config object for _bmad/bmm/config.yaml) and, if present and non-empty,
use that value as subagent_type, otherwise fall back to the existing
subagent_map.get(step, "coder"); apply the same change to the other occurrence
referenced around the later block (the similar map at lines ~101-108) so config
overrides the hardcoded mapping.
- Around line 130-133: The current try/except around read_text(config_path) in
opencode_dispatch.py swallows all Exceptions; change it to catch only expected
I/O/parsing errors (e.g., FileNotFoundError, OSError, UnicodeDecodeError) and
handle them by returning "" (or logging) while letting other unexpected
exceptions propagate so bugs aren’t hidden; specifically update the block around
the read_text call to catch those concrete exceptions and include a minimal log
via the existing logger (or re-raise) for any other exception types.
- Around line 149-174: The nesting flags (in_opencode/in_models) are being reset
because you strip leading whitespace too early; use the original raw_line to
determine indentation and only strip leading spaces when extracting key/value.
Specifically, keep raw_line as-is for indentation checks (use
raw_line.startswith(" ") / "\t" or count leading spaces) before calling
strip()—call strip_inline_yaml_comment(raw_line).rstrip() into a separate
variable (e.g., parsed_line) and then split parsed_line into key/value using
.split(":",1) after trimming surrounding quotes; update the checks that
set/reset in_opencode and in_models to use the raw_line indentation, and
populate models from parsed_line when in_opencode and in_models.
In `@skills/bmad-story-automator/src/story_automator/commands/orchestrator.py`:
- Around line 533-537: The status text in the "message" field in orchestrator.py
currently claims live streaming progress which OpenCode does not support; update
that "message" value to remove the phrase about "Progress is visible in the task
tool's streaming output" and instead state that tasks are fire-and-forget,
completion is detected via the task tool return value, and any progress should
be inferred from task status or post-run logs/outputs rather than live
streaming.
In `@skills/bmad-story-automator/src/story_automator/commands/tmux.py`:
- Around line 205-216: The opencode branch (agent == "opencode") creates a JSON
payload missing the "subagent_type" field; update the payload dict to include
"subagent_type" populated using the same mapping logic already applied to step
elsewhere in this module (reuse the existing step -> subagent mapping near where
step is defined) so the payload matches opencode_dispatch.py's structure; ensure
the field key is "subagent_type" and its value matches the same mapping used by
the other code path before printing _json.dumps(payload).
---
Outside diff comments:
In `@skills/bmad-story-automator/src/story_automator/core/tmux_runtime.py`:
- Around line 333-355: The session_status function currently misidentifies
OpenCode sessions because it checks session.startswith("opencode-") but session
names (from generate_session_name) don’t include agent type; update
session_status signature to accept an agent: str | None (e.g., def
session_status(session: str, *, full: bool, codex: bool, agent: str | None =
None, project_root: str | None = None, mode: str | None = None) ), replace the
prefix check with agent == "opencode" to short-circuit and return the native
dispatch dict, update all callers of session_status to pass the agent where
available, and adjust type hints and any tests; keep the legacy/runner flow
(_status_mode, _legacy_session_status, _runner_session_status) unchanged.
---
Nitpick comments:
In `@skills/bmad-story-automator/src/story_automator/core/tmux_runtime.py`:
- Around line 112-131: opencode_task_dispatch appears to be dead code: either
remove the unused function opencode_task_dispatch from tmux_runtime.py or
explicitly document why it is kept (e.g., public API for future use or
compatibility) by updating its docstring to state its intended role and
reference the opencode_dispatch command that currently builds the same payload;
choose one approach and apply it consistently so reviewers understand whether
opencode_task_dispatch is intentionally retained or should be deleted.
🪄 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: 9963872c-13f4-4714-a3f4-5d028fc068cd
📒 Files selected for processing (10)
docs/agents-and-monitoring.mdinstall.shskills/bmad-story-automator/src/story_automator/cli.pyskills/bmad-story-automator/src/story_automator/commands/opencode_dispatch.pyskills/bmad-story-automator/src/story_automator/commands/orchestrator.pyskills/bmad-story-automator/src/story_automator/commands/tmux.pyskills/bmad-story-automator/src/story_automator/core/runtime_layout.pyskills/bmad-story-automator/src/story_automator/core/runtime_policy.pyskills/bmad-story-automator/src/story_automator/core/stop_hooks.pyskills/bmad-story-automator/src/story_automator/core/tmux_runtime.py
|
Hi @lcapriottiunicef, |
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/commands/opencode_dispatch.py (2)
64-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFail fast when
--modelor--state-fileis missing its value.At Line 65 and Line 69, a dangling flag is currently treated as freeform
extratext. This silently hides CLI input errors and can produce incorrect dispatch payloads.Suggested fix
@@ while idx < len(tail): - if tail[idx] == "--model" and idx + 1 < len(tail): - model = tail[idx + 1] - idx += 2 - continue - if tail[idx] == "--state-file" and idx + 1 < len(tail): - state_file = tail[idx + 1] - idx += 2 - continue + if tail[idx] == "--model": + if idx + 1 >= len(tail): + print("--model requires a value", file=sys.stderr) + return 1 + model = tail[idx + 1] + idx += 2 + continue + if tail[idx] == "--state-file": + if idx + 1 >= len(tail): + print("--state-file requires a value", file=sys.stderr) + return 1 + state_file = tail[idx + 1] + idx += 2 + continue extra = f"{extra} {tail[idx]}".strip() idx += 1🤖 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/opencode_dispatch.py` around lines 64 - 74, The argument parsing loop in the while block currently treats dangling flags like --model and --state-file as extra text when their values are missing, which silently hides CLI input errors. Add explicit error handling after checking for these flags: if --model or --state-file is found without a value following it (when the condition idx + 1 < len(tail) would be false), raise an error or exit with a meaningful message instead of allowing the flag to fall through to the extra argument handling at the end of the loop.
169-186: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTop-level boundary is handled too late, which can misparse
models:outsideopencode.At Line 174,
in_opencodeis checked before the top-level reset at Line 179. A root-levelmodels:right after theopencodeblock can be incorrectly treated asopencode.models, causing wrong model resolution.Suggested fix
@@ - for raw_line in raw.splitlines(): + for raw_line in raw.splitlines(): cleaned = strip_inline_yaml_comment(raw_line).rstrip() line = cleaned.strip() if not line or ":" not in line: continue key, value = line.split(":", 1) key = key.strip() - value = value.strip().strip('"').strip("'") + value = unquote_scalar(value.strip()) + + is_top_level = raw_line == raw_line.lstrip(" \t") + if is_top_level: + in_models = False + in_opencode = key == "opencode" + continue # Track nesting: opencode > models - if key == "opencode": - in_opencode = True - in_models = False - continue if in_opencode and key == "models": in_models = True continue - - # Reset nesting when indent level decreases (new top-level key) - is_top_level = raw_line == raw_line.lstrip(" \t") - if is_top_level: - in_opencode = False - in_models = False # Capture model values under opencode.models if in_opencode and in_models and value: models[key] = value🤖 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/opencode_dispatch.py` around lines 169 - 186, The issue is that the top-level boundary reset for in_opencode and in_models flags happens after the nesting tracking logic. This causes a root-level models key appearing after an opencode block to be incorrectly captured as opencode.models. Move the top-level boundary check (the is_top_level calculation and the subsequent reset of in_opencode and in_models flags) to occur before the nesting tracking checks that set in_opencode and in_models to True, ensuring that any top-level key resets the state before new nesting flags are applied.
🤖 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/opencode_dispatch.py`:
- Around line 64-74: The argument parsing loop in the while block currently
treats dangling flags like --model and --state-file as extra text when their
values are missing, which silently hides CLI input errors. Add explicit error
handling after checking for these flags: if --model or --state-file is found
without a value following it (when the condition idx + 1 < len(tail) would be
false), raise an error or exit with a meaningful message instead of allowing the
flag to fall through to the extra argument handling at the end of the loop.
- Around line 169-186: The issue is that the top-level boundary reset for
in_opencode and in_models flags happens after the nesting tracking logic. This
causes a root-level models key appearing after an opencode block to be
incorrectly captured as opencode.models. Move the top-level boundary check (the
is_top_level calculation and the subsequent reset of in_opencode and in_models
flags) to occur before the nesting tracking checks that set in_opencode and
in_models to True, ensuring that any top-level key resets the state before new
nesting flags are applied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18883908-f5f2-477e-9671-905eb838248c
📒 Files selected for processing (4)
docs/agents-and-monitoring.mdskills/bmad-story-automator/src/story_automator/commands/opencode_dispatch.pyskills/bmad-story-automator/src/story_automator/commands/orchestrator.pyskills/bmad-story-automator/src/story_automator/commands/tmux.py
🚧 Files skipped from review as they are similar to previous changes (3)
- skills/bmad-story-automator/src/story_automator/commands/orchestrator.py
- skills/bmad-story-automator/src/story_automator/commands/tmux.py
- docs/agents-and-monitoring.md
Goal
Constraints & Preferences
Key Decisions
Critical Context
Files Modified (8)
docs/agents-and-monitoring.md: opencode instructions
core/runtime_layout.py .opencode parent name checks, skill root candidates, provider inference, marker paths, portable path prefixes
core/tmux_runtime.py agent_type() accepts opencode; agent_cli() returns native dispatch; skill_prefix() returns ""; new opencode_task_dispatch(); spawn_session() returns early for opencode; heartbeat_check() returns native; session_status() returns native
core/stop_hooks.py ensure_stop_hook() returns early with skip reason for opencode
core/runtime_policy.py VALID_PARSER_PROVIDERS now includes "opencode"
commands/tmux.py _build_cmd() generates opencode JSON payload; _raw_agent_selection() accepts opencode; _infer_agent_from_command() detects opencode
cli.py Imports and registers cmd_opencode_dispatch as "opencode-dispatch" command
commands/orchestrator.py New _opencode_status() action + usage update
install.sh .opencode/skills added to collect_target_skills_roots(), select_single_incomplete_diagnostic_root(), wrapper_points_to_skill_tree(), error messages, and usage
File Created (1)
commands/opencode_dispatch.py Generates JSON dispatch payload with step, storyId, prompt, model, subagent_type
Manual Step Required
_bmad/bmm/config.yaml — Add this block (was blocked by config zone restriction):
opencode:
models:
orchestrator: "" # empty = opencode default
create: ""
dev: ""
auto: ""
review: ""
retro: ""
subagent_type: "coder"
What Was NOT Done (and Why)
How It Works
When OpenCode is detected as the harness:
Summary by CodeRabbit
opencode-dispatchandopencode-statuscommands for JSON payload/status output.opencode/skillsas an additional supported skill root