Skip to content

fix(workflows): clean up download temp file on interrupt or typer.Exit - #4134

Open
chelsealong wants to merge 1 commit into
github:mainfrom
chelsealong:fix/4132-workflow-download-tmp-cleanup
Open

fix(workflows): clean up download temp file on interrupt or typer.Exit#4134
chelsealong wants to merge 1 commit into
github:mainfrom
chelsealong:fix/4132-workflow-download-tmp-cleanup

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Description

Fixes #4132.

specify workflow add <url> streams the download into a
tempfile.NamedTemporaryFile(..., delete=False). The file is created on
disk immediately (before any bytes are written), so any exit after that
point is supposed to clean it up.

Two exit paths skipped that cleanup:

  1. except typer.Exit: raise re-raised without unlinking tmp_path.
  2. KeyboardInterrupt is a BaseException, not an Exception, so
    pressing Ctrl+C during the size-limited read wasn't caught by the
    existing except Exception handler at all — the temp file leaked
    silently.

The later finally: tmp_path.unlink(...) only runs once installation has
started, i.e. after a successful download — it never covers these two
paths.

Fix

Factored the existing unlink-with-warn logic into a small
_cleanup_download_tmp_path() helper, and added a except BaseException
handler alongside the existing except typer.Exit and except Exception
handlers so any exit after the temp file is created — error, clean exit,
or interrupt — unlinks it.

Testing

Added test_add_from_url_interrupt_during_read_leaves_no_temp_file in
tests/test_workflows.py, which simulates a KeyboardInterrupt raised
mid-read and asserts no temp file is left behind.

Confirmed the new test fails without the fix (git checkout HEAD~1 -- src/specify_cli/workflows/_commands.py, rerun, restore):

FAILED tests/test_workflows.py::TestWorkflowCliAlignment::test_add_from_url_interrupt_during_read_leaves_no_temp_file
E   AssertionError: leaked temp files: [PosixPath('.../scratch-tmp/tmpmxi012rd.yml')]

With the fix:

tests/test_workflows.py::TestWorkflowCliAlignment::test_add_from_url_interrupt_during_read_leaves_no_temp_file PASSED
1 passed, 929 deselected in 2.78s

Full workflow suite:

.venv/bin/python -m pytest tests/test_workflows.py -q
929 passed, 1 skipped in 15.50s

Full test suite:

.venv/bin/python -m pytest tests -q
10 failed, 6921 passed, 9 skipped, 48 warnings in 467.78s

The 10 failures are pre-existing and unrelated to this change (template
composition / python-parity / terminal-width-dependent Rich formatting
tests in test_check_prerequisites_python_parity.py,
test_create_new_feature_python_parity.py, test_presets.py,
test_resolve_template_python_parity.py, test_setup_plan_python_parity.py,
test_setup_tasks_python_parity.py). Verified they fail identically on
this same environment with the fix reverted (unmodified tree), so they
are environment-specific and not caused by this change.

Lint:

uvx ruff@0.15.0 check src tests
All checks passed!

Agent config consistency:

.venv/bin/python -m pytest tests/test_agent_config_consistency.py -q
28 passed

AI Disclosure

This PR was written by an autonomous AI coding agent (Claude, Anthropic)
acting on behalf of the repository owner. The agent read the issue,
identified the missing BaseException/typer.Exit cleanup paths,
implemented the minimal fix, added a regression test, verified the test
fails without the fix and passes with it, and ran the project's lint and
test suites as described above.

`specify workflow add --from <url>` creates a delete=False temp file
before streaming the response body into it. The except clauses around
that read only handled typer.Exit (re-raise, no cleanup) and Exception
(cleanup + re-raise). KeyboardInterrupt is a BaseException, so Ctrl+C
during the size-limited read skipped both and left the file behind in
the system temp directory.

Adds a shared cleanup helper and a BaseException handler so any exit
path after the temp file is created -- error, typer.Exit, or interrupt
-- unlinks it, matching the existing best-effort cleanup on other
download errors.

Assisted-by: Claude Sonnet 5 (autonomous)
@chelsealong
chelsealong requested a review from mnriem as a code owner August 15, 2026 10:11
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.

[Bug]: Workflow URL download leaves a temp file after interrupt

1 participant