[codex] test: cover workflow validator rules

[stranske]

Source: Issue #2637

Closes #2637

Automated Status Summary

Scope

scripts/workflow_validator.py contains pure workflow-quality rules whose edge cases should be covered apart from live workflow files. This is a safe Route-Weight testgen opener because it is deterministic, file-local, and can use synthetic fixtures.

Tasks

• Cover load_workflow() valid YAML, invalid YAML, missing file, and non-mapping YAML.
• Cover deprecated action detection with explicit step names and fallback step-N names.
• Cover missing timeout detection across multiple jobs.
• Cover upload-artifact major-version detection with expected and unexpected majors.
• Cover hardcoded secret detection and unsafe string interpolation detection with synthetic workflow dictionaries.

Acceptance criteria

• Add focused pytest coverage in tests/scripts/test_workflow_validator.py for scripts/workflow_validator.py.
• Tests use temporary files, monkeypatches, or fake objects only.
• Tests do not require network access, GitHub credentials, macOS APIs, or live workflow mutation.
• Keep production-code changes out unless a tiny, clearly explained testability fix is unavoidable.

Head SHA: 70aa2eb
Latest Runs: ❔ in progress — Gate
Required: gate: ❔ in progress

Workflow / Job	Result	Logs
Auto-label dependency PRs	⏭️ skipped	View run
Gate	❔ in progress	View run
Health 40 Sweep	❔ startup failure	View run
Health 44 Gate Branch Protection	❔ in progress	View run
Health 45 Agents Guard	✅ success	View run
Health 50 Security Scan	✅ success	View run
Health 52 Semgrep Scan	✅ success	View run
Maint 52 Validate Workflows	✅ success	View run
PR 11 - Minimal invariant CI	✅ success	View run
Selftest CI	✅ success	View run

Summary by CodeRabbit

• Tests
  • Added coverage for workflow validation handling, including YAML edge cases and missing files.
  • Expanded checks for deprecated actions, missing job timeouts, artifact version consistency, and unsafe string interpolation.
  • Added validation for detecting hardcoded secret-like patterns in workflow environment values.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds tests/scripts/test_workflow_validator.py with six unit test functions covering all major rule functions in scripts/workflow_validator.py: YAML loading edge cases, deprecated action detection, missing timeout detection, upload-artifact major-version validation, hardcoded secret detection, and unsafe string interpolation detection.

Changes

Workflow Validator Unit Tests

Layer / File(s)	Summary
Module wiring and load_workflow tests tests/scripts/test_workflow_validator.py	Imports Path and scripts.workflow_validator; tests load_workflow for valid YAML (asserts name extraction), invalid/unterminated YAML, missing file path, and non-mapping YAML (all returning None).
Validator rule function tests tests/scripts/test_workflow_validator.py	Tests check_deprecated_actions for explicit and fallback step names; check_missing_timeout across multiple jobs; check_upload_artifact_major for expected v7, patch/beta/wrong-major variants; check_hardcoded_secrets for PAT, GitHub App token, and API key patterns; check_unsafe_string_interpolation for exactly two unsafe interpolation issues with correct job/step identity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• stranske/Workflows#2426: Changes check_upload_artifact_major to default to v7, directly affecting the expected-version assertions tested in this PR.

Suggested labels

codex

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately describes the main change: adding workflow validator rule tests.
Linked Issues check	✅ Passed	The new tests match issue #2637's requested workflow-validator coverage using synthetic, file-local fixtures.
Out of Scope Changes check	✅ Passed	The PR appears scoped to the requested test file and does not introduce unrelated code changes.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch orchestrator/issue-2637

---

_{Comment @coderabbitai help to get the list of available commands.}

[stranske]

Post-push review check: no inline review threads found; CodeRabbit skipped because the PR is draft. Local validation passed as listed in the PR body. GitHub status data currently shows Gate queued and several checks still in progress, so remote CI is not complete yet.

[stranske-keepalive]

Automated Status Summary

Head SHA: 507670d
Latest Runs: ⏳ pending — Gate
Required contexts: Gate / gate, Health 45 Agents Guard / guard
Required: core tests (3.12): ⏳ pending, core tests (3.13): ⏳ pending, docker smoke: ⏳ pending, gate: ⏳ pending

Workflow / Job	Result	Logs
(no jobs reported)	⏳ pending	—

Coverage Overview

• Coverage history entries: 1

Coverage Trend

Metric	Value
Current	72.89%
Baseline	85.00%
Delta	-12.11%
Minimum	70.00%
Status	✅ Pass

Top Coverage Hotspots (lowest coverage)

File	Coverage	Missing
scripts/cleanup_labels.py	0.0%	127
scripts/issue_dedup_smoke.py	0.0%	4
scripts/repo_review_round1_runner.py	0.0%	225
scripts/repo_review_round1_schema.py	0.0%	194
scripts/runner_lib/__main__.py	0.0%	3
scripts/update_langchain_versions.py	0.0%	34
scripts/validate_dependency_test_setup.py	0.0%	112
scripts/validate_template_sync.py	0.0%	77
scripts/langchain/topic_splitter.py	19.1%	57
tools/codex_log_analyzer.py	19.6%	140
scripts/repo_review_round2_runner.py	25.1%	344
scripts/validate_template_completeness.py	38.1%	53
scripts/prune_agent_stubs.py	39.7%	26
tools/ensure_workflow_timeout_variables.py	42.1%	74
scripts/sync_label_docs.py	42.9%	64

Low Coverage Files (<50.0%)

File	Coverage	Missing
scripts/cleanup_labels.py	0.0%	127
scripts/issue_dedup_smoke.py	0.0%	4
scripts/repo_review_round1_runner.py	0.0%	225
scripts/repo_review_round1_schema.py	0.0%	194
scripts/runner_lib/__main__.py	0.0%	3
scripts/update_langchain_versions.py	0.0%	34
scripts/validate_dependency_test_setup.py	0.0%	112
scripts/validate_template_sync.py	0.0%	77
scripts/langchain/topic_splitter.py	19.1%	57
tools/codex_log_analyzer.py	19.6%	140
scripts/repo_review_round2_runner.py	25.1%	344
scripts/validate_template_completeness.py	38.1%	53
scripts/prune_agent_stubs.py	39.7%	26
tools/ensure_workflow_timeout_variables.py	42.1%	74
scripts/sync_label_docs.py	42.9%	64

Updated automatically; will refresh on subsequent CI/Docker completions.

---

Keepalive checklist

Scope

scripts/workflow_validator.py contains pure workflow-quality rules whose edge cases should be covered apart from live workflow files. This is a safe Route-Weight testgen opener because it is deterministic, file-local, and can use synthetic fixtures.

Tasks

• Cover load_workflow() valid YAML, invalid YAML, missing file, and non-mapping YAML.
• Cover deprecated action detection with explicit step names and fallback step-N names.
• Cover missing timeout detection across multiple jobs.
• Cover upload-artifact major-version detection with expected and unexpected majors.
• Cover hardcoded secret detection and unsafe string interpolation detection with synthetic workflow dictionaries.

Acceptance criteria

• Add focused pytest coverage in tests/scripts/test_workflow_validator.py for scripts/workflow_validator.py.
• Tests use temporary files, monkeypatches, or fake objects only.
• Tests do not require network access, GitHub credentials, macOS APIs, or live workflow mutation.
• Keep production-code changes out unless a tiny, clearly explained testability fix is unavoidable.