fix(workflows): validate non-string step types - #4111
Conversation
There was a problem hiding this comment.
Pull request overview
Validates workflow step types before registry lookup, preventing unhashable YAML values from causing raw TypeError exceptions.
Changes:
- Rejects non-string step types with an actionable validation error.
- Adds regression tests for sequence and mapping values.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/workflows/engine.py |
Adds step-type validation. |
tests/test_workflows.py |
Tests non-string step types. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
mnriem
left a comment
There was a problem hiding this comment.
Please address test & lint errors
|
Ruff is green. Five of the six pytest matrix jobs pass. The only failure is |
Return an actionable validation error when a workflow step type is a YAML list or mapping instead of raising during registry membership checks. Assisted-by: OpenAI Codex (model: GPT-5, autonomous)
471a18f to
c03cb4c
Compare
|
@mnriem I rebased the PR onto the latest
GitHub is currently holding four workflows for maintainer approval. Could you approve them and take another look once the matrix completes? Current HEAD: |
|
Thank you! |
|
@mnriem Thanks for the review and merge! |
Description
A workflow step whose YAML
typeis a sequence or mapping currently reaches the step-registry membership check and raises a rawTypeErrorbecause the value is unhashable. This preventsworkflow run, validation, and installation paths from reporting their normal actionable configuration errors.This change validates that every step
typeis a string before registry lookup. Unknown string types keep the existinginvalid typebehavior, while list/mapping values now produce a typed validation error instead of a traceback.Testing
.venv\Scripts\specify.exe --helpuv sync && uv run pytestValidation performed:
.venv\Scripts\python -m pytest tests/test_workflows.py -k "invalid_step_type or non_string_step_type_reports_error" -q— 3 passed.venv\Scripts\python -m pytest tests/test_workflows.py -q -k "not symlink and not cross_project_registry_root"— 886 passed, 7 skipped, 36 deselectedtests/test_workflows.py— 902 passed, 7 skipped; 20 environment-only failures because this Windows session lacks symlink privilege (WinError 1314)uvx ruff@0.15.0 check src tests— passed.venv\Scripts\python -m compileall -q src— passedtype: [shell]— exits 1 with'type' must be a string, got listand no tracebackThe repository-wide suite was also attempted, but the local command runner timed out before completion; CI should provide the complete platform matrix.
AI Disclosure
OpenAI Codex (GPT-5, autonomous) independently identified the bug, authored the code and regression tests, ran the validations listed above, and prepared this pull request on behalf of @NgoQuocViet2001. The operator requested autonomous repository improvements and approved this candidate; the operator did not manually author or line-by-line review the change.