diff --git a/codex-rs/cli/src/workflow_cmd/compat.rs b/codex-rs/cli/src/workflow_cmd/compat.rs index 1754d32e247d..9fc8b68d0c7f 100644 --- a/codex-rs/cli/src/workflow_cmd/compat.rs +++ b/codex-rs/cli/src/workflow_cmd/compat.rs @@ -682,7 +682,7 @@ fn apply_compatibility_repairs(target: &RepairWorkflowTarget) -> anyhow::Result< if repaired != contents { fs::write(&workflow_yaml, repaired)?; repairs.push(format!( - "Updated {} with code-review autocomplete metadata", + "Updated {} with code-review workflow metadata", workflow_yaml.display() )); } @@ -719,80 +719,9 @@ fn repair_code_review_workflow_metadata(contents: &str) -> String { "userDescription", "Run a code review and repro workflow on the current branch.", ); - if !has_workflow_usage_options(&repaired) { - if !repaired.ends_with('\n') { - repaired.push('\n'); - } - repaired.push_str(CODE_REVIEW_USAGE_OPTIONS_YAML); - } repaired } -const CODE_REVIEW_USAGE_OPTIONS_YAML: &str = r#"usage: - options: - - flag: --action - valueHint: - description: Run mode: review, read-report, list-reports, incremental, or resume. - - flag: --working-directory - valueHint: - description: Repository path or report-list directory filter. - - flag: --target-ref - valueHint: - description: Branch or commit to review. - - flag: --base-ref - valueHint: - description: Upstream/base reference for branch comparison. - - flag: --scope - valueHint: - description: Review branch changes or the whole repository. - - flag: --review-id - valueHint: - description: Existing review ID for read-report, incremental, or resume. - - flag: --review-model - valueHint: - description: Model override for initial review agents. - - flag: --repro-model - valueHint: - description: Model override for reproduction agents. - - flag: --limit - valueHint: - description: Maximum kept findings sent to reproduction. - - flag: --chunk-size-bytes - valueHint: - description: Maximum chunk size in bytes. - - flag: --module-depth - valueHint: - description: Directory depth used for chunk grouping. - - flag: --severity-threshold - valueHint: - description: Minimum severity from 0 to 10. - - flag: --confidence-threshold - valueHint: - description: Minimum confidence from 0 to 100. - - flag: --include-preexisting - description: Keep preexisting findings in branch scope. - - flag: --include-skipped-by-limit - description: Incremental mode also replays findings skipped by the previous limit. - - flag: --allowed-areas - valueHint: - description: Allowed finding areas. - - flag: --database-path - valueHint: - description: SQLite audit store path. - - flag: --artifacts-dir - valueHint: - description: Artifact root directory. - - flag: --output - valueHint: - description: Return json or markdown wrapper output. - - flag: --report-type - valueHint: - description: Render default markdown or a GitHub review draft. - - flag: --findings - valueHint: - description: Return confirmed, filtered, or both finding sets. -"#; - fn default_command_from_id(id: &str) -> String { slugify(id.rsplit('/').next().unwrap_or(id)) } @@ -1195,32 +1124,6 @@ fn strip_inline_comment(value: &str) -> &str { value } -fn has_workflow_usage_options(contents: &str) -> bool { - let mut in_usage = false; - for line in contents.lines() { - let trimmed = line.trim(); - if trimmed.is_empty() || trimmed.starts_with('#') { - continue; - } - let indent = line.len().saturating_sub(line.trim_start().len()); - if indent == 0 { - in_usage = trimmed - .split_once(':') - .is_some_and(|(line_key, _)| line_key.trim() == "usage"); - continue; - } - if in_usage - && indent == 2 - && trimmed - .split_once(':') - .is_some_and(|(line_key, _)| line_key.trim() == "options") - { - return true; - } - } - false -} - fn set_top_level_yaml_scalar(contents: &str, key: &str, value: &str) -> String { let mut replaced = false; let mut lines = Vec::new(); diff --git a/codex-rs/cli/tests/workflows__cli.rs b/codex-rs/cli/tests/workflows__cli.rs index b8a3248ba36f..f6b4aa334dcb 100644 --- a/codex-rs/cli/tests/workflows__cli.rs +++ b/codex-rs/cli/tests/workflows__cli.rs @@ -107,7 +107,7 @@ fn write_workflow_source(workflow_dir: &Path) -> Result<()> { Ok(()) } -fn assert_code_review_autocomplete_metadata(workflow_dir: &Path) -> Result<()> { +fn assert_code_review_static_metadata_without_legacy_usage(workflow_dir: &Path) -> Result<()> { let workflow_yaml = fs::read_to_string(workflow_dir.join("workflow.yaml"))?; assert!( workflow_yaml.contains("id: code-review") || workflow_yaml.contains("id: \"code-review\"") @@ -116,14 +116,23 @@ fn assert_code_review_autocomplete_metadata(workflow_dir: &Path) -> Result<()> { workflow_yaml.contains("command: code-review") || workflow_yaml.contains("command: \"code-review\"") ); - assert!(workflow_yaml.contains("usage:")); - assert!(workflow_yaml.contains("options:")); - assert!(workflow_yaml.contains("flag: --action")); - assert!( - workflow_yaml.contains("valueHint: ") - ); - assert!(workflow_yaml.contains("flag: --review-id")); - assert!(workflow_yaml.contains("flag: --include-skipped-by-limit")); + assert!(workflow_yaml.contains("title:")); + assert!(workflow_yaml.contains("userDescription:")); + assert!(!workflow_yaml.contains("usage:\n options:")); + for legacy_fragment in [ + "read-report", + "list-reports", + "incremental", + "--action", + "--review-id", + "--target-ref", + "--base-ref", + "--review-model", + "--repro-model", + "--include-skipped-by-limit", + ] { + assert!(!workflow_yaml.contains(legacy_fragment)); + } Ok(()) } @@ -646,11 +655,46 @@ fn workflow_fix_repairs_workflow_without_running_unsupported_fix_action() -> Res "Repairing workflow code-review with compatibility mode.", )) .stdout(contains("Updated ")) - .stdout(contains("with code-review autocomplete metadata")) + .stdout(contains("with code-review workflow metadata")) .stdout(contains("code-review repair check completed.")); assert!(!fake_bun.was_invoked()); - assert_code_review_autocomplete_metadata(&workflow_dir)?; + assert_code_review_static_metadata_without_legacy_usage(&workflow_dir)?; + + Ok(()) +} + +#[test] +fn workflow_fix_keeps_valid_code_review_workflow_without_usage_options() -> Result<()> { + let codex_home = TempDir::new()?; + let project = TempDir::new()?; + enable_workflows(codex_home.path())?; + let workflow_yaml = r#"id: code-review +command: code-review +title: "/code-review" +userDescription: "Run a code review and repro workflow on the current branch." +"#; + let workflow_dir = write_workflow( + &codex_home.path().join("workflows"), + "code-review", + workflow_yaml, + )?; + write_workflow_source(&workflow_dir)?; + + let mut cmd = codex_command(codex_home.path(), project.path())?; + cmd.args(["workflow", "fix", "code-review"]) + .assert() + .success() + .stdout(contains( + "No compatibility repairs were needed for code-review.", + )) + .stdout(contains("code-review repair check completed.")); + + assert_eq!( + fs::read_to_string(workflow_dir.join("workflow.yaml"))?, + workflow_yaml + ); + assert_code_review_static_metadata_without_legacy_usage(&workflow_dir)?; Ok(()) } @@ -697,11 +741,11 @@ fn workflow_fix_tolerates_broken_metadata_and_source_without_running_workflow() "Repairing workflow code-review with compatibility mode.", )) .stdout(contains("Updated ")) - .stdout(contains("with code-review autocomplete metadata")) + .stdout(contains("with code-review workflow metadata")) .stdout(contains("code-review repair check completed.")); assert!(!fake_bun.was_invoked()); - assert_code_review_autocomplete_metadata(&workflow_dir)?; + assert_code_review_static_metadata_without_legacy_usage(&workflow_dir)?; Ok(()) } @@ -730,7 +774,7 @@ fn workflow_fix_scaffolds_missing_workflow_source_for_discovery_fallback() -> Re .stdout(contains("code-review repair check completed.")); assert!(workflow_dir.join("src").join("workflow.ts").is_file()); - assert_code_review_autocomplete_metadata(&workflow_dir)?; + assert_code_review_static_metadata_without_legacy_usage(&workflow_dir)?; Ok(()) } @@ -758,11 +802,11 @@ fn workflow_repair_alias_repairs_workflow_without_running_workflow_runtime() -> "Repairing workflow code-review with compatibility mode.", )) .stdout(contains("Updated ")) - .stdout(contains("with code-review autocomplete metadata")) + .stdout(contains("with code-review workflow metadata")) .stdout(contains("code-review repair check completed.")); assert!(!fake_bun.was_invoked()); - assert_code_review_autocomplete_metadata(&workflow_dir)?; + assert_code_review_static_metadata_without_legacy_usage(&workflow_dir)?; Ok(()) }