Skip to content

Gate Codex skills flag on CLI support #6520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
YusukeShimizu wants to merge 2 commits into from
Closed

Gate Codex skills flag on CLI support #6520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
468d8f8
codex: make skills flag usage conditional
YusukeShimizu Jan 16, 2026
feb0a1c
codex: add tests for skills feature flag handling
YusukeShimizu Jan 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 117 additions & 6 deletions crates/goose/src/providers/codex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,13 @@ pub struct CodexProvider {
model: ModelConfig,
#[serde(skip)]
name: String,
/// Reasoning effort level (low, medium, high)
reasoning_effort: String,
/// Whether to enable skills
enable_skills: bool,
// NOTE: Codex CLI removed the `skills` feature flag in openai/codex#8850 and now loads
// skills unconditionally. To stay compatible with older Codex CLIs (where `skills` was a
// feature flag), we only pass `--enable/--disable skills` if the installed CLI still
// advertises that flag via `codex features list`.
skills_feature_flag_supported: bool,
/// Whether to skip git repo check
skip_git_check: bool,
}
Expand Down Expand Up @@ -70,12 +73,16 @@ impl CodexProvider {
"high".to_string()
};

// Get enable_skills from config, default to true
let enable_skills = config
.get_codex_enable_skills()
.map(|s| s.to_lowercase() == "true")
.unwrap_or(true);

let skills_feature_flag_supported =
Self::supports_feature_flag(&resolved_command, "skills")
.await
.unwrap_or(false);

// Get skip_git_check from config, default to false
let skip_git_check = config
.get_codex_skip_git_check()
Expand All @@ -88,10 +95,66 @@ impl CodexProvider {
name: Self::metadata().name,
reasoning_effort,
enable_skills,
skills_feature_flag_supported,
skip_git_check,
})
}

fn feature_list_contains_feature(stdout: &str, feature: &str) -> bool {
// `codex features list` output is a whitespace-separated sequence of:
// `<name> <stability> <enabled>` repeated, e.g.:
// `undo stable false shell_tool stable true ...`
//
// We match by token to be robust to both newline- and space-delimited output.
stdout
.split_whitespace()
.collect::<Vec<_>>()
.windows(3)
.any(|w| {
w[0] == feature
&& matches!(w[1], "stable" | "beta" | "experimental")
&& matches!(w[2], "true" | "false")
})
Comment on lines +109 to +117
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If stdout has fewer than 3 tokens after split_whitespace(), the windows(3) iterator will be empty (not panic), but this edge case should be tested. An empty or malformed output from codex features list would silently return false, which is correct behavior but should be explicitly verified in tests.

Suggested change
stdout
.split_whitespace()
.collect::<Vec<_>>()
.windows(3)
.any(|w| {
w[0] == feature
&& matches!(w[1], "stable" | "beta" | "experimental")
&& matches!(w[2], "true" | "false")
})
let mut tokens = stdout.split_whitespace();
loop {
let name = match tokens.next() {
Some(t) => t,
None => break,
};
let stability = match tokens.next() {
Some(t) => t,
None => break,
};
let enabled = match tokens.next() {
Some(t) => t,
None => break,
};
if name == feature
&& matches!(stability, "stable" | "beta" | "experimental")
&& matches!(enabled, "true" | "false")
{
return true;
}
}
false

Copilot uses AI. Check for mistakes.
}
Comment on lines +103 to +118
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add test cases for edge scenarios: empty stdout, feature name appearing but with invalid stability/enabled values, feature name as substring of another word, and stdout with fewer than 3 tokens. The windows(3) call could panic on short input if the slice has fewer than 3 elements.

Copilot uses AI. Check for mistakes.

fn skills_feature_flag_args(
skills_feature_flag_supported: bool,
enable_skills: bool,
) -> Option<(&'static str, &'static str)> {
if !skills_feature_flag_supported {
return None;
}

Some((
if enable_skills {
"--enable"
} else {
"--disable"
},
"skills",
))
}

async fn supports_feature_flag(command: &PathBuf, feature: &str) -> Result<bool> {
let mut cmd = Command::new(command);
configure_command_no_window(&mut cmd);
cmd.arg("features")
.arg("list")
.stdout(Stdio::piped())
.stderr(Stdio::piped());

let output = cmd.output().await?;
if !output.status.success() {
return Ok(false);
}

let stdout = String::from_utf8_lossy(&output.stdout);
Ok(Self::feature_list_contains_feature(
stdout.as_ref(),
feature,
))
}

/// Convert goose messages to a simple text prompt format
/// Similar to Gemini CLI, we use Human:/Assistant: prefixes
fn messages_to_prompt(&self, system: &str, messages: &[Message]) -> String {
Expand Down Expand Up @@ -165,6 +228,10 @@ impl CodexProvider {
println!("Model: {}", self.model.model_name);
println!("Reasoning effort: {}", self.reasoning_effort);
println!("Enable skills: {}", self.enable_skills);
println!(
"Skills feature flag supported: {}",
self.skills_feature_flag_supported
);
println!("Skip git check: {}", self.skip_git_check);
println!("Prompt length: {} chars", prompt.len());
println!("Prompt: {}", prompt);
Expand All @@ -189,9 +256,10 @@ impl CodexProvider {
self.reasoning_effort
));

// Enable skills if configured
if self.enable_skills {
cmd.arg("--enable").arg("skills");
if let Some((flag, feature)) =
Self::skills_feature_flag_args(self.skills_feature_flag_supported, self.enable_skills)
{
cmd.arg(flag).arg(feature);
}

// JSON output format for structured parsing
Expand Down Expand Up @@ -527,6 +595,7 @@ impl Provider for CodexProvider {
"model": model_config.model_name,
"reasoning_effort": self.reasoning_effort,
"enable_skills": self.enable_skills,
"skills_feature_flag_supported": self.skills_feature_flag_supported,
"system_length": system.len(),
"messages_count": messages.len()
});
Expand Down Expand Up @@ -576,6 +645,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand All @@ -591,6 +661,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand All @@ -607,6 +678,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand Down Expand Up @@ -636,6 +708,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand All @@ -656,6 +729,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand Down Expand Up @@ -689,6 +763,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand Down Expand Up @@ -721,6 +796,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand All @@ -746,6 +822,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand All @@ -770,6 +847,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand All @@ -792,6 +870,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand Down Expand Up @@ -819,6 +898,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand Down Expand Up @@ -851,6 +931,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand Down Expand Up @@ -898,6 +979,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand All @@ -924,6 +1006,7 @@ mod tests {
name: "codex".to_string(),
reasoning_effort: "high".to_string(),
enable_skills: true,
skills_feature_flag_supported: false,
skip_git_check: false,
};

Expand Down Expand Up @@ -952,4 +1035,32 @@ mod tests {
fn test_default_model() {
assert_eq!(CODEX_DEFAULT_MODEL, "gpt-5.2-codex");
}

#[test]
fn test_skills_feature_flag_args_based_on_features_list_support() {
let stdout = "undo stable false shell_tool stable true skills stable true steer beta false";
let skills_supported = CodexProvider::feature_list_contains_feature(stdout, "skills");
assert!(skills_supported);

assert_eq!(
CodexProvider::skills_feature_flag_args(skills_supported, true),
Some(("--enable", "skills"))
);
assert_eq!(
CodexProvider::skills_feature_flag_args(skills_supported, false),
Some(("--disable", "skills"))
);

let stdout = "undo stable false shell_tool stable true steer beta false";
let skills_supported = CodexProvider::feature_list_contains_feature(stdout, "skills");
assert!(!skills_supported);
assert_eq!(
CodexProvider::skills_feature_flag_args(skills_supported, true),
None
);
assert_eq!(
CodexProvider::skills_feature_flag_args(skills_supported, false),
None
);
}
}
4 changes: 2 additions & 2 deletions documentation/docs/guides/cli-providers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ The Codex provider integrates with OpenAI's [Codex CLI tool](https://developers.
**Features:**
- Uses OpenAI's GPT-5 series models (gpt-5.2-codex, gpt-5.2, gpt-5.1-codex-max, gpt-5.1-codex-mini)
- Configurable reasoning effort levels (low, medium, high)
- Optional skills support for enhanced capabilities
- Skills support (Codex CLI native; legacy toggle only on older Codex CLIs)
- JSON output parsing for structured responses
- Automatic filtering of goose extensions from system prompts

Expand Down Expand Up @@ -311,7 +311,7 @@ The following models are recognized and passed to the Claude CLI via the `--mode
| `GOOSE_MODEL` | Model to use (only known models are passed to CLI) | `gpt-5.2-codex` |
| `CODEX_COMMAND` | Path to the Codex CLI command | `codex` |
| `CODEX_REASONING_EFFORT` | Reasoning effort level: `low`, `medium`, or `high` | `high` |
| `CODEX_ENABLE_SKILLS` | Enable Codex skills: `true` or `false` | `true` |
| `CODEX_ENABLE_SKILLS` | Legacy toggle for the Codex CLI `skills` feature flag (ignored on newer Codex CLIs where skills are always enabled): `true` or `false` | `true` |
| `CODEX_SKIP_GIT_CHECK` | Skip git repository requirement: `true` or `false` | `false` |

**Known Models:**
Expand Down