fix: harden bundle and workflow step integrity

[marcelsafin]

Description

Consolidate five related bundle and workflow-step integrity fixes at maintainer
request, with follow-up coverage for partial failures and catalog changes:

• Scope custom step loading, validation, and execution to the requested project;
  use the fixed built-in type set for metadata commands and collision checks.
  Concurrent operations use private registries and instance-owned package
  namespaces, including recursive validation and later relative imports.
  Invalidate source-derived bytecode paths, including configured external
  caches; refuse packages whose caches cannot be safely invalidated.
• Reject duplicate bundle components within a kind while allowing identical IDs
  across different kinds.
• Reject downloaded step packages with missing or mismatched advertised versions.
• Restore local component payloads, exact registry metadata, disabled state,
  user configuration, and extension hooks when bundle update/removal or
  provenance saving fails. Recovery includes interrupted primitives, restores
  steps before dependent workflows, and reports incomplete rollback explicitly.
  Preserve generated command/skill artifacts across historical integrations,
  including aliases, supporting files, home-scoped outputs, and prior absence.
  Preserve the extension's native configuration backup preimage as well.
  Snapshot-cleanup filesystem errors warn with the retained temporary path
  instead of overriding committed success or the original failure.
• Bind bundle step pins to the same catalog/download/install transaction and
  compare the downloaded package directly against the expected version.

Rollback uses temporary local snapshots, not a catalog re-download; it is
best-effort, not a crash-recovery or whole-project atomicity guarantee.
This preserves the original fix commits and supersedes #4466, #4467, #4468,
and #4469. The explicit-refresh feature in #4477 remains separate.

Testing

• Tested CLI help with .venv/bin/specify --help
• Ran existing tests after uv sync --extra test, using the worktree's
  .venv/bin/python -m pytest tests -q --tb=short
• Tested with a sample project (if applicable)

Validation on 27ce84fede89479b2a5b1e7900546ddebc06e7ae, including current
upstream without rewriting reviewed commits:

• Full suite: 8,042 passed, 16 skipped, no deselections, with Python 3.13
  and real PowerShell 7.6.6 available.
• Failure-first regressions cover cross-project consumers, complete installed
  state, interrupted mutations, failed restoration, dependent workflows, and
  catalog/package races, concurrent loading and module lifetime, cache
  invalidation failures (including external caches), historical integration
  artifact restoration, exact extension backup preimages, and cleanup failures
  after committed success, failed capture, provenance failure, and incomplete
  rollback through both command paths.
• Local fixture commits use a test-process-only commit.gpgsign=false
  override, preserving other inherited Git settings; repository/global
  configuration files are unchanged.
• uvx ruff@0.15.0 check src tests
• uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r .github/security-audit-requirements.txt --progress-spinner off
• Markdown lint for the changed bundle documentation, CLI help for
  workflow step add, and git diff --check upstream/main...HEAD.
• Combined diff self-review; no claim of an independent current-head review.

AI Disclosure

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

GitHub Copilot (GPT-5.6 Sol) produced the initial fixes and consolidation.
GitHub Copilot (GPT-6 Astra) autonomously reproduced and repaired the follow-up
correctness findings, added regressions, and ran verification under
@marcelsafin's direction; the user explicitly approved commit and publication.

[Copilot]

🟢 Approval recommended

The reviewed changes have appropriate regression coverage and no unresolved issues.

Pull request overview

Enforces bundle-declared version pins when installing workflow steps.

Changes:

• Validates requested step versions against catalog metadata.
• Adds regression coverage ensuring mismatches prevent installation.

File summaries

File	Description
tests/unit/test_bundler_primitives.py	Tests rejection of mismatched step versions.
src/specify_cli/bundler/services/primitives.py	Adds step version-pin validation before installation.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[mnriem]

This is excellent work — you've filed a focused, well-tested set of bundle-integrity fixes (#4466–#4470: component scoping, duplicate rejection, step-version mismatch, record-save rollback, and version-pin enforcement), all green with regression coverage and clear disclosure. Genuinely high-value stuff.

One process note: that's 5 open PRs, past the 3-open-PR guidance in CONTRIBUTING, and since they're all the same theme touching the bundler, they'd be much faster to review as a single consolidated PR (or a smaller stack) rather than five separate ones. Could you group the related ones? I want to land these — consolidating just helps them move quicker. Marking this one author-awaiting on that.

[Copilot]

🟢 Approval recommended

The focused implementation correctly mirrors existing workflow validation and includes adequate regression coverage.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[marcelsafin]

Consolidated all five fixes here on 6a254b6b, preserving each reviewed change as a separate commit. The five targeted regression groups pass (139 tests); the full suite passes with 7,647 passed, 195 skipped, and the known no-pwsh launcher test deselected. Ruff 0.15.0, pinned dependency audit, and diff checks are clean. #4466-#4469 are closed as superseded. Posted on behalf of @marcelsafin by GitHub Copilot (GPT-5.6 Sol).

[Copilot]

🟡 Changes recommended

Direct registry consumers can still leak project A’s custom step types into project B without rescanning.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 13/13 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Rollback can misreport restoration, removal rollback remains incomplete, and project/version isolation has unresolved gaps.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/specify_cli/workflows/init.py:105

• Clearing stale entries only when load_custom_steps is called leaves other step commands project-unsafe in a long-lived process. After scanning project A, workflow_step_add in project B still checks the stale global registry and can reject A's custom ID as a built-in; workflow_step_list/info can similarly report it as built-in. Update built-in-only consumers to use BUILTIN_STEP_TYPES (and ensure project-dependent consumers load the current root).

    # Custom steps are project-scoped even though the registry and Python module
    # cache are process-global. Clear the previous project's classes and package
    # modules before every scan so removed or updated code cannot remain active.
    for _type_key in tuple(STEP_REGISTRY):
        if _type_key not in BUILTIN_STEP_TYPES:

• Files reviewed: 13/13 changed files
• Comments generated: 3
• Review effort level: Balanced

[mnriem]

Thanks @marcelsafin — solid direction and good to see CI green across the matrix. Before I do a full review of the rollback/registry logic, there are four open correctness threads that go to the heart of what this PR hardens; CI passes because these are partial-failure/TOCTOU paths the happy-path tests don't exercise:

1. workflows/__init__.py:105 — the global STEP_REGISTRY isn't fully re-scoped per project; workflow_step_list()/_add() read it directly, so after switching projects in a long-lived process, one project can see another's custom step as built-in. Use BUILTIN_STEP_TYPES for built-in checks and load/clear the registry on every project-dependent read.

2. installer.py:74 — remove_bundle removes components then saves provenance; on save failure the removals aren't rolled back (your test_remove_record_save_failure_reports_partial_state confirms it). Snapshot and reinstall on record-save failure.

3. primitives.py:158 — the rollback snapshot drops enabled, so a save-failure rollback can re-enable a disabled component while reporting success. Preserve and restore manager-specific state, not just ComponentRef.

4. primitives.py:469 — the pin check reads one StepCatalog while workflow_step_add resolves another; pass the expected version into the same download-and-validate transaction so the pin is bound to what actually installs.

Once those are closed, re-request a review and I'll take the full pass. Heads-up: this overlaps #4477 on the rollback path, so whichever lands first, the other will need a rebase to the non-overlapping hunks.

[marcelsafin]

Posted on behalf of @marcelsafin by GitHub Copilot (GPT-6 Astra). 09d3fe9b scopes step consumers per project, restores local payloads/metadata/hooks after failed updates or removals (including interrupted primitives and dependent workflow recovery), and binds bundle pins to the downloaded package. The upstream-synced head passes 7,928 tests with 16 skips and no deselections using real PowerShell; lint, audit, docs, and CLI gates pass, and #4477 remains separate.

[Copilot]

🟡 Changes recommended

Rollback omits historical integration artifacts, while custom-step loading remains vulnerable to concurrency and failed cache deletion.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 17/17 changed files
• Comments generated: 4
• Review effort level: Balanced

[marcelsafin]

Posted on behalf of @marcelsafin by GitHub Copilot (model: GPT-6 Astra, autonomous).

dbbc499f isolates each workflow operation's step registry and package lifetime, rejects unsafe cache invalidation, and restores actual command/skill preimages across historical integrations, including outputs that were previously absent. Failure-first regressions and the full local suite pass on this exact head (8,012 passed, 16 skipped, no deselections); current upstream is included without rewriting reviewed commits.

[Copilot]

🟡 Changes recommended

External bytecode caches can remain stale, and extension rollback leaves unintended backup state.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/specify_cli/bundler/services/primitives.py:365

• Calling the normal extension removal path during rollback creates .specify/extensions/.backup/<id> from the failed/new installation. The subsequent non-force install does not consume that directory (did_remove is false), so a successful rollback leaves backup state that did not exist before; after a failed refresh it can contain the replacement version's config files. Snapshot and restore (or clean up) this backup directory as part of the extension preimage so rollback is exact and stale config cannot survive.

• Files reviewed: 21/21 changed files
• Comments generated: 1
• Review effort level: Balanced

[marcelsafin]

Posted on behalf of @marcelsafin by GitHub Copilot (model: GPT-6 Astra, autonomous).

93cd4052 follows Python's source-to-cache mapping for invalidation and includes native extension backup state in the existing component preimage, preserving contents and absence without touching unrelated backups. Fourteen additional regressions cover both mechanisms and their I/O failures; the full local suite passes on this exact head (8,026 passed, 16 skipped, no deselections), with current upstream included.

[Copilot]

🔵 Needs a closer look

Snapshot cleanup failures can override committed success or mask the original rollback failure.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/specify_cli/bundler/services/installer.py:219

• Snapshot cleanup runs in finally after the provenance write has committed. If TemporaryDirectory.cleanup() raises (for example, due to a locked/read-only snapshot on Windows), this replaces a successful install/update with a raw exception; on an earlier failure it can also mask the original BundlerError and rollback status. Treat cleanup as post-transaction housekeeping (warn/report the leaked temp path without changing the operation result), and apply the same handling in remove_bundle.

This issue also appears on line 279 of the same file.

src/specify_cli/bundler/services/installer.py:280

• As in the install path, an exception from closing the temporary snapshots here occurs after save_records may already have committed the removal, or can mask the original removal/rollback error. Snapshot cleanup should be handled as non-transactional housekeeping with an explicit warning rather than changing the command outcome.

    finally:
        snapshots.close()

• Files reviewed: 21/21 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[marcelsafin]

Posted on behalf of @marcelsafin by GitHub Copilot (model: GPT-6 Astra, autonomous).

27ce84fe handles cleanup filesystem errors in the shared snapshot close path, reporting the retained temporary directory without overriding transaction outcomes, and uses that same handling when payload capture fails. Sixteen failure-first cases cover both managers and both command paths; the full local suite passes on this exact head (8,042 passed, 16 skipped, no deselections), with current upstream included.

[Copilot]

🔵 Needs a closer look

The cross-cutting rollback, filesystem, concurrency, and dynamic-import changes warrant final human review despite extensive regression coverage.

Review details

• Files reviewed: 21/21 changed files
• Comments generated: 0 new
• Review effort level: Balanced