Skip to content

fix: scope custom steps to current project - #4466

Open
marcelsafin wants to merge 2 commits into
github:mainfrom
marcelsafin:fix/custom-step-registry-scope
Open

fix: scope custom steps to current project#4466
marcelsafin wants to merge 2 commits into
github:mainfrom
marcelsafin:fix/custom-step-registry-scope

Conversation

@marcelsafin

@marcelsafin marcelsafin commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Description

Scope dynamically loaded custom workflow steps to the current project.

STEP_REGISTRY and Python's module cache are process-global. Loading project
A followed by project B previously left A's step classes active, and a shared
type key could keep using A's imported helper code. Each scan now removes
non-built-in registry entries and generated custom-step modules before loading
the requested project.

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
  • Tested with a sample project (if applicable)

Targeted: TestLoadCustomSteps (12 passed).

Full suite: 7,533 passed, 195 skipped. One existing PowerShell-launcher test
was omitted because pwsh is unavailable locally; it fails identically on the
unchanged upstream commit.

AI Disclosure

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

GitHub Copilot (GPT-5.6 Sol) autonomously reproduced the bug, wrote the
regression test and implementation, and ran verification under
@marcelsafin's direction and review.

Assisted-by: GitHub Copilot (model: gpt-5.6-sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 8, 2026 14:37
@marcelsafin
marcelsafin requested a review from mnriem as a code owner September 8, 2026 14:37

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.

🟡 Changes recommended

Same-path reloads may reuse stale bytecode, and related comments must be updated.

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

Pull request overview

Scopes dynamically loaded workflow steps to the current project.

Changes:

  • Resets custom-step registrations and cached modules before each scan.
  • Adds cross-project isolation regression coverage.
File summaries
File Description
tests/test_workflows.py Tests replacing identically named steps across projects.
src/specify_cli/workflows/__init__.py Resets custom-step state; bytecode caching may still cause stale same-path reloads.
Review details

Suppressed comments (1)

src/specify_cli/workflows/init.py:104

  • This new removal behavior makes the existing explanations in src/specify_cli/bundler/services/references.py:50-53 and tests/unit/test_bundler_references.py:50-55 incorrect: both still state that load_custom_steps “never removes” custom entries. Please update those comments to explain that BUILTIN_STEP_TYPES remains the immutable built-in snapshot even though the active registry is reset between scans.
    for _type_key in tuple(STEP_REGISTRY):
        if _type_key not in BUILTIN_STEP_TYPES:
            STEP_REGISTRY.pop(_type_key, None)
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

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

Comment on lines +105 to +107
_module_prefix = "_speckit_custom_step_"
for _mod_key in [k for k in _sys.modules if k.startswith(_module_prefix)]:
_sys.modules.pop(_mod_key, None)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed on 7e2961ea: custom-step __pycache__ directories are purged before import, import caches are invalidated, and a same-path/same-mtime regression test now verifies updated helper code is loaded. I also updated the stale BUILTIN_STEP_TYPES comments identified in the review. Full runnable suite: 7,533 passed, 195 skipped; Ruff and CLI smoke are clean. Posted on behalf of @marcelsafin by GitHub Copilot (model: GPT-5.6 Sol).

Assisted-by: GitHub Copilot (model: gpt-5.6-sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 8, 2026 14:52

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.

🟢 Approval recommended

All reviewed changes are covered by regression tests with no unresolved issues.

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

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.

2 participants