Skip to content

fix(workflows): validate non-string step types - #4111

Open
NgoQuocViet2001 wants to merge 1 commit into
github:mainfrom
NgoQuocViet2001:fix/cli-edge-case-audit
Open

fix(workflows): validate non-string step types#4111
NgoQuocViet2001 wants to merge 1 commit into
github:mainfrom
NgoQuocViet2001:fix/cli-edge-case-audit

Conversation

@NgoQuocViet2001

Copy link
Copy Markdown
Contributor

Description

A workflow step whose YAML type is a sequence or mapping currently reaches the step-registry membership check and raises a raw TypeError because the value is unhashable. This prevents workflow run, validation, and installation paths from reporting their normal actionable configuration errors.

This change validates that every step type is a string before registry lookup. Unknown string types keep the existing invalid type behavior, while list/mapping values now produce a typed validation error instead of a traceback.

Testing

  • Tested locally with .venv\Scripts\specify.exe --help
  • Ran existing tests with uv sync && uv run pytest
  • Tested with a sample workflow

Validation 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 deselected
  • Full tests/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 — passed
  • CLI repro with type: [shell] — exits 1 with 'type' must be a string, got list and no traceback

The repository-wide suite was also attempted, but the local command runner timed out before completion; CI should provide the complete platform matrix.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

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.

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)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address test & lint errors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants