fix(bundler): re-read the step registry when rolling back a failed step refresh - #4139
Open
jawwad-ali wants to merge 1 commit into
Open
fix(bundler): re-read the step registry when rolling back a failed step refresh#4139jawwad-ali wants to merge 1 commit into
jawwad-ali wants to merge 1 commit into
Conversation
…fresh
`_StepKindManager.refresh` documents that it keeps a backup and restores it
"if the remove+reinstall path fails". The package half of that rollback
works; the registry half was unreachable.
`StepRegistry.__init__` snapshots the file once (`self.data = self._load()`)
and `is_installed` consults only that snapshot. Measured:
snapshot at construction: is_installed('my-step') = True
after the entry is deleted on disk: same object = True <-- stale
a fresh StepRegistry: = False
By rollback time `self.remove()` has already deleted the entry from disk,
but `self._registry`'s snapshot still contains it — so
`not self._registry.is_installed(...)` was always False and the restore
never ran, in exactly the failure case it was written for.
The user was left with the step package back on disk but unregistered:
`workflow step list` no longer shows it, the engine cannot resolve it, and
a later `workflow step add <id>` refuses with "Step directory already
exists".
Read the registry fresh at rollback time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
_StepKindManager.refreshdocuments its own contract:The package half of that rollback works. The registry half is dead code:
StepRegistry.__init__snapshots the file once (self.data = self._load()) andis_installedreads only that snapshot.Reproduction on current
main(bf88c9f)By the time the rollback runs,
self.remove(component)has already deleted the entry from disk viaworkflow_step_remove— butself._registry's snapshot still contains it. Sonot self._registry.is_installed(...)is always False, and the restore never executes, in precisely the failure case it exists for.Why it matters
After a failed
specify bundle update, the user is left in a broken half-state: the step package is back on disk but unregistered.specify workflow step listno longer shows itspecify workflow step add <id>refuses with "Step directory already exists"…leaving them to clean up by hand.
Fix
Read the registry fresh at rollback time, so
is_installedreflects whatremove()actually did. The lazy import matches this class's own style —__init__importsStepRegistrythe same way.No breaking change. Nothing on any success path is touched; only the already-failing rollback branch changes, and it changes from silently doing nothing to doing what it says.
Verification
src(assert StepRegistry(tmp_path).is_installed("my-step")with the on-disk registry showing"steps": {}) and passes with the fix — 1 failed → 21 passed.type_keymismatch produces.tests/unit: no new failures vs a clean-mainbaseline captured onbf88c9f9.uvx ruff@0.15.0 check src tests→ cleanWritten with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main.