Skip to content

fix(agent): review fixes for Codex and OpenCode support#44

Merged
MiniCodeMonkey merged 19 commits intomainfrom
feature/opencode-support-v2
Mar 7, 2026
Merged

fix(agent): review fixes for Codex and OpenCode support#44
MiniCodeMonkey merged 19 commits intomainfrom
feature/opencode-support-v2

Conversation

@MiniCodeMonkey
Copy link
Owner

Summary

Review fixes for #43 (Codex and OpenCode support). Based on the same branch with bug fixes, deduplication, and improved test coverage.

  • Fix OpenCode double-prompt bug (prompt passed as both CLI arg and stdin)
  • Fix Claude ConvertCommand reverting incorrect --tools "" args
  • Fix OpenCode parser to emit EventToolStart when tools begin, not only on completion
  • Refactor extract shared parseAgentFlags() to eliminate 3x duplicated flag parsing in main.go
  • Refactor extract runAgentCommand() to deduplicate convert.go helpers
  • Improve OpenCode CleanOutput to use JSON parsing instead of string matching
  • Add Claude provider tests, OpenCode CleanOutput tests, parser tool_use tests
  • Docs update troubleshooting to clarify provider-specific log file names

Test plan

  • All 9 packages pass (go test ./...)
  • go build and go vet clean
  • Manual test with --agent codex and --agent opencode flags
  • Verify chief new --agent opencode works end-to-end

Builds on #43

Simon-BEE and others added 19 commits February 25, 2026 09:38
Introduce an agent abstraction to support Claude and OpenAI Codex CLIs. Added internal/agent providers (ClaudeProvider, CodexProvider), resolution and installation checks (Resolve, CheckInstalled), and tests. Wire provider selection via --agent / --agent-path flags, CHIEF_AGENT env vars, and .chief/config.yaml (agent.provider, agent.cliPath). Propagate the provider into TUI, new, edit, and convert flows; convert and fix-json now run through the provider. Added Codex JSONL parser for the loop output and accompanying tests. Updated config struct, docs (README, installation, configuration, troubleshooting), and various command code to use the provider abstraction. Key new files: internal/agent/*.go, internal/cmd/convert.go, internal/loop/codex_parser*.go and tests; updated main.go, cmd handlers, prd conversion wiring and docs to reflect multi-agent support.
Propagate errors from provider commands and improve provider resolution flow.

- Extended loop.Provider ConvertCommand and FixJSONCommand to return an error and updated implementations (Claude, Codex) to follow the new signature. Codex now returns explicit errors when temp files cannot be created.
- Updated callers (internal/cmd/convert.go, runFixJSONWithProvider, loop tests, agent tests) to handle the new error return values.
- Changed Resolve to return (loop.Provider, error) and make unknown providers produce a clear error; added mustResolve helper in tests and a TestResolve_unknownProvider.
- Added resolveProvider helper in cmd/chief to load config and resolve the provider (exiting on error), and improved CLI flag parsing for --agent/--agent-path (support both --flag value and --flag=value). Minor test and docstring tweaks to reflect agent-agnostic language.
Add runtime validation and better error handling across CLI and loop components. Key changes:

- cmd/chief: validate presence of values for --agent and --agent-path flags (print error and exit) and handle config.Load errors with a clear message.
- internal/cmd: enforce non-nil Provider for RunNew, RunEdit and runInteractiveAgent (return explicit errors) and add corresponding tests that assert provider is required. Minor comment/spacing cleanups in option structs.
- internal/cmd/convert: ensure temporary output files are removed on failed command start to avoid leftover artifacts.
- internal/loop: return an error if Loop.Run is invoked without a configured provider; require provider for Manager.Start; add tests for both failure cases. Also minor struct field alignment and test provider method formatting.

These changes make failures explicit and fail fast with clearer messages, and ensure temporary files are cleaned up on early command failures.
Remove the deprecated --output-last-message option from CodexProvider CLI invocations and simplify error messages in WaitWithSpinner and WaitWithPanel from "Claude failed" to the more generic "agent failed" for compatibility and clearer messaging. Changes in internal/agent/codex.go and internal/prd/generator.go.
…rser event timing

- Remove duplicate stdin in OpenCode ConvertCommand and FixJSONCommand
  where the prompt was passed both as a CLI argument and via stdin
- Revert Claude ConvertCommand to use `-p` with stdin instead of the
  incorrect `-p --tools ""` which passed an empty string as a separate arg
- Fix OpenCode parser to emit EventToolStart when tools begin (not only
  on completion) and EventToolResult when completed, so the TUI shows
  activity during tool execution
…e test coverage

- Extract parseAgentFlags() in main.go to eliminate triple-duplicated
  --agent/--agent-path parsing across parseTUIFlags, runNew, and runEdit
- Extract runAgentCommand() in convert.go to share stdout/file output
  handling between runConversionWithProvider and runFixJSONWithProvider
- Improve OpenCode CleanOutput to use JSON parsing instead of fragile
  string matching heuristics
- Add Claude provider tests (claude_test.go) covering all Provider methods
- Add OpenCode CleanOutput tests for NDJSON extraction and edge cases
- Add OpenCode parser tests for tool_use start/no-state/completed events
- Update troubleshooting docs to clarify provider-specific log file names
Add acknowledgments for the contributors who designed and built the
multi-agent architecture, Codex CLI integration, and OpenCode CLI support.
@MiniCodeMonkey MiniCodeMonkey merged commit 0e15001 into main Mar 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants