feat: Codex cross-platform support (Phase 1)#12
Conversation
Replace all ${DROID_PLUGIN_ROOT:-${CLAUDE_PLUGIN_ROOT}}/bin/flowctl
references in skills/ with $HOME/.flow/bin/flowctl for cross-platform
consistency. Also add YAML frontmatter to cross-model-reviewer.md agent.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…bcommand Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 4 new unit tests for parse_agent (valid, no frontmatter, invalid yaml, missing model) to codex_sync.rs. Create scripts/codex_smoke_test.sh that validates TOML generation, sandbox modes, hooks patching, and dry-run. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces initial OpenAI Codex support to the flowctl toolchain and updates the Flow-Code skill docs to use a unified flowctl location, enabling Codex-compatible agent artifact generation and discovery.
Changes:
- Standardize skill documentation to call
flowctlvia$HOME/.flow/bin/flowctl. - Add
flowctl codex sync(core implementation + CLI wiring) to convert agent Markdown + patch hooks for Codex consumption. - Add Codex plugin discovery artifacts and a smoke test script; bump CLI version in lockfile.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/flow-code/SKILL.md | Update documented FLOWCTL path to $HOME/.flow/bin/flowctl. |
| skills/flow-code-work/SKILL.md | Update documented FLOWCTL path. |
| skills/flow-code-work/phases.md | Update documented FLOWCTL path. |
| skills/flow-code-sync/SKILL.md | Update documented FLOWCTL path (multiple occurrences). |
| skills/flow-code-retro/SKILL.md | Update documented FLOWCTL path. |
| skills/flow-code-ralph-init/SKILL.md | Copy flowctl from $HOME/.flow/bin/flowctl in setup instructions. |
| skills/flow-code-plan/steps.md | Update documented FLOWCTL path. |
| skills/flow-code-plan/SKILL.md | Update documented FLOWCTL path. |
| skills/flow-code-plan-review/workflow.md | Update documented FLOWCTL path. |
| skills/flow-code-plan-review/SKILL.md | Update documented FLOWCTL path (multiple occurrences). |
| skills/flow-code-interview/SKILL.md | Update documented FLOWCTL path (multiple occurrences). |
| skills/flow-code-impl-review/workflow.md | Update documented FLOWCTL path. |
| skills/flow-code-impl-review/SKILL.md | Update documented FLOWCTL path (multiple occurrences). |
| skills/flow-code-export-context/SKILL.md | Update documented FLOWCTL path. |
| skills/flow-code-epic-review/workflow.md | Update documented FLOWCTL path (multiple occurrences). |
| skills/flow-code-epic-review/SKILL.md | Update documented FLOWCTL path (multiple occurrences). |
| skills/flow-code-deps/SKILL.md | Update documented FLOWCTL path (multiple occurrences). |
| skills/flow-code-brainstorm/SKILL.md | Update documented FLOWCTL path (multiple occurrences). |
| skills/flow-code-auto-improve/SKILL.md | Update documented FLOWCTL path. |
| skills/_shared/rp-review-protocol.md | Update documented FLOWCTL path. |
| scripts/codex_smoke_test.sh | Add smoke tests for flowctl codex sync. |
| scripts/bump-version.sh | Include .codex-plugin/plugin.json in version bump/check. |
| flowctl/crates/flowctl-core/src/lib.rs | Export new codex_sync module. |
| flowctl/crates/flowctl-core/src/codex_sync.rs | Implement agent MD → TOML conversion + hooks patching + unit tests. |
| flowctl/crates/flowctl-cli/src/commands/codex/sync.rs | Add CLI command handler for flowctl codex sync. |
| flowctl/crates/flowctl-cli/src/commands/codex/review.rs | Refactor review implementation into codex/ module structure. |
| flowctl/crates/flowctl-cli/src/commands/codex/mod.rs | Add Sync subcommand and shared helper functions after module split. |
| flowctl/Cargo.lock | Bump flowctl-cli version to 0.1.31. |
| agents/cross-model-reviewer.md | Add YAML frontmatter for Codex/agent tooling compatibility. |
| .codex-plugin/plugin.json | Add Codex plugin manifest. |
| .agents/plugins/marketplace.json | Add plugin discovery entry for Codex/agents marketplace indexing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if !dry_run { | ||
| let out_path = agents_out.join(format!("{}.toml", doc.frontmatter.name)); | ||
| std::fs::write(&out_path, &toml_content).map_err(|e| { | ||
| CoreError::FrontmatterParse(format!( | ||
| "cannot write {}: {e}", | ||
| out_path.display() | ||
| )) |
There was a problem hiding this comment.
sync_all uses doc.frontmatter.name directly when constructing the output filename. If an agent frontmatter name contains path separators or .., this can write outside output_dir/agents (path traversal) or fail unexpectedly on Windows. Validate/sanitize the name (e.g., allow only [A-Za-z0-9_-]+) and/or derive the filename from the source .md basename instead of the frontmatter value before joining paths.
| #[test] | ||
| fn test_map_model_opus() { | ||
| let m = map_model("opus", "worker"); | ||
| assert_eq!(m.codex_model, "gpt-5.4"); | ||
| assert_eq!(m.reasoning_effort.as_deref(), Some("high")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_map_model_sonnet_fast() { | ||
| let m = map_model("sonnet", "worker"); | ||
| assert_eq!(m.codex_model, "gpt-5.4-mini"); | ||
| assert!(m.reasoning_effort.is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_map_model_sonnet_intelligent_scout() { | ||
| let m = map_model("sonnet", "epic-scout"); | ||
| assert_eq!(m.codex_model, "gpt-5.4"); | ||
| assert_eq!(m.reasoning_effort.as_deref(), Some("high")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_map_model_haiku() { | ||
| let m = map_model("haiku", "worker"); | ||
| assert_eq!(m.codex_model, "gpt-5.4-mini"); | ||
| assert!(m.reasoning_effort.is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_map_model_inherit() { | ||
| let m = map_model("inherit", "worker"); | ||
| assert!(m.codex_model.is_empty()); | ||
| assert!(m.reasoning_effort.is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_map_model_empty() { | ||
| let m = map_model("", "worker"); | ||
| assert!(m.codex_model.is_empty()); | ||
| assert!(m.reasoning_effort.is_none()); |
There was a problem hiding this comment.
These tests assert the default model strings (e.g. gpt-5.4) but map_model() reads CODEX_MODEL_INTELLIGENT/CODEX_MODEL_FAST from the process environment. If a developer or CI sets those env vars, the tests will fail. Consider clearing/restoring these env vars within the tests (or refactor model selection so tests can inject defaults) to keep the unit tests deterministic.
| #[test] | |
| fn test_map_model_opus() { | |
| let m = map_model("opus", "worker"); | |
| assert_eq!(m.codex_model, "gpt-5.4"); | |
| assert_eq!(m.reasoning_effort.as_deref(), Some("high")); | |
| } | |
| #[test] | |
| fn test_map_model_sonnet_fast() { | |
| let m = map_model("sonnet", "worker"); | |
| assert_eq!(m.codex_model, "gpt-5.4-mini"); | |
| assert!(m.reasoning_effort.is_none()); | |
| } | |
| #[test] | |
| fn test_map_model_sonnet_intelligent_scout() { | |
| let m = map_model("sonnet", "epic-scout"); | |
| assert_eq!(m.codex_model, "gpt-5.4"); | |
| assert_eq!(m.reasoning_effort.as_deref(), Some("high")); | |
| } | |
| #[test] | |
| fn test_map_model_haiku() { | |
| let m = map_model("haiku", "worker"); | |
| assert_eq!(m.codex_model, "gpt-5.4-mini"); | |
| assert!(m.reasoning_effort.is_none()); | |
| } | |
| #[test] | |
| fn test_map_model_inherit() { | |
| let m = map_model("inherit", "worker"); | |
| assert!(m.codex_model.is_empty()); | |
| assert!(m.reasoning_effort.is_none()); | |
| } | |
| #[test] | |
| fn test_map_model_empty() { | |
| let m = map_model("", "worker"); | |
| assert!(m.codex_model.is_empty()); | |
| assert!(m.reasoning_effort.is_none()); | |
| fn with_default_model_env_cleared<T>(f: impl FnOnce() -> T) -> T { | |
| struct EnvRestore { | |
| intelligent: Option<std::ffi::OsString>, | |
| fast: Option<std::ffi::OsString>, | |
| } | |
| impl Drop for EnvRestore { | |
| fn drop(&mut self) { | |
| match &self.intelligent { | |
| Some(value) => std::env::set_var("CODEX_MODEL_INTELLIGENT", value), | |
| None => std::env::remove_var("CODEX_MODEL_INTELLIGENT"), | |
| } | |
| match &self.fast { | |
| Some(value) => std::env::set_var("CODEX_MODEL_FAST", value), | |
| None => std::env::remove_var("CODEX_MODEL_FAST"), | |
| } | |
| } | |
| } | |
| let restore = EnvRestore { | |
| intelligent: std::env::var_os("CODEX_MODEL_INTELLIGENT"), | |
| fast: std::env::var_os("CODEX_MODEL_FAST"), | |
| }; | |
| std::env::remove_var("CODEX_MODEL_INTELLIGENT"); | |
| std::env::remove_var("CODEX_MODEL_FAST"); | |
| let result = f(); | |
| drop(restore); | |
| result | |
| } | |
| #[test] | |
| fn test_map_model_opus() { | |
| with_default_model_env_cleared(|| { | |
| let m = map_model("opus", "worker"); | |
| assert_eq!(m.codex_model, "gpt-5.4"); | |
| assert_eq!(m.reasoning_effort.as_deref(), Some("high")); | |
| }); | |
| } | |
| #[test] | |
| fn test_map_model_sonnet_fast() { | |
| with_default_model_env_cleared(|| { | |
| let m = map_model("sonnet", "worker"); | |
| assert_eq!(m.codex_model, "gpt-5.4-mini"); | |
| assert!(m.reasoning_effort.is_none()); | |
| }); | |
| } | |
| #[test] | |
| fn test_map_model_sonnet_intelligent_scout() { | |
| with_default_model_env_cleared(|| { | |
| let m = map_model("sonnet", "epic-scout"); | |
| assert_eq!(m.codex_model, "gpt-5.4"); | |
| assert_eq!(m.reasoning_effort.as_deref(), Some("high")); | |
| }); | |
| } | |
| #[test] | |
| fn test_map_model_haiku() { | |
| with_default_model_env_cleared(|| { | |
| let m = map_model("haiku", "worker"); | |
| assert_eq!(m.codex_model, "gpt-5.4-mini"); | |
| assert!(m.reasoning_effort.is_none()); | |
| }); | |
| } | |
| #[test] | |
| fn test_map_model_inherit() { | |
| with_default_model_env_cleared(|| { | |
| let m = map_model("inherit", "worker"); | |
| assert!(m.codex_model.is_empty()); | |
| assert!(m.reasoning_effort.is_none()); | |
| }); | |
| } | |
| #[test] | |
| fn test_map_model_empty() { | |
| with_default_model_env_cleared(|| { | |
| let m = map_model("", "worker"); | |
| assert!(m.codex_model.is_empty()); | |
| assert!(m.reasoning_effort.is_none()); | |
| }); |
| //! Codex sync command — generates Codex artifacts from agent `.md` files. | ||
|
|
||
| use std::path::Path; | ||
|
|
||
| use serde_json::json; | ||
|
|
||
| use flowctl_core::codex_sync::sync_all; | ||
|
|
||
| use crate::output::{error_exit, json_output}; | ||
|
|
||
| pub fn cmd_sync( | ||
| json_mode: bool, | ||
| agents_dir: &str, | ||
| output_dir: &str, | ||
| hooks: &str, | ||
| dry_run: bool, | ||
| verbose: bool, | ||
| ) { | ||
| let agents_path = Path::new(agents_dir); | ||
| let output_path = Path::new(output_dir); | ||
| let hooks_path = Path::new(hooks); | ||
|
|
||
| let hooks_arg = if hooks_path.exists() { | ||
| Some(hooks_path) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| let summary = match sync_all(agents_path, hooks_arg, output_path, dry_run) { | ||
| Ok(s) => s, | ||
| Err(e) => { | ||
| error_exit(&format!("codex sync failed: {e}")); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
The PR description and core module docs mention generating .codex-plugin/plugin.json and .agents/plugins/marketplace.json, but cmd_sync currently only calls sync_all, which (at least in this PR) writes agent TOMLs and an optional hooks.json only. Either implement generation of the plugin/marketplace manifests as part of flowctl codex sync or update the docs/PR description so expectations match actual behavior.
| /// Sync agent .md files to Codex artifacts. | ||
| Sync { | ||
| /// Directory containing agent .md files. | ||
| #[arg(long, default_value = "agents")] | ||
| agents_dir: String, | ||
| /// Output directory for generated Codex artifacts. | ||
| #[arg(long, default_value = "codex")] | ||
| output_dir: String, | ||
| /// Source hooks.json file to patch. | ||
| #[arg(long, default_value = "hooks/hooks.json")] | ||
| hooks: String, | ||
| /// Validate without writing files. | ||
| #[arg(long)] | ||
| dry_run: bool, | ||
| /// Show per-file details. | ||
| #[arg(long)] | ||
| verbose: bool, | ||
| }, |
There was a problem hiding this comment.
output_dir for flowctl codex sync defaults to codex, but the rest of this PR (and the PR description) centers around .codex-plugin/ as the Codex discovery directory. Consider changing the default to .codex-plugin (or documenting that callers must pass --output-dir .codex-plugin) to avoid generating artifacts into a non-discoverable folder by default.
| } | ||
|
|
||
| TEST_DIR=$(mktemp -d) | ||
| trap "rm -rf $TEST_DIR" EXIT |
There was a problem hiding this comment.
The cleanup trap uses an unquoted variable (rm -rf $TEST_DIR). Quoting this prevents edge-case failures if the temp path contains spaces or glob characters and matches the quoting style used in other repo scripts.
| trap "rm -rf $TEST_DIR" EXIT | |
| trap 'rm -rf "$TEST_DIR"' EXIT |
P0 fixes (state loss — root cause of 5 issues): - get_flow_dir() now walks up directory tree (FLOW_STATE_DIR env → walk-up → CWD) Fixes: #1 state loss, #3 state not persistent, #5 worker parallel fail, #9 .flow symlink issues. Same pattern as git finding .git. - flowctl recover --epic <id> [--dry-run]: rebuilds task completion status from git log. Fixes #11 no recovery after state loss. P1 fixes (guard + review): - Guard graceful fallback: missing tools → "skipped" (not "failed"). Only actual failures block pipeline. Fixes #8. - Review-backend availability check: if rp-cli/codex not in PATH, auto-fallback to "none" with warning. Fixes #7. P2 fixes (UX): - Slug max length 40→20 chars. "Django+React platform with account management" → "fn-3-django-react-plat" not 40-char monster. Fixes #2 #12. - Brainstorm auto-skip: trivial tasks (≤10 words, contains "fix"/"typo"/etc) skip brainstorm entirely. Fixes #6. - --interactive flag: pause at key decisions. Fixes #10. 370 tests pass. Zero new dependencies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
$HOME/.flow/bin/flowctl(30 replacements across 20 skills)flowctl codex synccommand that converts agent .md files to Codex .toml format.codex-plugin/plugin.jsonand.agents/plugins/marketplace.jsonfor Codex plugin discoverycodex.rsintocodex/module directory (mod.rs + review.rs + sync.rs)cross-model-reviewer.mdWhat this enables
flow-code can now generate Codex-compatible agent definitions from existing Claude Code agents. The
flowctl codex synccommand handles:Test plan
cargo test -p flowctl-core codex_sync— 21 tests passscripts/codex_smoke_test.sh— 5 smoke tests passcargo clippy --all— no warningsPhase 2 (future)
scripts/install.sh)🤖 Generated with Claude Code