-
Notifications
You must be signed in to change notification settings - Fork 1
fix runtime module discovery reliability #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
2 changes: 2 additions & 0 deletions
2
openspec/changes/runtime-01-discovery-reliability/.openspec.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-05-07 |
43 changes: 43 additions & 0 deletions
43
openspec/changes/runtime-01-discovery-reliability/TDD_EVIDENCE.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # TDD Evidence: runtime-01-discovery-reliability | ||
|
|
||
| ## Scope Decision | ||
|
|
||
| - `#552` and `#554` remain in `nold-ai/specfact-cli`: the installed `specfact-codebase` artifact is present, but core runtime loading and diagnostics decide whether `specfact code` is registered and importable. | ||
| - `#553` remains in `nold-ai/specfact-cli`: environment-manager detection and `specfact init ide` option handling are core CLI behavior. | ||
| - No transfer to `nold-ai/specfact-cli-modules` is required unless implementation proves signed module manifests or payloads must change. | ||
|
|
||
| ## GitHub Readiness | ||
|
|
||
| - Parent feature: `#353 [Feature] Marketplace Module Distribution`. | ||
| - Change user story: `#557 [Story] Runtime Discovery Reliability for Installed Modules and Monorepos`. | ||
| - Source bug reports: `#552`, `#553`, and `#554`. | ||
| - Labels and SpecFact CLI project assignment were present on the reported issues; `#557` was created with `openspec`, `change-proposal`, `marketplace`, `dependencies`, and `module-system` labels and assigned to the SpecFact CLI project with `Todo` status. | ||
| - Corrected hierarchy on 2026-05-07: removed direct sub-issue links from bug reports to epic `#285`; linked `#557` under feature `#353`; linked `#557` as blocking `#552`, `#553`, and `#554`; commented source tracking back to all three bug reports. | ||
|
|
||
| ## Failing Evidence | ||
|
|
||
| - `hatch run pytest tests/unit/specfact_cli/registry/test_module_packages.py::test_installed_group_loader_adds_enabled_dependency_module_src_roots tests/unit/specfact_cli/registry/test_module_packages.py::test_lazy_loader_failure_is_recorded_for_availability_diagnostics tests/unit/utils/test_env_manager.py::TestDetectEnvManager::test_detect_uv_from_path_when_no_project_markers tests/unit/utils/test_env_manager.py::TestDetectEnvManager::test_detect_uv_from_rootless_monorepo_pyproject tests/unit/utils/test_env_manager.py::TestDetectEnvManager::test_detect_uv_from_second_level_monorepo_lock tests/e2e/test_init_command.py::TestInitCommandE2E::test_init_no_warning_with_explicit_uv_env_manager tests/e2e/test_init_command.py::TestInitCommandE2E::test_init_no_warning_with_rootless_monorepo_uv -q` | ||
| - Result before production edits: 7 failed. Failures covered installed module dependency `src/` importability, lazy loader failure diagnostics, PATH/monorepo environment detection, and explicit `init ide --env-manager uv`. | ||
|
|
||
| ## Passing Evidence | ||
|
|
||
| - `hatch run pytest tests/unit/specfact_cli/registry/test_module_packages.py::test_installed_group_loader_adds_enabled_dependency_module_src_roots tests/unit/specfact_cli/registry/test_module_packages.py::test_lazy_loader_failure_is_recorded_for_availability_diagnostics tests/unit/utils/test_env_manager.py::TestDetectEnvManager::test_detect_uv_from_path_when_no_project_markers tests/unit/utils/test_env_manager.py::TestDetectEnvManager::test_detect_uv_from_rootless_monorepo_pyproject tests/unit/utils/test_env_manager.py::TestDetectEnvManager::test_detect_uv_from_second_level_monorepo_lock tests/e2e/test_init_command.py::TestInitCommandE2E::test_init_no_warning_with_explicit_uv_env_manager tests/e2e/test_init_command.py::TestInitCommandE2E::test_init_no_warning_with_rootless_monorepo_uv -q` -> 7 passed. | ||
| - `hatch run pytest tests/unit/specfact_cli/registry/test_module_packages.py tests/unit/specfact_cli/registry/test_module_availability.py -q` -> 50 passed. | ||
| - `hatch run pytest tests/e2e/test_init_command.py -q` -> 20 passed, 2 warnings. | ||
| - `hatch run pytest tests/unit/utils/test_env_manager.py -q` -> 34 passed. | ||
| - `hatch run pytest tests/integration/test_bundle_install.py::test_installing_spec_bundle_auto_installs_project_dependency tests/integration/test_bundle_install.py::test_installing_spec_bundle_skips_dependency_when_already_present tests/unit/modules/module_registry/test_commands.py::test_install_command_project_scope_reenable_uses_selected_repo tests/unit/modules/module_registry/test_commands.py::test_install_command_project_scope_installs_to_project_modules_root tests/unit/modules/module_registry/test_official_tier_display.py::test_module_install_reports_verified_official_tier -q` -> 5 passed. | ||
| - `hatch run env HOME=/tmp/specfact-test-home-runtime-01 pytest tests/integration/test_core_slimming.py::test_fresh_install_cli_app_registered_commands_only_three_core tests/integration/test_core_slimming.py::test_stale_flat_shim_plan_exits_with_install_instructions tests/unit/cli/test_lean_help_output.py::test_stale_lazy_flat_shim_prints_install_guidance tests/unit/registry/test_category_groups.py::test_bootstrap_with_category_grouping_enabled_registers_group_commands tests/unit/registry/test_category_groups.py::test_bootstrap_with_category_grouping_disabled_still_has_no_flat_shims -q` -> 5 passed. | ||
| - Added `scripts/runtime_discovery_smoke.py` and Hatch script `runtime-discovery-smoke` for CI-capable real-world coverage. The script creates an isolated HOME, builds a rootless monorepo fixture from `specfact-cli-demo` when available, adds multiple package-level `pyproject.toml`/lock markers, serves a local file-backed marketplace from `specfact-cli-modules`, installs `nold-ai/specfact-project`, `nold-ai/specfact-codebase`, and `nold-ai/specfact-code-review`, checks upgrade command availability, runs `specfact init ide` with auto and explicit `--env-manager uv`, and verifies installed `specfact code`, `code review run`, and `code import` command loading. | ||
| - `.github/workflows/pr-orchestrator.yml` now runs `python scripts/runtime_discovery_smoke.py --launcher direct --launcher pip-editable --launcher uvx` so installer, module discovery, init, and environment-manager regressions fail fast in CI across Hatch/current-interpreter, pip editable, and uvx launch paths. | ||
| - `hatch run pytest tests/integration/scripts/test_runtime_discovery_smoke.py -q` -> 1 passed. | ||
| - `hatch run runtime-discovery-smoke --modules-repo /home/dom/git/nold-ai/specfact-cli-modules --demo-repo /home/dom/git/nold-ai/specfact-demo-repo --launcher direct` -> passed against a real demo-repo copy and sibling module artifacts. | ||
| - `hatch run runtime-discovery-smoke --modules-repo /home/dom/git/nold-ai/specfact-cli-modules --demo-repo /home/dom/git/nold-ai/specfact-demo-repo --launcher pip-editable` -> passed with a temporary editable install and isolated module HOME. | ||
| - `hatch run runtime-discovery-smoke --modules-repo /home/dom/git/nold-ai/specfact-cli-modules --demo-repo /home/dom/git/nold-ai/specfact-demo-repo --launcher uvx` -> passed with `uvx --from <repo>` and isolated module HOME. | ||
| - `openspec validate runtime-01-discovery-reliability --strict` -> valid. | ||
| - `hatch run format` -> all checks passed. | ||
| - `hatch run type-check` -> 0 errors, 1572 existing repository-wide warnings. | ||
| - Touched-file `ruff format --check`, `ruff check`, and `pylint` -> clean; Pylint rated touched files 10.00/10. | ||
| - `hatch run workflows-lint` -> passed. | ||
| - `hatch run contract-test` -> no modified files detected; cached results used. | ||
| - `hatch run smart-test-auto` attempted a full baseline because no incremental baseline existed; it failed in the local developer HOME due pre-existing installed user modules being discovered by clean-runtime tests. The same failed subset passed with an isolated HOME, and all change-targeted suites passed. | ||
| - SpecFact code review: `SPECFACT_MODULES_ROOTS=/home/dom/git/nold-ai/specfact-cli-modules/packages hatch run python -m specfact_cli.cli code review run --json --out .specfact/code-review.runtime-01.changed.json --scope changed` -> 0 blocking findings; 499 warnings remain, dominated by existing repository-wide type-safety warnings. New contract warnings introduced by this change were fixed before the final run. |
46 changes: 46 additions & 0 deletions
46
openspec/changes/runtime-01-discovery-reliability/proposal.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| ## Why | ||
|
|
||
| Three user bug reports show clean installed-runtime discovery gaps in SpecFact CLI 0.46.18: | ||
|
|
||
| - `#552` and `#554`: `nold-ai/specfact-codebase` is installed and enabled, but `specfact code` can report that the module is not installed or can show only timing output instead of command help. | ||
| - `#553`: `specfact init ide` reports no compatible environment manager in rootless monorepos even when `uv` is available on `PATH` and package-level `pyproject.toml` files exist. | ||
|
|
||
| The failures are core runtime issues, not module ownership issues. Installed module command loading, missing-command diagnostics, and environment-manager detection live in `specfact-cli`. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - **EXTEND** installed module runtime loading so lazy command import makes all enabled discovered module `src/` roots importable before loading a module command app. | ||
| - **EXTEND** missing command diagnostics so an installed-but-unloadable module reports the real runtime/import cause instead of a false "not installed" message. | ||
| - **NEW** environment-manager detection behavior for rootless monorepos and PATH-only tool availability. | ||
| - **EXTEND** `specfact init ide` with `--env-manager <auto|uv|hatch|poetry|pip>` while keeping automatic detection as the default. | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| - `installed-runtime-module-discovery` | ||
| - `module-installation` | ||
| - `module-owned-ide-prompts` | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| - `environment-manager-detection` | ||
|
|
||
| ## Impact | ||
|
|
||
| - Affected code: module discovery/command loading, module availability diagnostics, environment-manager detection, and `init ide` option wiring. | ||
| - Affected tests: targeted unit/e2e tests for installed module runtime loading, missing command diagnostics, monorepo environment detection, and `init ide --env-manager`. | ||
| - GitHub scope: fixes `#552`, `#553`, and `#554`; all remain in `nold-ai/specfact-cli` and are blocked by dedicated user-story issue `#557`, which is tracked under feature parent `#353`. | ||
|
|
||
| --- | ||
|
|
||
| ## Source Tracking | ||
|
|
||
| <!-- source_repo: nold-ai/specfact-cli --> | ||
| - **Parent Feature**: [#353](https://github.com/nold-ai/specfact-cli/issues/353) | ||
| - **Change User Story**: [#557](https://github.com/nold-ai/specfact-cli/issues/557) | ||
| - **GitHub Issues**: [#552](https://github.com/nold-ai/specfact-cli/issues/552), [#553](https://github.com/nold-ai/specfact-cli/issues/553), [#554](https://github.com/nold-ai/specfact-cli/issues/554) | ||
| - **Issue Relationships**: `#557` blocks `#552`, `#553`, and `#554`; no direct user bug report is nested under an epic or feature. | ||
| - **Repository**: nold-ai/specfact-cli | ||
| - **Last Synced Status**: GitHub story and dependencies synced | ||
| - **Sanitized**: false |
43 changes: 43 additions & 0 deletions
43
...es/runtime-01-discovery-reliability/specs/environment-manager-detection/spec.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: Environment Manager Detection Supports Rootless Monorepos | ||
|
|
||
| The system SHALL detect supported Python environment managers in repositories that do not have a root-level Python project file but do contain package-level project files. | ||
|
|
||
| #### Scenario: Rootless monorepo with uv package files | ||
|
|
||
| - **GIVEN** a repository root has no `pyproject.toml` | ||
| - **AND** a first-level package directory contains `pyproject.toml` | ||
| - **AND** `uv` is available on `PATH` | ||
| - **WHEN** environment manager detection runs for the repository root | ||
| - **THEN** the detected manager is `uv` | ||
| - **AND** the command prefix is `uv run` | ||
| - **AND** `specfact init ide` does not report "No Compatible Environment Manager Detected" | ||
|
|
||
| #### Scenario: Rootless monorepo with nested uv lock | ||
|
|
||
| - **GIVEN** a repository root has no `pyproject.toml` | ||
| - **AND** a first-level or second-level package directory contains `uv.lock` or `uv.toml` | ||
| - **AND** `uv` is available on `PATH` | ||
| - **WHEN** environment manager detection runs for the repository root | ||
| - **THEN** the detected manager is `uv` | ||
|
|
||
| ### Requirement: Environment Manager Detection Falls Back To PATH Tools | ||
|
|
||
| When no project marker identifies an environment manager, the system SHALL detect supported tools available on `PATH` before returning `unknown`. | ||
|
|
||
| #### Scenario: PATH-only uv detection | ||
|
|
||
| - **GIVEN** a repository has no supported root or package-level Python project marker | ||
| - **AND** `uv` is available on `PATH` | ||
| - **WHEN** environment manager detection runs | ||
| - **THEN** the detected manager is `uv` | ||
| - **AND** the command prefix is `uv run` | ||
|
|
||
| #### Scenario: No markers or tools remain unknown | ||
|
|
||
| - **GIVEN** a repository has no supported Python project marker | ||
| - **AND** no supported environment manager executable is available on `PATH` | ||
| - **WHEN** environment manager detection runs | ||
| - **THEN** the detected manager remains `unknown` | ||
| - **AND** existing direct-invocation fallback behavior is preserved |
22 changes: 22 additions & 0 deletions
22
...ntime-01-discovery-reliability/specs/installed-runtime-module-discovery/spec.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| ## MODIFIED Requirements | ||
|
|
||
| ### Requirement: Module Discovery Roots | ||
|
|
||
| The system SHALL discover and load module packages consistently between development and installed runtime contexts. | ||
|
|
||
| #### Scenario: Installed runtime loads dependent module packages | ||
|
|
||
| - **GIVEN** user-scope modules `nold-ai/specfact-project` and `nold-ai/specfact-codebase` are installed and enabled | ||
| - **AND** no sibling `specfact-cli-modules` source checkout contributes bundle paths to `sys.path` | ||
| - **WHEN** the user invokes `specfact code --help` | ||
| - **THEN** the codebase module command app loads from the installed module artifact | ||
| - **AND** imports of installed dependency packages such as `specfact_project` resolve without manual `PYTHONPATH` | ||
| - **AND** the command help includes codebase subcommands such as `import`, `analyze`, `drift`, `validate`, and `repro` | ||
|
|
||
| #### Scenario: Development source paths do not mask installed-runtime validation | ||
|
|
||
| - **GIVEN** tests configure explicit installed module roots | ||
| - **AND** development-only sibling module source paths are disabled for that runtime | ||
| - **WHEN** module command loading is validated | ||
| - **THEN** success depends on the installed module artifacts under the configured roots | ||
| - **AND** missing installed dependencies fail the validation instead of being satisfied by a sibling checkout |
14 changes: 14 additions & 0 deletions
14
...spec/changes/runtime-01-discovery-reliability/specs/module-installation/spec.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| ## MODIFIED Requirements | ||
|
|
||
| ### Requirement: Missing Command Diagnostics Explain Installed-Unavailable Causes | ||
|
|
||
| When a known module-provided command group is not registered, the system SHALL distinguish an absent module from an installed module that is unavailable for another local reason. | ||
|
|
||
| #### Scenario: Missing command provider fails during lazy command load | ||
|
|
||
| - **GIVEN** a known command group is provided by an installed and enabled module | ||
| - **AND** the module command app cannot be imported because a runtime dependency or module package import is missing | ||
| - **WHEN** the user invokes the command group | ||
| - **THEN** the CLI SHALL report that the module is installed but unavailable | ||
| - **AND** the diagnostic SHALL include the failing load reason when it can be captured without retrying destructive installation | ||
| - **AND** the diagnostic SHALL NOT report only that the module is not installed |
12 changes: 12 additions & 0 deletions
12
...changes/runtime-01-discovery-reliability/specs/module-owned-ide-prompts/spec.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| ## MODIFIED Requirements | ||
|
|
||
| ### Requirement: IDE prompt export SHALL use installed module resources | ||
|
|
||
| `specfact init ide` SHALL discover prompt templates from installed module packages and their packaged resource directories. The export flow SHALL not depend on workflow prompt files stored under the core CLI package for bundle-owned commands. | ||
|
|
||
| #### Scenario: IDE setup accepts explicit environment manager | ||
|
|
||
| - **GIVEN** prompt templates are available for export | ||
| - **WHEN** the user runs `specfact init ide --env-manager uv` | ||
| - **THEN** IDE prompt export uses the selected `uv` environment manager metadata for dependency setup decisions | ||
| - **AND** the command does not emit the "No Compatible Environment Manager Detected" warning for that explicit manager |
33 changes: 33 additions & 0 deletions
33
openspec/changes/runtime-01-discovery-reliability/tasks.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| # Tasks: runtime-01-discovery-reliability | ||
|
|
||
| ## 1. Readiness and spec validation | ||
|
|
||
| - [x] 1.1 Confirm issues `#552`, `#553`, and `#554` are correctly scoped to `specfact-cli`, not `specfact-cli-modules`, and record that decision in `TDD_EVIDENCE.md`. | ||
| - [x] 1.2 Confirm public GitHub metadata is complete: dedicated user-story issue `#557`, feature parent `#353`, labels, project assignment, issue dependencies, and Todo/not-in-progress status. | ||
| - [x] 1.3 Validate the OpenSpec change with `openspec validate runtime-01-discovery-reliability --strict`. | ||
|
|
||
| ## 2. Failing-first tests | ||
|
|
||
| - [x] 2.1 Add tests for clean installed module runtime loading with temp installed `specfact-project` and `specfact-codebase` modules, no sibling module source path, and `specfact code` exposing `import`, `analyze`, `drift`, `validate`, and `repro`. | ||
| - [x] 2.2 Add tests proving module load/import failures are classified as installed-unavailable instead of absent. | ||
| - [x] 2.3 Add tests for rootless monorepo environment detection with `uv` on `PATH`, package-level `pyproject.toml`/`uv.lock`, and explicit `init ide --env-manager uv`. | ||
| - [x] 2.4 Run the targeted tests before production edits and record failing evidence in `TDD_EVIDENCE.md`. | ||
|
|
||
| ## 3. Runtime discovery fixes | ||
|
|
||
| - [x] 3.1 Add a focused helper that prepends enabled discovered module `src/` roots to `sys.path` before lazy-loading installed module command apps. | ||
| - [x] 3.2 Preserve existing development behavior but prevent sibling `specfact-cli-modules` source paths from hiding installed-runtime test failures. | ||
| - [x] 3.3 Capture lazy loader failures in availability diagnostics so known module commands distinguish absent, disabled, skipped, and load-failed providers. | ||
|
|
||
| ## 4. Environment manager fixes | ||
|
|
||
| - [x] 4.1 Extend environment-manager detection to scan rootless monorepo package directories up to two levels deep. | ||
| - [x] 4.2 Add PATH fallback detection for supported tools, preferring `uv`, then `hatch`, `poetry`, and `pip`. | ||
| - [x] 4.3 Add `specfact init ide --env-manager <auto|uv|hatch|poetry|pip>` and use the explicit manager when provided. | ||
|
|
||
| ## 5. Passing evidence and quality gates | ||
|
|
||
| - [x] 5.1 Re-run targeted tests and record passing evidence in `TDD_EVIDENCE.md`. | ||
| - [x] 5.2 Run required quality gates for touched scope: formatting, type-check, lint, contract-test, smart-test or targeted equivalent, and SpecFact code review JSON. | ||
| - [x] 5.3 Add a CI-capable runtime discovery smoke script that exercises module install, upgrade/init-adjacent discovery, rootless monorepo environment-manager detection, and installed `specfact code` command loading against a real demo checkout. | ||
| - [x] 5.4 Update task checkboxes and prepare the branch for PR to `dev`. |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.