Skip to content

[Bug]: Bundle step refresh rollback is not transactional with step operations #4815

Description

@mnriem

Bug Description

Bundle step refresh in src/specify_cli/bundles/primitives.py snapshots, removes, and restores an installed custom step without treating the rollback sequence as one transaction relative to other step operations.

StepRegistry.save() uses atomic file replacement, which prevents a partially written registry but does not prevent a stale in-memory registry snapshot from overwriting entries committed concurrently. After #4769, step add and remove use the shared step lock, but bundle refresh still reads and restores state outside that locking protocol.

The same rollback path can also mask the original reinstall failure if copying the backup or saving the restored registry entry fails.

Steps to Reproduce

The races should be reproduced with deterministic synchronization rather than timing sleeps:

  1. Install a custom step and begin a bundle refresh of it.
  2. Pause the refresh after it snapshots the package and registry metadata but before removal.
  3. Concurrently force-install a newer version of the same step.
  4. Resume the refresh and make its reinstall fail.
  5. Observe that the refresh can remove the newer package and restore the stale backup and metadata.

A second synchronized regression can pause rollback after a fresh StepRegistry snapshot is loaded, commit another step from a concurrent operation, then resume save(). The stale snapshot can discard the concurrently committed registry entry.

For the diagnostic failure, make backup restoration or StepRegistry.save() fail after the reinstall has already raised BundlerError.

Expected Behavior

  • Snapshot and removal are atomic relative to other step directory and registry mutations.
  • Rollback never removes or overwrites a package or registry entry committed by a later operation.
  • Restoring one entry cannot discard concurrently committed registry entries.
  • If restoration fails, the original reinstall error remains the primary error and the restoration failure is retained as secondary diagnostic context.

Actual Behavior

  • Snapshot and backup occur before the locked removal operation.
  • Rollback copies the backup and reloads, mutates, and saves the registry outside the step lock.
  • A stale rollback can overwrite newer package state or unrelated concurrent registry changes.
  • A restoration exception can replace the original reinstall error; cleanup then removes the backup.

Specify CLI Version

Current main; also present in #4769 at 996981d4 because this pre-existing code was intentionally reverted from that PR for separate handling.

AI Agent

GitHub Copilot

Operating System

Not operating-system-specific.

Python Version

All supported Python versions.

Error Logs

N/A — this is a deterministic concurrency and error-preservation defect.

Additional Context

Related review findings:

The existing rollback regression verifies ordinary serial restoration, but does not cover concurrent step mutations or restoration failure. This issue should require positive and negative deterministic regression tests that fail before the fix and pass afterward.

The separate cross-cutting local-directory hardening and extension/preset locking work is already tracked by #4793.

AI Disclosure

Drafted and filed by GitHub Copilot CLI using GPT-5.6 Sol in interactive mode on behalf of @mnriem. AI assistance covered review of the linked findings and affected code, duplicate-issue research, and drafting and filing this issue.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions