Skip to content

fix(extensions): restore core-command discovery broken by #3014 module move#3292

Open
dhruv-15-03 wants to merge 1 commit into
github:mainfrom
dhruv-15-03:fix/3274-extensions-core-command-discovery
Open

fix(extensions): restore core-command discovery broken by #3014 module move#3292
dhruv-15-03 wants to merge 1 commit into
github:mainfrom
dhruv-15-03:fix/3274-extensions-core-command-discovery

Conversation

@dhruv-15-03

@dhruv-15-03 dhruv-15-03 commented Jul 1, 2026

Copy link
Copy Markdown

What

Fix the off-by-one path resolution in specify_cli.extensions._load_core_command_names() so it actually discovers the bundled core command templates instead of silently returning the hardcoded fallback set. Fixes #3294.

Why

When this code moved from src/specify_cli/extensions.py to src/specify_cli/extensions/__init__.py (#3014, 826e193), the file went one directory deeper but the bespoke Path(__file__) parent counts were not updated. Both candidate dirs now resolve to non-existent paths:

  • wheel -> specify_cli/extensions/core_pack/commands (real: specify_cli/core_pack/commands)
  • source -> src/templates/commands (real: repo-root templates/commands)

So _load_core_command_names() always falls through to _FALLBACK_CORE_COMMAND_NAMES. It only looks healthy because the fallback currently equals the 10 real command stems. CORE_COMMAND_NAMES (built from this function) guards extensions against shadowing core commands (#1994); with discovery dead, that guard depends on the fallback being hand-maintained -- converge was manually added to it in #3001 (0c29d89).

Same off-by-one class @mnriem diagnosed for the presets loader (re #3086, #2826), but in a module the preset-scoped fix does not touch.

How

  • Delegate path resolution to the canonical _assets resolvers (_locate_core_pack / _repo_root), exactly as presets/__init__.py does. These are anchored to the package root, so discovery survives future module moves.
  • Add tests/test_extensions.py::TestCoreCommandDiscovery covering the source, wheel, and fallback branches, plus a sentinel test proving discovery returns the real bundled set rather than the fallback.

Verification

  • New regression tests fail on the pre-fix code (the sentinel test fails with a clean assertion -- it returns the fallback sentinel instead of the bundled set) and pass after the fix.
  • tests/test_extensions.py: 328 passed, 3 skipped.
  • Broader relevant suites (imports, extensions, presets, integration registry/manifest): 887 passed. The only failures are 6 pre-existing Windows symlink-privilege errors in test_manifest.py that fail identically on the unmodified base (unrelated to this change).

Behavior change

None for end users today (the fallback already matches the real commands). This restores the intended dynamic discovery and removes the silent manual-maintenance dependency on the fallback list.


Authored with assistance from GitHub Copilot (model: Claude Opus 4.8), acting autonomously under the direction of @dhruv-15-03. Commits carry Assisted-by: trailers and the change is agent-generated; please review accordingly.

`_load_core_command_names()` resolved its command dirs with bespoke
Path(__file__) math that was correct only while the code lived at
src/specify_cli/extensions.py. The github#3014 refactor moved it into the
specify_cli/extensions/ package one directory deeper without updating the
parent counts, so both candidate dirs pointed at non-existent paths and
discovery always fell through to _FALLBACK_CORE_COMMAND_NAMES.

Delegate path resolution to the canonical _assets resolvers
(_locate_core_pack / _repo_root), mirroring presets/__init__.py. They are
anchored to the package root, so discovery is immune to future module
moves. Add regression tests that pin live discovery across the
wheel/source/fallback branches; they fail on the pre-fix code.

Fixes github#3274

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

Pull request overview

This PR fixes dynamic discovery of bundled core command templates used by the extensions system to prevent core-command shadowing. It corrects broken path resolution introduced when the extensions module moved into a package, by delegating lookup to the canonical _assets helpers and adding regression tests that ensure discovery doesn’t silently fall back to a hardcoded list.

Changes:

  • Update _load_core_command_names() to resolve command template directories via _locate_core_pack() (wheel) and _repo_root() (source) instead of Path(__file__) math.
  • Add regression tests covering: real discovery vs fallback, source-tree discovery, wheel core_pack preference, and fallback behavior/lockstep.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/specify_cli/extensions/__init__.py Fixes core-command discovery by using _assets path resolvers (wheel + source) instead of brittle relative parent traversal.
tests/test_extensions.py Adds regression tests to ensure discovery reads real bundled templates and doesn’t silently rely on the fallback set.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Low
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants