Skip to content

fix(workflows): handle an unreadable run state in workflow status - #3999

Merged
mnriem merged 1 commit into
github:mainfrom
Noor-ul-ain001:fix/status-oserror-runstate
Aug 6, 2026
Merged

fix(workflows): handle an unreadable run state in workflow status#3999
mnriem merged 1 commit into
github:mainfrom
Noor-ul-ain001:fix/status-oserror-runstate

Conversation

@Noor-ul-ain001

Copy link
Copy Markdown
Contributor

Problem

workflow status <run_id> and workflow resume <run_id> both load the same run state via RunState.load(). A prior fix aligned the two on the FileNotFoundError and ValueError boundaries, but resume also handles OSError and status never gained that handler:

# workflow_resume
except FileNotFoundError: ...
except ValueError as exc: ...
except OSError as exc:          # <-- present here
    err.print(f"[red]Resume failed:[/red] ...")
    raise typer.Exit(1)

# workflow_status
except FileNotFoundError: ...
except ValueError as exc: ...
                                # <-- missing

So an unreadable state.json — wrong permissions, an I/O error, or a directory sitting where the file belongs — escapes workflow status as a raw traceback, while resume on the very same run exits cleanly.

state_path.exists() returns True for a directory, so the existing guard passes and open() raises.

Reproduction

With a run directory whose state.json is a directory, on unmodified main:

workflow status abc123     exit=1  LEAKED PermissionError   output: ''
workflow resume abc123     exit=1  clean                    output: 'Resume failed: [Errno 13] Permission denied: ...'

status prints nothing at all and dumps a traceback; resume prints a clean error. Reproduced through the real Typer CLI, no mocking.

Fix

Add the missing except OSError beside its siblings, using the same _escape_markup + typer.Exit(1) shape as the neighbouring handlers, and routing through err so the message goes to stderr under --json and the stdout JSON stream stays parseable.

Tests

Two regression tests in TestWorkflowCliAlignment, next to the existing status/resume alignment tests:

  • test_status_unreadable_run_state_exits_cleanly — end-to-end CLI, a directory in place of state.json
  • test_status_json_unreadable_run_state_error_goes_to_stderr--json stderr routing, mirroring the sibling ValueError test

Both fail without the source change (verified by reverting _commands.py alone) and pass with it.

tests/test_workflows.py: 898 passed, up from 896 on the same tree, with an unchanged set of 20 pre-existing failures — all Windows symlink tests that need elevation and fail identically on unmodified main.

🤖 Generated with Claude Code

`workflow status <run_id>` and `workflow resume <run_id>` both call
`RunState.load()`, and a prior fix aligned them on the FileNotFoundError
and ValueError boundaries. `resume` also handles OSError; `status` never
gained that handler.

So an unreadable `state.json` -- wrong permissions, an I/O error, or a
directory sitting where the file belongs -- escapes as a raw traceback
with no output at all, while `resume` on the same run prints a clean
`Error:` line and exits 1. `state_path.exists()` is True for a directory,
so the existing guard passes and `open()` raises.

Add the missing `except OSError` next to its siblings, using the same
`_escape_markup` + `typer.Exit(1)` shape, and routing through `err` so
the message lands on stderr under `--json` and the stdout JSON stream
stays parseable.

Two regression tests: the end-to-end CLI path (a directory in place of
state.json) and the `--json` stderr-routing path. Both fail without the
source change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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

Handles unreadable workflow run state files without leaking raw tracebacks.

Changes:

  • Catches OSError in workflow status.
  • Routes JSON-mode errors to stderr.
  • Adds CLI and stderr-routing regression tests.
Show a summary per file
File Description
src/specify_cli/workflows/_commands.py Adds clean OSError handling.
tests/test_workflows.py Covers unreadable state and JSON output.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@mnriem
mnriem merged commit 204d94f into github:main Aug 6, 2026
14 checks passed
@mnriem

mnriem commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

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