diff --git a/CHANGELOG.md b/CHANGELOG.md index 631e0709..58794826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,41 @@ All notable changes to this project will be documented in this file. --- +## [0.46.22] - 2026-05-12 + +### Fixed + +- **Module review hardening**: avoid false versioned dependency failures when + callers do not provide installed-version maps, and require exact namespace + matches for fully qualified `module doctor` IDs. + +--- + +## [0.46.21] - 2026-05-12 + +### Fixed + +- **Module diagnostics CI hardening**: keep JSON request payload typing, + legacy category grouping, and stale shim delegation compatible with the + required lint, type-check, and test gates. + +--- + +## [0.46.20] - 2026-05-12 + +### Added + +- **Module scope diagnostics**: add `specfact module doctor`. + +### Fixed + +- **Module scope diagnostics**: enforce versioned module dependency mismatches + across project and user scoped modules. +- **Module signature PR verification**: keep the relaxed PR verifier aligned + with its version-bump-only policy when signatures are deferred to CI signing. + +--- + ## [0.46.19] - 2026-05-07 ### Added diff --git a/docs/module-system/installing-modules.md b/docs/module-system/installing-modules.md index f9bc4162..5dbe4815 100644 --- a/docs/module-system/installing-modules.md +++ b/docs/module-system/installing-modules.md @@ -44,10 +44,11 @@ Notes: - If the requested module is already installed but disabled in `modules.json`, install repairs the lifecycle state by enabling the manifest module id and reports that action. - Missing command guidance distinguishes truly absent modules from installed-but-disabled or installed-but-skipped modules and suggests the matching recovery command. - Invalid ids show an explicit error (`name` or `namespace/name` only). +- Use `specfact module doctor [module-id]` to inspect the effective module copy, shadowed duplicates, and configured development source roots. ## Dependency resolution -Before installing a marketplace module, SpecFact resolves its dependencies (other modules and optional pip packages) from manifest `pip_dependencies` and `module_dependencies`. If conflicts are detected (e.g. incompatible versions), install fails unless you override. +Before installing a marketplace module, SpecFact resolves its dependencies (other modules and optional pip packages) from manifest `pip_dependencies`, `module_dependencies`, and versioned bundle dependency declarations. If conflicts are detected (e.g. incompatible versions), install fails unless you override. ```bash # Install with dependency resolution (default) @@ -62,6 +63,7 @@ specfact module install nold-ai/specfact-backlog --force - Use `--skip-deps` when you want to install a single module without pulling its dependencies or when you manage dependencies yourself. - Use `--force` to proceed when resolution reports conflicts (e.g. for local overrides or known-compatible versions). Enable/disable and dependency-aware cascades still respect `--force` where applicable. +- If a dependency already exists in the selected install scope but its version does not satisfy a declared bundle dependency range, reinstall or upgrade that dependency in the same scope before retrying. See [Dependency resolution](../reference/dependency-resolution.md) for how resolution works and conflict detection. @@ -106,6 +108,7 @@ specfact module list specfact module list --show-origin specfact module list --source marketplace specfact module list --show-bundled-available +specfact module doctor nold-ai/specfact-codebase ``` Default columns: @@ -116,7 +119,9 @@ Default columns: - `Trust` (`official`, `community`, `local-dev`) - `Publisher` -With `--show-origin`, an additional `Origin` column is shown (`built-in`, `marketplace`, `custom`). +With `--show-origin`, an additional `Origin` column is shown (`built-in`, `project`, `user`, `marketplace`, `custom`). + +`module doctor` keeps discovery metadata-only and reports effective vs shadowed duplicate copies, exact manifest versions, paths, enabled state, configured development source roots, and recovery commands. Use it when project-scoped modules under `/.specfact/modules` and user-scoped modules under `~/.specfact/modules` disagree. ## Show Detailed Module Info diff --git a/docs/module-system/module-marketplace.md b/docs/module-system/module-marketplace.md index 3ce148ff..90ba3414 100644 --- a/docs/module-system/module-marketplace.md +++ b/docs/module-system/module-marketplace.md @@ -10,8 +10,6 @@ audience: [solo, team, enterprise] expertise_level: [beginner, intermediate] --- -# Module Marketplace - SpecFact supports centralized marketplace distribution with local multi-source discovery. For the curated official bundle list and trust/dependency quick reference, see @@ -38,8 +36,9 @@ Local module discovery scans these roots in priority order: 1. `built-in` modules (`src/specfact_cli/modules`) 2. `project` modules (`/.specfact/modules`) -3. `user` modules (`~/.specfact/modules`) -4. legacy/custom roots (`~/.specfact/marketplace-modules`, `~/.specfact/custom-modules`, `SPECFACT_MODULES_ROOTS`) +3. explicit development/custom roots (`SPECFACT_MODULES_ROOTS`) +4. `user` modules (`~/.specfact/modules`) +5. marketplace and custom roots (`~/.specfact/marketplace-modules`, `~/.specfact/custom-modules`) If module names collide, higher-priority sources win and lower-priority entries are shadowed. @@ -48,14 +47,17 @@ If module names collide, higher-priority sources win and lower-priority entries SpecFact shows both trust semantics and origin details: - `Trust` column (default): `official`, `community`, `local-dev` -- `Origin` column (`--show-origin`): `built-in`, `marketplace`, `custom` +- `Origin` column (`--show-origin`): `built-in`, `project`, `user`, `marketplace`, `custom` Use: ```bash specfact module list --show-origin +specfact module doctor nold-ai/specfact-codebase ``` +`module doctor` additionally reports shadowed duplicate copies, exact manifest versions, paths, enabled state, configured development source roots, and recovery commands. + ## Security Model Install workflow enforces integrity and compatibility checks: @@ -123,5 +125,4 @@ Scope boundary: > Modules docs handoff: this page remains in the core docs set as release-line overview content. > Canonical bundle-specific deep guidance now lives in the canonical modules docs site, currently > published at `https://modules.specfact.io/`. - > for the current release line and are planned to migrate to `specfact-cli-modules`. diff --git a/openspec/CHANGE_ORDER.md b/openspec/CHANGE_ORDER.md index 82535a77..9c1ec55e 100644 --- a/openspec/CHANGE_ORDER.md +++ b/openspec/CHANGE_ORDER.md @@ -8,7 +8,7 @@ should be implemented. | Bucket | Count | Location | |---|---|---| -| **Active** | 24 | [`openspec/changes/`](changes/) | +| **Active** | 25 | [`openspec/changes/`](changes/) | | **Parked** | 20 | [`openspec/parking-lot/`](parking-lot/) | | **Archived** | 105 | [`openspec/changes/archive/`](changes/archive/) | @@ -25,7 +25,7 @@ for the un-park trigger of each parked change. ## Active tracks -The 24 active changes group into five independent tracks. Tracks can run in +The 25 active changes group into five independent tracks. Tracks can run in parallel; within a track, follow the order column. ### Track A — Full-chain traceability (core thesis) @@ -90,6 +90,7 @@ README. Snapshot/CI work folds into 03 and 04 if and when needed. |---|---|---| | `dep-security-cleanup` | 62/69 done | Apache-2.0 license-cleanliness pass | | `marketplace-07-module-install-state-consistency` | ✓ archive pending | Resolves install-state disagreement across scopes | +| `module-scope-version-diagnostics` | active | Resolves project/user module version mismatch diagnostics and enforcement under #565 / #353 | | `upgrade-01-install-method-aware` | ✓ archive pending | Bug-fix for uvx/uv users | | `governance-02-exception-management` | active | Time-bound policy exceptions | | `architecture-02-well-architected-review` | **gated on architecture-01 shipping + 1 cycle of usage** | Boundary/ADR review pillar | diff --git a/openspec/changes/module-scope-version-diagnostics/.openspec.yaml b/openspec/changes/module-scope-version-diagnostics/.openspec.yaml new file mode 100644 index 00000000..40cc12f4 --- /dev/null +++ b/openspec/changes/module-scope-version-diagnostics/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-12 diff --git a/openspec/changes/module-scope-version-diagnostics/TDD_EVIDENCE.md b/openspec/changes/module-scope-version-diagnostics/TDD_EVIDENCE.md new file mode 100644 index 00000000..eaa40c94 --- /dev/null +++ b/openspec/changes/module-scope-version-diagnostics/TDD_EVIDENCE.md @@ -0,0 +1,41 @@ +# TDD Evidence + +## Failing Before + +- `hatch run pytest tests/unit/specfact_cli/registry/test_module_dependencies.py::test_validate_module_dependencies_detects_version_mismatch tests/unit/registry/test_module_installer.py::test_install_module_rejects_existing_bundle_dependency_version_mismatch tests/unit/modules/module_registry/test_commands.py::test_doctor_reports_effective_and_shadowed_duplicate_modules -q` + - Result: FAIL (`3 failed`) before production edits. + - Failures: `_validate_module_dependencies` rejected the new version map argument; install accepted an out-of-range existing dependency; `module doctor` did not exist. + +## Passing After + +- `hatch run pytest tests/unit/specfact_cli/registry/test_module_dependencies.py::test_validate_module_dependencies_detects_version_mismatch tests/unit/registry/test_module_installer.py::test_install_module_rejects_existing_bundle_dependency_version_mismatch tests/unit/modules/module_registry/test_commands.py::test_doctor_reports_effective_and_shadowed_duplicate_modules -q` + - Result: PASS (`3 passed`). +- `hatch run pytest tests/unit/specfact_cli/registry/test_module_dependencies.py tests/unit/registry/test_module_installer.py tests/unit/modules/module_registry/test_commands.py -q` + - Result: PASS (`96 passed`). + +## Quality Gates + +- `hatch run format` + - Result: PASS (`All checks passed!`). +- `openspec validate module-scope-version-diagnostics --strict` + - Result: PASS. +- `basedpyright src/specfact_cli/modules/module_registry/src/commands.py src/specfact_cli/registry/module_installer.py src/specfact_cli/registry/module_packages.py src/specfact_cli/registry/module_state.py tests/unit/modules/module_registry/test_commands.py tests/unit/registry/test_module_installer.py tests/unit/specfact_cli/registry/test_module_dependencies.py` + - Result: PASS (`0 errors`, existing pytest monkeypatch typing warnings only). +- `hatch run specfact code review run --json --out .specfact/code-review.changed.json --scope changed` + - Result: PASS (`0 blocking`, 17 warning-only findings). + - Warning disposition: remaining findings are pre-existing local patterns in touched modules (parameter-count helpers, duplicate Typer validator shapes, existing naming heuristics, and legacy helper exposure). They were not expanded by this change except the validated dependency complexity, which was refactored before the passing review run. + +## Real-World Tmp Smoke + +- Created a temporary git repo with project module `nold-ai/specfact-codebase` version `0.41.0` and isolated user-scope module `nold-ai/specfact-codebase` version `0.40.0`. + - Command: `hatch run env HOME= specfact module doctor nold-ai/specfact-codebase --repo ` + - Result: PASS. Output showed project `0.41.0` as `effective`, user `0.40.0` as `shadowed`, and recovery command `specfact module uninstall nold-ai/specfact-codebase --scope user`. +- Created a temporary install root with existing dependency `nold-ai/dep` version `1.0.0` and installed a local tarball declaring `bundle_dependencies: [{id: nold-ai/dep, version: ">=2.0.0"}]` through the worktree installer. + - Command: `hatch run python ` + - Result: PASS. Install failed with `Dependency nold-ai/dep requires >=2.0.0, but installed version is 1.0.0. Reinstall or upgrade the dependency in the same module scope.` + +## Known Gate Caveat + +- `hatch run lint` + - Result: FAIL on repository-wide pre-existing `JsonType` basedpyright errors in `src/specfact_cli/adapters/ado.py`, `src/specfact_cli/adapters/github.py`, and `src/specfact_cli/validators/change_proposal_integration.py`. + - Scoped basedpyright for this change passes with 0 errors. diff --git a/openspec/changes/module-scope-version-diagnostics/design.md b/openspec/changes/module-scope-version-diagnostics/design.md new file mode 100644 index 00000000..711dfd7f --- /dev/null +++ b/openspec/changes/module-scope-version-diagnostics/design.md @@ -0,0 +1,16 @@ +## Overview + +Keep module precedence deterministic while making the effective runtime explainable. The CLI will continue to select the first discovered module id by source priority, but diagnostics will expose duplicate copies and version drift. Dependency enforcement will use manifest metadata already parsed by the runtime. + +## Decisions + +- Add `specfact module doctor [MODULE_ID] [--repo PATH]` as the focused diagnostic entrypoint instead of overloading `module list`. +- Use metadata-only discovery with shadowed duplicates retained, so diagnostics do not import module command code. +- Report development source roots from `SPECFACT_MODULES_REPO`, `SPECFACT_CLI_MODULES_REPO`, and `SPECFACT_MODULES_ROOTS` when configured because those can affect Python import resolution even when install manifests come from `.specfact/modules`. +- Enforce versioned module dependencies with `packaging.specifiers.SpecifierSet` and `packaging.version.Version`; malformed specifiers remain non-blocking with debug logging, matching existing `core_compatibility` tolerance. +- Install-time enforcement checks existing dependencies before accepting them, installs missing dependencies as today, then validates the installed manifest version. + +## Risks + +- Users with stale duplicate installs may see new warnings or skipped modules. Mitigation: diagnostics include exact uninstall/reinstall guidance. +- Marketplace install cannot select the newest version satisfying a range yet. Mitigation: install the normal resolved artifact, then fail with an actionable version mismatch if it does not satisfy the manifest requirement. diff --git a/openspec/changes/module-scope-version-diagnostics/proposal.md b/openspec/changes/module-scope-version-diagnostics/proposal.md new file mode 100644 index 00000000..838ec491 --- /dev/null +++ b/openspec/changes/module-scope-version-diagnostics/proposal.md @@ -0,0 +1,40 @@ +## Why + +A key user reported module conflicts and version mismatches when SpecFact modules exist in both project-local and user scopes across split repositories and monorepos. Today project scope correctly wins, but lower-priority copies and implicit development source roots are hard to see, and declared module dependency version ranges are not enforced consistently. + +## What Changes + +- Add a user-facing module diagnostics command that shows effective and shadowed module copies across project, user, marketplace, custom, and development source roots. +- Surface exact module versions, origins, paths, and recovery guidance when duplicate module ids exist. +- Enforce versioned module dependency requirements from module manifests during marketplace install and command registration. +- Preserve current project-over-user precedence; this change makes mismatches visible and prevents incompatible dependency sets from loading silently. + +## Capabilities + +### New Capabilities + +- `module-scope-diagnostics`: Diagnose effective module origin, shadowed duplicates, development source roots, and version mismatch recovery steps. + +### Modified Capabilities + +- `module-packages`: Module registration enforces declared module dependency version ranges instead of checking presence only. +- `module-installation`: Module install enforces declared bundle dependency version ranges after dependency discovery/install. + +## Impact + +- Affected code: module discovery diagnostics, `specfact module` command surface, module dependency validation, and marketplace install dependency handling. +- Affected tests: unit tests for module diagnostics output, registration skip behavior, and install-time dependency version mismatch handling. +- Documentation: module installation/marketplace docs should mention diagnostics and duplicate-scope remediation. + +--- + +## Source Tracking + + +- **Parent Feature**: [#353](https://github.com/nold-ai/specfact-cli/issues/353) +- **Change User Story**: [#565](https://github.com/nold-ai/specfact-cli/issues/565) +- **GitHub Issue**: [#565](https://github.com/nold-ai/specfact-cli/issues/565) +- **Issue Relationships**: `#565` is a sub-issue of Feature `#353`; Feature `#353` is a sub-issue of Epic `#194`. +- **Repository**: nold-ai/specfact-cli +- **Last Synced Status**: GitHub story, labels, parent relationship, and source tracking synced +- **Sanitized**: false diff --git a/openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md b/openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md new file mode 100644 index 00000000..06930ed5 --- /dev/null +++ b/openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md @@ -0,0 +1,20 @@ +## MODIFIED Requirements + +### Requirement: Module install enforces versioned bundle dependencies + +The system SHALL validate versioned bundle dependency declarations during module installation. + +#### Scenario: Existing dependency version is too old + +- **GIVEN** a module being installed declares a bundle dependency with a version range +- **AND** that dependency already exists in the target install root with a version outside the range +- **WHEN** the user installs the dependent module +- **THEN** install fails before accepting the dependency set +- **AND** the error identifies the dependency id, required version range, and installed version + +#### Scenario: Newly installed dependency version is validated + +- **GIVEN** a module being installed declares a missing bundle dependency with a version range +- **WHEN** dependency installation completes +- **THEN** the installed dependency version is validated against the declared range +- **AND** install fails if the installed dependency still does not satisfy the range diff --git a/openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md b/openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md new file mode 100644 index 00000000..0d27d767 --- /dev/null +++ b/openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md @@ -0,0 +1,20 @@ +## MODIFIED Requirements + +### Requirement: Module registration enforces versioned module dependencies + +The system SHALL skip registering an enabled module when its declared versioned module dependency is absent, disabled, or present at a version that does not satisfy the declared specifier. + +#### Scenario: Enabled dependency version is too old + +- **GIVEN** an enabled module declares a dependency on another module with a minimum version +- **AND** the dependency is enabled but its manifest version is below that minimum +- **WHEN** module package commands are registered +- **THEN** the dependent module is skipped +- **AND** diagnostics report the required version and the discovered version + +#### Scenario: Dependency version satisfies the declared range + +- **GIVEN** an enabled module declares a versioned module dependency +- **AND** the dependency is enabled and its version satisfies the declared range +- **WHEN** module package commands are registered +- **THEN** the dependent module remains eligible for registration diff --git a/openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md b/openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md new file mode 100644 index 00000000..55cd1850 --- /dev/null +++ b/openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md @@ -0,0 +1,20 @@ +## ADDED Requirements + +### Requirement: Module doctor reports effective and shadowed module copies + +The system SHALL provide a `specfact module doctor` diagnostic that reports module origin, version, path, and shadowing state without importing module command code. + +#### Scenario: Duplicate project and user module copies are visible + +- **GIVEN** a module id exists in project scope and user scope with different versions +- **WHEN** the user runs `specfact module doctor ` +- **THEN** the output identifies the project copy as effective +- **AND** the output identifies the user copy as shadowed +- **AND** the output shows both versions and paths +- **AND** the output includes a recovery command for removing the stale user-scope copy + +#### Scenario: Development source roots are disclosed + +- **GIVEN** development source root environment variables are configured +- **WHEN** the user runs `specfact module doctor` +- **THEN** the output lists the configured development source roots that may influence import resolution diff --git a/openspec/changes/module-scope-version-diagnostics/tasks.md b/openspec/changes/module-scope-version-diagnostics/tasks.md new file mode 100644 index 00000000..f6b2b61d --- /dev/null +++ b/openspec/changes/module-scope-version-diagnostics/tasks.md @@ -0,0 +1,24 @@ +## 1. Governance + +- [x] 1.1 Create GitHub issue #565 with labels, parent Feature #353, and proposal source tracking. +- [x] 1.2 Keep internal wiki source tracking aligned for `module-scope-version-diagnostics`. + +## 2. Specs and Tests + +- [x] 2.1 Add OpenSpec proposal, design, and spec deltas for diagnostics and version enforcement. +- [x] 2.2 Add failing tests for module doctor duplicate-scope diagnostics. +- [x] 2.3 Add failing tests for install-time versioned bundle dependency enforcement. +- [x] 2.4 Add failing tests for registration-time versioned module dependency enforcement. + +## 3. Implementation + +- [x] 3.1 Implement `specfact module doctor`. +- [x] 3.2 Enforce versioned bundle dependency declarations during install. +- [x] 3.3 Enforce versioned module dependency declarations during registration and lifecycle checks. +- [x] 3.4 Update module docs with duplicate-scope remediation guidance. + +## 4. Evidence and Gates + +- [x] 4.1 Record failing-before and passing-after test evidence in `TDD_EVIDENCE.md`. +- [x] 4.2 Run targeted tests for touched module registry/installer behavior. +- [x] 4.3 Run required scoped quality gates and SpecFact code review. diff --git a/pyproject.toml b/pyproject.toml index 1dd737dd..34575021 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "specfact-cli" -version = "0.46.19" +version = "0.46.22" description = "The swiss knife CLI for agile DevOps teams. Keep backlog, specs, tests, and code in sync with validation and contract enforcement for new projects and long-lived codebases." readme = "README.md" requires-python = ">=3.11" diff --git a/scripts/publish-module.py b/scripts/publish-module.py old mode 100755 new mode 100644 index a676a9c8..c53d233b --- a/scripts/publish-module.py +++ b/scripts/publish-module.py @@ -377,11 +377,32 @@ def _build_publish_entry( "checksum_sha256": checksum, "tier": manifest.get("tier", "community"), "publisher": manifest.get("publisher", "unknown"), - "bundle_dependencies": manifest.get("bundle_dependencies", []), + "bundle_dependencies": _bundle_dependency_ids_for_registry(manifest), "description": (manifest.get("description") or "").strip(), } +def _bundle_dependency_ids_for_registry(manifest: dict[str, Any]) -> list[str]: + """Return registry-compatible bundle dependency IDs from string or object manifest entries.""" + raw_dependencies = manifest.get("bundle_dependencies", []) + if not isinstance(raw_dependencies, list): + raise ValueError( + f"bundle_dependencies must be a list; got {type(raw_dependencies).__name__}: {raw_dependencies!r}" + ) + dependency_ids: list[str] = [] + for entry in raw_dependencies: + if isinstance(entry, dict): + raw_id = entry.get("id") + if raw_id is None or not str(raw_id).strip(): + raise ValueError(f"bundle_dependencies object entry must include non-empty 'id'; got {entry!r}") + dependency_ids.append(str(raw_id).strip()) + continue + dependency_id = str(entry).strip() + if dependency_id: + dependency_ids.append(dependency_id) + return dependency_ids + + @beartype @require(lambda bundle_name: cast(str, bundle_name).strip() != "", "bundle_name must be non-empty") def publish_bundle( diff --git a/scripts/verify-modules-signature.py b/scripts/verify-modules-signature.py index 377b5445..c4c31376 100755 --- a/scripts/verify-modules-signature.py +++ b/scripts/verify-modules-signature.py @@ -308,12 +308,6 @@ def verify_manifest( if not checksum: raise ValueError("missing integrity.checksum") _parse_checksum(checksum) - signature = str(integrity.get("signature", "")).strip() - if signature: - if not public_key_pem: - raise ValueError("public key required to verify integrity.signature") - payload = _module_payload(manifest_path.parent, payload_from_filesystem=payload_from_filesystem) - _verify_signature(payload, signature, public_key_pem) return integrity_raw = data.get("integrity") if not isinstance(integrity_raw, dict): diff --git a/setup.py b/setup.py index 9cf48ca8..daedff32 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ if __name__ == "__main__": _setup = setup( name="specfact-cli", - version="0.46.19", + version="0.46.22", description=( "The swiss knife CLI for agile DevOps teams. Keep backlog, specs, tests, and code in sync with " "validation and contract enforcement for new projects and long-lived codebases." diff --git a/src/__init__.py b/src/__init__.py index 1f744e48..6783909e 100644 --- a/src/__init__.py +++ b/src/__init__.py @@ -3,4 +3,4 @@ """ # Package version: keep in sync with pyproject.toml, setup.py, src/specfact_cli/__init__.py -__version__ = "0.46.19" +__version__ = "0.46.22" diff --git a/src/specfact_cli/__init__.py b/src/specfact_cli/__init__.py index b85421a9..7b059f74 100644 --- a/src/specfact_cli/__init__.py +++ b/src/specfact_cli/__init__.py @@ -45,6 +45,6 @@ def _bootstrap_bundle_paths() -> None: _bootstrap_bundle_paths() -__version__ = "0.46.19" +__version__ = "0.46.22" __all__ = ["__version__"] diff --git a/src/specfact_cli/adapters/ado.py b/src/specfact_cli/adapters/ado.py index 1acfa405..6e94ada8 100644 --- a/src/specfact_cli/adapters/ado.py +++ b/src/specfact_cli/adapters/ado.py @@ -1625,7 +1625,7 @@ def _patch_work_item( headers = {"Content-Type": "application/json-patch+json", **self._auth_headers()} try: response = self._request_with_retry( - lambda: requests.patch(url, json=patch_document, headers=headers, timeout=30) + lambda: requests.patch(url, json=cast(Any, patch_document), headers=headers, timeout=30) ) except requests.RequestException as exc: resp = getattr(exc, "response", None) @@ -1939,7 +1939,7 @@ def _try_refresh_oauth_token(self) -> dict[str, Any] | None: # Refresh failed (no cached refresh token, refresh token expired, etc.) return None - def _auth_headers(self) -> dict[str, str]: + def _auth_headers(self) -> dict[str, str | bytes]: """Return authorization headers based on token type.""" if not self.api_token: return {} @@ -1953,7 +1953,7 @@ def _ado_get( self, url: str, *, - headers: dict[str, str] | None = None, + headers: dict[str, str | bytes] | None = None, params: dict[str, Any] | None = None, timeout: int = 30, retry_on_ambiguous_transport: bool = True, @@ -1973,7 +1973,7 @@ def _ado_post( self, url: str, *, - headers: dict[str, str] | None = None, + headers: dict[str, str | bytes] | None = None, params: dict[str, Any] | None = None, json: dict[str, Any] | None = None, timeout: int = 30, @@ -2213,7 +2213,7 @@ def _create_work_item_from_proposal( try: response = self._request_with_retry( - lambda: requests.post(url, json=patch_document, headers=headers, timeout=30), + lambda: requests.post(url, json=cast(Any, patch_document), headers=headers, timeout=30), retry_on_ambiguous_transport=False, ) if is_debug_mode(): @@ -2670,7 +2670,7 @@ def _add_work_item_comment( try: response = self._request_with_retry( - lambda: requests.post(url, json=comment_body, headers=headers, timeout=30), + lambda: requests.post(url, json=cast(Any, comment_body), headers=headers, timeout=30), retry_on_ambiguous_transport=False, ) comment_data = response.json() @@ -3478,7 +3478,7 @@ def create_issue(self, project_id: str, payload: dict[str, Any]) -> dict[str, An **self._auth_headers(), } response = self._request_with_retry( - lambda: requests.post(url, json=patch_document, headers=headers, timeout=30), + lambda: requests.post(url, json=cast(Any, patch_document), headers=headers, timeout=30), retry_on_ambiguous_transport=False, ) created = response.json() @@ -3782,7 +3782,7 @@ def _backlog_patch_try_without_multiline_format( if operations_no_format == operations: return None try: - resp = requests.patch(url, headers=headers, json=operations_no_format, timeout=30) + resp = requests.patch(url, headers=headers, json=cast(Any, operations_no_format), timeout=30) resp.raise_for_status() return resp except requests.HTTPError as retry_error: @@ -3802,7 +3802,7 @@ def _backlog_patch_try_replace_multiline_add( ) -> requests.Response | None: operations_replace = self._backlog_ops_replace_multiline_add_with_replace(operations) try: - resp = requests.patch(url, headers=headers, json=operations_replace, timeout=30) + resp = requests.patch(url, headers=headers, json=cast(Any, operations_replace), timeout=30) resp.raise_for_status() return resp except requests.HTTPError: @@ -3820,7 +3820,7 @@ def _backlog_patch_try_html_conversion( ) operations_html = self._backlog_ops_convert_markdown_fields_to_html(operations) try: - resp = requests.patch(url, headers=headers, json=operations_html, timeout=30) + resp = requests.patch(url, headers=headers, json=cast(Any, operations_html), timeout=30) resp.raise_for_status() return resp except requests.HTTPError: @@ -3834,7 +3834,9 @@ def _execute_backlog_patch_with_fallbacks( operations: list[dict[str, Any]], ) -> requests.Response: try: - return self._request_with_retry(lambda: requests.patch(url, headers=headers, json=operations, timeout=30)) + return self._request_with_retry( + lambda: requests.patch(url, headers=headers, json=cast(Any, operations), timeout=30) + ) except requests.HTTPError as e: user_msg = _log_ado_patch_failure(e.response, operations, url) e.ado_user_message = user_msg # type: ignore[attr-defined] diff --git a/src/specfact_cli/adapters/github.py b/src/specfact_cli/adapters/github.py index da260908..7d1e1859 100644 --- a/src/specfact_cli/adapters/github.py +++ b/src/specfact_cli/adapters/github.py @@ -561,7 +561,7 @@ def _search_github_issues(self, query: str) -> list[BacklogItem]: from specfact_cli.backlog.converter import convert_github_issue_to_backlog_item url = f"{self.base_url}/search/issues" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -1232,7 +1232,7 @@ def fetch_backlog_item(self, item_ref: str) -> dict[str, Any]: repo_owner, repo_name, issue_number = self._parse_issue_reference(item_ref) url = f"{self.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number}" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -1498,7 +1498,7 @@ def _create_issue_from_proposal( # Create issue via GitHub API url = f"{self.base_url}/repos/{repo_owner}/{repo_name}/issues" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -1592,7 +1592,7 @@ def _update_issue_status( # Update issue state url = f"{self.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number}" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -1601,7 +1601,9 @@ def _update_issue_status( payload["state_reason"] = state_reason try: - response = self._request_with_retry(lambda: requests.patch(url, json=payload, headers=headers, timeout=30)) + response = self._request_with_retry( + lambda: requests.patch(url, json=cast(Any, payload), headers=headers, timeout=30) + ) issue_data = response.json() # Add comment explaining status change @@ -1634,7 +1636,7 @@ def _get_issue_comments(self, repo_owner: str, repo_name: str, issue_number: int return [] url = f"{self.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number}/comments" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -1658,7 +1660,7 @@ def _add_issue_comment(self, repo_owner: str, repo_name: str, issue_number: int, comment: Comment text """ url = f"{self.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number}/comments" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -1666,7 +1668,7 @@ def _add_issue_comment(self, repo_owner: str, repo_name: str, issue_number: int, try: self._request_with_retry( - lambda: requests.post(url, json=payload, headers=headers, timeout=30), + lambda: requests.post(url, json=cast(Any, payload), headers=headers, timeout=30), retry_on_ambiguous_transport=False, ) except requests.RequestException as e: @@ -1676,7 +1678,7 @@ def _add_issue_comment(self, repo_owner: str, repo_name: str, issue_number: int, def _fetch_issue_snapshot(self, repo_owner: str, repo_name: str, issue_number: int) -> tuple[str, str, str]: """Fetch current issue body, title, and state for preservation-aware updates.""" url = f"{self.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number}" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -1759,7 +1761,7 @@ def _update_issue_body( # Update issue body via GitHub API PATCH url = f"{self.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number}" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -1969,7 +1971,7 @@ def sync_status_to_github( # Get current issue to retrieve existing labels url = f"{self.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number}" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -1987,7 +1989,9 @@ def sync_status_to_github( patch_url = f"{self.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number}" patch_payload = {"labels": all_labels} - self._request_with_retry(lambda: requests.patch(patch_url, json=patch_payload, headers=headers, timeout=30)) + self._request_with_retry( + lambda: requests.patch(patch_url, json=cast(Any, patch_payload), headers=headers, timeout=30) + ) return { "issue_number": current_issue.get("number", issue_number), # Use API response number (int) @@ -2623,7 +2627,7 @@ def _fetch_backlog_item_by_id(self, issue_id: str) -> BacklogItem | None: return None url = f"{self.base_url}/repos/{self.repo_owner}/{self.repo_name}/issues/{normalized_id}" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -2761,7 +2765,7 @@ def _issue_relationship_edges(self, issue: dict[str, Any], issue_id: str) -> lis @beartype def _github_graphql(self, query: str, variables: dict[str, Any]) -> dict[str, Any]: """Execute GitHub GraphQL request and return `data` payload.""" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github+json", } @@ -2976,14 +2980,14 @@ def create_issue(self, project_id: str, payload: dict[str, Any]) -> dict[str, An issue_type, str(payload.get("priority") or "").strip(), payload.get("story_points") ) url = f"{self.base_url}/repos/{owner}/{repo}/issues" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } response = self._request_with_retry( lambda: requests.post( url, - json={"title": title, "body": body, "labels": labels}, + json=cast(Any, {"title": title, "body": body, "labels": labels}), headers=headers, timeout=30, ), @@ -3245,7 +3249,7 @@ def update_backlog_item(self, item: BacklogItem, update_fields: list[str] | None issue_number = int(item.id) url = f"{self.base_url}/repos/{self.repo_owner}/{self.repo_name}/issues/{issue_number}" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {self.api_token}", "Accept": "application/vnd.github.v3+json", } diff --git a/src/specfact_cli/cli.py b/src/specfact_cli/cli.py index 3d9271d7..7264ec22 100644 --- a/src/specfact_cli/cli.py +++ b/src/specfact_cli/cli.py @@ -698,6 +698,8 @@ def get_command(self, ctx: click.Context, cmd_name: str) -> click.Command | None real_group = self._get_real_click_group() if real_group is not None: return real_group.get_command(ctx, cmd_name) + if self._lazy_cmd_name in KNOWN_BUNDLE_GROUP_OR_SHIM_NAMES: + return self._delegate_cmd return None def _get_real_click_group(self) -> click.Group | None: diff --git a/src/specfact_cli/modules/module_registry/module-package.yaml b/src/specfact_cli/modules/module_registry/module-package.yaml index 29c594bf..ad1a954d 100644 --- a/src/specfact_cli/modules/module_registry/module-package.yaml +++ b/src/specfact_cli/modules/module_registry/module-package.yaml @@ -1,5 +1,5 @@ name: module-registry -version: 0.1.24 +version: 0.1.26 commands: - module category: core diff --git a/src/specfact_cli/modules/module_registry/src/commands.py b/src/specfact_cli/modules/module_registry/src/commands.py index b1be2ee3..1e199e2c 100644 --- a/src/specfact_cli/modules/module_registry/src/commands.py +++ b/src/specfact_cli/modules/module_registry/src/commands.py @@ -28,7 +28,12 @@ from specfact_cli.registry.custom_registries import add_registry, fetch_all_indexes, list_registries, remove_registry from specfact_cli.registry.help_cache import run_discovery_and_write_cache from specfact_cli.registry.marketplace_client import fetch_registry_index -from specfact_cli.registry.module_discovery import discover_all_modules, discover_all_modules_for_project +from specfact_cli.registry.module_discovery import ( + DiscoveredModule, + discover_all_modules, + discover_all_modules_for_project, + discover_all_modules_for_project_with_shadowed, +) from specfact_cli.registry.module_installer import ( REGISTRY_ID_FILE, USER_MODULES_ROOT, @@ -1066,6 +1071,107 @@ def _print_bundled_available_table(available: list[ModulePackageMetadata]) -> No console.print("[dim]Install bundled modules into project scope: specfact module init --scope project[/dim]") +def _doctor_entry_matches(entry: DiscoveredModule, module_id: str | None) -> bool: + if module_id is None: + return True + requested = module_id.strip() + if not requested: + return True + discovered_id = entry.metadata.name + if requested == discovered_id: + return True + if "/" in requested: + return False + return requested.rsplit("/", 1)[-1] == discovered_id.rsplit("/", 1)[-1] + + +def _doctor_dev_roots() -> list[tuple[str, str]]: + roots: list[tuple[str, str]] = [] + for env_name in ("SPECFACT_MODULES_REPO", "SPECFACT_CLI_MODULES_REPO"): + value = os.environ.get(env_name, "").strip() + if value: + roots.append((env_name, value)) + for raw_root in os.environ.get("SPECFACT_MODULES_ROOTS", "").split(os.pathsep): + value = raw_root.strip() + if value: + roots.append(("SPECFACT_MODULES_ROOTS", value)) + return roots + + +def _doctor_status( + entry: DiscoveredModule, + seen_by_module_id: set[str], + enabled_state: dict[str, dict[str, Any]], +) -> str: + module_id = entry.metadata.name + if module_id in seen_by_module_id: + return "shadowed" + seen_by_module_id.add(module_id) + if enabled_state.get(module_id, {}).get("enabled", True) is False: + return "disabled" + return "effective" + + +def _print_doctor_recovery(entries: list[tuple[DiscoveredModule, str]]) -> None: + for entry, status in entries: + if status != "shadowed" or entry.source != "user": + continue + console.print(f"[yellow]Recovery:[/yellow] specfact module uninstall {entry.metadata.name} --scope user") + + +@app.command(name="doctor") +@beartype +@require(_module_id_optional_nonempty, "module_id must be non-empty if provided") +def doctor( + module_id: str | None = typer.Argument(None, help="Optional module id to inspect"), + repo: Path | None = typer.Option(None, "--repo", help="Repository path for project scope (default: current dir)"), +) -> None: + """Diagnose module scope, shadowing, and development source roots.""" + repo_path = (repo or Path.cwd()).resolve() + discovered = [ + entry + for entry in discover_all_modules_for_project_with_shadowed(repo_path) + if _doctor_entry_matches(entry, module_id) + ] + if not discovered: + console.print("[yellow]No matching modules discovered.[/yellow]") + else: + state = read_modules_state() + seen_by_module_id: set[str] = set() + rows: list[tuple[DiscoveredModule, str]] = [] + table = Table(title="Module Doctor") + table.add_column("Module", style="cyan") + table.add_column("Version", style="magenta") + table.add_column("Status", style="yellow") + table.add_column("Origin", style="blue") + table.add_column("Enabled", style="green") + table.add_column("Path", overflow="fold") + for entry in discovered: + status = _doctor_status(entry, seen_by_module_id, state) + enabled = state.get(entry.metadata.name, {}).get("enabled", True) is not False + rows.append((entry, status)) + table.add_row( + entry.metadata.name, + entry.metadata.version, + status, + entry.source, + "yes" if enabled else "no", + str(entry.package_dir), + ) + console.print(table) + _print_doctor_recovery(rows) + + dev_roots = _doctor_dev_roots() + if not dev_roots: + return + dev_table = Table(title="Development Source Roots") + dev_table.add_column("Source", style="cyan") + dev_table.add_column("Path", overflow="fold") + for source, path in dev_roots: + dev_table.add_row(source, path) + console.print(dev_table) + + @app.command(name="list") @beartype @require( diff --git a/src/specfact_cli/registry/module_installer.py b/src/specfact_cli/registry/module_installer.py index 8b5382d1..2c2cedce 100644 --- a/src/specfact_cli/registry/module_installer.py +++ b/src/specfact_cli/registry/module_installer.py @@ -19,8 +19,8 @@ import yaml from beartype import beartype from icontract import ensure, require -from packaging.specifiers import SpecifierSet -from packaging.version import Version +from packaging.specifiers import InvalidSpecifier, SpecifierSet +from packaging.version import InvalidVersion, Version from specfact_cli import __version__ as cli_version from specfact_cli.common import get_bridge_logger @@ -74,6 +74,12 @@ class _BundleDepsInstallContext: logger: logging.Logger +@dataclass(frozen=True, slots=True) +class _BundleDependencySpec: + module_id: str + version_specifier: str = "" + + @beartype def _path_is_under_user_modules_install_tree(module_dir: Path) -> bool: """True when *module_dir* resolves under :data:`USER_MODULES_ROOT` (``--scope user`` tree).""" @@ -189,11 +195,18 @@ def _extract_bundle_dependencies(metadata: dict[str, Any]) -> list[str]: Supports both plain string entries ("namespace/name") and versioned object entries ({"id": "namespace/name", "version": ">=x.y.z"}). """ + return [dependency.module_id for dependency in _extract_bundle_dependency_specs(metadata)] + + +@beartype +def _extract_bundle_dependency_specs(metadata: dict[str, Any]) -> list[_BundleDependencySpec]: + """Extract validated bundle dependency ids and optional version specifiers.""" raw_dependencies = metadata.get("bundle_dependencies", []) if not isinstance(raw_dependencies, list): return [] - dependencies: list[str] = [] + dependencies: list[_BundleDependencySpec] = [] for index, value in enumerate(raw_dependencies): + version_specifier = "" if isinstance(value, dict): entry = cast(dict[str, Any], value) raw_id = entry.get("id") @@ -203,6 +216,7 @@ def _extract_bundle_dependencies(metadata: dict[str, Any]) -> list[str]: f"(invalid manifest; got {value!r})" ) dep = str(raw_id).strip() + version_specifier = str(entry.get("version") or entry.get("version_specifier") or "").strip() else: dep = str(value).strip() if not dep: @@ -210,7 +224,7 @@ def _extract_bundle_dependencies(metadata: dict[str, Any]) -> list[str]: f"bundle_dependencies[{index}]: string entry must be non-empty (invalid manifest; got {value!r})" ) _validate_marketplace_namespace_format(dep) - dependencies.append(dep) + dependencies.append(_BundleDependencySpec(dep, version_specifier)) return dependencies @@ -228,6 +242,39 @@ def _installed_dependency_version(manifest_path: Path) -> str: return "unknown" +@beartype +def _dependency_version_satisfies(installed_version: str, version_specifier: str) -> bool: + """Return True when installed_version satisfies version_specifier or no specifier is declared.""" + if not version_specifier: + return True + try: + return Version(installed_version) in SpecifierSet(version_specifier) + except (InvalidSpecifier, InvalidVersion) as exc: + get_bridge_logger(__name__).debug( + "Ignoring malformed bundle dependency version comparison: installed_version=%r, " + "version_specifier=%r, error=%s", + installed_version, + version_specifier, + exc, + ) + return False + + +@beartype +def _raise_if_dependency_version_mismatch( + dependency_module_id: str, + installed_version: str, + version_specifier: str, +) -> None: + if _dependency_version_satisfies(installed_version, version_specifier): + return + raise ValueError( + f"Dependency {dependency_module_id} requires {version_specifier}, " + f"but installed version is {installed_version}. " + f"Reinstall or upgrade the dependency in the same module scope." + ) + + @beartype def _integrity_debug_details_enabled() -> bool: """Return True when verbose integrity diagnostics should be shown.""" @@ -822,15 +869,27 @@ def _metadata_obj_from_install_dict(metadata: dict[str, Any], manifest_module_na def _install_bundle_dependencies_for_module(module_id: str, ctx: _BundleDepsInstallContext) -> None: - for dependency_module_id in _extract_bundle_dependencies(ctx.metadata): + for dependency in _extract_bundle_dependency_specs(ctx.metadata): + dependency_module_id = dependency.module_id if dependency_module_id == module_id: continue dependency_name = dependency_module_id.split("/", 1)[1] dependency_manifest = ctx.target_root / dependency_name / "module-package.yaml" if dependency_manifest.exists(): - dependency_version = _installed_dependency_version(dependency_manifest) - ctx.logger.info("Dependency %s already satisfied (version %s)", dependency_module_id, dependency_version) - continue + dependency_id_file = dependency_manifest.parent / REGISTRY_ID_FILE + if dependency_id_file.exists(): + installed_registry_id = dependency_id_file.read_text(encoding="utf-8").strip() + if installed_registry_id == dependency_module_id: + dependency_version = _installed_dependency_version(dependency_manifest) + _raise_if_dependency_version_mismatch( + dependency_module_id, + dependency_version, + dependency.version_specifier, + ) + ctx.logger.info( + "Dependency %s already satisfied (version %s)", dependency_module_id, dependency_version + ) + continue try: install_module( dependency_module_id, @@ -844,6 +903,12 @@ def _install_bundle_dependencies_for_module(module_id: str, ctx: _BundleDepsInst ) except Exception as dep_exc: raise ValueError(f"Dependency install failed for {dependency_module_id}: {dep_exc}") from dep_exc + dependency_version = _installed_dependency_version(dependency_manifest) + _raise_if_dependency_version_mismatch( + dependency_module_id, + dependency_version, + dependency.version_specifier, + ) try: all_metas = [e.metadata for e in discover_all_modules()] all_metas.append(ctx.metadata_obj) diff --git a/src/specfact_cli/registry/module_packages.py b/src/specfact_cli/registry/module_packages.py index 0a07dd7a..314bd0df 100644 --- a/src/specfact_cli/registry/module_packages.py +++ b/src/specfact_cli/registry/module_packages.py @@ -25,7 +25,7 @@ from beartype import beartype from icontract import ensure, require -from packaging.specifiers import SpecifierSet +from packaging.specifiers import InvalidSpecifier, SpecifierSet from packaging.version import InvalidVersion, Version from specfact_cli import __version__ as cli_version @@ -85,6 +85,7 @@ class _ModuleIntegrityContext: @dataclass class _PackageRegistrationContext: enabled_map: dict[str, bool] + module_versions: dict[str, str] allow_unsigned: bool is_test_mode: bool logger: Any @@ -347,6 +348,12 @@ def _validated_schema_extensions_from_raw(raw: dict[str, Any]) -> list[SchemaExt def _apply_category_manifest_postprocess(meta: ModulePackageMetadata) -> ModulePackageMetadata: if meta.category is None: + inferred_category = _infer_category_from_legacy_group(meta) + if inferred_category is not None: + meta.category = inferred_category + meta = normalize_legacy_bundle_group_command(meta) + validate_module_category_manifest(meta) + return meta logger = get_bridge_logger(__name__) logger.warning( "Module '%s' has no category field; mounting as flat top-level command.", @@ -358,6 +365,22 @@ def _apply_category_manifest_postprocess(meta: ModulePackageMetadata) -> ModuleP return meta +def _infer_category_from_legacy_group(meta: ModulePackageMetadata) -> str | None: + """Infer category for legacy manifests that already declare a group command.""" + group = meta.bundle_group_command + if group is None: + return None + group_to_category = { + "project": "project", + "backlog": "backlog", + "code": "codebase", + "codebase": "codebase", + "spec": "spec", + "govern": "govern", + } + return group_to_category.get(group) + + def _raw_opt_str(raw: dict[str, Any], key: str) -> str | None: v = raw.get(key) return str(v) if v else None @@ -480,18 +503,86 @@ def _check_core_compatibility(meta: Any, current_cli_version: str) -> bool: def _validate_module_dependencies( meta: Any, enabled_map: dict[str, bool], + module_versions: dict[str, str] | None = None, ) -> tuple[bool, list[str]]: """Validate that declared dependencies exist and are enabled.""" - missing: list[str] = [] module_dependencies = getattr(meta, "module_dependencies", []) if not isinstance(module_dependencies, list): return False, ["invalid metadata: module_dependencies must be a list"] + versioned_dependencies = getattr(meta, "module_dependencies_versioned", []) + if not isinstance(versioned_dependencies, list): + return False, ["invalid metadata: module_dependencies_versioned must be a list"] + missing = _missing_plain_module_dependencies(module_dependencies, enabled_map) + missing.extend(_missing_versioned_module_dependencies(versioned_dependencies, enabled_map, module_versions)) + return len(missing) == 0, missing + + +@beartype +def _missing_plain_module_dependencies(module_dependencies: list[Any], enabled_map: dict[str, bool]) -> list[str]: + """Return missing or disabled plain module dependencies.""" + missing: list[str] = [] for dep_id in module_dependencies: if dep_id not in enabled_map: missing.append(f"{dep_id} (not found)") elif not enabled_map[dep_id]: missing.append(f"{dep_id} (disabled)") - return len(missing) == 0, missing + return missing + + +@beartype +def _missing_versioned_module_dependencies( + versioned_dependencies: list[Any], + enabled_map: dict[str, bool], + module_versions: dict[str, str] | None, +) -> list[str]: + """Return missing, disabled, or incompatible versioned module dependencies.""" + missing: list[str] = [] + for dep in versioned_dependencies: + problem = _versioned_module_dependency_problem(dep, enabled_map, module_versions) + if problem is None: + continue + missing.append(problem) + return missing + + +@beartype +def _versioned_module_dependency_problem( + dep: Any, + enabled_map: dict[str, bool], + module_versions: dict[str, str] | None, +) -> str | None: + """Return the validation problem for one versioned dependency, if any.""" + dep_name = str(getattr(dep, "name", "")).strip() + version_specifier = str(getattr(dep, "version_specifier", "") or "").strip() + if not dep_name: + return None + if dep_name not in enabled_map: + return f"{dep_name} (not found)" + if not enabled_map[dep_name]: + return f"{dep_name} (disabled)" + if not version_specifier: + return None + if module_versions is None: + return None + if dep_name not in module_versions: + return f"{dep_name} (requires {version_specifier}, found unknown)" + found_version = module_versions.get(dep_name) + if not found_version: + return f"{dep_name} (requires {version_specifier}, found unknown)" + try: + if Version(found_version) not in SpecifierSet(version_specifier): + return f"{dep_name} (requires {version_specifier}, found {found_version})" + except (InvalidVersion, InvalidSpecifier) as exc: + get_bridge_logger(__name__).debug( + "Ignoring malformed module dependency version comparison: dep_name=%r, " + "version_specifier=%r, found_version=%r, error=%s", + dep_name, + version_specifier, + found_version, + exc, + ) + return None + return None @beartype @@ -533,13 +624,14 @@ def validate_enable_safe( Empty dict means all enables are dependency-safe in the effective map. """ meta_by_name: dict[str, ModulePackageMetadata] = {meta.name: meta for _package_dir, meta in packages} + module_versions = {meta.name: meta.version for _package_dir, meta in packages} blocked: dict[str, list[str]] = {} for mid in enable_ids: meta = meta_by_name.get(mid) if meta is None: blocked[mid] = ["module not found"] continue - deps_ok, missing = _validate_module_dependencies(meta, enabled_map) + deps_ok, missing = _validate_module_dependencies(meta, enabled_map, module_versions) if not deps_ok: blocked[mid] = missing return blocked @@ -562,7 +654,10 @@ def expand_disable_with_dependents( reverse_deps: dict[str, set[str]] = {} for _package_dir, meta in packages: name = meta.name - for dep in meta.module_dependencies: + for dep in [ + *list(meta.module_dependencies), + *[versioned.name for versioned in meta.module_dependencies_versioned if versioned.name], + ]: reverse_deps.setdefault(dep, set()).add(name) expanded: set[str] = set(disable_ids) @@ -592,7 +687,13 @@ def expand_enable_with_dependencies( Used by --force mode so enabling a module also enables required upstream dependency providers. """ - dep_map: dict[str, list[str]] = {meta.name: list(meta.module_dependencies) for _package_dir, meta in packages} + dep_map: dict[str, list[str]] = { + meta.name: [ + *list(meta.module_dependencies), + *[dep.name for dep in meta.module_dependencies_versioned if dep.name], + ] + for _package_dir, meta in packages + } expanded: set[str] = set(enable_ids) queue = list(enable_ids) while queue: @@ -1421,7 +1522,7 @@ def _register_one_package_if_eligible(package_dir: Path, meta: Any, reg: _Packag if not compatible: reg.skipped.append((meta.name, f"requires {meta.core_compatibility}, cli is {cli_version}")) return - deps_ok, missing = _validate_module_dependencies(meta, reg.enabled_map) + deps_ok, missing = _validate_module_dependencies(meta, reg.enabled_map, reg.module_versions) if not deps_ok: reg.skipped.append((meta.name, f"missing dependencies: {', '.join(missing)}")) return @@ -1515,6 +1616,7 @@ def register_module_package_commands( if not packages: return discovered_list: list[tuple[str, str]] = [(meta.name, meta.version) for _dir, meta in packages] + module_versions = {meta.name: meta.version for _dir, meta in packages} state = read_modules_state() enabled_map = merge_module_state(discovered_list, state, enable_ids, disable_ids) logger = get_bridge_logger(__name__) @@ -1531,6 +1633,7 @@ def register_module_package_commands( } reg_ctx = _PackageRegistrationContext( enabled_map=enabled_map, + module_versions=module_versions, allow_unsigned=allow_unsigned, is_test_mode=is_test_mode, logger=logger, diff --git a/src/specfact_cli/registry/module_state.py b/src/specfact_cli/registry/module_state.py index 2c43ce03..45b6d2e4 100644 --- a/src/specfact_cli/registry/module_state.py +++ b/src/specfact_cli/registry/module_state.py @@ -93,6 +93,10 @@ def find_dependents( if not enabled_map.get(name, True): continue module_dependencies = list(getattr(meta, "module_dependencies", [])) - if module_id in module_dependencies: + versioned_dependencies = [ + str(getattr(dependency, "name", "")) + for dependency in list(getattr(meta, "module_dependencies_versioned", [])) + ] + if module_id in {*module_dependencies, *versioned_dependencies}: dependents.append(name) return sorted(dependents) diff --git a/src/specfact_cli/sync/bridge_sync.py b/src/specfact_cli/sync/bridge_sync.py index 5678acf9..57706db7 100644 --- a/src/specfact_cli/sync/bridge_sync.py +++ b/src/specfact_cli/sync/bridge_sync.py @@ -2445,7 +2445,7 @@ def _search_existing_github_issue( # Search for issues containing the change proposal ID in the footer search_url = f"{adapter_instance_any.base_url}/search/issues" search_query = f'repo:{repo_owner}/{repo_name} "OpenSpec Change Proposal: `{change_id}`" in:body' - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {adapter_instance_any.api_token}", "Accept": "application/vnd.github.v3+json", } @@ -2681,7 +2681,7 @@ def _fetch_github_issue_sync_state( import requests url = f"{adapter_inst_any.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_num}" - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {adapter_inst_any.api_token}", "Accept": "application/vnd.github.v3+json", } diff --git a/src/specfact_cli/validators/change_proposal_integration.py b/src/specfact_cli/validators/change_proposal_integration.py index 322e8ba5..34fae3eb 100644 --- a/src/specfact_cli/validators/change_proposal_integration.py +++ b/src/specfact_cli/validators/change_proposal_integration.py @@ -16,7 +16,7 @@ import re from contextlib import suppress from pathlib import Path -from typing import Any +from typing import Any, cast import requests from beartype import beartype @@ -166,7 +166,7 @@ def _post_github_validation_comment_and_labels( with suppress(Exception): url = f"{adapter_instance.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number_int}" # type: ignore[attr-defined] - headers = { + headers: dict[str, str | bytes] = { "Authorization": f"token {adapter_instance.api_token}", # type: ignore[attr-defined] "Accept": "application/vnd.github.v3+json", } @@ -188,7 +188,7 @@ def _post_github_validation_comment_and_labels( all_labels = [*current_labels, "validation-failed"] patch_url = f"{adapter_instance.base_url}/repos/{repo_owner}/{repo_name}/issues/{issue_number_int}" # type: ignore[attr-defined] patch_payload = {"labels": all_labels} - patch_response = requests.patch(patch_url, json=patch_payload, headers=headers, timeout=30) + patch_response = requests.patch(patch_url, json=cast(Any, patch_payload), headers=headers, timeout=30) patch_response.raise_for_status() diff --git a/tests/unit/adapters/test_ado_backlog_adapter.py b/tests/unit/adapters/test_ado_backlog_adapter.py index 257604f5..9c657e64 100644 --- a/tests/unit/adapters/test_ado_backlog_adapter.py +++ b/tests/unit/adapters/test_ado_backlog_adapter.py @@ -773,7 +773,7 @@ def test_auth_headers_basic_pat(self) -> None: adapter.auth_scheme = "basic" headers = adapter._auth_headers() assert "Authorization" in headers - assert headers["Authorization"].startswith("Basic ") + assert str(headers["Authorization"]).startswith("Basic ") @beartype def test_auth_headers_bearer_oauth(self) -> None: @@ -782,7 +782,7 @@ def test_auth_headers_bearer_oauth(self) -> None: adapter.auth_scheme = "bearer" headers = adapter._auth_headers() assert "Authorization" in headers - assert headers["Authorization"].startswith("Bearer ") + assert str(headers["Authorization"]).startswith("Bearer ") @beartype def test_auth_headers_no_token(self) -> None: diff --git a/tests/unit/modules/module_registry/test_commands.py b/tests/unit/modules/module_registry/test_commands.py index ff6b932e..37a63ce9 100644 --- a/tests/unit/modules/module_registry/test_commands.py +++ b/tests/unit/modules/module_registry/test_commands.py @@ -9,6 +9,7 @@ from specfact_cli.models.module_package import ModulePackageMetadata from specfact_cli.modules.module_registry.src.commands import app +from specfact_cli.registry.module_discovery import DiscoveredModule from specfact_cli.registry.module_installer import USER_MODULES_ROOT, InstallModuleOptions @@ -70,6 +71,88 @@ def test_install_command_rejects_invalid_module_id(monkeypatch) -> None: assert "Invalid module id" in result.stdout +def test_doctor_reports_effective_and_shadowed_duplicate_modules(monkeypatch, tmp_path: Path) -> None: + project_dir = tmp_path / "repo" / ".specfact" / "modules" / "specfact-codebase" + user_dir = tmp_path / "user-modules" / "specfact-codebase" + project_dir.mkdir(parents=True) + user_dir.mkdir(parents=True) + entries = [ + DiscoveredModule( + project_dir, + ModulePackageMetadata(name="nold-ai/specfact-codebase", version="0.41.0", commands=["code"]), + "project", + ), + DiscoveredModule( + user_dir, + ModulePackageMetadata(name="nold-ai/specfact-codebase", version="0.40.0", commands=["code"]), + "user", + ), + ] + + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.discover_all_modules_for_project_with_shadowed", + lambda _repo: entries, + ) + monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.read_modules_state", dict) + + result = runner.invoke(app, ["doctor", "nold-ai/specfact-codebase", "--repo", str(tmp_path / "repo")]) + + assert result.exit_code == 0 + assert "effective" in result.stdout + assert "shadowed" in result.stdout + assert "0.41.0" in result.stdout + assert "0.40.0" in result.stdout + assert "specfact module uninstall nold-ai/specfact-codebase --scope user" in result.stdout + + +def test_doctor_fully_qualified_module_id_matches_exact_namespace(monkeypatch, tmp_path: Path) -> None: + entries = [ + DiscoveredModule( + tmp_path / "repo" / ".specfact" / "modules" / "foo", + ModulePackageMetadata(name="n1/foo", version="1.0.0", commands=["foo"]), + "project", + ), + DiscoveredModule( + tmp_path / "user-modules" / "foo", + ModulePackageMetadata(name="n2/foo", version="2.0.0", commands=["foo"]), + "user", + ), + ] + + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.discover_all_modules_for_project_with_shadowed", + lambda _repo: entries, + ) + monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.read_modules_state", dict) + + result = runner.invoke(app, ["doctor", "n1/foo", "--repo", str(tmp_path / "repo")]) + + assert result.exit_code == 0 + assert "n1/foo" in result.stdout + assert "1.0.0" in result.stdout + assert "n2/foo" not in result.stdout + assert "2.0.0" not in result.stdout + + +def test_doctor_reports_configured_development_source_roots(monkeypatch, tmp_path: Path) -> None: + modules_repo = tmp_path / "specfact-cli-modules" + extra_root = tmp_path / "extra-modules" + monkeypatch.setenv("SPECFACT_MODULES_REPO", str(modules_repo)) + monkeypatch.setenv("SPECFACT_MODULES_ROOTS", str(extra_root)) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.discover_all_modules_for_project_with_shadowed", + lambda _repo: [], + ) + + result = runner.invoke(app, ["doctor", "--repo", str(tmp_path)]) + + assert result.exit_code == 0 + assert "Development Source Roots" in result.stdout + assert "SPECFACT_MODULES_REPO" in result.stdout + assert "specfact-cli-modules" in result.stdout + assert "extra-modules" in result.stdout + + def test_install_command_skips_when_module_already_available_locally(monkeypatch, tmp_path: Path) -> None: class _Meta: name = "bundle-mapper" @@ -479,7 +562,7 @@ class _Entry: assert "local module roots" in result.stdout -def test_uninstall_command_namespace_input_normalizes_name(monkeypatch) -> None: +def test_uninstall_command_namespace_input_normalizes_name(monkeypatch, tmp_path: Path) -> None: class _Meta: name = "bundle-mapper" @@ -487,6 +570,7 @@ class _Entry: metadata = _Meta() source = "custom" + monkeypatch.chdir(tmp_path) monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", lambda: [_Entry()]) result = runner.invoke(app, ["uninstall", "specfact/bundle-mapper"]) diff --git a/tests/unit/registry/test_module_installer.py b/tests/unit/registry/test_module_installer.py index 0475c089..62755774 100644 --- a/tests/unit/registry/test_module_installer.py +++ b/tests/unit/registry/test_module_installer.py @@ -140,6 +140,7 @@ def test_install_module_logs_satisfied_dependencies_without_warning(monkeypatch, "name: specfact-project\nversion: '0.40.16'\ncommands: [project]\n", encoding="utf-8", ) + (dependency_dir / module_installer.REGISTRY_ID_FILE).write_text("nold-ai/specfact-project", encoding="utf-8") installed = install_module( "nold-ai/specfact-backlog", @@ -155,6 +156,91 @@ def test_install_module_logs_satisfied_dependencies_without_warning(monkeypatch, ) +def test_install_module_reinstalls_dependency_when_registry_id_mismatch(monkeypatch, tmp_path: Path) -> None: + root_tarball = _create_module_tarball( + tmp_path / "root", + "backlog", + bundle_dependencies=["nold-ai/specfact-project"], + ) + dependency_tarball = _create_module_tarball(tmp_path / "dep", "project") + downloaded_modules: list[str] = [] + + def _download(module_id: str, **_kwargs: object) -> Path: + downloaded_modules.append(module_id) + if module_id == "nold-ai/specfact-backlog": + return root_tarball + if module_id == "nold-ai/specfact-project": + return dependency_tarball + raise AssertionError(f"Unexpected module download request: {module_id}") + + monkeypatch.setattr("specfact_cli.registry.module_installer.download_module", _download) + monkeypatch.setattr("specfact_cli.registry.module_installer.verify_module_artifact", lambda *_args, **_kwargs: True) + monkeypatch.setattr( + "specfact_cli.registry.module_installer.ensure_publisher_trusted", lambda *_args, **_kwargs: None + ) + monkeypatch.setattr("specfact_cli.registry.module_installer.resolve_dependencies", lambda *_args, **_kwargs: None) + monkeypatch.setattr( + "specfact_cli.registry.module_installer.install_resolved_pip_requirements", lambda *_args, **_kwargs: None + ) + monkeypatch.setattr("specfact_cli.registry.module_installer.discover_all_modules", list) + + install_root = tmp_path / "marketplace-modules" + dependency_dir = install_root / "specfact-project" + dependency_dir.mkdir(parents=True, exist_ok=True) + (dependency_dir / "module-package.yaml").write_text( + "name: specfact-project\nversion: '0.40.16'\ncommands: [project]\n", + encoding="utf-8", + ) + (dependency_dir / module_installer.REGISTRY_ID_FILE).write_text("other-org/specfact-project", encoding="utf-8") + + with pytest.raises(ValueError, match="Module namespace collision"): + install_module( + "nold-ai/specfact-backlog", + InstallModuleOptions(install_root=install_root, reinstall=True), + ) + + assert downloaded_modules == ["nold-ai/specfact-backlog"] + + +def test_install_module_rejects_existing_bundle_dependency_version_mismatch(monkeypatch, tmp_path: Path) -> None: + module_dir = tmp_path / "review-pkg" / "specfact-code-review" + module_dir.mkdir(parents=True, exist_ok=True) + (module_dir / "module-package.yaml").write_text( + "name: specfact-code-review\n" + "version: '0.47.0'\n" + "commands: [code]\n" + 'core_compatibility: ">=0.1.0,<1.0.0"\n' + "bundle_dependencies:\n" + " - id: nold-ai/specfact-codebase\n" + " version: '>=0.41.0'\n", + encoding="utf-8", + ) + (module_dir / "src").mkdir(parents=True, exist_ok=True) + tarball = tmp_path / "review.tar.gz" + with tarfile.open(tarball, "w:gz") as archive: + archive.add(module_dir, arcname="specfact-code-review") + + monkeypatch.setattr("specfact_cli.registry.module_installer.download_module", lambda *_args, **_kwargs: tarball) + monkeypatch.setattr("specfact_cli.registry.module_installer.verify_module_artifact", lambda *_args, **_kwargs: True) + monkeypatch.setattr( + "specfact_cli.registry.module_installer.ensure_publisher_trusted", lambda *_args, **_kwargs: None + ) + + install_root = tmp_path / "marketplace-modules" + dependency_dir = install_root / "specfact-codebase" + dependency_dir.mkdir(parents=True, exist_ok=True) + (dependency_dir / "module-package.yaml").write_text( + "name: specfact-codebase\nversion: '0.40.9'\ncommands: [code]\n", + encoding="utf-8", + ) + + with pytest.raises(ValueError, match=r"nold-ai/specfact-codebase.*>=0.41.0.*0.40.9"): + install_module( + "nold-ai/specfact-code-review", + InstallModuleOptions(install_root=install_root, reinstall=True), + ) + + def test_install_module_rejects_archive_path_traversal(monkeypatch, tmp_path: Path) -> None: tarball = tmp_path / "unsafe.tar.gz" with tarfile.open(tarball, "w:gz") as archive: diff --git a/tests/unit/scripts/test_publish_module_bundle.py b/tests/unit/scripts/test_publish_module_bundle.py index e0a77b15..a428cded 100644 --- a/tests/unit/scripts/test_publish_module_bundle.py +++ b/tests/unit/scripts/test_publish_module_bundle.py @@ -102,6 +102,55 @@ def test_tarball_checksum_matches_generated_index_entry(tmp_path: Path, monkeypa assert checksum == entry["checksum_sha256"] +def test_publish_entry_serializes_bundle_dependency_objects_as_ids() -> None: + module = _load_script_module() + + entry = module._build_publish_entry( + { + "bundle_dependencies": [ + "nold-ai/specfact-project", + {"id": "nold-ai/specfact-codebase", "version": ">=0.41.0"}, + ] + }, + "nold-ai/specfact-review", + "0.47.0", + Path("specfact-review-0.47.0.tar.gz"), + "abc123", + ) + + assert entry["bundle_dependencies"] == ["nold-ai/specfact-project", "nold-ai/specfact-codebase"] + + +def test_publish_entry_rejects_malformed_bundle_dependency_object() -> None: + module = _load_script_module() + + with pytest.raises(ValueError, match="non-empty 'id'"): + module._build_publish_entry( + { + "bundle_dependencies": [ + {"version": "1.0"}, + ] + }, + "nold-ai/specfact-review", + "0.47.0", + Path("specfact-review-0.47.0.tar.gz"), + "abc123", + ) + + with pytest.raises(ValueError, match="non-empty 'id'"): + module._build_publish_entry( + { + "bundle_dependencies": [ + {"id": ""}, + ] + }, + "nold-ai/specfact-review", + "0.47.0", + Path("specfact-review-0.47.0.tar.gz"), + "abc123", + ) + + def test_tarball_has_no_path_traversal_entries(tmp_path: Path) -> None: module = _load_script_module() bundle_dir = _create_bundle_package(tmp_path, "specfact-codebase") diff --git a/tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py b/tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py index f8836b2c..d7ca5e1e 100644 --- a/tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py +++ b/tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py @@ -256,7 +256,7 @@ def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwa ) result = runner.invoke(module_app, ["upgrade", "backlog"]) assert result.exit_code == 0 - out = (result.stdout or "") + (result.stderr or "") + out = result.output or "" assert "unavailable" in out.lower() or "network error" in out.lower() diff --git a/tests/unit/specfact_cli/registry/test_module_dependencies.py b/tests/unit/specfact_cli/registry/test_module_dependencies.py index 81f4eed9..fffb7f09 100644 --- a/tests/unit/specfact_cli/registry/test_module_dependencies.py +++ b/tests/unit/specfact_cli/registry/test_module_dependencies.py @@ -4,8 +4,10 @@ from pathlib import Path +from specfact_cli.models.module_package import VersionedModuleDependency from specfact_cli.registry.module_packages import ( ModulePackageMetadata, + _missing_versioned_module_dependencies, _validate_module_dependencies, expand_disable_with_dependents, expand_enable_with_dependencies, @@ -46,6 +48,102 @@ def test_validate_module_dependencies_detects_missing_and_disabled() -> None: assert "ghost (not found)" in missing +def test_validate_module_dependencies_detects_version_mismatch() -> None: + meta = ModulePackageMetadata( + name="review", + version="0.47.0", + commands=["code"], + module_dependencies_versioned=[ + VersionedModuleDependency(name="codebase", version_specifier=">=0.41.0"), + ], + ) + + ok, missing = _validate_module_dependencies( + meta, + {"review": True, "codebase": True}, + {"review": "0.47.0", "codebase": "0.40.9"}, + ) + + assert ok is False + assert "codebase (requires >=0.41.0, found 0.40.9)" in missing + + +def test_validate_module_dependencies_accepts_satisfied_versioned_dependency() -> None: + meta = ModulePackageMetadata( + name="review", + version="0.47.0", + commands=["code"], + module_dependencies_versioned=[ + VersionedModuleDependency(name="codebase", version_specifier=">=0.41.0"), + ], + ) + + ok, missing = _validate_module_dependencies( + meta, + {"review": True, "codebase": True}, + {"review": "0.47.0", "codebase": "0.41.2"}, + ) + + assert ok is True + assert missing == [] + + +def test_validate_module_dependencies_skips_version_compare_without_versions() -> None: + meta = ModulePackageMetadata( + name="review", + version="0.47.0", + commands=["code"], + module_dependencies_versioned=[ + VersionedModuleDependency(name="codebase", version_specifier=">=0.41.0"), + ], + ) + + ok, missing = _validate_module_dependencies(meta, {"review": True, "codebase": True}) + + assert ok is True + assert missing == [] + + +def test_missing_versioned_module_dependencies_handles_specifier_forms() -> None: + enabled_map = { + "exact": True, + "range": True, + "range_bad": True, + "caret": True, + "tilde": True, + "missing": True, + "disabled": False, + } + module_versions = { + "exact": "1.2.3", + "range": "1.4.0", + "range_bad": "2.0.0", + "caret": "1.2.3", + "tilde": "1.2.3", + } + + missing = _missing_versioned_module_dependencies( + [ + VersionedModuleDependency(name="exact", version_specifier="==1.2.3"), + VersionedModuleDependency(name="range", version_specifier=">=1.0,<2.0"), + VersionedModuleDependency(name="range_bad", version_specifier=">=1.0,<2.0"), + VersionedModuleDependency(name="caret", version_specifier="^1.2.0"), + VersionedModuleDependency(name="tilde", version_specifier="~1.2.0"), + VersionedModuleDependency(name="missing", version_specifier=">=1.0"), + VersionedModuleDependency(name="disabled", version_specifier=">=1.0"), + VersionedModuleDependency(name="ghost", version_specifier=">=1.0"), + ], + enabled_map, + module_versions, + ) + + assert "range_bad (requires >=1.0,<2.0, found 2.0.0)" in missing + assert "missing (requires >=1.0, found unknown)" in missing + assert "disabled (disabled)" in missing + assert "ghost (not found)" in missing + assert not any(item.startswith(("exact ", "range ", "caret ", "tilde ")) for item in missing) + + def test_validate_disable_safe_blocks_enabled_dependents() -> None: packages = [ _pkg("plan", ["sync"]), diff --git a/tests/unit/specfact_cli/registry/test_module_packages.py b/tests/unit/specfact_cli/registry/test_module_packages.py index f4dd7bc8..b788ae67 100644 --- a/tests/unit/specfact_cli/registry/test_module_packages.py +++ b/tests/unit/specfact_cli/registry/test_module_packages.py @@ -655,6 +655,40 @@ def _fake_loader(_package_dir: Path, package_name: str, _cmd_name: str): assert "ext_cmd" in command_names +def test_registration_skips_package_with_unsatisfied_versioned_module_dependency( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Registration should skip modules whose versioned module dependency is not satisfied.""" + packages = [ + ( + tmp_path / "codebase", + ModulePackageMetadata(name="codebase", version="0.40.9", commands=["codebase"]), + ), + ( + tmp_path / "review", + ModulePackageMetadata( + name="review", + version="0.47.0", + commands=["review"], + module_dependencies_versioned=[ + VersionedModuleDependency(name="codebase", version_specifier=">=0.41.0"), + ], + ), + ), + ] + + monkeypatch.setattr(module_packages_impl, "discover_all_package_metadata", lambda: packages) + monkeypatch.setattr(module_packages_impl, "verify_module_artifact", lambda _dir, _meta, allow_unsigned=False: True) + monkeypatch.setattr(module_packages_impl, "read_modules_state", dict) + monkeypatch.setattr(module_packages_impl, "_check_protocol_compliance_from_source", lambda *_args, **_kwargs: []) + + register_module_package_commands() + + names = set(CommandRegistry.list_commands()) + assert "codebase" in names + assert "review" not in names + + def test_mount_installed_groups_preserves_bundle_native_group_command( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py b/tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py index 700c20ea..d6070370 100644 --- a/tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py +++ b/tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py @@ -10,7 +10,8 @@ import pytest -from specfact_cli.registry.module_installer import _extract_bundle_dependencies +from specfact_cli.registry import module_installer +from specfact_cli.registry.module_installer import _dependency_version_satisfies, _extract_bundle_dependencies # ── Scenario: Registry entry declares a versioned bundle dependency ─────────── @@ -74,6 +75,25 @@ def test_extract_bundle_dependencies_rejects_empty_string_entry() -> None: _extract_bundle_dependencies(metadata) +def test_dependency_version_satisfies_logs_malformed_inputs( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + logger = module_installer.logging.getLogger("test.module_installer.version") + logger.propagate = True + monkeypatch.setattr(module_installer, "get_bridge_logger", lambda _name: logger) + caplog.set_level("DEBUG") + + assert _dependency_version_satisfies("not-a-version", ">=1.0") is False + + assert "not-a-version" in caplog.text + assert ">=1.0" in caplog.text + + assert _dependency_version_satisfies("1.2.3", "not-a-specifier") is False + + assert "1.2.3" in caplog.text + assert "not-a-specifier" in caplog.text + + # ── core_compatibility actionable error ─────────────────────────────────────── diff --git a/tests/unit/validators/test_bundle_dependency_install.py b/tests/unit/validators/test_bundle_dependency_install.py index e9902ce2..26498770 100644 --- a/tests/unit/validators/test_bundle_dependency_install.py +++ b/tests/unit/validators/test_bundle_dependency_install.py @@ -8,7 +8,7 @@ import pytest -from specfact_cli.registry.module_installer import InstallModuleOptions, install_module +from specfact_cli.registry.module_installer import REGISTRY_ID_FILE, InstallModuleOptions, install_module def _create_module_tarball( @@ -143,6 +143,7 @@ def _download(module_id: str, version: str | None = None) -> Path: "name: specfact-project\nversion: '0.39.0'\ncommands: [project]\n", encoding="utf-8", ) + (dep_dir / REGISTRY_ID_FILE).write_text("nold-ai/specfact-project", encoding="utf-8") install_module("nold-ai/specfact-spec", InstallModuleOptions(install_root=install_root))