Skip to content

fix(extensions): treat an unreadable staged backup as a conflict - #3962

Merged
mnriem merged 1 commit into
github:mainfrom
marcelsafin:fix/rescue-staging-read
Aug 4, 2026
Merged

fix(extensions): treat an unreadable staged backup as a conflict#3962
mnriem merged 1 commit into
github:mainfrom
marcelsafin:fix/rescue-staging-read

Conversation

@marcelsafin

Copy link
Copy Markdown
Contributor

Description

The rescue-retry loop in install_from_directory() reads each staged backup with bare stat()/read_bytes() calls, so a staged config that cannot be read crashes the reinstall with a raw OSError. Every sibling read in this path — the live twin four lines below, the packaged baseline check, the mode sidecar — already catches OSError.

Fix: treat an unreadable staged file like an uncomparable live config: add it to the conflict set so both copies are preserved and the retry aborts with the existing resolution guidance while dest_dir is still untouched.

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest (6,310 passed, 176 skipped)
  • New regression test test_retry_with_unreadable_staged_config_aborts_and_preserves_both (fails on main with raw PermissionError, passes with fix)
  • ruff check src tests clean

AI Disclosure

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

Implemented autonomously by GitHub Copilot CLI (model: Claude Fable 5) under human direction; TDD (failing test first), full suite and lint verified locally. Commit includes Assisted-by/Co-authored-by trailers.

The rescue-retry loop in install_from_directory() reads each staged
backup with bare stat()/read_bytes() calls, so a staged config that
cannot be read crashed the reinstall with a raw OSError. Every sibling
read in this path — the live twin four lines below, the packaged
baseline check, the mode sidecar — already catches OSError.

Treat an unreadable staged file like an uncomparable live config:
add it to the conflict set so both copies are preserved and the retry
aborts with the existing resolution guidance while dest_dir is still
untouched.

Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 3, 2026 19:56
@marcelsafin
marcelsafin requested a review from mnriem as a code owner August 3, 2026 19:56

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 staged extension backups safely during reinstall retries.

Changes:

  • Converts staged-file OSError failures into preserved-config conflicts.
  • Adds regression coverage verifying both copies remain intact.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/specify_cli/extensions/__init__.py Handles staged backup read/stat failures.
tests/test_extensions.py Tests unreadable staged backup recovery.

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

@mnriem
mnriem requested a balanced review from Copilot August 4, 2026 13:08

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.

Review details

Suppressed comments (1)

src/specify_cli/extensions/init.py:2106

  • The stat() handler is reached only after _recognized_config_names(rescue_staging_dir) has already called entry.is_file() on the same staged path (lines 2014–2021). On supported Python 3.11–3.13, a PermissionError/other non-missing-path OSError from that implicit stat propagates there, so the retry still crashes before this new try; on runtimes that suppress the error, the name is omitted and the rescued config can be treated as absent. Please make the staged-name scan report matching entries whose metadata cannot be read as conflicts (and add a Path.stat regression case), rather than relying on this later stat boundary.
                try:
                    staged_stat = staged_file.stat()
                    staged_bytes = staged_file.read_bytes()
                except OSError:
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@mnriem
mnriem merged commit f245c6c into github:main Aug 4, 2026
14 checks passed
@mnriem

mnriem commented Aug 4, 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