You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Implements #4695 only. This is the scoped successor to #4757 (and #4754), based directly on current main. It intentionally excludes the unrelated workflow-composition implementation from #4680.
Adds workflow step add --dev <directory> and --from <archive-url> with shared package validation, provenance, default-deny trust confirmation, and --force handling.
Makes step registry updates atomic, validates persisted metadata, and hardens staging/publish/cleanup behavior.
Serializes step registry and directory mutations with a per-project file lock (.specify/.step-install.lock). step add and step remove both take it, so concurrent installs and removals cannot lose or resurrect registry entries. Other pre-existing step-registry writers, such as the bundle step-refresh rollback, are out of scope (see Deferred).
Bounds package traversal: --dev validation and staging are iterative, with a 512-entry budget (directories included) and a nesting-depth limit of 32.
Refreshes project-local custom step modules, documents package transport and loading behavior, and adds regression coverage.
Changes made while rescouting this branch (relative to #4757):
Corrected the forced-reinstall registry-failure message, which incorrectly claimed the step was "not registered" even when the previous entry is retained.
Added deterministic concurrency regression tests proving the install lock serializes overlapping installs and that the second install reloads committed registry state.
Changes from review rounds on this PR:
step remove now runs under the same step lock as installs, and reads the registry and step directory only after acquiring it (review thread r4133293814). The earlier "install lock covers installs only" scope no longer applies.
The workflow and step install locks share one helper, shared_infra._exclusive_project_lock, while keeping separate lock files (.workflow-install.lock, .step-install.lock), so workflow and step installs do not block each other. Only lock-acquisition failures are reported as lock errors.
A step lock failure is reported once. The shared helper's message is operation-neutral (Failed to acquire the step lock: ...), and step remove shows the underlying error after its own Failed to lock step removal '<id>' prefix instead of nesting the helper's message (review thread r4146392458). The helper's inner lock context is step rather than step install, so a symlinked-lock rejection during removal doesn't describe the lock as an install lock (r4146949727).
Evidence
Regression coverage exercises YAML-native metadata rejection, atomic registry serialization failure, staged metadata changes, archive declaration mismatch after redirects, Rich markup package names, runtime refresh between projects, and bundle step delegation.
Lock coverage:
Install vs. install: test_install_lock_blocks_concurrent_duplicate and test_install_lock_serializes_force_replace (synchronized threads); neutralizing the lock makes them fail.
Install vs. remove: test_remove_waiting_on_install_does_not_resurrect_entry and test_install_waiting_on_remove_is_not_unregistered (synchronized threads). Against the previous unlocked remove, the first ends with the removed entry back in the registry and the second fails on the registry write.
Remove failure paths: test_remove_fails_cleanly_when_lock_cannot_be_acquired and test_remove_restores_registry_entry_when_directory_delete_fails.
Lock-failure messages: test_remove_fails_cleanly_when_lock_cannot_be_acquired and test_dev_lock_failure_is_reported_once_without_installing assert a single lock message for remove and add; both fail against the previous doubled wording. test_remove_symlinked_lock_error_uses_neutral_lock_wording fails against the old step install lock context.
Shared helper (tests/test_shared_infra_lock.py): a second holder blocks until release, release on exception, symlinked lock file / .specify rejection, private lock-file permissions.
Traversal limits: depth boundary for validation and copy, default limit rejecting a 33-level tree, and directories counting toward the entry budget.
Validation
Run on Linux at 996981d4 (markdownlint last run at f7c4f1b7; docs unchanged since):
The recursive-copy symlink race raised in the feat(workflows): install custom step types from local dirs and archives #4757 review is cross-cutting (the sibling workflow/preset/extension local installers share or exceed it, and there is no shared safe-copy primitive for live directories). It is deferred to a dedicated proposal for a shared, fd-relative local-install primitive rather than being patched only here, as agreed with the maintainer.
Extending the shared project lock to extension and preset installs belongs with that same proposal.
The bundle step-refresh rollback in bundles/primitives.py writes the step registry without the step lock. This code is unchanged by this PR and behaves the same on main, where no step operation is locked. Making bundle refresh atomic with respect to step operations (snapshot, removal, and restore, plus not masking the reinstall error when the restore fails) is proposed as a separate change (review threads r4147053376, r4147291538, r4147291622). A lock on the restore alone was tried in c7f8a513 and reverted in 996981d4 to keep this PR scoped to [Feature]: Add local (--dev) installation for custom workflow step types #4695.
AI Disclosure
Implementation was generated with OpenCode (models: deepseek-v4.1-flash for the original feature, gpt-5.6-terra for remediation, and deepseek-v4.1-flash for the rescoping), autonomous mode. Later review rounds were generated with GitHub Copilot CLI (models: unknown for the traversal-limit round, Claude Opus 5.5 for the removal-lock round), autonomous after @markuswondrak approved each plan. Subsequent rounds, including the lock-message fix in a0f3c1cf, the bundle-rollback lock in c7f8a513, and its partial revert in 996981d4, were generated with OpenCode (model: Claude Opus 5.5), autonomous after @markuswondrak approved each plan. This PR description was last updated by OpenCode (model: Claude Opus 5.5, autonomous, at @markuswondrak's request) after the bundle-rollback revert; the pytest and ruff validation above was re-run by the agent. Commit f79dcc29 (lock context string) was applied by @markuswondrak from a GitHub Copilot Autofix suggestion. The AI authored code, tests, documentation, commits, and this PR description on behalf of @markuswondrak.
…es (github#4695)
`specify workflow step add` gains `--dev <directory>` and `--from <archive-url>` alongside the existing catalog source. All three converge on a new `step/installer.py` domain module that owns package validation (shape, symlink/special-file rejection, 512-file/50 MiB limits), same-filesystem staging with revalidation, atomic commit, `--force` replacement, and source-kind-only registry provenance. Direct URLs require a default-deny trust prompt before any request.
Docs document the local-authoring flow and the deferred bundle-local limitation.
Assisted-by: opencode (model: deepseek-v4.1-flash, autonomous)
Adds local-directory and archive-URL installation for custom workflow step types with shared validation, provenance, locking, and atomic registry persistence.
Changes:
Adds --dev, --from, and --force installation flows.
Refreshes project-local custom-step modules between projects.
Documents package behavior and adds extensive regression coverage.
Case-folded IDs (--force): reject collisions against registered IDs and distinct on-disk directory names; exact-ID replacement remains allowed. Added guard and force regression tests.
Temporary-directory allocation: convert catalog and archive temp-dir creation failures to StepInstallError.
Streaming byte limit: copy in bounded chunks against a shared remaining package budget; abort as soon as it is exceeded. Added source-growth and exact-limit tests.
Cleanup diagnostics: installer staging cleanup now reports the residual path, warning after commit and preserving an active primary error. The archive and catalog temp-directory paths in command_add.py each have source-specific cleanup handling; they intentionally remain separate implementations for now, but share similar warning/path/primary-error behavior and should stay aligned if either changes.
Force documentation: distinguish failure to remove the old package from publication/registry failures that occur after removal.
AI-generated review-round summary on behalf of @markuswondrak. Agent: OpenCode, model: deepseek-v4.1-flash, autonomous. Extent: implemented the fixes and regression tests, ran validation, and drafted this summary.
StepInstallError messages include user-controlled local paths, URLs, catalog paths, and step IDs, but this renders the exception as Rich markup. A failed --dev path containing [/], for example, is parsed as a closing tag and can turn the intended user-facing validation error into a Rich MarkupError. Escape the exception at this common rendering boundary (and keep presentation markup out of domain error strings) so all failure paths remain safe.
Add Windows coverage for concurrent install locking
These are the only regression tests that prove overlapping installs serialize, but both skip on Windows while the new msvcrt.locking implementation has distinct behavior. The repository runs pytest on windows-latest, so the Windows critical section currently has no concurrent positive/negative coverage. Add an equivalent synchronized Windows test (or make the helper exercise the platform lock abstraction) to verify the second installer blocks and reloads committed registry state.
Review-round update — commit a78b752\n\nAddressed the remaining installer findings: reject case-folded orphan-directory collisions (including on case-insensitive filesystems), detect archive format from bounded bytes when URL and Content-Type provide no hint, escape installer errors at CLI output boundaries, and run the synchronized install-lock tests through the Windows locking primitive as well.\n\nValidation: .venv/bin/python -m pytest tests/specify_cli/workflows/step -q (165 passed); uv run ruff check on the changed files passed; git diff --check passed. Windows-specific execution is covered by CI but was not run in this Linux environment.\n\nAI disclosure: This review-round update and its code/tests were generated on behalf of @markuswondrak by OpenCode using github-copilot/gpt-6-luna in autonomous mode. Extent: implementation, regression tests, validation, and drafting this summary.
A sufficiently deep local package tree raises an uncaught RecursionError here instead of StepInstallError. Empty directories do not count toward _MAX_STEP_PACKAGE_FILES, so on POSIX a package can contain roughly 1,000 one-character nested directories within the path-length limit; _walk() then exceeds Python's recursion limit, and _copy() below has the same failure mode if the tree changes after validation. Use iterative traversal (and preferably a directory/depth budget) in both validation and staging so malformed --dev input fails cleanly.
@mnriem : Regarding the deep-tree review finding: local package installation follows very similar copy/traversal patterns across steps, presets, extensions, and workflow packages, but each currently implements them separately.
Does it make sense to fix this only in the step installer here, or would you prefer a separate issue to consolidate and harden local package installation more generally first?
Please address Copilot feedback. And on your question I prefer to first land this so we have it and then yes a refactoring along those lines is appreciated after we scope it properly
Replace recursive package validation and staging walks with iterative
traversal so deeply nested --dev trees fail with StepInstallError instead
of RecursionError. Enforce a 32-level directory depth limit and count
directories toward the 512-entry package budget in both validation and
copy, covering trees that change after validation.
Assisted-by: GitHub Copilot CLI (model: unknown, Auto mode, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The installer operates on resolved step paths, so on macOS (where the temp
directory lives under the /var -> /private/var symlink) the rmtree and
os.replace mocks never matched the unresolved target and the expected
StepInstallError was not raised. Compare resolved paths instead.
Assisted-by: GitHub Copilot CLI (model: unknown, Auto mode, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Traversal:_walk_package_tree (validation) and _copy_package_tree (staging) now loop instead of calling themselves. A deeply nested --dev tree fails with StepInstallError instead of RecursionError. File order is unchanged.
Depth budget: new _MAX_STEP_PACKAGE_DEPTH = 32, enforced in both validation and staging, so a tree that grows deeper after validation is also rejected.
Entry budget: directories now count toward the 512-entry package budget in both places, so wide trees of empty directories are bounded too. The error message still reports the existing 512-file limit.
Docs:docs/reference/workflows.md now lists the entry and depth limits.
2c612fee: CI fix for the force-reinstall failure tests
test_force_removal_failure_warns_reinstall and test_force_publication_failure_warns_reinstall failed with DID NOT RAISE in CI. The problem was in the tests, not the installer.
The installer operates on resolved paths, while the rmtree and os.replace mocks compared against the unresolved target. On macOS the temp directory sits under the /var → /private/var symlink, so the mocks never matched.
The mocks now compare resolved paths.
Evidence
Five new tests in tests/specify_cli/workflows/step/test_installer.py cover:
the depth limit at its boundary, for both validation and copy;
the default limit rejecting a 33-level tree cleanly;
directories counting toward the entry budget, for both validation and copy.
All five fail against the previous installer.py and pass with cf42dcc1.
The CI failure was reproduced locally by pointing TEMP at a directory junction: 2 failed before 2c612fee, and all force_ tests passed after.
The 11 failures are symlink-creation tests. They fail the same way on the parent commit because this Windows environment lacks the permission to create symlinks (WinError 1314); they are unrelated to these changes.
The full CI matrix has not been re-run yet, and Ruff was not run in this environment.
AI disclosure: posted on behalf of @markuswondrak. The investigation, fixes, tests, docs update, commits, and this comment were generated by GitHub Copilot CLI in Auto mode (model: unknown), acting autonomously after @markuswondrak approved the plan (depth limit of 32, and counting directories toward the entry limit). No line-by-line human review was done before commit.
Use immutable built-in step types in workflow commands
src/specify_cli/workflows/__init__.py:81
The project refresh is still incomplete for workflow step list and workflow step info: those commands classify built-ins from the mutable STEP_REGISTRY (command_list.py:26, command_info.py:41) and do not call load_custom_steps. After project A loads a custom type, invoking either command for project B in the same process can therefore report A's stale type as built-in. Switch those checks to BUILTIN_STEP_TYPES and add a cross-project command regression.
Preserve source file permissions when staging packages
This copy strips every source file's permission bits: target.open("xb") creates a mode based on the process umask, while expected_mode is used only to check the file type. A --dev package with a private data file becomes more broadly readable, and an executable helper loses its execute bit after installation. Preserve stat.S_IMODE(opened.st_mode) on the staged file (with portable handling) and cover restrictive/executable modes in a regression test.
Call the budget an entry limit instead of a file limit: fixed. The validation, staging and catalog-preflight errors now report an N-entry limit (files and directories combined), and the test assertions are updated. With the old messages, the updated assertions fail in 7 tests. With the new wording, tests/specify_cli/workflows tests/specify_cli/bundles tests/test_workflows.py gives 1602 passed, 1 skipped, and ruff is clean.
Posted on behalf of @markuswondrak. This change and this comment were generated by OpenCode (model: Claude Opus 5.5) in autonomous mode, after @markuswondrak approved the plan. The AI wrote the code, tests, commit and this comment. No line-by-line human review was done before the commit.
On a case-insensitive filesystem, registry lookup remains case-sensitive. Removing foo when Foo is registered can therefore treat the same on-disk directory as an orphan, delete it, and leave the Foo registry entry behind. Reject a case-folded registry match unless the exact ID was supplied (as the installer already does), and add a regression covering wrong-case removal.
Commit:f2a02847006c24009501011d20195d03222f0759 (no code changes this round)
This round responds to the Copilot review at f2a02847. Both findings describe behaviour that already exists on main and that this PR doesn't introduce, so both are declined here and proposed as separate bug issues:
Prevent case-insensitive registry mismatches from deleting valid directories: the orphan-removal decision in step remove is unchanged from main. This PR only moved it under the install lock and escaped its output. On a case-insensitive filesystem, step remove foo with Foo registered treats the shared directory as an orphan, deletes it, and leaves the Foo entry behind. Proposed fix for a separate issue: reject a case-folded registry or directory match unless the exact ID was supplied, as step add already does.
I'm happy to open both issues, or to fold the step remove fix into this PR if that's preferred.
Posted on behalf of @markuswondrak. This comment was generated by OpenCode (model: Claude Opus 5.5) in autonomous mode, after @markuswondrak approved declining both findings. No code was changed this round.
`step remove` shares the step lock with `step add`, but the shared helper
labelled every acquisition failure as "Failed to lock step installation".
`step remove` then added its own prefix, printing "Failed to lock step
removal '<id>': Failed to lock step installation: <error>".
- Make the helper's message operation-neutral ("Failed to acquire the
step lock"), since install and remove both use it.
- In `step remove`, report the underlying acquisition error after the
removal prefix instead of the helper's message.
- Assert a single lock message on the remove path, and add a `step add
--dev` lock-failure regression. Both fail against the previous wording.
Assisted-by: OpenCode (model: Claude Opus 5.5, autonomous)
Avoid double-prefixing removal lock failure message (r4146392458): fixed. The shared step lock helper now raises an operation-neutral Failed to acquire the step lock: <error>, since step add and step remove both use it. step remove prints the underlying acquisition error after its own prefix, so the output is Failed to lock step removal '<id>': <error>. The remove lock-failure test now asserts a single lock message, and a new step add --dev test covers the add path. Both fail against the previous wording.
Validation at a0f3c1cf: the step, custom-step, bundle and shared-lock tests listed in the PR description give 229 passed, and uvx ruff@0.15.0 check src tests passes. In the full suite, 3 tests in tests/test_github_workflows.py::test_community_checksum_command_rejects_invalid_digest fail on this machine with or without this change, because a German locale makes sha256sum print GESCHEITERT instead of FAILED. Everything else passes (8541 passed).
Posted on behalf of @markuswondrak. This change and this comment were generated by OpenCode (model: Claude Opus 5.5) in autonomous mode, after @markuswondrak approved the fix approach. The AI wrote the code, tests, commit, PR-description update and this comment. No line-by-line human review was done before the commit.
When a bundle step refresh removes a step and the reinstall fails, the
rollback reloaded `step-registry.json`, put the old entry back and saved,
all without the step lock that `step add` and `step remove` hold. A
concurrent step operation could commit between that reload and save, or
save a stale snapshot over the restored entry, and lose a registry entry.
- Run the rollback's directory and registry restore inside
`_step_install_transaction`, reloading the registry after the lock is
taken.
- Skip the restore if the step was registered again after the removal,
so a newer package and entry are not overwritten by the stale backup.
- If the lock can't be acquired, leave the project untouched and add a
note to the reinstall error instead of restoring unlocked.
- Add regressions for a concurrent writer holding the lock during the
rollback, a concurrent reinstall of the same step, and lock failure;
all three fail against the previous rollback. Also cover `step remove`
with a symlinked lock file, which fails against the old `step install`
lock context.
Assisted-by: OpenCode (model: Claude Opus 5.5, autonomous)
Lock registry rollback to prevent overwriting concurrent installs (r4147053376): fixed. The rollback in bundles/primitives.py is unchanged from main, but it was the one step-registry writer left outside the lock this PR introduces. When a bundle step refresh fails its reinstall, the directory and registry restore now run inside _step_install_transaction, with the registry reloaded after the lock is taken. The restore is skipped if the step was registered again after the removal, so a newer package isn't overwritten by the stale backup. If the lock can't be acquired, the project is left untouched and the reinstall error gets a note saying the step was not restored. Three new regressions cover a concurrent writer holding the lock during the rollback, a concurrent reinstall of the same step, and lock failure; all three fail against the previous rollback.
Use operation-neutral lock context for step removal errors (r4146949727): the context change landed in f79dcc29. This round adds the requested coverage: test_remove_symlinked_lock_error_uses_neutral_lock_wording checks that a symlinked lock file during step remove is reported as Refusing to use symlinked step lock, and it fails against the old step install context.
Validation at c7f8a513: the test set listed in the PR description gives 233 passed, and uvx ruff@0.15.0 check src tests passes. The full suite gives 8545 passed; the 3 test_community_checksum_command_rejects_invalid_digest failures come from this machine's German locale and pass with LC_ALL=C.UTF-8.
Posted on behalf of @markuswondrak. This change and this comment were generated by OpenCode (model: Claude Opus 5.5) in autonomous mode, after @markuswondrak approved the fix. The AI wrote the code, tests, commit, PR-description update and this comment. No line-by-line human review was done before the commit.
Reverts the `bundles/primitives.py` part of c7f8a51 and its three
rollback tests. The step refresh rollback is pre-existing code that this
PR did not otherwise touch; locking it pulled the bundle refresh flow into
review scope, and further findings there (snapshot and removal before the
lock, restore failures masking the reinstall error) are also pre-existing
on `main`. It belongs in a separate change for bundle refresh atomicity.
The `step remove` symlinked-lock test from c7f8a51 is kept: it covers the
step lock and removal locking that this PR adds.
Assisted-by: OpenCode (model: Claude Opus 5.5, autonomous)
This round narrows the PR back to #4695. The previous round's lock on the bundle step-refresh rollback (c7f8a513) is reverted, because it changed pre-existing bundle code and brought the rest of the refresh flow into review.
Lock registry rollback to prevent overwriting concurrent installs (r4147053376): declined for this PR. The rollback in bundles/primitives.py is unchanged from main, where no step operation is locked. src/specify_cli/bundles is now identical to the PR's base.
Acquire rollback lock before snapshot and package removal (r4147291538) and Preserve reinstall error when rollback restoration fails (r4147291622): declined. Both describe pre-existing refresh behaviour and are anchored on the reverted code.
The PR description now limits the lock guarantee to step add and step remove, and lists bundle refresh atomicity under Deferred as a separate change. I'm happy to open an issue for it.
Kept from c7f8a513: test_remove_symlinked_lock_error_uses_neutral_lock_wording, which covers the step lock this PR adds (r4146949727).
Validation at 996981d4: the test set listed in the PR description gives 230 passed, tests/specify_cli/bundles passes, and uvx ruff@0.15.0 check src tests passes.
Posted on behalf of @markuswondrak. The revert and this comment were generated by OpenCode (model: Claude Opus 5.5) in autonomous mode, after @markuswondrak decided to revert and decline. The AI wrote the revert commit, the PR-description update, the inline replies and this comment. No line-by-line human review was done before the commit.
@mnriem all review findings are either fixed, or argued against.
This is the summary of the deffered ones and their arguments against fixing:
1. Bundle step-refresh rollback is not serialized with the step lock bundles/primitives.py reloads, mutates, and save()s the step registry without
taking the step lock. This code is unchanged by this PR and behaves identically on main. Reverted from this PR in 996981d4; proposed as a separate change.
2. Recursive-copy symlink TOCTOU race in local package install
Cross-cutting: the sibling workflow/preset/extension local installers share or
exceed it, and there is no shared safe-copy primitive. Deferred to a dedicated
proposal for a shared, fd-relative local-install primitive (agreed with the maintainer).
3. Extending the shared project lock to extension and preset installs
Belongs with the same shared-install-primitive proposal as (2); no separate thread.
This branch has not been deployed
No deployments
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
triage-can-waitVerdict: valid and in-scope but deprioritized; held behind the evidence gate
3 participants
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.
mnriem
added
the
triage-can-wait
Summary
Implements #4695 only. This is the scoped successor to #4757 (and #4754), based directly on current
main. It intentionally excludes the unrelated workflow-composition implementation from #4680.workflow step add --dev <directory>and--from <archive-url>with shared package validation, provenance, default-deny trust confirmation, and--forcehandling..specify/.step-install.lock).step addandstep removeboth take it, so concurrent installs and removals cannot lose or resurrect registry entries. Other pre-existing step-registry writers, such as the bundle step-refresh rollback, are out of scope (see Deferred).--devvalidation and staging are iterative, with a 512-entry budget (directories included) and a nesting-depth limit of 32.Changes made while rescouting this branch (relative to #4757):
step removerefactor. Removal keeps its pre-existing in-place delete + registry rollback, so no staged-removal directory is created that the custom-step loader could re-discover.workflowstep is registered here), leaving only the custom-step package reference.Changes from review rounds on this PR:
step removenow runs under the same step lock as installs, and reads the registry and step directory only after acquiring it (review thread r4133293814). The earlier "install lock covers installs only" scope no longer applies.shared_infra._exclusive_project_lock, while keeping separate lock files (.workflow-install.lock,.step-install.lock), so workflow and step installs do not block each other. Only lock-acquisition failures are reported as lock errors.Failed to acquire the step lock: ...), andstep removeshows the underlying error after its ownFailed to lock step removal '<id>'prefix instead of nesting the helper's message (review thread r4146392458). The helper's inner lock context issteprather thanstep install, so a symlinked-lock rejection during removal doesn't describe the lock as an install lock (r4146949727).Evidence
Regression coverage exercises YAML-native metadata rejection, atomic registry serialization failure, staged metadata changes, archive declaration mismatch after redirects, Rich markup package names, runtime refresh between projects, and bundle step delegation.
Lock coverage:
test_install_lock_blocks_concurrent_duplicateandtest_install_lock_serializes_force_replace(synchronized threads); neutralizing the lock makes them fail.test_remove_waiting_on_install_does_not_resurrect_entryandtest_install_waiting_on_remove_is_not_unregistered(synchronized threads). Against the previous unlocked remove, the first ends with the removed entry back in the registry and the second fails on the registry write.test_remove_fails_cleanly_when_lock_cannot_be_acquiredandtest_remove_restores_registry_entry_when_directory_delete_fails.test_remove_fails_cleanly_when_lock_cannot_be_acquiredandtest_dev_lock_failure_is_reported_once_without_installingassert a single lock message for remove and add; both fail against the previous doubled wording.test_remove_symlinked_lock_error_uses_neutral_lock_wordingfails against the oldstep installlock context.tests/test_shared_infra_lock.py): a second holder blocks until release, release on exception, symlinked lock file /.specifyrejection, private lock-file permissions.Traversal limits: depth boundary for validation and copy, default limit rejecting a 33-level tree, and directories counting toward the entry budget.
Validation
Run on Linux at
996981d4(markdownlint last run atf7c4f1b7; docs unchanged since):.venv/bin/python -m pytest tests/specify_cli/workflows/step tests/specify_cli/workflows/test_custom_steps.py tests/specify_cli/bundles/test_primitives.py tests/specify_cli/bundles/test_references.py tests/test_shared_infra_lock.py -q230 passeduvx ruff@0.15.0 check src tests(CI gate): all checks passed.markdownlint-cli2 docs/reference/workflows.md docs/reference/bundles.md: 0 issues.Deferred
workflow/preset/extensionlocal installers share or exceed it, and there is no shared safe-copy primitive for live directories). It is deferred to a dedicated proposal for a shared, fd-relative local-install primitive rather than being patched only here, as agreed with the maintainer.bundles/primitives.pywrites the step registry without the step lock. This code is unchanged by this PR and behaves the same onmain, where no step operation is locked. Making bundle refresh atomic with respect to step operations (snapshot, removal, and restore, plus not masking the reinstall error when the restore fails) is proposed as a separate change (review threads r4147053376, r4147291538, r4147291622). A lock on the restore alone was tried inc7f8a513and reverted in996981d4to keep this PR scoped to [Feature]: Add local (--dev) installation for custom workflow step types #4695.AI Disclosure
Implementation was generated with OpenCode (models:
deepseek-v4.1-flashfor the original feature,gpt-5.6-terrafor remediation, anddeepseek-v4.1-flashfor the rescoping), autonomous mode. Later review rounds were generated with GitHub Copilot CLI (models: unknown for the traversal-limit round, Claude Opus 5.5 for the removal-lock round), autonomous after @markuswondrak approved each plan. Subsequent rounds, including the lock-message fix ina0f3c1cf, the bundle-rollback lock inc7f8a513, and its partial revert in996981d4, were generated with OpenCode (model: Claude Opus 5.5), autonomous after @markuswondrak approved each plan. This PR description was last updated by OpenCode (model: Claude Opus 5.5, autonomous, at @markuswondrak's request) after the bundle-rollback revert; the pytest and ruff validation above was re-run by the agent. Commitf79dcc29(lock context string) was applied by @markuswondrak from a GitHub Copilot Autofix suggestion. The AI authored code, tests, documentation, commits, and this PR description on behalf of @markuswondrak.