diff --git a/CHANGELOG.md b/CHANGELOG.md index 631e0709..fa870e97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,73 @@ All notable changes to this project will be documented in this file. --- +## [0.46.25] - 2026-05-17 + +### Fixed + +- **Dependency resolver compatibility**: pin `click` below the Typer/Semgrep + conflict boundary and move optional OpenTelemetry packages out of the base + install so Semgrep-compatible environments resolve cleanly. + +--- + +## [0.46.24] - 2026-05-17 + +### Fixed + +- **Module dependency review follow-up**: preserve versioned dependency + constraints in tests, fail closed for malformed dependency version checks, + and cover the large-bundle manifest persistence branch. + +--- + +## [0.46.23] - 2026-05-17 + +### Fixed + +- **Module dependency review hardening**: reject malformed published bundle + dependency IDs, tolerate malformed installed dependency version comparisons + consistently, and avoid unhashable dependency crashes in dependent lookup. +- **Bundle save stability**: avoid reused YAML writer state and keep very large + bundle manifest saves within the test timeout. + +--- + +## [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..a6c1d66d 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.25" 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" @@ -65,8 +65,9 @@ dependencies = [ # CLI framework # Typer 0.24+ requires click>=8.2.1 (generic Choice[...]); dev semgrep pins click~=8.1.8 — cap Typer. + "click>=8.1.8,<8.2", "typer>=0.20.0,<0.24", - "rich>=13.5.2,<13.6.0", # Compatible with semgrep (requires rich~=13.5.2) + "rich>=13.5.2,<16.0.0", # Semgrep allows rich>=13.5.2; keep below next major. "questionary>=2.0.1", # Interactive prompts with arrow key navigation # Template engine @@ -94,10 +95,6 @@ dependencies = [ # File system watching "watchdog>=6.0.0", - - # Telemetry (opt-in; only active when SPECFACT_TELEMETRY_OPT_IN=true) - "opentelemetry-sdk>=1.27.0", - "opentelemetry-exporter-otlp-proto-http>=1.27.0", ] [project.optional-dependencies] @@ -106,6 +103,11 @@ contracts = [ "hypothesis>=6.142.4", ] +telemetry = [ + "opentelemetry-sdk>=1.27.0", + "opentelemetry-exporter-otlp-proto-http>=1.27.0", +] + dev = [ "setuptools>=69.0.0,<82", "pytest>=8.4.2", diff --git a/scripts/publish-module.py b/scripts/publish-module.py old mode 100755 new mode 100644 index a676a9c8..f901ce15 --- a/scripts/publish-module.py +++ b/scripts/publish-module.py @@ -13,6 +13,7 @@ import sys import tarfile import tempfile +from collections.abc import Mapping from pathlib import Path from typing import Any, cast @@ -92,12 +93,13 @@ def _load_manifest(manifest_path: Path) -> dict[str, Any]: def _official_nold_publisher_manifest(manifest: dict[str, Any]) -> bool: """True when ``publisher`` matches shipped nold-ai in-repo bundles (slug ``name`` is allowed).""" pub = manifest.get("publisher") - if not isinstance(pub, dict): + if not isinstance(pub, Mapping): return False - email = str(pub.get("email", "")).strip().lower() + publisher = cast(Mapping[str, object], pub) + email = str(publisher.get("email", "")).strip().lower() if email and email == OFFICIAL_PUBLISHER_EMAIL.strip().lower(): return True - url = str(pub.get("url", "")).strip().lower() + url = str(publisher.get("url", "")).strip().lower() return OFFICIAL_MODULES_REPO_URL_MARKER in url.replace(" ", "") @@ -377,11 +379,45 @@ 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, Mapping): + dependency_entry = cast(Mapping[str, object], entry) + raw_id = dependency_entry.get("id") + if not isinstance(raw_id, str): + raise ValueError( + "bundle_dependencies object entry must include non-empty 'id' str; " + f"got {type(raw_id).__name__}: {entry!r}" + ) + dependency_id = raw_id.strip() + if not dependency_id: + raise ValueError(f"bundle_dependencies object entry must include non-empty 'id'; got {entry!r}") + dependency_ids.append(dependency_id) + continue + if not isinstance(entry, str): + raise ValueError( + "bundle_dependencies entries must be strings or objects with non-empty 'id'; " + f"got {type(entry).__name__}: {entry!r}" + ) + dependency_id = entry.strip() + if not dependency_id: + raise ValueError("bundle_dependencies string entries must be non-empty") + 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..6c0e6abb 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.25", 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." @@ -22,8 +22,9 @@ "azure-identity>=1.17.1", "cryptography>=43.0.0", "packaging>=24.0", + "click>=8.1.8,<8.2", "typer>=0.20.0,<0.24", - "rich>=13.5.2,<13.6.0", + "rich>=13.5.2,<16.0.0", "questionary>=2.0.1", "jinja2>=3.1.6", "networkx>=3.4.2", @@ -35,7 +36,5 @@ "icontract>=2.7.1", "beartype>=0.22.4", "watchdog>=6.0.0", - "opentelemetry-sdk>=1.27.0", - "opentelemetry-exporter-otlp-proto-http>=1.27.0", ], ) diff --git a/src/__init__.py b/src/__init__.py index 1f744e48..bc2989ff 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.25" diff --git a/src/specfact_cli/__init__.py b/src/specfact_cli/__init__.py index b85421a9..23b09028 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.25" __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/models/project.py b/src/specfact_cli/models/project.py index e822ac5b..ce6dd072 100644 --- a/src/specfact_cli/models/project.py +++ b/src/specfact_cli/models/project.py @@ -662,7 +662,13 @@ def save_to_directory( if progress_callback: progress_callback(total_artifacts, total_artifacts, "bundle.manifest.yaml") manifest_path = bundle_dir / "bundle.manifest.yaml" - dump_structured_file(self.manifest.model_dump(mode="json"), manifest_path) + manifest_data = self.manifest.model_dump(mode="json") + if num_features > 1000: + import json + + manifest_path.write_text(json.dumps(manifest_data, indent=2), encoding="utf-8") + else: + dump_structured_file(manifest_data, manifest_path) @beartype @require(lambda self, key: isinstance(key, str) and len(key) > 0, "Feature key must be non-empty string") diff --git a/src/specfact_cli/modules/module_registry/module-package.yaml b/src/specfact_cli/modules/module_registry/module-package.yaml index 29c594bf..57d16e73 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 @@ -17,5 +17,5 @@ publisher: description: 'Manage modules: search, list, show, install, and upgrade.' license: Apache-2.0 integrity: - checksum: sha256:bc1293efa20676657f440f5c8f21583fcf7c5c2138d754e0262934417c92cfbb - signature: 7hALhs1hTjtnz3SdYmzKIqoniyvJDk5LyFcauv/wnVJRyZEQyPv42sb0Zk30zXAZ8QPE/g43/g4l78nv0AqNBQ== + checksum: sha256:52a65869fb3d2d36910308c954e0ba199cb1c73f24136ee0d4aa7254f0996a22 + signature: Uo2eJ40HiWBqe77N8gjiDg7zGhsvJE/hN82+Jcto7yUoqbH7bidLw9DfVQ0CrJfgM9Y0XHAUS1+LMyPr1jAaBg== 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..c54a5f46 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).""" @@ -183,17 +189,19 @@ def _download_archive_with_cache(module_id: str, version: str | None = None) -> @beartype -def _extract_bundle_dependencies(metadata: dict[str, Any]) -> list[str]: - """Extract validated bundle dependency module ids from raw manifest metadata. - - Supports both plain string entries ("namespace/name") and versioned object entries - ({"id": "namespace/name", "version": ">=x.y.z"}). - """ - raw_dependencies = metadata.get("bundle_dependencies", []) - if not isinstance(raw_dependencies, list): +def _extract_bundle_dependency_specs(metadata: dict[str, Any]) -> list[_BundleDependencySpec]: + """Extract validated bundle dependency ids and optional version specifiers.""" + if "bundle_dependencies" not in metadata: return [] - dependencies: list[str] = [] + raw_dependencies = metadata["bundle_dependencies"] + if not isinstance(raw_dependencies, list): + raise ValueError( + "bundle_dependencies must be a list of module ids or dependency objects " + f"(invalid manifest; got {type(raw_dependencies).__name__})" + ) + 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 +211,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 +219,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 +237,53 @@ 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( + "Skipping malformed bundle dependency version comparison (treating as unsatisfied): " + "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 _verify_installed_dependency_identity(dependency_module_id: str, dependency_id_file: Path) -> None: + if not dependency_id_file.exists(): + raise ValueError( + f"Dependency install failed for {dependency_module_id}: missing {REGISTRY_ID_FILE} identity file" + ) + installed_registry_id = dependency_id_file.read_text(encoding="utf-8").strip() + if installed_registry_id != dependency_module_id: + raise ValueError( + f"Dependency install failed for {dependency_module_id}: {REGISTRY_ID_FILE} contains " + f"{installed_registry_id!r}" + ) + + @beartype def _integrity_debug_details_enabled() -> bool: """Return True when verbose integrity diagnostics should be shown.""" @@ -822,15 +878,26 @@ 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_manifest.exists() and 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 +911,13 @@ 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 + _verify_installed_dependency_identity(dependency_module_id, dependency_id_file) + 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..63af431b 100644 --- a/src/specfact_cli/registry/module_state.py +++ b/src/specfact_cli/registry/module_state.py @@ -92,7 +92,16 @@ def find_dependents( continue if not enabled_map.get(name, True): continue - module_dependencies = list(getattr(meta, "module_dependencies", [])) - if module_id in module_dependencies: + module_dependencies = [ + str(dependency).strip() + for dependency in list(getattr(meta, "module_dependencies", [])) + if str(dependency).strip() + ] + versioned_dependencies = [ + str(getattr(dependency, "name", "")).strip() + for dependency in list(getattr(meta, "module_dependencies_versioned", [])) + if str(getattr(dependency, "name", "")).strip() + ] + if module_id in module_dependencies or module_id in 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/utils/yaml_utils.py b/src/specfact_cli/utils/yaml_utils.py index d7f985ba..d3ae4f6d 100644 --- a/src/specfact_cli/utils/yaml_utils.py +++ b/src/specfact_cli/utils/yaml_utils.py @@ -2,6 +2,8 @@ YAML utilities. This module provides helpers for YAML parsing and serialization. + +CrossHair: skip (ruamel.yaml initialization performs plugin filesystem discovery) """ from __future__ import annotations @@ -31,13 +33,21 @@ def __init__(self, preserve_quotes: bool = True, indent_mapping: int = 2, indent indent_mapping: Indentation for mappings (must be > 0) indent_sequence: Indentation for sequences (must be > 0) """ - self.yaml = YAML() - self.yaml.preserve_quotes = preserve_quotes - cast(Any, self.yaml).indent(mapping=indent_mapping, sequence=indent_sequence) - self.yaml.default_flow_style = False + self.preserve_quotes = preserve_quotes + self.indent_mapping = indent_mapping + self.indent_sequence = indent_sequence + self.yaml = self._new_yaml() + + def _new_yaml(self) -> YAML: + """Return a fresh ruamel YAML writer/reader for one operation.""" + yaml = YAML() + yaml.preserve_quotes = self.preserve_quotes + cast(Any, yaml).indent(mapping=self.indent_mapping, sequence=self.indent_sequence) + yaml.default_flow_style = False # Configure to quote boolean-like strings to prevent YAML parsing issues # YAML parsers interpret "Yes", "No", "True", "False", "On", "Off" as booleans - self.yaml.default_style = None # Let ruamel.yaml decide, but we'll quote manually + yaml.default_style = None # Let ruamel.yaml decide, but we'll quote manually + return yaml @beartype @require(lambda file_path: isinstance(file_path, (Path, str)), "File path must be Path or str") @@ -60,8 +70,9 @@ def load(self, file_path: Path | str) -> Any: if not file_path.exists(): raise FileNotFoundError(f"YAML file not found: {file_path}") + yaml = self._new_yaml() with open(file_path, encoding="utf-8") as f: - loader = cast(Callable[[Any], Any], self.yaml.load) + loader = cast(Callable[[Any], Any], yaml.load) return loader(f) @beartype @@ -77,7 +88,8 @@ def load_string(self, yaml_string: str) -> Any: Returns: Parsed YAML content """ - loader = cast(Callable[[Any], Any], self.yaml.load) + yaml = self._new_yaml() + loader = cast(Callable[[Any], Any], yaml.load) return loader(yaml_string) @beartype @@ -98,8 +110,9 @@ def dump(self, data: Any, file_path: Path | str) -> None: # Use context manager for proper file handling # Thread-local YAML instances ensure thread-safety + yaml = self._new_yaml() with open(file_path, "w", encoding="utf-8") as f: - dumper = cast(Callable[..., None], self.yaml.dump) + dumper = cast(Callable[..., None], yaml.dump) dumper(data, f) # Explicit flush to ensure data is written before context exits # This helps prevent "I/O operation on closed file" errors in parallel operations @@ -167,8 +180,9 @@ def dump_string(self, data: Any) -> str: """ from io import StringIO + yaml = self._new_yaml() stream = StringIO() - dumper = cast(Callable[..., None], self.yaml.dump) + dumper = cast(Callable[..., None], yaml.dump) dumper(data, stream) return stream.getvalue() 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/integration/test_bundle_install.py b/tests/integration/test_bundle_install.py index a7597d76..4c115c06 100644 --- a/tests/integration/test_bundle_install.py +++ b/tests/integration/test_bundle_install.py @@ -11,6 +11,7 @@ from typer.testing import CliRunner from specfact_cli.modules.module_registry.src.commands import app +from specfact_cli.registry.module_installer import REGISTRY_ID_FILE runner = CliRunner() @@ -133,6 +134,7 @@ def _download(module_id: str, version: str | None = None) -> Path: dep_dir = install_root / "specfact-project" dep_dir.mkdir(parents=True, exist_ok=True) (dep_dir / "module-package.yaml").write_text("name: specfact-project\nversion: '0.39.0'\ncommands: [project]\n") + (dep_dir / REGISTRY_ID_FILE).write_text("nold-ai/specfact-project", encoding="utf-8") result = runner.invoke( app, 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/models/test_project.py b/tests/unit/models/test_project.py index af5a071c..462eff73 100644 --- a/tests/unit/models/test_project.py +++ b/tests/unit/models/test_project.py @@ -8,6 +8,7 @@ import hashlib from datetime import UTC, datetime from pathlib import Path +from typing import Any import pytest @@ -416,7 +417,7 @@ def test_save_to_directory_checksums_computed_during_write(self, tmp_path: Path) assert checksum == file_checksum, f"Checksum mismatch for {artifact_name}" @pytest.mark.timeout(30) # Increase timeout for large bundle test - def test_save_to_directory_large_bundle_worker_reduction(self, tmp_path: Path): + def test_save_to_directory_large_bundle_worker_reduction(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch): """Test that large bundles (1000+ features) use fewer workers for memory optimization.""" bundle_dir = tmp_path / "test-bundle" @@ -437,6 +438,17 @@ def test_save_to_directory_large_bundle_worker_reduction(self, tmp_path: Path): ) bundle.add_feature(feature) + from specfact_cli.utils import structured_io + + dumped_paths: list[Path] = [] + original_dump_structured_file = structured_io.dump_structured_file + + def tracked_dump_structured_file(data: Any, file_path: Path, *args: Any, **kwargs: Any) -> None: + dumped_paths.append(file_path) + original_dump_structured_file(data, file_path, *args, **kwargs) + + monkeypatch.setattr(structured_io, "dump_structured_file", tracked_dump_structured_file) + # Save should complete successfully with reduced workers # Note: This test takes longer due to large bundle size (30s timeout) bundle.save_to_directory(bundle_dir) @@ -450,6 +462,15 @@ def test_save_to_directory_large_bundle_worker_reduction(self, tmp_path: Path): # Features + product + manifest (and potentially idea/business if present) assert len(bundle.manifest.checksums.files) >= num_features + manifest_path = bundle_dir / "bundle.manifest.yaml" + assert manifest_path not in dumped_paths + + from specfact_cli.utils.structured_io import load_structured_file + + expected_manifest = bundle.manifest.model_dump(mode="json") + assert load_structured_file(manifest_path) == expected_manifest + assert ProjectBundle.load_from_directory(bundle_dir).manifest.model_dump(mode="json") == expected_manifest + class TestBundleFormat: """Tests for BundleFormat enum.""" 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/packaging/test_core_package_includes.py b/tests/unit/packaging/test_core_package_includes.py index a94bdb53..1fea06d7 100644 --- a/tests/unit/packaging/test_core_package_includes.py +++ b/tests/unit/packaging/test_core_package_includes.py @@ -93,3 +93,30 @@ def test_setup_py_version_matches_pyproject() -> None: assert version_in_pyproject is not None setup_text = SETUP_PY.read_text(encoding="utf-8") assert f'version="{version_in_pyproject}"' in setup_text or f"version='{version_in_pyproject}'" in setup_text + + +def test_core_dependency_bounds_avoid_semgrep_cli_conflicts() -> None: + """Core install requirements must not permit known Typer/Semgrep resolver conflicts.""" + data = tomllib.loads(PYPROJECT.read_text(encoding="utf-8")) + dependencies = set(data["project"]["dependencies"]) + + assert "click>=8.1.8,<8.2" in dependencies + assert "typer>=0.20.0,<0.24" in dependencies + assert "rich>=13.5.2,<16.0.0" in dependencies + assert not any(dependency.startswith("opentelemetry-") for dependency in dependencies) + + setup_text = SETUP_PY.read_text(encoding="utf-8") + assert '"click>=8.1.8,<8.2"' in setup_text + assert '"typer>=0.20.0,<0.24"' in setup_text + assert '"rich>=13.5.2,<16.0.0"' in setup_text + assert '"opentelemetry-sdk' not in setup_text + assert '"opentelemetry-exporter-otlp-proto-http' not in setup_text + + +def test_telemetry_dependencies_are_opt_in_extra() -> None: + """OpenTelemetry should stay out of core but remain available for explicit telemetry installs.""" + data = tomllib.loads(PYPROJECT.read_text(encoding="utf-8")) + telemetry = set(data["project"]["optional-dependencies"]["telemetry"]) + + assert "opentelemetry-sdk>=1.27.0" in telemetry + assert "opentelemetry-exporter-otlp-proto-http>=1.27.0" in telemetry diff --git a/tests/unit/registry/test_module_installer.py b/tests/unit/registry/test_module_installer.py index 0475c089..c7ea764d 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,155 @@ 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", + ) + (dependency_dir / module_installer.REGISTRY_ID_FILE).write_text("nold-ai/specfact-codebase", 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_bundle_dependency_install_requires_post_install_registry_identity( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + install_root = tmp_path / "marketplace-modules" + + def _install_without_registry_id(module_id: str, options: InstallModuleOptions | None = None) -> Path: + _ = module_id, options + 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", + ) + return dependency_dir + + monkeypatch.setattr(module_installer, "install_module", _install_without_registry_id) + + ctx = module_installer._BundleDepsInstallContext( + metadata={"bundle_dependencies": ["nold-ai/specfact-project"]}, + metadata_obj=ModulePackageMetadata(name="specfact-spec", version="0.1.0", commands=["spec"]), + target_root=install_root, + trust_non_official=False, + non_interactive=True, + force=False, + logger=MagicMock(), + ) + + with pytest.raises(ValueError, match=module_installer.REGISTRY_ID_FILE): + module_installer._install_bundle_dependencies_for_module("nold-ai/specfact-spec", ctx) + + +def test_bundle_dependency_install_rejects_post_install_registry_identity_mismatch( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + install_root = tmp_path / "marketplace-modules" + + def _install_wrong_registry_id(module_id: str, options: InstallModuleOptions | None = None) -> Path: + _ = module_id, options + 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") + return dependency_dir + + monkeypatch.setattr(module_installer, "install_module", _install_wrong_registry_id) + + ctx = module_installer._BundleDepsInstallContext( + metadata={"bundle_dependencies": ["nold-ai/specfact-project"]}, + metadata_obj=ModulePackageMetadata(name="specfact-spec", version="0.1.0", commands=["spec"]), + target_root=install_root, + trust_non_official=False, + non_interactive=True, + force=False, + logger=MagicMock(), + ) + + with pytest.raises(ValueError, match="other-org/specfact-project"): + module_installer._install_bundle_dependencies_for_module("nold-ai/specfact-spec", ctx) + + 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..2931ffed 100644 --- a/tests/unit/scripts/test_publish_module_bundle.py +++ b/tests/unit/scripts/test_publish_module_bundle.py @@ -102,6 +102,94 @@ 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_publish_entry_rejects_non_string_bundle_dependency_ids() -> None: + module = _load_script_module() + + with pytest.raises(ValueError, match=r"id'.*str"): + module._build_publish_entry( + { + "bundle_dependencies": [ + {"id": 123}, + ] + }, + "nold-ai/specfact-review", + "0.47.0", + Path("specfact-review-0.47.0.tar.gz"), + "abc123", + ) + + with pytest.raises(ValueError, match="entries must be strings"): + module._build_publish_entry( + { + "bundle_dependencies": [123], + }, + "nold-ai/specfact-review", + "0.47.0", + Path("specfact-review-0.47.0.tar.gz"), + "abc123", + ) + + with pytest.raises(ValueError, match="string entries must be non-empty"): + module._build_publish_entry( + { + "bundle_dependencies": [" "], + }, + "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..724aee72 100644 --- a/tests/unit/specfact_cli/registry/test_module_dependencies.py +++ b/tests/unit/specfact_cli/registry/test_module_dependencies.py @@ -3,15 +3,19 @@ from __future__ import annotations from pathlib import Path +from types import SimpleNamespace +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, validate_disable_safe, validate_enable_safe, ) +from specfact_cli.registry.module_state import find_dependents def _pkg(name: str, deps: list[str] | None = None) -> tuple[Path, ModulePackageMetadata]: @@ -46,6 +50,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_supports_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"]), @@ -59,6 +159,21 @@ def test_validate_disable_safe_blocks_enabled_dependents() -> None: assert blocked == {"sync": ["plan"]} +def test_find_dependents_supports_unhashable_dependency_entries() -> None: + packages = [ + ( + Path("/tmp/plan"), + SimpleNamespace( + name="plan", + module_dependencies=[["sync"]], + module_dependencies_versioned=[SimpleNamespace(name="sync")], + ), + ), + ] + + assert find_dependents("sync", packages, {"plan": True}) == ["plan"] + + def test_validate_disable_safe_allows_batch_disable_of_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..21d60914 100644 --- a/tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py +++ b/tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py @@ -10,68 +10,101 @@ 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_dependency_specs # ── Scenario: Registry entry declares a versioned bundle dependency ─────────── -def test_extract_bundle_dependencies_handles_versioned_object() -> None: - """_extract_bundle_dependencies must handle {"id": "...", "version": ">=x.y.z"} form.""" +def _dependency_ids(metadata: dict[str, Any]) -> dict[str, str]: + return { + dependency.module_id: dependency.version_specifier for dependency in _extract_bundle_dependency_specs(metadata) + } + + +def test_extract_bundle_dependency_specs_supports_versioned_object() -> None: + """_extract_bundle_dependency_specs must support {"id": "...", "version": ">=x.y.z"} form.""" metadata: dict[str, Any] = {"bundle_dependencies": [{"id": "nold-ai/specfact-project", "version": ">=0.41.0"}]} - deps = _extract_bundle_dependencies(metadata) + deps = _dependency_ids(metadata) assert "nold-ai/specfact-project" in deps, f"Versioned object form not handled; got {deps}" + assert deps["nold-ai/specfact-project"] == ">=0.41.0" -def test_extract_bundle_dependencies_handles_plain_string() -> None: - """_extract_bundle_dependencies must still handle plain string entries (backward compat).""" +def test_extract_bundle_dependency_specs_supports_plain_string() -> None: + """_extract_bundle_dependency_specs must still support plain string entries (backward compat).""" metadata: dict[str, Any] = {"bundle_dependencies": ["nold-ai/specfact-project"]} - deps = _extract_bundle_dependencies(metadata) + deps = _dependency_ids(metadata) assert "nold-ai/specfact-project" in deps -def test_extract_bundle_dependencies_handles_mixed_list() -> None: - """_extract_bundle_dependencies must handle a mix of string and versioned object entries.""" +def test_extract_bundle_dependency_specs_supports_mixed_list() -> None: + """_extract_bundle_dependency_specs must support a mix of string and versioned object entries.""" metadata: dict[str, Any] = { "bundle_dependencies": [ "nold-ai/specfact-project", {"id": "nold-ai/specfact-codebase", "version": ">=0.40.0"}, ] } - deps = _extract_bundle_dependencies(metadata) + deps = _dependency_ids(metadata) assert "nold-ai/specfact-project" in deps assert "nold-ai/specfact-codebase" in deps def test_extract_bundle_dependencies_empty_list() -> None: metadata: dict[str, Any] = {"bundle_dependencies": []} - deps = _extract_bundle_dependencies(metadata) - assert deps == [] + deps = _dependency_ids(metadata) + assert deps == {} def test_extract_bundle_dependencies_missing_key() -> None: metadata: dict[str, Any] = {} - deps = _extract_bundle_dependencies(metadata) - assert deps == [] + deps = _dependency_ids(metadata) + assert deps == {} + + +def test_extract_bundle_dependencies_rejects_malformed_shape() -> None: + metadata: dict[str, Any] = {"bundle_dependencies": {"id": "nold-ai/specfact-project"}} + with pytest.raises(ValueError, match="bundle_dependencies"): + _extract_bundle_dependency_specs(metadata) def test_extract_bundle_dependencies_rejects_object_without_id() -> None: """Malformed bundle_dependencies objects must fail manifest validation, not be skipped.""" metadata: dict[str, Any] = {"bundle_dependencies": [{"version": ">=1.0.0"}]} with pytest.raises(ValueError, match="non-empty 'id'"): - _extract_bundle_dependencies(metadata) + _extract_bundle_dependency_specs(metadata) def test_extract_bundle_dependencies_rejects_empty_id_object() -> None: metadata: dict[str, Any] = {"bundle_dependencies": [{"id": "", "version": ">=1.0.0"}]} with pytest.raises(ValueError, match="non-empty 'id'"): - _extract_bundle_dependencies(metadata) + _extract_bundle_dependency_specs(metadata) def test_extract_bundle_dependencies_rejects_empty_string_entry() -> None: metadata: dict[str, Any] = {"bundle_dependencies": ["nold-ai/specfact-project", ""]} with pytest.raises(ValueError, match="string entry must be non-empty"): - _extract_bundle_dependencies(metadata) + _extract_bundle_dependency_specs(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))