[codex] Add module scope version diagnostics

[djm81]

Summary

Adds module-scope diagnostics and version mismatch enforcement for project/user scoped SpecFact modules.

• Adds specfact module doctor to show effective vs shadowed module copies across project, user, and configured development roots.
• Enforces versioned bundle dependency declarations during module install and versioned module dependency declarations during registration/lifecycle checks.
• Updates module documentation, OpenSpec artifacts, source tracking, and module/package version metadata.
• Fixes the relaxed PR module-signature verifier so --skip-checksum-verification really performs version-bump-only verification while CI signing handles signatures.

Root Cause

Users can work across split repos and monorepos with both project-local and user-scoped modules installed. Discovery priority can make one copy shadow another, while older dependency checks did not consistently enforce declared module version ranges, causing confusing command availability and runtime mismatches.

Tracking

Fixes #565
Parent feature: #353
OpenSpec change: module-scope-version-diagnostics

Validation

• openspec validate module-scope-version-diagnostics --strict
• 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 (96 passed)
• Scoped basedpyright on touched implementation/tests (0 errors)
• hatch run specfact code review run --json --out .specfact/code-review.changed.json --scope changed (0 blocking)
• Real tmp smoke: project/user duplicate module doctor scenario reports project as effective and user as shadowed with remediation
• Real tmp smoke: installer rejects existing dependency 1.0.0 when tarball requires >=2.0.0
• Pre-commit passed on commit 500337e5

[strix-security]

Strix is installed on this repository, but we could not run this PR security review because this workspace does not have an active plan. If you'd like to continue receiving code reviews, you can add a payment method or manage billing here.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a CLI diagnostic specfact module doctor, enforces declared versioned bundle/module dependencies at install and registration, updates OpenSpec/specs and user docs, adjusts adapter typing and CLI fallback behavior, normalizes publish bundle_dependencies, and adds tests covering diagnostics and version checks.

Changes

Module Scope Version Diagnostics and Dependency Enforcement

Layer / File(s)	Summary
Specification, design, and OpenSpec traceability openspec/changes/module-scope-version-diagnostics/*, openspec/CHANGE_ORDER.md, CHANGELOG.md	OpenSpec proposal, design, specs, tasks, and TDD evidence added; CHANGELOG and CHANGE_ORDER updated to record module-scope diagnostics and enforcement requirements.
User-facing documentation updates docs/module-system/installing-modules.md, docs/module-system/module-marketplace.md	Documentation adds specfact module doctor usage/examples, updates dependency-resolution notes (including reinstall/upgrade when installed versions conflict with bundle ranges), clarifies discovery priority and --show-origin metadata, and documents recovery commands for shadowed duplicates.
Module doctor CLI command and helpers src/specfact_cli/modules/module_registry/src/commands.py, tests/unit/modules/module_registry/test_commands.py	New doctor command discovers modules with shadow-aware discovery, supports exact fully-qualified and short-id matching, classifies status (effective/shadowed/disabled), prints recovery uninstall guidance, and reports configured development source roots.
Bundle dependency parsing & publish normalization src/specfact_cli/registry/module_installer.py, scripts/publish-module.py, tests/unit/scripts/test_publish_module_bundle.py	Introduce _BundleDependencySpec, parse/normalize bundle_dependencies (dict or string) into structured specs or flat ID lists for registry entries; publish script normalizes mixed inputs into ID strings and rejects malformed objects.
Bundle dependency enforcement at install-time src/specfact_cli/registry/module_installer.py, tests/unit/registry/test_module_installer.py	When a bundled dependency is present, installer verifies registry id matches expected module id and validates the installed module-package.yaml version against the declared specifier using packaging; malformed spec/version treated as not satisfied and mismatches raise ValueError. After install, the installer re-checks installed version and raises if still mismatched.
Module package registration and dependency validation src/specfact_cli/registry/module_packages.py, src/specfact_cli/registry/module_state.py, tests/unit/specfact_cli/registry/*	Registration precomputes discovered module_versions, _validate_module_dependencies() now validates module_dependencies_versioned against discovered versions (logs malformed specs non-blockingly), expands transitive enable/disable helpers to include versioned dependency names, and skips registering packages with unsatisfied versioned dependencies.
CLI lazy delegation fallback and adapter typing adjustments src/specfact_cli/cli.py, src/specfact_cli/adapters/ado.py, src/specfact_cli/adapters/github.py, src/specfact_cli/validators/change_proposal_integration.py, scripts/verify-modules-signature.py	Add fallback in _LazyDelegateGroup.get_command for known bundle/shim names; wrap adapter json= payloads with cast(Any, ...) for typing compatibility; require/parse checksum when checksum verification is disabled (removes conditional signature check in that path); minor validator typing cast and test capture update.
Test coverage for diagnostics and enforcement tests/unit/specfact_cli/registry/*, tests/unit/registry/*, tests/unit/modules/*, tests/unit/scripts/*	New/updated tests verify doctor outputs (effective/shadowed/dev roots), installer rejects mismatched bundle dependency versions and namespace collisions, registration skips packages with unsatisfied versioned deps, dependency-version malformed-input logging, and publish serialization.
Package and manifest version bumps pyproject.toml, setup.py, src/__init__.py, src/specfact_cli/__init__.py, src/specfact_cli/modules/module_registry/module-package.yaml	Project/package version updated to 0.46.22; module registry manifest version bumped.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• nold-ai/specfact-cli#541: Both PRs modify the CLI lazy-delegation routing in src/specfact_cli/cli.py (the _LazyDelegateGroup behavior for resolving/delegating commands).
• nold-ai/specfact-cli#535: Adds shadow-capable discovery entrypoints used by the new module doctor diagnostics.

Suggested labels

QA, codex

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 46.58% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title clearly summarizes the main change: addition of module scope version diagnostics, which is the primary focus of the PR changeset.
Description check	✅ Passed	Description provides comprehensive coverage of summary, root cause, tracking (linked issues #565, #353, OpenSpec change), and validation steps executed.
Linked Issues check	✅ Passed	Changes fully implement #565 requirements: adds module doctor diagnostics, enforces versioned dependencies at install/registration, updates documentation, and preserves project-over-user precedence.
Out of Scope Changes check	✅ Passed	All changes align with stated objectives. Version bumps (0.46.19→0.46.22), signature verifier fix, and adapter JSON casting are necessary supporting changes directly tied to module diagnostics implementation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/module-scope-version-diagnostics

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e6f147be9b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

openspec/CHANGE_ORDER.md (1)

28-29: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Active-count prose is stale and now contradicts the snapshot table.

After updating Line 11 to 25 active changes, this sentence still says 24. Please update it to keep governance docs internally consistent.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openspec/CHANGE_ORDER.md` around lines 28 - 29, Update the stale count in the
sentence that begins "The 24 active changes group into five independent tracks."
to match the updated snapshot count (change "24" to "25") so the prose aligns
with the Line 11 update to 25 active changes; keep the rest of the sentence
unchanged.

src/specfact_cli/registry/module_packages.py (1)

1603-1639: ⚠️ Potential issue | 🔴 Critical

Test coverage gap for versioned dependency validation logic. The PR adds _missing_versioned_module_dependencies() and integrates versioned dependency checking into the registration flow, but there are no corresponding tests in test_module_packages.py for this validation path. Per coding guidelines, critical paths (dependency resolution) require 100% coverage.

Add tests covering:

• _validate_module_dependencies() with module_versions parameter populated
• _missing_versioned_module_dependencies() with various version specifier scenarios
• Dependency resolution failures in the registration context

Note on cross-repo impact: The linked nold-ai/specfact-cli-modules tests validate bundle_dependencies (not module_dependencies_versioned), so registry manifest validation in that repo will not be affected by versioned module dependency changes here.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/specfact_cli/registry/module_packages.py` around lines 1603 - 1639, Add
unit tests in test_module_packages.py to exercise the new versioned dependency
validation paths: write tests that call _validate_module_dependencies() with a
populated module_versions dict to assert successful validation and failure
cases, and tests that directly call _missing_versioned_module_dependencies()
with multiple version specifiers (exact, caret, tilde, ranges) to assert correct
missing/unsatisfied results; also add an integration-style test that runs the
registration flow (e.g., invoking _register_one_package_if_eligible or the
top-level registration loop) with a package whose manifest declares a versioned
dependency not present or not matching module_versions and assert the
registration is skipped/fails and that the appropriate error is recorded in the
registration context (skipped list or logger/counters). Ensure each test
isolates module_versions input and asserts both positive and negative outcomes
for dependency resolution.

src/specfact_cli/registry/module_installer.py (1)

202-228: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Linked-repo manifest validators cannot handle object-form bundle_dependencies; must coordinate with upstream.

This parser now supports bundle_dependencies entries as objects ({id: "...", version: ">=x.y.z"}), but the linked nold-ai/specfact-cli-modules validators (tools/validate_repo_manifests.py:269) assume string-only format. Passing a dict to str(dep).strip() produces a string representation that cannot match registry IDs, causing validation failure. Before object-form entries appear in manifests pushed to the linked repo, its validators must be updated to extract the id field from objects, or this feature must remain internal/undocumented pending that coordination.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/specfact_cli/registry/module_installer.py` around lines 202 - 228, The
new object-form handling in _extract_bundle_dependency_specs allows dict entries
but upstream validators expect only string-form bundle_dependencies, so either
revert/disable object-form exposure in published manifests or add a
compatibility conversion step: ensure any dict entry parsed by
_extract_bundle_dependency_specs yields the same string ID representation before
manifest serialization (or update tools/validate_repo_manifests.py to extract
entry["id"]); locate usages of _extract_bundle_dependency_specs and
_BundleDependencySpec and ensure _validate_marketplace_namespace_format is
applied to the id string and that any manifest output sent to the linked repo
contains only string IDs (not raw dicts) unless the upstream validator is also
updated to accept objects.

🧹 Nitpick comments (2)

CHANGELOG.md (1)

25-30: ⚡ Quick win

Classify specfact module doctor under Added for release traceability

Line 27 introduces a new command surface, but it is currently logged under ### Fixed. Please split this entry so net-new command functionality is under ### Added, and keep mismatch/signature behavior under ### Fixed/### Changed as appropriate.

As per coding guidelines, "Update CHANGELOG.md to document all significant changes under Added, Fixed, Changed, or Removed sections when making a version change".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CHANGELOG.md` around lines 25 - 30, The CHANGELOG entry wrongly places the
new CLI command "specfact module doctor" under "### Fixed"; update CHANGELOG.md
by splitting that bullet: move the "- **Module scope diagnostics**: add
`specfact module doctor`..." line into a new or existing "### Added" section and
leave only the version-mismatch/signature behavior text (or split it into "###
Fixed" / "### Changed" as appropriate) under "### Fixed"/"### Changed" so that
net-new command functionality is documented under Added while the behavior fixes
remain under Fixed/Changed.

src/specfact_cli/registry/module_packages.py (1)

569-574: ⚡ Quick win

Consider logging malformed version specifiers for observability.

Catching InvalidVersion and InvalidSpecifier silently (returning None) hides manifest configuration errors. While non-blocking tolerance is intentional per the PR objectives, users have no visibility into why a version constraint was ignored.

A debug-level log would aid troubleshooting without blocking registration:

🔧 Proposed observability improvement

     try:
         if Version(found_version) not in SpecifierSet(version_specifier):
             return f"{dep_name} (requires {version_specifier}, found {found_version})"
     except (InvalidVersion, InvalidSpecifier):
+        from specfact_cli.common import get_bridge_logger
+        get_bridge_logger(__name__).debug(
+            "Ignoring malformed version specifier for dependency '%s': specifier=%s, found=%s",
+            dep_name,
+            version_specifier,
+            found_version,
+        )
         return None
     return None

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/specfact_cli/registry/module_packages.py` around lines 569 - 574, Update
the except block that catches InvalidVersion and InvalidSpecifier around the
Version(found_version) / SpecifierSet(version_specifier) check to emit a
debug-level log containing the package name (dep_name), the offending
version_specifier, the found_version, and the exception message instead of
failing silently; keep the return None behavior so registration remains
tolerant, but call the module logger (or existing logger) in the except branch
to record the malformed specifier for observability.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/specfact_cli/modules/module_registry/src/commands.py`:
- Around line 1074-1084: The function _doctor_entry_matches currently treats any
requested name as matching by basename which means a full module-id like
"nold-ai/specfact-codebase" will match "other/specfact-codebase"; change the
logic so that if requested contains a slash (i.e. is a full module-id) it must
match discovered_id exactly, otherwise (no slash) allow the basename fallback:
keep the early returns for None/empty requested, and alter the final comparison
to only perform requested.rsplit("/", 1)[-1] == discovered_id.rsplit("/", 1)[-1]
when "/" not in requested; otherwise require requested == discovered_id. This
affects _doctor_entry_matches and the variables requested and discovered_id.

In `@src/specfact_cli/registry/module_installer.py`:
- Around line 246-253: The helper _dependency_version_satisfies currently
swallows InvalidSpecifier/InvalidVersion exceptions; modify its except block to
emit a debug-level log including the installed_version, version_specifier, and
the caught exception (while still returning True to preserve non-blocking
behavior). Use the module logger (e.g., logger = logging.getLogger(__name__))
and log the exception details when catching InvalidSpecifier or InvalidVersion
in _dependency_version_satisfies so malformed inputs are recorded for
diagnostics.

---

Outside diff comments:
In `@openspec/CHANGE_ORDER.md`:
- Around line 28-29: Update the stale count in the sentence that begins "The 24
active changes group into five independent tracks." to match the updated
snapshot count (change "24" to "25") so the prose aligns with the Line 11 update
to 25 active changes; keep the rest of the sentence unchanged.

In `@src/specfact_cli/registry/module_installer.py`:
- Around line 202-228: The new object-form handling in
_extract_bundle_dependency_specs allows dict entries but upstream validators
expect only string-form bundle_dependencies, so either revert/disable
object-form exposure in published manifests or add a compatibility conversion
step: ensure any dict entry parsed by _extract_bundle_dependency_specs yields
the same string ID representation before manifest serialization (or update
tools/validate_repo_manifests.py to extract entry["id"]); locate usages of
_extract_bundle_dependency_specs and _BundleDependencySpec and ensure
_validate_marketplace_namespace_format is applied to the id string and that any
manifest output sent to the linked repo contains only string IDs (not raw dicts)
unless the upstream validator is also updated to accept objects.

In `@src/specfact_cli/registry/module_packages.py`:
- Around line 1603-1639: Add unit tests in test_module_packages.py to exercise
the new versioned dependency validation paths: write tests that call
_validate_module_dependencies() with a populated module_versions dict to assert
successful validation and failure cases, and tests that directly call
_missing_versioned_module_dependencies() with multiple version specifiers
(exact, caret, tilde, ranges) to assert correct missing/unsatisfied results;
also add an integration-style test that runs the registration flow (e.g.,
invoking _register_one_package_if_eligible or the top-level registration loop)
with a package whose manifest declares a versioned dependency not present or not
matching module_versions and assert the registration is skipped/fails and that
the appropriate error is recorded in the registration context (skipped list or
logger/counters). Ensure each test isolates module_versions input and asserts
both positive and negative outcomes for dependency resolution.

---

Nitpick comments:
In `@CHANGELOG.md`:
- Around line 25-30: The CHANGELOG entry wrongly places the new CLI command
"specfact module doctor" under "### Fixed"; update CHANGELOG.md by splitting
that bullet: move the "- **Module scope diagnostics**: add `specfact module
doctor`..." line into a new or existing "### Added" section and leave only the
version-mismatch/signature behavior text (or split it into "### Fixed" / "###
Changed" as appropriate) under "### Fixed"/"### Changed" so that net-new command
functionality is documented under Added while the behavior fixes remain under
Fixed/Changed.

In `@src/specfact_cli/registry/module_packages.py`:
- Around line 569-574: Update the except block that catches InvalidVersion and
InvalidSpecifier around the Version(found_version) /
SpecifierSet(version_specifier) check to emit a debug-level log containing the
package name (dep_name), the offending version_specifier, the found_version, and
the exception message instead of failing silently; keep the return None behavior
so registration remains tolerant, but call the module logger (or existing
logger) in the except branch to record the malformed specifier for
observability.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 005cc5b9-a58f-4d0a-a26a-e5fb349c74d8

📥 Commits

Reviewing files that changed from the base of the PR and between a021c53 and e6f147b.

📒 Files selected for processing (30)

• CHANGELOG.md
• docs/module-system/installing-modules.md
• docs/module-system/module-marketplace.md
• openspec/CHANGE_ORDER.md
• openspec/changes/module-scope-version-diagnostics/.openspec.yaml
• openspec/changes/module-scope-version-diagnostics/TDD_EVIDENCE.md
• openspec/changes/module-scope-version-diagnostics/design.md
• openspec/changes/module-scope-version-diagnostics/proposal.md
• openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md
• openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md
• openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md
• openspec/changes/module-scope-version-diagnostics/tasks.md
• pyproject.toml
• scripts/verify-modules-signature.py
• setup.py
• src/__init__.py
• src/specfact_cli/__init__.py
• src/specfact_cli/adapters/ado.py
• src/specfact_cli/adapters/github.py
• src/specfact_cli/cli.py
• src/specfact_cli/modules/module_registry/module-package.yaml
• 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
• src/specfact_cli/validators/change_proposal_integration.py
• tests/unit/modules/module_registry/test_commands.py
• tests/unit/registry/test_module_installer.py
• tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py

💤 Files with no reviewable changes (1)

• scripts/verify-modules-signature.py

📜 Review details

🧰 Additional context used

📓 Path-based instructions (20)

**/*.py

📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)

**/*.py: Maintain minimum 80% test coverage, with 100% coverage for critical paths in Python code
Use clear naming and self-documenting code, preferring clear names over comments
Ensure each function/class has a single clear purpose (Single Responsibility Principle)
Extract common patterns to avoid code duplication (DRY principle)
Apply SOLID object-oriented design principles in Python code
Use type hints everywhere in Python code and enable basedpyright strict mode
Use Pydantic models for data validation and serialization in Python
Use async/await for I/O operations in Python code
Use context managers for resource management in Python
Use dataclasses for simple data containers in Python
Enforce maximum line length of 120 characters in Python code
Use 4 spaces for indentation in Python code (no tabs)
Use 2 blank lines between classes and 1 blank line between methods in Python
Organize imports in order: Standard library → Third party → Local in Python files
Use snake_case for variables and functions in Python
Use PascalCase for class names in Python
Use UPPER_SNAKE_CASE for constants in Python
Use leading underscore (_) for private methods in Python classes
Use snake_case for Python file names
Enable basedpyright strict mode with strict type checking configuration in Python
Use Google-style docstrings for functions and classes in Python
Include comprehensive exception handling with specific exception types in Python code
Use logging with structured context (extra parameters) instead of print statements
Use retry logic with tenacity decorators (@retry) for operations that might fail
Use Pydantic BaseSettings for environment-based configuration in Python
Validate user input using Pydantic validators in Python models
Use @lru_cache and Redis-based caching for expensive calculations in Python
Run code formatting with Black (120 character line length) and isort in Python
Run type checking with basedpyright on all Python files
Run linting with ruff and pylint on all Pyth...

Files:

• src/__init__.py
• setup.py
• src/specfact_cli/registry/module_state.py
• src/specfact_cli/validators/change_proposal_integration.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py
• src/specfact_cli/cli.py
• tests/unit/registry/test_module_installer.py
• tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py
• src/specfact_cli/__init__.py
• src/specfact_cli/adapters/github.py
• tests/unit/modules/module_registry/test_commands.py
• src/specfact_cli/adapters/ado.py
• src/specfact_cli/registry/module_installer.py
• src/specfact_cli/modules/module_registry/src/commands.py
• src/specfact_cli/registry/module_packages.py

{src/__init__.py,pyproject.toml,setup.py}

📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)

{src/__init__.py,pyproject.toml,setup.py}: Update src/init.py first as primary source of truth for package version, then pyproject.toml and setup.py
Maintain version synchronization across src/init.py, pyproject.toml, and setup.py

Files:

• src/__init__.py
• setup.py
• pyproject.toml

src/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

src/**/*.py: All code changes must be followed by running the full test suite using the smart test system.
All Python files in src/ and tools/ directories must have corresponding test files in tests/ directory. If you modify src/common/logger_setup.py, you MUST have tests/unit/common/test_logger_setup.py. NO EXCEPTIONS - even small changes require tests.
All new Python runtime code files must have corresponding test files created BEFORE committing the code. NO EXCEPTIONS - no code without tests.
Test Coverage Validation: Run hatch run smart-test-unit for modified files, hatch run smart-test-folder for modified directories, and hatch run smart-test-full before committing. ALL TESTS MUST PASS.
All components must support TEST_MODE=true environment variable with test-specific behavior defined as: if os.environ.get('TEST_MODE') == 'true': # test-specific behavior
Use src/common/logger_setup.py for all logging via: from common.logger_setup import get_logger; logger = get_logger(name)
Use src/common/redis_client.py with fallback for Redis operations via: from common.redis_client import get_redis_client; redis_client = get_redis_client()
Type checking must pass with no errors using: mypy .
Test coverage must meet or exceed 80% total coverage. New code must have corresponding tests. Modified code must maintain or improve coverage. Critical paths must have 100% coverage.
Use Pydantic v2 validation for all context and data schemas.

Add/update contracts on new or modified public APIs, stateful classes and adapters using icontract decorators and beartype runtime type checks

src/**/*.py: Meaningful Naming — identifiers reveal intent; avoid abbreviations. Identifiers in src/ must use snake_case (modules/functions), PascalCase (classes), UPPER_SNAKE_CASE (constants). Avoid single-letter names outside short loop variables.
KISS — keep functions and modules small and single-purpose. Maximum function length: 120 lines (Phase A error threshold). Maximum cyclomati...

Files:

• src/__init__.py
• src/specfact_cli/registry/module_state.py
• src/specfact_cli/validators/change_proposal_integration.py
• src/specfact_cli/cli.py
• src/specfact_cli/__init__.py
• src/specfact_cli/adapters/github.py
• src/specfact_cli/adapters/ado.py
• src/specfact_cli/registry/module_installer.py
• src/specfact_cli/modules/module_registry/src/commands.py
• src/specfact_cli/registry/module_packages.py

@(src|tests)/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

Linting must pass with no errors using: pylint src tests

Files:

• src/__init__.py
• src/specfact_cli/registry/module_state.py
• src/specfact_cli/validators/change_proposal_integration.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py
• src/specfact_cli/cli.py
• tests/unit/registry/test_module_installer.py
• tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py
• src/specfact_cli/__init__.py
• src/specfact_cli/adapters/github.py
• tests/unit/modules/module_registry/test_commands.py
• src/specfact_cli/adapters/ado.py
• src/specfact_cli/registry/module_installer.py
• src/specfact_cli/modules/module_registry/src/commands.py
• src/specfact_cli/registry/module_packages.py

{pyproject.toml,setup.py,src/__init__.py}

📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)

Manually update version numbers in pyproject.toml, setup.py, and src/init.py when making a formal version change

Files:

• src/__init__.py
• setup.py
• pyproject.toml

**/*.{py,pyi}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{py,pyi}: After any code changes, follow these steps in order: (1) Apply linting and formatting to ensure code quality: hatch run format, (2) Type checking: hatch run type-check (basedpyright), (3) Contract-first approach: Run hatch run contract-test for contract validation, (4) Run full test suite: hatch test --cover -v, (5) Verify all tests pass and contracts are satisfied, (6) Fix any issues and repeat steps until all tests pass
All public APIs must have @icontract decorators and @beartype type checking
Use Pydantic models for all data structures with data validation
Only write high-value comments if at all. Avoid talking to the user through comments

Files:

• src/__init__.py
• setup.py
• src/specfact_cli/registry/module_state.py
• src/specfact_cli/validators/change_proposal_integration.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py
• src/specfact_cli/cli.py
• tests/unit/registry/test_module_installer.py
• tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py
• src/specfact_cli/__init__.py
• src/specfact_cli/adapters/github.py
• tests/unit/modules/module_registry/test_commands.py
• src/specfact_cli/adapters/ado.py
• src/specfact_cli/registry/module_installer.py
• src/specfact_cli/modules/module_registry/src/commands.py
• src/specfact_cli/registry/module_packages.py

**/*.yaml

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

YAML files must pass linting using: hatch run yaml-lint with relaxed policy.

Files:

• src/specfact_cli/modules/module_registry/module-package.yaml

**/*.{yml,yaml}

📄 CodeRabbit inference engine (.cursor/rules/testing-and-build-guide.mdc)

Validate YAML configuration files locally using hatch run yaml-lint before committing

**/*.{yml,yaml}: Format all YAML and workflow files using hatch run yaml-fix-all before committing
Use Prettier to fix whitespace, indentation, and final newline across YAML files
Use yamllint with the repo .yamllint configuration (line-length 140, trailing spaces and final newline enforced) to lint non-workflow YAML files

Files:

• src/specfact_cli/modules/module_registry/module-package.yaml

**/*.{md,mdc}

📄 CodeRabbit inference engine (.cursor/rules/markdown-rules.mdc)

**/*.{md,mdc}: Do not use more than one consecutive blank line anywhere in the document (MD012: No Multiple Consecutive Blank Lines)
Fenced code blocks should be surrounded by blank lines (MD031: Fenced Code Blocks)
Lists should be surrounded by blank lines (MD032: Lists)
Files must end with a single empty line (MD047: Files Must End With Single Newline)
Lines should not have trailing spaces (MD009: No Trailing Spaces)
Use asterisks (**) for strong emphasis, not underscores (__) (MD050: Strong Style)
Fenced code blocks must have a language specified (MD040: Fenced Code Language)
Headers should increment by one level at a time (MD001: Header Increment)
Headers should be surrounded by blank lines (MD022: Headers Should Be Surrounded By Blank Lines)
Only one top-level header (H1) is allowed per document (MD025: Single H1 Header)
Use consistent list markers, preferring dashes (-) for unordered lists (MD004: List Style)
Nested unordered list items should be indented consistently, typically by 2 spaces (MD007: Unordered List Indentation)
Use exactly one space after the list marker (e.g., -, *, +, 1.) (MD030: Spaces After List Markers)
Use incrementing numbers for ordered lists (MD029: Ordered List Item Prefix)
Enclose bare URLs in angle brackets or format them as links (MD034: Bare URLs)
Don't use spaces immediately inside code spans (MD038: Spaces Inside Code Spans)
Use consistent indentation (usually 2 or 4 spaces) throughout markdown files
Keep line length under 120 characters in markdown files
Use reference-style links for better readability in markdown files
Use a trailing slash for directory paths in markdown files
Ensure proper escaping of special characters in markdown files

Files:

• openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md
• docs/module-system/module-marketplace.md
• openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md
• openspec/CHANGE_ORDER.md
• openspec/changes/module-scope-version-diagnostics/design.md
• openspec/changes/module-scope-version-diagnostics/proposal.md
• openspec/changes/module-scope-version-diagnostics/TDD_EVIDENCE.md
• openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md
• openspec/changes/module-scope-version-diagnostics/tasks.md
• CHANGELOG.md
• docs/module-system/installing-modules.md

**/*.md

📄 CodeRabbit inference engine (.cursorrules)

Avoid markdown linting errors (refer to markdown-rules)

Files:

• openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md
• docs/module-system/module-marketplace.md
• openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md
• openspec/CHANGE_ORDER.md
• openspec/changes/module-scope-version-diagnostics/design.md
• openspec/changes/module-scope-version-diagnostics/proposal.md
• openspec/changes/module-scope-version-diagnostics/TDD_EVIDENCE.md
• openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md
• openspec/changes/module-scope-version-diagnostics/tasks.md
• CHANGELOG.md
• docs/module-system/installing-modules.md

openspec/changes/**/*.md

📄 CodeRabbit inference engine (.cursorrules)

For /opsx:archive (Archive change): Include module signing and cleanup in final tasks. Agents MUST run openspec archive <change-id> from repo root (no manual mv under openspec/changes/archive/)

Files:

• openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md
• openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md
• openspec/changes/module-scope-version-diagnostics/design.md
• openspec/changes/module-scope-version-diagnostics/proposal.md
• openspec/changes/module-scope-version-diagnostics/TDD_EVIDENCE.md
• openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md
• openspec/changes/module-scope-version-diagnostics/tasks.md

openspec/**/{proposal.md,tasks.md,design.md,spec.md}

📄 CodeRabbit inference engine (.cursor/rules/automatic-openspec-workflow.mdc)

openspec/**/{proposal.md,tasks.md,design.md,spec.md}: Apply openspec/config.yaml project context and per-artifact rules (for proposal, specs, design, tasks) when creating or updating any OpenSpec change artifact in the specfact-cli codebase
After implementation, validate the change with openspec validate <change-id> --strict; fix validation errors in proposal, specs, design, or tasks and re-validate until passing before considering the change complete

Files:

• openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md
• openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md
• openspec/changes/module-scope-version-diagnostics/design.md
• openspec/changes/module-scope-version-diagnostics/proposal.md
• openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md
• openspec/changes/module-scope-version-diagnostics/tasks.md

openspec/**/*.md

⚙️ CodeRabbit configuration file

openspec/**/*.md: Treat as specification source of truth: proposal/tasks/spec deltas vs. code behavior,
CHANGE_ORDER consistency, and scenario coverage. Surface drift between OpenSpec and
implementation.

Files:

• openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md
• openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md
• openspec/CHANGE_ORDER.md
• openspec/changes/module-scope-version-diagnostics/design.md
• openspec/changes/module-scope-version-diagnostics/proposal.md
• openspec/changes/module-scope-version-diagnostics/TDD_EVIDENCE.md
• openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md
• openspec/changes/module-scope-version-diagnostics/tasks.md

pyproject.toml

📄 CodeRabbit inference engine (.cursorrules)

When updating the version in pyproject.toml, ensure it's newer than the latest PyPI version. The CI/CD pipeline will automatically publish to PyPI only if the new version is greater than the published version

Files:

• pyproject.toml

docs/**/*.md

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

Update architecture documentation in docs/ for architecture changes, state machine documentation for FSM modifications, interface documentation for API changes, and configuration guides for configuration changes. DO NOT create internal docs in specfact-cli repo folder that should not be visible to end users; use the respective internal repository instead.

Files:

• docs/module-system/module-marketplace.md
• docs/module-system/installing-modules.md

---

⚙️ CodeRabbit configuration file

docs/**/*.md: User-facing accuracy: CLI examples match current behavior; preserve Jekyll front matter;
call out when README/docs index need sync.

Files:

• docs/module-system/module-marketplace.md
• docs/module-system/installing-modules.md

src/specfact_cli/**/*.py

⚙️ CodeRabbit configuration file

src/specfact_cli/**/*.py: Focus on modular CLI architecture: lazy module loading, registry/bootstrap patterns, and
dependency direction. Flag breaking changes to public APIs, Pydantic models, and resource
bundling. Verify @icontract + @beartype on public surfaces; prefer centralized logging
(get_bridge_logger) over print().

Files:

• src/specfact_cli/registry/module_state.py
• src/specfact_cli/validators/change_proposal_integration.py
• src/specfact_cli/cli.py
• src/specfact_cli/__init__.py
• src/specfact_cli/adapters/github.py
• src/specfact_cli/adapters/ado.py
• src/specfact_cli/registry/module_installer.py
• src/specfact_cli/modules/module_registry/src/commands.py
• src/specfact_cli/registry/module_packages.py

CHANGELOG.md

📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)

Include new version entries at the top of CHANGELOG.md when updating versions

Update CHANGELOG.md with all code changes as part of version control requirements.

Update CHANGELOG.md to document all significant changes under Added, Fixed, Changed, or Removed sections when making a version change

Files:

• CHANGELOG.md

**/test_*.py

📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)

**/test_*.py: Write tests first in test-driven development (TDD) using the Red-Green-Refactor cycle
Ensure each test is independent and repeatable with no shared state between tests
Organize Python imports in tests using unittest.mock for Mock and patch
Use setup_method() for test initialization and Arrange-Act-Assert pattern in test files
Use @pytest.mark.asyncio decorator for async test functions in Python
Organize test files in structure: tests/unit/, tests/integration/, tests/e2e/ by module

Files:

• tests/unit/specfact_cli/registry/test_module_dependencies.py
• tests/unit/registry/test_module_installer.py
• tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py
• tests/unit/modules/module_registry/test_commands.py

tests/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

Tests must be meaningful and test actual functionality, cover both success and failure cases, be independent and repeatable, and have clear, descriptive names. NO EXCEPTIONS - no placeholder or empty tests.

tests/**/*.py: Trim low-value unit tests when a contract covers the same assertion (type/shape/raises on negative checks)
Delete tests that only assert input validation, datatype/shape enforcement, or raises on negative conditions now guarded by contracts and runtime typing
Convert repeated edge-case permutations into one Hypothesis property with contracts acting as oracles

Secret redaction via LoggerSetup.redact_secrets must be covered by unit tests

Files:

• tests/unit/specfact_cli/registry/test_module_dependencies.py
• tests/unit/registry/test_module_installer.py
• tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py
• tests/unit/modules/module_registry/test_commands.py

---

⚙️ CodeRabbit configuration file

tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by
contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior.

Files:

• tests/unit/specfact_cli/registry/test_module_dependencies.py
• tests/unit/registry/test_module_installer.py
• tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py
• tests/unit/modules/module_registry/test_commands.py

**/*cli*.{py,pyi}

📄 CodeRabbit inference engine (.cursorrules)

Commands should follow typer patterns with rich console output

Files:

• src/specfact_cli/cli.py

🪛 LanguageTool

docs/module-system/module-marketplace.md

[style] ~59-~59: This phrase is redundant. Consider writing “duplicates” or “copies”.
Context: ...e doctor` additionally reports shadowed duplicate copies, exact manifest versions, paths, enable...

(DUPLICATE_COPY)

openspec/changes/module-scope-version-diagnostics/design.md

[style] ~3-~3: This phrase is redundant. Consider writing “duplicates” or “copies”.
Context: ...e priority, but diagnostics will expose duplicate copies and version drift. Dependency enforceme...

(DUPLICATE_COPY)

openspec/changes/module-scope-version-diagnostics/proposal.md

[style] ~3-~3: To elevate your writing, try using a synonym here.
Context: ...d implicit development source roots are hard to see, and declared module dependency ...

(HARD_TO)

openspec/changes/module-scope-version-diagnostics/TDD_EVIDENCE.md

[uncategorized] ~40-~40: The official name of this software platform is spelled with a capital “H”.
Context: ... in src/specfact_cli/adapters/ado.py, src/specfact_cli/adapters/github.py, and `src/specfact_cli/validators/c...

(GITHUB)

docs/module-system/installing-modules.md

[style] ~124-~124: This phrase is redundant. Consider writing “duplicates” or “copies”.
Context: ...-only and reports effective vs shadowed duplicate copies, exact manifest versions, paths, enable...

(DUPLICATE_COPY)

🔀 Multi-repo context nold-ai/specfact-cli-modules

nold-ai/specfact-cli-modules

• tools/validate_repo_manifests.py: validates bundle_dependencies presence and content; will interact with any change that alters bundle_dependencies format/semantics or adds versioned bundle dependency entries. [::nold-ai/specfact-cli-modules::tools/validate_repo_manifests.py:256-275]

• registry/index.json and packages/*/module-package.yaml: many registry entries and package manifests include bundle_dependencies; installer/registry changes that start accepting version specifiers could require registry/manifest updates or additional validation. Example files: [::nold-ai/specfact-cli-modules::registry/index.json], [::nold-ai/specfact-cli-modules::packages/specfact-code-review/module-package.yaml], [::nold-ai/specfact-cli-modules::packages/specfact-project/module-package.yaml]

• tests referencing bundle_dependencies and verifier:

  • tests/unit/test_registry_manifest_bundle_dependencies.py checks registry bundle_dependencies match module-package.yaml — may need updates if versioned dependency syntax is introduced. [::nold-ai/specfact-cli-modules::tests/unit/test_registry_manifest_bundle_dependencies.py:1..]
  • tests/unit/test_validate_repo_manifests_bundle_deps.py — related validation tests. [::nold-ai/specfact-cli-modules::tests/unit/test_validate_repo_manifests_bundle_deps.py:37..296]
  • scripts/verify-modules-signature.py and its unit tests exist here (tests/unit/test_verify_modules_signature_script.py, tests/unit/test_pre_commit_verify_modules_signature_script.py) — the PR’s change to verify_manifest behavior (checksum vs signature) is mirrored by a script copy here; verify-related behavior/tests may need to stay in sync. [::nold-ai/specfact-cli-modules::scripts/verify-modules-signature.py:350,396,462,467,473,510], [::nold-ai/specfact-cli-modules::tests/unit/test_verify_modules_signature_script.py:15..]

• docs/reference/module-security.md and docs/reference/dependency-resolution.md: mention module_dependencies_versioned / pip_dependencies_versioned and dependency resolution behavior; documentation consumers should be checked for wording consistency with the stricter enforcement behavior introduced in the PR. [::nold-ai/specfact-cli-modules::docs/reference/module-security.md], [::nold-ai/specfact-cli-modules::docs/reference/dependency-resolution.md]

• packaging/version parsing usage: scripts/publish_module.py and publish_bundle_selection.py already import packaging.Version / InvalidVersion — indicates existing handling of versions in this repo that could be relevant if version specifiers appear in bundle_dependencies. [::nold-ai/specfact-cli-modules::publish_bundle_selection.py:5], [::nold-ai/specfact-cli-modules::scripts/publish_module.py:15]

Summary assessment

• This repository contains manifest/registry validation, package manifests, docs, and signature verification scripts that reference bundle_dependencies and version parsing. The PR’s changes (enforcing versioned bundle/module dependencies and signature/ checksum behavior) have direct, observable touchpoints here: manifest/registry validation and tests, package manifests in packages/, and the verify-modules-signature script and its tests. These files/tests should be reviewed/updated if the upstream format/validation rules or verify_manifest semantics change.

🔇 Additional comments (28)

src/specfact_cli/modules/module_registry/module-package.yaml (1)

2-2: Version metadata bump looks safe and scoped.

This change is a clean package version increment with no manifest contract or dependency-surface regression in this file.

src/specfact_cli/validators/change_proposal_integration.py (1)

19-19: Typing-only request payload cast looks safe

This is a non-behavioral typing adjustment; request payload shape and GitHub patch flow remain unchanged.

Also applies to: 191-191

src/specfact_cli/adapters/ado.py (1)

1628-1628: Request payload cast updates are consistent and low-risk

These changes are type-system compatibility updates only; HTTP behavior, retry policy, and ADO write semantics are preserved.

Also applies to: 2216-2216, 2673-2673, 3481-3481, 3785-3785, 3805-3805, 3823-3823, 3837-3839

src/specfact_cli/adapters/github.py (1)

1604-1606: Type-safety adjustment looks good and behavior-preserving.

These json=cast(Any, ...) updates keep strict typing happy without changing request semantics at runtime. No regression risk identified in these call sites.

Also applies to: 1670-1672, 1992-1994, 2988-2991

tests/unit/specfact_cli/modules/test_module_upgrade_improvements.py (1)

259-260: Good output-capture change for this warning-path test.

Using result.output or "" keeps the assertion path robust and avoids brittle stream-concatenation assumptions.

pyproject.toml (1)

7-7: Version bump looks consistent and safe.

0.46.21 is synchronized with the other version sources in this PR and remains ahead of the latest published PyPI release.

src/__init__.py (1)

6-6: Root version source update is correct.

This aligns with the coordinated package version bump across metadata files.

src/specfact_cli/__init__.py (1)

48-48: Version export update is in sync.

No concerns on this changed segment.

setup.py (1)

10-10: Setup metadata version bump is consistent.

This is aligned with the other declared version sources.

openspec/changes/module-scope-version-diagnostics/.openspec.yaml (1)

1-2: OpenSpec metadata is correctly set for this change.

schema and created match the required spec-driven track metadata.

openspec/CHANGE_ORDER.md (1)

11-11: Track E update is well integrated.

The new active item entry and snapshot table update are aligned.

Also applies to: 93-93

src/specfact_cli/cli.py (1)

696-703: Fallback logic looks correct for known bundle commands.

The added fallback at lines 701-702 ensures that when the real Click group cannot be materialized (e.g., during help generation for a newly registered command like module doctor), Click still receives a command object for known bundle/shim names instead of None. This prevents command resolution failures while preserving the lazy delegation pattern.

The logic is safe because:

• KNOWN_BUNDLE_GROUP_OR_SHIM_NAMES is a well-defined frozenset
• self._delegate_cmd is guaranteed to exist (initialized in __init__)
• The fallback only applies when real_group is None, which is the failure case

openspec/changes/module-scope-version-diagnostics/tasks.md (1)

1-25: Tasks checklist is complete and well-aligned with the PR scope.

The OpenSpec tasks document correctly tracks the four phases (Governance, Specs and Tests, Implementation, Evidence and Gates) and references the key implementation areas:

• specfact module doctor command
• Versioned bundle dependency enforcement (install-time)
• Versioned module dependency enforcement (registration-time)
• Documentation updates

All items are marked complete and the checklist aligns with the TDD_EVIDENCE.md validation guidance referenced in the coding guidelines.

openspec/changes/module-scope-version-diagnostics/proposal.md (1)

1-41: Proposal document is complete and well-structured.

The proposal clearly articulates the problem (module conflicts/shadowing across scopes with inconsistent version enforcement), the solution (diagnostics via module doctor + stricter enforcement), and the impact scope. The source tracking section properly links to GitHub issue #565, parent Feature #353, and Epic #194.

The document aligns with the OpenSpec guidelines and provides clear traceability for the change.

Note: The static analysis hint about "hard to see" on line 3 is a style suggestion and not material for this review.

src/specfact_cli/registry/module_state.py (1)

96-100: Dependent discovery correctly extended to include versioned dependencies.

The change properly extends find_dependents to consider both module_dependencies (list of module IDs) and module_dependencies_versioned (list of dependency objects with name attribute). The extracted names are combined via set union for the membership check.

This aligns with the PR's goal of enforcing versioned module dependencies and ensures that modules with versioned dependency declarations are correctly identified as dependents during disable operations.

Minor observation: The code uses str(getattr(dependency, "name", "")) which will safely handle malformed entries (returning empty string), preventing crashes. This defensive approach is appropriate for discovery logic.

openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md (1)

1-21: Installation spec clearly defines versioned dependency enforcement.

The spec properly defines the install-time validation requirements with two critical scenarios:

1. Pre-installation check that fails when an existing dependency version is outside the declared range
2. Post-installation validation that verifies newly installed dependencies satisfy the declared range

The error reporting requirement (dependency id, required range, installed version) provides actionable diagnostics for users.

This aligns with the guidance from openspec/changes/module-scope-version-diagnostics/specs/module-installation/spec.md and supports the PR's goal of preventing incompatible dependency sets from loading silently.

openspec/changes/module-scope-version-diagnostics/specs/module-scope-diagnostics/spec.md (1)

1-21: Module doctor spec provides clear diagnostic requirements.

The spec properly defines the specfact module doctor command requirements with two key scenarios:

1. Shadowing detection: Identifies effective vs shadowed copies across project/user scopes with versions, paths, and recovery commands
2. Development roots disclosure: Lists configured environment variables that affect import resolution

The requirement that diagnostics operate without importing module command code is critical for avoiding bootstrap/circular dependency issues and aligns with the design guidance.

This spec supports the PR's goal of making module conflicts and version mismatches visible and actionable.

openspec/changes/module-scope-version-diagnostics/specs/module-packages/spec.md (1)

1-21: Registration-time enforcement spec is clear and well-scoped.

The spec properly defines how versioned module dependencies are enforced during command registration:

1. Skip behavior: Modules with unsatisfied dependencies are skipped (not failed), maintaining graceful degradation
2. Diagnostics: Reports both required and discovered versions for troubleshooting
3. Eligibility check: Dependency must be enabled AND satisfy the declared version range

The skip-on-mismatch approach aligns with the existing module eligibility patterns and prevents hard failures during CLI initialization, while the diagnostic output makes version mismatches visible via module doctor.

docs/module-system/module-marketplace.md (2)

39-42: Discovery priority ordering correctly reflects the new behavior.

The updated discovery roots list now includes:

1. built-in modules
2. project modules
3. explicit development/custom roots (via SPECFACT_MODULES_ROOTS)
4. user modules
5. marketplace and custom roots

This ordering makes the environment-driven development roots explicit and places them at the correct priority level (after project, before user). This aligns with the module doctor requirement to disclose configured development source roots and helps users understand shadowing behavior.

---

50-59: Origin column and module doctor documentation enhance user guidance.

The additions properly document:

• Origin column (--show-origin): Lists the five possible origins (built-in, project, user, marketplace, custom) for module discovery transparency
• module doctor command: Provides the new diagnostic tool with its key outputs (shadowed duplicates, versions, paths, enabled state, development roots, recovery commands)

These updates align with the PR's goal of making module conflicts and version mismatches visible and actionable. The example commands (module list --show-origin, module doctor) give users concrete ways to inspect their module configuration.

Note: The LanguageTool hint about "duplicate copies" on line 59 is a style suggestion and not material for this review.

src/specfact_cli/registry/module_packages.py (6)

28-29: LGTM!

The InvalidSpecifier and SpecifierSet imports from packaging.specifiers are correctly added to support the new versioned dependency validation logic. The packaging library is already in use.

---

85-96: LGTM!

The module_versions field cleanly threads version information through the registration context, enabling versioned dependency validation during eligibility checks.

---

349-382: LGTM!

The _infer_category_from_legacy_group helper provides clean backward compatibility for legacy manifests that declare bundle_group_command without explicit category. The fallback to flat top-level mounting preserves behavior for unmapped groups.

---

615-626: LGTM!

Correctly computes and passes module_versions to enable versioned dependency validation during enable safety checks. This aligns with the PR objective to enforce declared version constraints.

---

643-663: LGTM!

The expansion to include module_dependencies_versioned in reverse dependency computation ensures --force mode correctly cascades disables to modules with versioned dependency declarations. The defensive if versioned.name guard adds extra safety.

---

679-695: LGTM!

Symmetric with the disable expansion changes. Correctly includes versioned dependencies in transitive upstream dependency computation for --force mode enables.

tests/unit/registry/test_module_installer.py (1)

158-194: Strong regression test for install-time version-range enforcement.

This directly validates the new contract (reject existing out-of-range dependency and report required vs installed versions) with a realistic installer path.

tests/unit/specfact_cli/registry/test_module_dependencies.py (1)

50-68: Good contract test for versioned dependency mismatch in lifecycle validation.

This is the right failure-mode assertion for the new module_dependencies_versioned enforcement path.

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/publish-module.py`:
- Around line 385-400: The function _bundle_dependency_ids_for_registry
currently silently returns [] when bundle_dependencies is not a list and skips
dict entries lacking a non-empty "id", which allows malformed manifests to pass;
change it to fail-fast: if raw_dependencies is not a list raise a ValueError (or
custom ManifestError) describing the expected list shape, and when iterating
entries if an entry is a dict but missing or empty "id" raise a ValueError
identifying the offending entry (include the entry value in the error message);
keep trimming/accepting valid string ids and preserve the returned list behavior
for well-formed entries.

In `@tests/unit/scripts/test_publish_module_bundle.py`:
- Around line 105-122: Add a negative unit test alongside
test_publish_entry_serializes_bundle_dependency_objects_as_ids that calls
module._build_publish_entry with bundle_dependencies containing a malformed
object (e.g. {"version": "1.0"} or {"id": ""}) and assert it raises a ValueError
(use pytest.raises) so the publish contract fails fast for missing/empty id;
reference the existing test name and the _build_publish_entry symbol when adding
the new test to ensure it lives in
tests/unit/scripts/test_publish_module_bundle.py near the positive case.

In `@tests/unit/specfact_cli/registry/test_module_dependencies.py`:
- Around line 91-104: The test reveals that _validate_module_dependencies
currently treats missing module_versions as a pass for
VersionedModuleDependency; update _validate_module_dependencies so that when
meta.module_dependencies_versioned is non-empty and the module_versions mapping
is missing entries (or entirely None/empty) for those dependencies, the function
marks them as missing (i.e., treat as "found unknown" / enforcement failure)
instead of skipping version comparison; locate logic that iterates
meta.module_dependencies_versioned in _validate_module_dependencies and add a
check that module_versions contains the dependency name and a non-null version
before attempting comparison, and if not present add the dependency to the
missing list returned.

In `@tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py`:
- Around line 78-90: The test currently asserts that a malformed installed
version ("not-a-version") is considered satisfied by
_dependency_version_satisfies, which weakens enforcement; change the test to
assert that _dependency_version_satisfies("not-a-version", ">=1.0") returns
False (i.e., treat malformed versions as unsatisfied) and keep the existing
caplog assertions so diagnostics still appear (ensure the test references the
same function name _dependency_version_satisfies and the same logger setup so
the log contains "not-a-version" and ">=1.0").

🪄 Autofix (Beta)

✅ Autofix completed

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bd03c573-6949-4faf-bdcd-6a58e9be5ccd

📥 Commits

Reviewing files that changed from the base of the PR and between e6f147b and e41f6db.

📒 Files selected for processing (16)

• CHANGELOG.md
• openspec/CHANGE_ORDER.md
• pyproject.toml
• scripts/publish-module.py
• setup.py
• src/__init__.py
• src/specfact_cli/__init__.py
• src/specfact_cli/modules/module_registry/module-package.yaml
• src/specfact_cli/modules/module_registry/src/commands.py
• src/specfact_cli/registry/module_installer.py
• src/specfact_cli/registry/module_packages.py
• tests/unit/modules/module_registry/test_commands.py
• tests/unit/scripts/test_publish_module_bundle.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py
• tests/unit/specfact_cli/registry/test_module_packages.py
• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py

✅ Files skipped from review due to trivial changes (5)

• pyproject.toml
• src/init.py
• src/specfact_cli/init.py
• openspec/CHANGE_ORDER.md
• CHANGELOG.md

🚧 Files skipped from review as they are similar to previous changes (5)

• src/specfact_cli/modules/module_registry/module-package.yaml
• setup.py
• src/specfact_cli/modules/module_registry/src/commands.py
• src/specfact_cli/registry/module_installer.py
• src/specfact_cli/registry/module_packages.py

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Tests (Python 3.12)
• GitHub Check: Compatibility (Python 3.11)

🧰 Additional context used

📓 Path-based instructions (6)

**/test_*.py

📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)

**/test_*.py: Write tests first in test-driven development (TDD) using the Red-Green-Refactor cycle
Ensure each test is independent and repeatable with no shared state between tests
Organize Python imports in tests using unittest.mock for Mock and patch
Use setup_method() for test initialization and Arrange-Act-Assert pattern in test files
Use @pytest.mark.asyncio decorator for async test functions in Python
Organize test files in structure: tests/unit/, tests/integration/, tests/e2e/ by module

Files:

• tests/unit/specfact_cli/registry/test_module_packages.py
• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py
• tests/unit/scripts/test_publish_module_bundle.py
• tests/unit/modules/module_registry/test_commands.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py

**/*.py

📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)

**/*.py: Maintain minimum 80% test coverage, with 100% coverage for critical paths in Python code
Use clear naming and self-documenting code, preferring clear names over comments
Ensure each function/class has a single clear purpose (Single Responsibility Principle)
Extract common patterns to avoid code duplication (DRY principle)
Apply SOLID object-oriented design principles in Python code
Use type hints everywhere in Python code and enable basedpyright strict mode
Use Pydantic models for data validation and serialization in Python
Use async/await for I/O operations in Python code
Use context managers for resource management in Python
Use dataclasses for simple data containers in Python
Enforce maximum line length of 120 characters in Python code
Use 4 spaces for indentation in Python code (no tabs)
Use 2 blank lines between classes and 1 blank line between methods in Python
Organize imports in order: Standard library → Third party → Local in Python files
Use snake_case for variables and functions in Python
Use PascalCase for class names in Python
Use UPPER_SNAKE_CASE for constants in Python
Use leading underscore (_) for private methods in Python classes
Use snake_case for Python file names
Enable basedpyright strict mode with strict type checking configuration in Python
Use Google-style docstrings for functions and classes in Python
Include comprehensive exception handling with specific exception types in Python code
Use logging with structured context (extra parameters) instead of print statements
Use retry logic with tenacity decorators (@retry) for operations that might fail
Use Pydantic BaseSettings for environment-based configuration in Python
Validate user input using Pydantic validators in Python models
Use @lru_cache and Redis-based caching for expensive calculations in Python
Run code formatting with Black (120 character line length) and isort in Python
Run type checking with basedpyright on all Python files
Run linting with ruff and pylint on all Pyth...

Files:

• tests/unit/specfact_cli/registry/test_module_packages.py
• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py
• scripts/publish-module.py
• tests/unit/scripts/test_publish_module_bundle.py
• tests/unit/modules/module_registry/test_commands.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py

tests/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

Tests must be meaningful and test actual functionality, cover both success and failure cases, be independent and repeatable, and have clear, descriptive names. NO EXCEPTIONS - no placeholder or empty tests.

tests/**/*.py: Trim low-value unit tests when a contract covers the same assertion (type/shape/raises on negative checks)
Delete tests that only assert input validation, datatype/shape enforcement, or raises on negative conditions now guarded by contracts and runtime typing
Convert repeated edge-case permutations into one Hypothesis property with contracts acting as oracles

Secret redaction via LoggerSetup.redact_secrets must be covered by unit tests

Files:

• tests/unit/specfact_cli/registry/test_module_packages.py
• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py
• tests/unit/scripts/test_publish_module_bundle.py
• tests/unit/modules/module_registry/test_commands.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py

---

⚙️ CodeRabbit configuration file

tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by
contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior.

Files:

• tests/unit/specfact_cli/registry/test_module_packages.py
• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py
• tests/unit/scripts/test_publish_module_bundle.py
• tests/unit/modules/module_registry/test_commands.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py

@(src|tests)/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

Linting must pass with no errors using: pylint src tests

Files:

• tests/unit/specfact_cli/registry/test_module_packages.py
• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py
• tests/unit/scripts/test_publish_module_bundle.py
• tests/unit/modules/module_registry/test_commands.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py

**/*.{py,pyi}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{py,pyi}: After any code changes, follow these steps in order: (1) Apply linting and formatting to ensure code quality: hatch run format, (2) Type checking: hatch run type-check (basedpyright), (3) Contract-first approach: Run hatch run contract-test for contract validation, (4) Run full test suite: hatch test --cover -v, (5) Verify all tests pass and contracts are satisfied, (6) Fix any issues and repeat steps until all tests pass
All public APIs must have @icontract decorators and @beartype type checking
Use Pydantic models for all data structures with data validation
Only write high-value comments if at all. Avoid talking to the user through comments

Files:

• tests/unit/specfact_cli/registry/test_module_packages.py
• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py
• scripts/publish-module.py
• tests/unit/scripts/test_publish_module_bundle.py
• tests/unit/modules/module_registry/test_commands.py
• tests/unit/specfact_cli/registry/test_module_dependencies.py

scripts/**/*.py

⚙️ CodeRabbit configuration file

scripts/**/*.py: Deterministic tooling: subprocess safety, Hatch integration, and parity with documented
quality gates (format, type-check, module signing).

Files:

• scripts/publish-module.py

🔀 Multi-repo context nold-ai/specfact-cli-modules

nold-ai/specfact-cli-modules

• tools/validate_repo_manifests.py: validates bundle_dependencies must be a list and that each bundle_dependencies entry matches a module id present in registry/index.json — potential mismatch if specfact-cli begins to accept versioned bundle dependency entries (ids + version specifiers). [::nold-ai/specfact-cli-modules::tools/validate_repo_manifests.py:256-275]

• registry/index.json: registry entries include bundle_dependencies arrays (string IDs, no version specifiers in current file). If core starts writing versioned bundle dependency objects, registry and this validation will need to accept/normalize that shape. Example entry with bundle_dependencies: "nold-ai/specfact-code-review" lists ["nold-ai/specfact-codebase"]. [::nold-ai/specfact-cli-modules::registry/index.json: entire file; sample module entries shown]

• tests referencing bundle_dependencies:

  • tests/unit/test_registry_manifest_bundle_dependencies.py ensures registry.bundle_dependencies equals package/module-package.yaml values and expects a list of strings. These tests will fail if specfact-cli/registry starts using objects with version fields instead of flat IDs. [::nold-ai/specfact-cli-modules::tests/unit/test_registry_manifest_bundle_dependencies.py:1..62]
  • tests/unit/test_validate_repo_manifests_bundle_deps.py validates manifests' bundle_dependencies shape/content. [::nold-ai/specfact-cli-modules::tests/unit/test_validate_repo_manifests_bundle_deps.py:1..296]

• scripts/verify-modules-signature.py: contains _parse_checksum, verify_manifest, and verify_manifest_integrity_shape_only. The PR in specfact-cli changed verify_manifest behavior when checksum verification is disabled (now requires integrity.checksum to exist and parse, and avoids signature-only verification when verify_checksum=False). There is a near-identical script here — its verify_manifest / verify_manifest_integrity_shape_only call sites and semantics should be reviewed to keep behavior consistent across repos/tests. [::nold-ai/specfact-cli-modules::scripts/verify-modules-signature.py:350-396, 411-419]

• docs/reference/module-security.md and docs/reference/dependency-resolution.md: document presence of module_dependencies_versioned / pip_dependencies_versioned and dependency resolution behavior; wording/validation expectations should be reviewed to align with stricter enforcement introduced in specfact-cli. [::nold-ai/specfact-cli-modules::docs/reference/module-security.md:71], [::nold-ai/specfact-cli-modules::docs/reference/dependency-resolution.md:20..48]

Summary assessment

• Relevant cross-repo touchpoints found: manifest/registry validation and tests assume bundle_dependencies are lists of IDs; registry data currently uses ID-only bundle_dependencies; module-signature verification script exists here and may need its verify_manifest semantics aligned with specfact-cli changes. These will likely need updates if specfact-cli introduces versioned bundle dependency objects or changes signature/checksum verification semantics.

🔇 Additional comments (3)

tests/unit/modules/module_registry/test_commands.py (3)

74-106: Good coverage for scope-precedence diagnostics and remediation path.

This test materially protects the new contract: project copy remains effective, user copy is reported shadowed, and actionable uninstall guidance is present.

---

108-135: Exact-namespace matching check is a strong regression guard.

Validating n1/foo does not leak n2/foo output closes a common identifier-collision edge case for module doctor filtering.

---

137-154: Nice isolation improvements for environment/cwd-sensitive paths.

The development-roots test and the tmp_path + chdir update make CLI behavior checks more deterministic and reduce environment-coupled flakiness.

Also applies to: 565-580

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 5 file(s) based on 4 unresolved review comments.

Files modified:

• scripts/publish-module.py
• src/specfact_cli/registry/module_installer.py
• src/specfact_cli/registry/module_packages.py
• tests/unit/scripts/test_publish_module_bundle.py
• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py

Commit: 8e053f7139c285cb76d05aa93d0feaf7cc315af6

The changes have been pushed to the feature/module-scope-version-diagnostics branch.

Time taken: 4m 18s

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

scripts/publish-module.py (1)

380-405: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail closed for non-string scalar dependency entries.

At Line 402, arbitrary values are coerced via str(entry), so values like None, 0, or True become published dependency IDs. That can mask malformed manifests and undermine strict dependency enforcement and registry integrity.

Suggested fix

 def _bundle_dependency_ids_for_registry(manifest: dict[str, Any]) -> list[str]:
     """Return registry-compatible bundle dependency IDs from string or object manifest entries."""
     raw_dependencies = manifest.get("bundle_dependencies", [])
     if not isinstance(raw_dependencies, list):
         raise ValueError(
             f"bundle_dependencies must be a list; got {type(raw_dependencies).__name__}: {raw_dependencies!r}"
         )
     dependency_ids: list[str] = []
     for entry in raw_dependencies:
         if isinstance(entry, dict):
             raw_id = entry.get("id")
             if raw_id is None or not str(raw_id).strip():
                 raise ValueError(
                     f"bundle_dependencies object entry must include non-empty 'id'; got {entry!r}"
                 )
             dependency_ids.append(str(raw_id).strip())
             continue
-        dependency_id = str(entry).strip()
-        if dependency_id:
-            dependency_ids.append(dependency_id)
+        if not isinstance(entry, str):
+            raise ValueError(
+                f"bundle_dependencies entries must be strings or objects with non-empty 'id'; got {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

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/publish-module.py` around lines 380 - 405, The function
_bundle_dependency_ids_for_registry currently coerces arbitrary non-dict entries
to strings (via str(entry)) which allows None/0/True to become dependency IDs;
change the loop to only accept dict entries with a non-empty 'id' and plain
string entries (check isinstance(entry, str)), rejecting any other scalar types
by raising a ValueError with a clear message referencing the offending entry;
preserve the existing behavior of stripping and skipping empty strings for
string entries and keep building dependency_ids from valid values.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/specfact_cli/registry/module_installer.py`:
- Around line 877-885: The fast-path that treats dependency_manifest
(ctx.target_root / dependency_name / "module-package.yaml") as satisfied must
first verify the on-disk registry ID: read the REGISTRY_ID_FILE in that same
directory and compare it to dependency_module_id before trusting the version;
only call _installed_dependency_version and
_raise_if_dependency_version_mismatch and log the satisfied message if the
stored registry id equals dependency_module_id, otherwise skip this
short-circuit and let the normal install resolution proceed (use the same
directory as dependency_manifest to locate REGISTRY_ID_FILE).

In `@tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py`:
- Around line 78-90: Add a second malformed-specifier case to the
test_dependency_version_satisfies_logs_malformed_inputs test: call
_dependency_version_satisfies("1.2.3", "not-a-specifier") and assert it returns
False, then assert that both "1.2.3" and "not-a-specifier" appear in
caplog.text; this covers the InvalidSpecifier branch in
_dependency_version_satisfies and mirrors the existing InvalidVersion
assertions.

---

Duplicate comments:
In `@scripts/publish-module.py`:
- Around line 380-405: The function _bundle_dependency_ids_for_registry
currently coerces arbitrary non-dict entries to strings (via str(entry)) which
allows None/0/True to become dependency IDs; change the loop to only accept dict
entries with a non-empty 'id' and plain string entries (check isinstance(entry,
str)), rejecting any other scalar types by raising a ValueError with a clear
message referencing the offending entry; preserve the existing behavior of
stripping and skipping empty strings for string entries and keep building
dependency_ids from valid values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2f530834-ad99-4304-a688-b07b1848ae4e

📥 Commits

Reviewing files that changed from the base of the PR and between e41f6db and 8e053f7.

📒 Files selected for processing (5)

• scripts/publish-module.py
• src/specfact_cli/registry/module_installer.py
• src/specfact_cli/registry/module_packages.py
• tests/unit/scripts/test_publish_module_bundle.py
• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/unit/scripts/test_publish_module_bundle.py
• src/specfact_cli/registry/module_packages.py

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Tests (Python 3.12)
• GitHub Check: Compatibility (Python 3.11)
• GitHub Check: Type Checking (basedpyright)

🧰 Additional context used

📓 Path-based instructions (8)

**/test_*.py

📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)

**/test_*.py: Write tests first in test-driven development (TDD) using the Red-Green-Refactor cycle
Ensure each test is independent and repeatable with no shared state between tests
Organize Python imports in tests using unittest.mock for Mock and patch
Use setup_method() for test initialization and Arrange-Act-Assert pattern in test files
Use @pytest.mark.asyncio decorator for async test functions in Python
Organize test files in structure: tests/unit/, tests/integration/, tests/e2e/ by module

Files:

• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py

**/*.py

📄 CodeRabbit inference engine (.cursor/rules/python-github-rules.mdc)

**/*.py: Maintain minimum 80% test coverage, with 100% coverage for critical paths in Python code
Use clear naming and self-documenting code, preferring clear names over comments
Ensure each function/class has a single clear purpose (Single Responsibility Principle)
Extract common patterns to avoid code duplication (DRY principle)
Apply SOLID object-oriented design principles in Python code
Use type hints everywhere in Python code and enable basedpyright strict mode
Use Pydantic models for data validation and serialization in Python
Use async/await for I/O operations in Python code
Use context managers for resource management in Python
Use dataclasses for simple data containers in Python
Enforce maximum line length of 120 characters in Python code
Use 4 spaces for indentation in Python code (no tabs)
Use 2 blank lines between classes and 1 blank line between methods in Python
Organize imports in order: Standard library → Third party → Local in Python files
Use snake_case for variables and functions in Python
Use PascalCase for class names in Python
Use UPPER_SNAKE_CASE for constants in Python
Use leading underscore (_) for private methods in Python classes
Use snake_case for Python file names
Enable basedpyright strict mode with strict type checking configuration in Python
Use Google-style docstrings for functions and classes in Python
Include comprehensive exception handling with specific exception types in Python code
Use logging with structured context (extra parameters) instead of print statements
Use retry logic with tenacity decorators (@retry) for operations that might fail
Use Pydantic BaseSettings for environment-based configuration in Python
Validate user input using Pydantic validators in Python models
Use @lru_cache and Redis-based caching for expensive calculations in Python
Run code formatting with Black (120 character line length) and isort in Python
Run type checking with basedpyright on all Python files
Run linting with ruff and pylint on all Pyth...

Files:

• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py
• scripts/publish-module.py
• src/specfact_cli/registry/module_installer.py

tests/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

Tests must be meaningful and test actual functionality, cover both success and failure cases, be independent and repeatable, and have clear, descriptive names. NO EXCEPTIONS - no placeholder or empty tests.

tests/**/*.py: Trim low-value unit tests when a contract covers the same assertion (type/shape/raises on negative checks)
Delete tests that only assert input validation, datatype/shape enforcement, or raises on negative conditions now guarded by contracts and runtime typing
Convert repeated edge-case permutations into one Hypothesis property with contracts acting as oracles

Secret redaction via LoggerSetup.redact_secrets must be covered by unit tests

Files:

• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py

---

⚙️ CodeRabbit configuration file

tests/**/*.py: Contract-first testing: meaningful scenarios, not redundant assertions already covered by
contracts. Flag flakiness, environment coupling, and missing coverage for changed behavior.

Files:

• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py

@(src|tests)/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

Linting must pass with no errors using: pylint src tests

Files:

• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py
• src/specfact_cli/registry/module_installer.py

**/*.{py,pyi}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{py,pyi}: After any code changes, follow these steps in order: (1) Apply linting and formatting to ensure code quality: hatch run format, (2) Type checking: hatch run type-check (basedpyright), (3) Contract-first approach: Run hatch run contract-test for contract validation, (4) Run full test suite: hatch test --cover -v, (5) Verify all tests pass and contracts are satisfied, (6) Fix any issues and repeat steps until all tests pass
All public APIs must have @icontract decorators and @beartype type checking
Use Pydantic models for all data structures with data validation
Only write high-value comments if at all. Avoid talking to the user through comments

Files:

• tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py
• scripts/publish-module.py
• src/specfact_cli/registry/module_installer.py

scripts/**/*.py

⚙️ CodeRabbit configuration file

scripts/**/*.py: Deterministic tooling: subprocess safety, Hatch integration, and parity with documented
quality gates (format, type-check, module signing).

Files:

• scripts/publish-module.py

src/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/spec-fact-cli-rules.mdc)

src/**/*.py: All code changes must be followed by running the full test suite using the smart test system.
All Python files in src/ and tools/ directories must have corresponding test files in tests/ directory. If you modify src/common/logger_setup.py, you MUST have tests/unit/common/test_logger_setup.py. NO EXCEPTIONS - even small changes require tests.
All new Python runtime code files must have corresponding test files created BEFORE committing the code. NO EXCEPTIONS - no code without tests.
Test Coverage Validation: Run hatch run smart-test-unit for modified files, hatch run smart-test-folder for modified directories, and hatch run smart-test-full before committing. ALL TESTS MUST PASS.
All components must support TEST_MODE=true environment variable with test-specific behavior defined as: if os.environ.get('TEST_MODE') == 'true': # test-specific behavior
Use src/common/logger_setup.py for all logging via: from common.logger_setup import get_logger; logger = get_logger(name)
Use src/common/redis_client.py with fallback for Redis operations via: from common.redis_client import get_redis_client; redis_client = get_redis_client()
Type checking must pass with no errors using: mypy .
Test coverage must meet or exceed 80% total coverage. New code must have corresponding tests. Modified code must maintain or improve coverage. Critical paths must have 100% coverage.
Use Pydantic v2 validation for all context and data schemas.

Add/update contracts on new or modified public APIs, stateful classes and adapters using icontract decorators and beartype runtime type checks

src/**/*.py: Meaningful Naming — identifiers reveal intent; avoid abbreviations. Identifiers in src/ must use snake_case (modules/functions), PascalCase (classes), UPPER_SNAKE_CASE (constants). Avoid single-letter names outside short loop variables.
KISS — keep functions and modules small and single-purpose. Maximum function length: 120 lines (Phase A error threshold). Maximum cyclomati...

Files:

• src/specfact_cli/registry/module_installer.py

src/specfact_cli/**/*.py

⚙️ CodeRabbit configuration file

src/specfact_cli/**/*.py: Focus on modular CLI architecture: lazy module loading, registry/bootstrap patterns, and
dependency direction. Flag breaking changes to public APIs, Pydantic models, and resource
bundling. Verify @icontract + @beartype on public surfaces; prefer centralized logging
(get_bridge_logger) over print().

Files:

• src/specfact_cli/registry/module_installer.py

🔀 Multi-repo context nold-ai/specfact-cli-modules

nold-ai/specfact-cli-modules

• tools/validate_repo_manifests.py enforces that manifest bundle_dependencies, when present, must be a list and that each entry is treated as an ID string; callers/tests expect ID-only entries (not object-with-version). Potential break if specfact-cli begins emitting objects with version_specifier fields. [::nold-ai/specfact-cli-modules::tools/validate_repo_manifests.py:264-275]

• registry/index.json entries currently store bundle_dependencies as arrays of ID strings (no version objects). Many registry/manifest alignment rules and tests assume this shape. If specfact-cli starts publishing versioned bundle dependency objects, registry publishing/consumers/tests here will need updates to accept/normalize that shape. [::nold-ai/specfact-cli-modules::registry/index.json:1..99]

• Tests in this repo validate that registry.bundle_dependencies equals package/module-package.yaml and assert the registry field is a list of strings; these tests will likely fail until this repo’s validation/tests are updated to accept versioned bundle dep forms. [::nold-ai/specfact-cli-modules::tests/unit/test_registry_manifest_bundle_dependencies.py:45-51]

• The publish workflow and publish-modules action copy manifest["bundle_dependencies"] into registry entries (publish step uses the manifest value directly). That means specfact-cli emitting object-shaped bundle_dependencies would be propagated into registry/index.json by the publish pipeline unless this repo updates publish logic/validation. [::nold-ai/specfact-cli-modules::.github/workflows/publish-modules.yml:393-394]

• scripts/verify-modules-signature.py exists here and contains verify_manifest / integrity-shape helpers; specfact-cli modified similar verification behavior when checksum verification is disabled. Review this script to ensure consistent semantics across repos (avoid diverging verification behavior). [::nold-ai/specfact-cli-modules::scripts/verify-modules-signature.py:350-396, 411-419]

• Documentation in this repo already references versioned dependency concepts (module_dependencies_versioned, etc.) but repository validation code and registry/test assets currently operate on ID-only bundle_dependencies arrays; alignment required between docs/specfact-cli enforcement and modules-registry validation/tests if versioned bundle deps are adopted. [::nold-ai/specfact-cli-modules::docs/reference/dependency-resolution.md:20..48]

Conclusion: I found cross-repo touchpoints likely affected by the PR changes — primarily the bundle_dependencies shape and signature/ checksum verification semantics. Updates will be needed in this repo’s validators, tests, and publish pipeline if specfact-cli starts emitting versioned bundle dependency objects or changes verification semantics.

[djm81]

@codex fix the failures and quality gates then push directly to this branch as part of the PR

[chatgpt-codex-connector]

Summary

• Fixed the bundle dependency install fast-path so an on-disk dependency is only treated as already satisfied when its REGISTRY_ID_FILE matches the expected full module id; otherwise normal install resolution proceeds.
• Expanded malformed-input coverage for version checks by adding an InvalidSpecifier case ("not-a-specifier") to the existing malformed version test.
• Updated the installer test fixture to include the dependency’s registry id file so the satisfied-dependency short-circuit remains valid under the stricter identity check.
• Committed changes on branch work in commit a729c5015fc66acf13ccd8d2519fab68d2f4a7c0.

Testing

• ⚠️ hatch run pytest tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py -q (warning: hatch is not installed in this environment)
• ✅ python -m pytest tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py -q
• ❌ python -m pytest tests/unit/registry/test_module_installer.py -q (initial run surfaced one failing test due to tightened fast-path identity behavior before test fixture update)
• ✅ python -m pytest tests/unit/registry/test_module_installer.py::test_install_module_logs_satisfied_dependencies_without_warning tests/unit/specfact_cli/registry/test_versioned_bundle_deps.py -q

View task →