From b5ad2ea0d5e0ee062906e0c7b2f156330ea1a39f Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sat, 29 Aug 2026 22:56:03 +0200 Subject: [PATCH 01/10] test(modules): capture user-scope preservation regression --- .../.openspec.yaml | 2 + .../design.md | 21 + .../proposal.md | 38 ++ .../requirements-evidence.yaml | 31 + .../requirements-proof/review-evidence.json | 9 + .../specs/module-scope-diagnostics/spec.md | 30 + .../modules/module_registry/test_commands.py | 550 ++++++++++-------- tests/unit/registry/test_module_discovery.py | 4 +- 8 files changed, 433 insertions(+), 252 deletions(-) create mode 100644 openspec/changes/module-scope-02-preserve-user-installs/.openspec.yaml create mode 100644 openspec/changes/module-scope-02-preserve-user-installs/design.md create mode 100644 openspec/changes/module-scope-02-preserve-user-installs/proposal.md create mode 100644 openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml create mode 100644 openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json create mode 100644 openspec/changes/module-scope-02-preserve-user-installs/specs/module-scope-diagnostics/spec.md diff --git a/openspec/changes/module-scope-02-preserve-user-installs/.openspec.yaml b/openspec/changes/module-scope-02-preserve-user-installs/.openspec.yaml new file mode 100644 index 00000000..50adc910 --- /dev/null +++ b/openspec/changes/module-scope-02-preserve-user-installs/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-29 diff --git a/openspec/changes/module-scope-02-preserve-user-installs/design.md b/openspec/changes/module-scope-02-preserve-user-installs/design.md new file mode 100644 index 00000000..2776ad1d --- /dev/null +++ b/openspec/changes/module-scope-02-preserve-user-installs/design.md @@ -0,0 +1,21 @@ +## Overview + +Preserve the existing deterministic discovery order while correcting the meaning of a shadowed user module. A project copy is effective only for the current repository; the user copy is neither stale nor invalid by default and remains the effective installation elsewhere. + +## Decisions + +- Keep all discovery roots, precedence, deduplication, and shadowed-entry reporting unchanged. +- Keep the discovery signal user-visible, but describe it as workspace-local precedence and explicitly state that no action is required. +- Replace the doctor recovery command with explanatory shadowing guidance. `module list --show-origin` remains the diagnostic path for inspecting exact sources. +- Do not weaken or remove explicit `specfact module uninstall --scope user`; this defect concerns automatic/routine advice, not intentional lifecycle commands. +- Test both message producers directly so future wording changes cannot reintroduce the destructive recommendation. + +## Risks + +- Users with a genuinely unwanted duplicate no longer receive a one-line delete command. Mitigation: diagnostics still show both origins and explicit uninstall remains available in lifecycle documentation. +- Warning language could become noisy despite being safe. Mitigation: existing once-per-process deduplication remains unchanged. +- Only one repository could merge, temporarily leaving inconsistent guidance. Mitigation: paired issues and PRs are cross-linked and independently safe to merge. + +## Rollback + +Revert the diagnostic text and tests. No persisted module state or installation files are changed by this patch. diff --git a/openspec/changes/module-scope-02-preserve-user-installs/proposal.md b/openspec/changes/module-scope-02-preserve-user-installs/proposal.md new file mode 100644 index 00000000..2e1622b3 --- /dev/null +++ b/openspec/changes/module-scope-02-preserve-user-installs/proposal.md @@ -0,0 +1,38 @@ +## Why + +Core module discovery and `specfact module doctor` describe normal project-over-user shadowing as stale state and recommend uninstalling the user-scoped copy. Review/bootstrap workflows surface and follow that recommendation, repeatedly removing `specfact-codebase` and `specfact-code-review` from the user scope even though those installations are still needed in other repositories. + +## What Changes + +- Keep project-over-user precedence unchanged. +- Replace user-scope uninstall recovery advice with non-destructive scope guidance in module discovery warnings and doctor output. +- State explicitly that the user-scoped copy remains installed and available outside the current workspace and that no action is required for normal shadowing. +- Add regression tests that reject destructive user-scope uninstall recommendations while preserving origin diagnostics. + +## Capabilities + +### Modified Capabilities + +- `module-scope-diagnostics`: Discovery and doctor diagnostics report shadowing without treating a valid user installation as cleanup residue. + +## Impact + +- Affected code: `src/specfact_cli/registry/module_discovery.py` and `src/specfact_cli/modules/module_registry/src/commands.py`. +- Affected tests: focused module discovery and module doctor unit tests. +- Paired modules guidance: `nold-ai/specfact-cli-modules#452` corrects the repository bootstrap surfaces that trigger this behavior during review work. +- Compatibility and data impact: none. Discovery order, module state, explicit uninstall behavior, manifests, and persistent installation data remain unchanged. + +--- + +## Source Tracking + + +- **Parent Feature**: [#353](https://github.com/nold-ai/specfact-cli/issues/353) +- **Parent Epic**: [#194](https://github.com/nold-ai/specfact-cli/issues/194) +- **Bug Issue**: [#699](https://github.com/nold-ai/specfact-cli/issues/699) +- **Paired Modules Bug**: [nold-ai/specfact-cli-modules#452](https://github.com/nold-ai/specfact-cli-modules/issues/452) +- **Issue Relationships**: `#699` is a sub-issue of Feature `#353`; Feature `#353` is a sub-issue of Epic `#194`. +- **Blocked By**: none +- **Repository**: nold-ai/specfact-cli +- **Last Synced Status**: issue type, labels, assignee, parent, project assignment, In Progress status, and blocker metadata verified on 2026-08-29 +- **Sanitized**: false diff --git a/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml b/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml new file mode 100644 index 00000000..def55705 --- /dev/null +++ b/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml @@ -0,0 +1,31 @@ +schema_version: "2" +requirements: + openspec:module-scope-02-preserve-user-installs:module-scope-diagnostics:module-doctor-reports-effective-and-shadowed-module-copies: + rationale: "Module diagnostics must explain project precedence without treating valid user installations as stale." + stakeholder_refs: + - "nold-ai/specfact-cli#699" + - "nold-ai/specfact-cli-modules#452" + touchpoints: + - id: discovery-shadow-warning + kind: source_file + locator: "src/specfact_cli/registry/module_discovery.py" + - id: module-doctor-guidance + kind: source_file + locator: "src/specfact_cli/modules/module_registry/src/commands.py" + verification_cases: + - case_id: MSI-CORE-001 + scenario_id: module-doctor-reports-effective-and-shadowed-module-copies + method: test + intent: "Report both copies and preserve the user-scoped installation." + observable: "Doctor states that no action is required and emits no uninstall advice." + selector: + runner: pytest + node_id: tests/unit/modules/module_registry/test_commands.py::test_doctor_reports_effective_and_shadowed_duplicate_modules + - case_id: MSI-CORE-002 + scenario_id: module-doctor-reports-effective-and-shadowed-module-copies + method: test + intent: "Keep runtime precedence diagnostics non-destructive." + observable: "Discovery states that the user copy remains installed and emits no uninstall advice." + selector: + runner: pytest + node_id: tests/unit/registry/test_module_discovery.py::test_project_shadow_warning_is_actionable_and_emitted_once diff --git a/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json b/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json new file mode 100644 index 00000000..80c4dc14 --- /dev/null +++ b/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json @@ -0,0 +1,9 @@ +{ + "schema_version": "1", + "decision": "accepted", + "reviewer_id": "djm81", + "reviewer_role": "product-owner", + "recorded_at": "2026-08-29T22:20:04+02:00", + "reference": "https://github.com/nold-ai/specfact-cli/issues/699", + "mapping_digest": "sha256:d25cd13f7e7fb1e327e2d9ba6c38eab85f5ba7ae870bd4a61efc13e05446f8f4" +} diff --git a/openspec/changes/module-scope-02-preserve-user-installs/specs/module-scope-diagnostics/spec.md b/openspec/changes/module-scope-02-preserve-user-installs/specs/module-scope-diagnostics/spec.md new file mode 100644 index 00000000..c36b63e5 --- /dev/null +++ b/openspec/changes/module-scope-02-preserve-user-installs/specs/module-scope-diagnostics/spec.md @@ -0,0 +1,30 @@ +## MODIFIED Requirements + +### Requirement: Module doctor reports effective and shadowed module copies + +The system SHALL provide module-scope diagnostics that report module origin, version, path, and shadowing state without importing module command code or treating a valid lower-priority installation as stale by default. + +#### Scenario: Duplicate project and user module copies are visible + +- **GIVEN** a module id exists in project scope and user scope with different versions +- **WHEN** the user runs `specfact module doctor ` +- **THEN** the output identifies the project copy as effective +- **AND** the output identifies the user copy as shadowed +- **AND** the output shows both versions and paths +- **AND** the output states that the user-scoped copy remains installed and available outside the current workspace +- **AND** the output states that no action is required for normal shadowing +- **AND** the output does not recommend uninstalling the user-scoped copy + +#### Scenario: Runtime discovery reports project-over-user precedence + +- **GIVEN** a module id exists in project scope and user scope +- **WHEN** runtime discovery selects the project-scoped copy +- **THEN** the diagnostic identifies project scope as effective in the current workspace +- **AND** it states that the user-scoped copy remains installed and available outside the workspace +- **AND** it does not recommend uninstalling the user-scoped copy + +#### Scenario: Development source roots are disclosed + +- **GIVEN** development source root environment variables are configured +- **WHEN** the user runs `specfact module doctor` +- **THEN** the output lists the configured development source roots that may influence import resolution diff --git a/tests/unit/modules/module_registry/test_commands.py b/tests/unit/modules/module_registry/test_commands.py index bd733c2d..eb7414b1 100644 --- a/tests/unit/modules/module_registry/test_commands.py +++ b/tests/unit/modules/module_registry/test_commands.py @@ -7,17 +7,18 @@ import pytest from typer.testing import CliRunner -from specfact_cli.models.module_package import ModulePackageMetadata +from specfact_cli.models.module_package import ModulePackageMetadata, PublisherInfo from specfact_cli.modules.module_registry.src.commands import app from specfact_cli.registry.module_discovery import DiscoveredModule from specfact_cli.registry.module_installer import USER_MODULES_ROOT, InstallModuleOptions runner = CliRunner() +CODEBASE_MODULE_ID = "nold-ai/specfact-codebase" @pytest.fixture(autouse=True) -def _isolate_user_modules_root(monkeypatch, tmp_path: Path) -> None: +def isolate_user_modules_root(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: """Isolate user module root so tests do not depend on machine-local installs.""" user_root = tmp_path / "user-modules" user_root.mkdir(parents=True, exist_ok=True) @@ -25,11 +26,178 @@ def _isolate_user_modules_root(monkeypatch, tmp_path: Path) -> None: monkeypatch.setattr("specfact_cli.registry.module_installer.USER_MODULES_ROOT", user_root, raising=False) -def test_install_command_integration(monkeypatch, tmp_path: Path) -> None: +def _write_disabled_codebase(install_root: Path) -> None: + installed_module = install_root / "specfact-codebase" + installed_module.mkdir(parents=True) + (installed_module / "module-package.yaml").write_text( + f"name: {CODEBASE_MODULE_ID}\nversion: '0.1.0'\ncommands: [analyze]\n", + encoding="utf-8", + ) + + +def _patch_user_reenable_state( + monkeypatch: pytest.MonkeyPatch, + install_root: Path, + enabled: list[list[str]], + captured_state: list[list[dict[str, object]]], +) -> None: + def discover_state(*, enable_ids: list[str], **_kwargs: object) -> list[dict[str, object]]: + enabled.append(list(enable_ids)) + return [ + {"id": CODEBASE_MODULE_ID, "version": "0.1.0", "enabled": True}, + {"id": "unrelated-module", "version": "9.9.9", "enabled": False}, + ] + + monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.USER_MODULES_ROOT", install_root) + monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", list) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.read_modules_state", + lambda: {CODEBASE_MODULE_ID: {"version": "0.1.0", "enabled": False}}, + ) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.get_discovered_modules_for_state", + discover_state, + ) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.write_modules_state", + lambda modules: captured_state.append(modules), + ) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.run_discovery_and_write_cache", + lambda _version: None, + ) + + +def _patch_project_reenable_state( + monkeypatch: pytest.MonkeyPatch, + base_paths: list[Path | None], + state_by_id: dict[str, dict[str, object]], +) -> None: + def discover_state(*, base_path: Path | None = None, **_kwargs: object) -> list[dict[str, object]]: + base_paths.append(base_path) + return [{"id": CODEBASE_MODULE_ID, "version": "0.1.0", "enabled": True}] + + def write_state(modules: list[dict[str, object]]) -> None: + for row in modules: + state_by_id[str(row["id"])] = { + "version": str(row["version"]), + "enabled": bool(row["enabled"]), + } + + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.discover_all_modules_for_project", + lambda _path: [], + ) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.read_modules_state", + lambda: dict(state_by_id), + ) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.get_discovered_modules_for_state", + discover_state, + ) + monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.write_modules_state", write_state) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.run_discovery_and_write_cache", + lambda _version: None, + ) + + +def _module_registry_metadata( + *, + name: str = "module-registry", + commands: list[str] | None = None, + command_help: dict[str, str] | None = None, +) -> ModulePackageMetadata: + return ModulePackageMetadata( + name=name, + description="Manage modules", + license="Apache-2.0", + tier="community", + commands=commands or [], + command_help=command_help, + core_compatibility=">=0.28.0,<1.0.0", + publisher=PublisherInfo( + name="nold-ai", + email="opensource@nold.ai", + attributes={"url": "https://github.com/nold-ai/specfact-cli-modules"}, + ), + ) + + +def _patch_show_module( + monkeypatch: pytest.MonkeyPatch, + metadata: ModulePackageMetadata, + *, + source: str = "builtin", +) -> None: + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", + lambda: [ + { + "id": metadata.name, + "version": metadata.version, + "enabled": True, + "source": source, + "official": True, + "publisher": "nold-ai", + } + ], + ) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.discover_all_modules", + lambda: [DiscoveredModule(Path("/modules") / metadata.name, metadata, source)], + ) + + +class _CommandInfo: + def __init__(self, name: str, help_text: str) -> None: + self.name = name + self.help = help_text + self.callback = None + + +class _GroupInfo: + def __init__(self, name: str, typer_instance: object) -> None: + self.name = name + self.typer_instance = typer_instance + + +class _FakeTyper: + def __init__(self, commands: list[tuple[str, str]], groups: list[object]) -> None: + self.registered_commands = [_CommandInfo(name, help_text) for name, help_text in commands] + self.registered_groups = groups + + +def _version_state_trust_rows() -> list[dict[str, object]]: + return [ + { + "id": "init", + "version": "0.1.0", + "enabled": True, + "source": "builtin", + "official": True, + "publisher": "nold-ai", + }, + { + "id": "backlog", + "version": "0.2.0", + "enabled": False, + "source": "marketplace", + "official": False, + "publisher": "community-dev", + }, + ] + + +def test_install_command_integration(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + def install_module_stub(module_id: str, _options: InstallModuleOptions, **_kwargs: object) -> Path: + return tmp_path / module_id.split("/")[-1] + monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", list) monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.install_module", - lambda module_id, options=None, **_kwargs: tmp_path / module_id.split("/")[-1], + install_module_stub, ) result = runner.invoke(app, ["install", "specfact/backlog"]) @@ -39,10 +207,10 @@ def test_install_command_integration(monkeypatch, tmp_path: Path) -> None: assert "specfact/backlog" in result.stdout -def test_install_command_accepts_bare_module_name(monkeypatch, tmp_path: Path) -> None: +def test_install_command_accepts_bare_module_name(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: captured: dict[str, str | None] = {"module_id": None} - def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _install(module_id: str, options: InstallModuleOptions, **_kwargs): captured["module_id"] = module_id return tmp_path / module_id.split("/")[-1] @@ -60,7 +228,7 @@ def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwa assert "Installed" in result.stdout -def test_install_command_rejects_invalid_module_id(monkeypatch) -> None: +def test_install_command_rejects_invalid_module_id(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.install_module", lambda *_args, **_kwargs: None ) @@ -71,7 +239,9 @@ def test_install_command_rejects_invalid_module_id(monkeypatch) -> None: assert "Invalid module id" in result.stdout -def test_doctor_reports_effective_and_shadowed_duplicate_modules(monkeypatch, tmp_path: Path) -> None: +def test_doctor_reports_effective_and_shadowed_duplicate_modules( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: project_dir = tmp_path / "repo" / ".specfact" / "modules" / "specfact-codebase" user_dir = tmp_path / "user-modules" / "specfact-codebase" project_dir.mkdir(parents=True) @@ -102,10 +272,15 @@ def test_doctor_reports_effective_and_shadowed_duplicate_modules(monkeypatch, tm assert "shadowed" in result.stdout assert "0.41.0" in result.stdout assert "0.40.0" in result.stdout - assert "specfact module uninstall nold-ai/specfact-codebase --scope user" in result.stdout + normalized_output = " ".join(result.stdout.split()) + assert "remains installed and available outside this workspace" in normalized_output + assert "No action is required" in normalized_output + assert "module uninstall" not in normalized_output -def test_doctor_fully_qualified_module_id_matches_exact_namespace(monkeypatch, tmp_path: Path) -> None: +def test_doctor_fully_qualified_module_id_matches_exact_namespace( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: entries = [ DiscoveredModule( tmp_path / "repo" / ".specfact" / "modules" / "foo", @@ -134,7 +309,7 @@ def test_doctor_fully_qualified_module_id_matches_exact_namespace(monkeypatch, t assert "2.0.0" not in result.stdout -def test_doctor_reports_configured_development_source_roots(monkeypatch, tmp_path: Path) -> None: +def test_doctor_reports_configured_development_source_roots(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: modules_repo = tmp_path / "specfact-cli-modules" extra_root = tmp_path / "extra-modules" monkeypatch.setenv("SPECFACT_MODULES_REPO", str(modules_repo)) @@ -153,7 +328,9 @@ def test_doctor_reports_configured_development_source_roots(monkeypatch, tmp_pat assert "extra-modules" in result.stdout -def test_install_command_skips_when_module_already_available_locally(monkeypatch, tmp_path: Path) -> None: +def test_install_command_skips_when_module_already_available_locally( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: class _Meta: name = "bundle-mapper" @@ -163,7 +340,7 @@ class _Entry: called = {"install": False} - def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _install(module_id: str, options: InstallModuleOptions, **_kwargs): called["install"] = True return tmp_path / module_id.split("/")[-1] @@ -177,111 +354,53 @@ def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwa assert "already installed" in result.stdout or "already available" in result.stdout -def test_install_command_existing_disabled_module_enables_state(monkeypatch, tmp_path: Path) -> None: +def test_install_command_existing_disabled_module_enables_state( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: install_root = tmp_path / "user-modules" - installed_module = install_root / "specfact-codebase" - installed_module.mkdir(parents=True) - (installed_module / "module-package.yaml").write_text( - "name: nold-ai/specfact-codebase\nversion: '0.1.0'\ncommands: [analyze]\n", - encoding="utf-8", - ) enabled: list[list[str]] = [] captured_state: list[list[dict[str, object]]] = [] - monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.USER_MODULES_ROOT", install_root) - monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", list) - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.install_module", lambda *_args, **_kwargs: None - ) - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.read_modules_state", - lambda: {"nold-ai/specfact-codebase": {"version": "0.1.0", "enabled": False}}, - ) - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.get_discovered_modules_for_state", - lambda *, enable_ids, disable_ids, base_path=None, preserve_existing: ( - enabled.append(list(enable_ids)) - or [ - {"id": "nold-ai/specfact-codebase", "version": "0.1.0", "enabled": True}, - {"id": "unrelated-module", "version": "9.9.9", "enabled": False}, - ] - ), - ) - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.write_modules_state", - lambda modules: captured_state.append(modules), - ) - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.run_discovery_and_write_cache", lambda _: None - ) - - result = runner.invoke(app, ["install", "nold-ai/specfact-codebase"]) + _write_disabled_codebase(install_root) + _patch_user_reenable_state(monkeypatch, install_root, enabled, captured_state) + result = runner.invoke(app, ["install", CODEBASE_MODULE_ID]) assert result.exit_code == 0 - assert enabled == [["nold-ai/specfact-codebase"]] + assert enabled == [[CODEBASE_MODULE_ID]] assert captured_state == [ [ - {"id": "nold-ai/specfact-codebase", "version": "0.1.0", "enabled": True}, + {"id": CODEBASE_MODULE_ID, "version": "0.1.0", "enabled": True}, {"id": "unrelated-module", "version": "9.9.9", "enabled": False}, ] ] assert "enabled" in result.stdout.lower() -def test_install_command_project_scope_reenable_uses_selected_repo(monkeypatch, tmp_path: Path) -> None: +def test_install_command_project_scope_reenable_uses_selected_repo( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: repo_path = tmp_path / "repo" install_root = repo_path / ".specfact" / "modules" - installed_module = install_root / "specfact-codebase" - installed_module.mkdir(parents=True) - (installed_module / "module-package.yaml").write_text( - "name: nold-ai/specfact-codebase\nversion: '0.1.0'\ncommands: [analyze]\n", - encoding="utf-8", - ) base_paths: list[Path | None] = [] - state_by_id = {"nold-ai/specfact-codebase": {"version": "0.1.0", "enabled": False}} + state_by_id = {CODEBASE_MODULE_ID: {"version": "0.1.0", "enabled": False}} - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.discover_all_modules_for_project", lambda path: [] - ) - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.install_module", lambda *_args, **_kwargs: None - ) - - def _read_state(): - return dict(state_by_id) - - def _discover_state(*, enable_ids, disable_ids, base_path=None, preserve_existing): - base_paths.append(base_path) - return [{"id": "nold-ai/specfact-codebase", "version": "0.1.0", "enabled": True}] + _write_disabled_codebase(install_root) + _patch_project_reenable_state(monkeypatch, base_paths, state_by_id) - def _write_state(modules): - for row in modules: - state_by_id[str(row["id"])] = {"version": str(row["version"]), "enabled": bool(row["enabled"])} - - monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.read_modules_state", _read_state) - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.get_discovered_modules_for_state", - _discover_state, - ) - monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.write_modules_state", _write_state) - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.run_discovery_and_write_cache", lambda _: None - ) - - result = runner.invoke( - app, ["install", "nold-ai/specfact-codebase", "--scope", "project", "--repo", str(repo_path)] - ) + result = runner.invoke(app, ["install", CODEBASE_MODULE_ID, "--scope", "project", "--repo", str(repo_path)]) assert result.exit_code == 0 assert base_paths == [repo_path] - assert state_by_id["nold-ai/specfact-codebase"]["enabled"] is True + assert state_by_id[CODEBASE_MODULE_ID]["enabled"] is True assert "enabled" in result.stdout.lower() -def test_install_command_project_scope_installs_to_project_modules_root(monkeypatch, tmp_path: Path) -> None: +def test_install_command_project_scope_installs_to_project_modules_root( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: captured: dict[str, object] = {"install_root": None, "module_id": None} - def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _install(module_id: str, options: InstallModuleOptions, **_kwargs): o = options or InstallModuleOptions() captured["module_id"] = module_id captured["install_root"] = o.install_root @@ -304,14 +423,16 @@ def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwa assert captured["install_root"] == repo_path / ".specfact" / "modules" -def test_install_command_project_scope_normalizes_nested_repo_path(monkeypatch, tmp_path: Path) -> None: +def test_install_command_project_scope_normalizes_nested_repo_path( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: captured: dict[str, object] = {"install_root": None, "discovery_repo": None} repo_root = tmp_path / "repo" nested_dir = repo_root / "services" / "api" nested_dir.mkdir(parents=True) (repo_root / ".git").mkdir() - def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _install(module_id: str, options: InstallModuleOptions, **_kwargs): o = options or InstallModuleOptions() captured["install_root"] = o.install_root return tmp_path / module_id.split("/")[-1] @@ -335,7 +456,7 @@ def _discover(repo: Path | None): assert captured["install_root"] == repo_root / ".specfact" / "modules" -def test_install_command_prefers_bundled_source_when_available(monkeypatch, tmp_path: Path) -> None: +def test_install_command_prefers_bundled_source_when_available(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", list) monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_bundled_module_metadata", @@ -373,7 +494,9 @@ def _install_marketplace(*_args, **_kwargs): assert called["marketplace"] is False -def test_install_command_project_scope_does_not_skip_when_user_scope_module_exists(monkeypatch, tmp_path: Path) -> None: +def test_install_command_project_scope_does_not_skip_when_user_scope_module_exists( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: class _Meta: name = "bundle-mapper" @@ -385,7 +508,7 @@ class _Entry: called = {"marketplace": False} - def _install_marketplace(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _install_marketplace(module_id: str, options: InstallModuleOptions, **_kwargs): called["marketplace"] = True return tmp_path / module_id.split("/")[-1] @@ -403,7 +526,9 @@ def _install_marketplace(module_id: str, options: InstallModuleOptions | None = assert called["marketplace"] is True -def test_install_command_source_marketplace_skips_bundled_resolution(monkeypatch, tmp_path: Path) -> None: +def test_install_command_source_marketplace_skips_bundled_resolution( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", list) monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_bundled_module_metadata", @@ -416,7 +541,7 @@ def _bundled(*_args, **_kwargs): called["bundled"] = True return True - def _marketplace(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _marketplace(module_id: str, options: InstallModuleOptions, **_kwargs): called["marketplace"] = True return tmp_path / module_id.split("/")[-1] @@ -431,12 +556,12 @@ def _marketplace(module_id: str, options: InstallModuleOptions | None = None, ** def test_install_command_requires_explicit_trust_for_non_official_in_non_interactive( - monkeypatch, tmp_path: Path + monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", list) monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.is_non_interactive", lambda: True) - def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _install(module_id: str, options: InstallModuleOptions, **_kwargs): o = options or InstallModuleOptions() if not o.trust_non_official and o.non_interactive: raise ValueError("requires --trust-non-official") @@ -455,12 +580,14 @@ def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwa assert "--trust-non-official" in result.stdout -def test_install_command_passes_trust_flag_to_marketplace_installer(monkeypatch, tmp_path: Path) -> None: +def test_install_command_passes_trust_flag_to_marketplace_installer( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", list) monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.is_non_interactive", lambda: True) captured: dict[str, bool | None] = {"trust_non_official": None, "non_interactive": None} - def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _install(module_id: str, options: InstallModuleOptions, **_kwargs): o = options or InstallModuleOptions() captured["trust_non_official"] = o.trust_non_official captured["non_interactive"] = o.non_interactive @@ -480,7 +607,7 @@ def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwa assert captured["non_interactive"] is True -def test_module_init_passes_trust_flag_and_non_interactive(monkeypatch, tmp_path: Path) -> None: +def test_module_init_passes_trust_flag_and_non_interactive(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: captured: dict[str, object] = {"trust_non_official": None, "non_interactive": None} def _sync(*, target_root, trust_non_official=False, non_interactive=False): @@ -498,7 +625,7 @@ def _sync(*, target_root, trust_non_official=False, non_interactive=False): assert captured["non_interactive"] is True -def test_uninstall_command_with_source_validation(monkeypatch) -> None: +def test_uninstall_command_with_source_validation(monkeypatch: pytest.MonkeyPatch) -> None: called = {"ok": False} class _Meta: @@ -520,7 +647,9 @@ def fake_uninstall(module_name: str, **_kwargs) -> None: assert called["ok"] is True -def test_uninstall_command_requires_scope_when_module_exists_in_user_and_project(monkeypatch, tmp_path: Path) -> None: +def test_uninstall_command_requires_scope_when_module_exists_in_user_and_project( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: repo_path = tmp_path / "repo" project_modules = repo_path / ".specfact" / "modules" / "bundle-mapper" user_modules = tmp_path / "user-modules" / "bundle-mapper" @@ -544,7 +673,7 @@ def test_uninstall_command_requires_scope_when_module_exists_in_user_and_project assert "project" in result.stdout -def test_uninstall_command_custom_module_has_clear_guidance(monkeypatch, tmp_path: Path) -> None: +def test_uninstall_command_custom_module_has_clear_guidance(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: class _Meta: name = "bundle-mapper" @@ -562,7 +691,7 @@ class _Entry: assert "local module roots" in result.stdout -def test_uninstall_command_namespace_input_normalizes_name(monkeypatch, tmp_path: Path) -> None: +def test_uninstall_command_namespace_input_normalizes_name(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: class _Meta: name = "bundle-mapper" @@ -579,7 +708,7 @@ class _Entry: assert "Cannot uninstall custom module 'bundle-mapper'" in result.stdout -def test_uninstall_command_unknown_module_has_clear_guidance(monkeypatch) -> None: +def test_uninstall_command_unknown_module_has_clear_guidance(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", list) result = runner.invoke(app, ["uninstall", "specfact/missing-module"]) @@ -589,7 +718,7 @@ def test_uninstall_command_unknown_module_has_clear_guidance(monkeypatch) -> Non assert "module list --show-origin" in result.stdout -def test_search_command_filters_registry(monkeypatch) -> None: +def test_search_command_filters_registry(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.fetch_all_indexes", lambda: [ @@ -625,7 +754,7 @@ def test_search_command_filters_registry(monkeypatch) -> None: assert "specfact/policy" not in result.stdout -def test_search_command_sorts_results_alphabetically(monkeypatch) -> None: +def test_search_command_sorts_results_alphabetically(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.fetch_all_indexes", lambda: [ @@ -663,7 +792,7 @@ def test_search_command_sorts_results_alphabetically(monkeypatch) -> None: assert pos_alpha < pos_zeta -def test_search_command_finds_installed_module_when_not_in_registry(monkeypatch) -> None: +def test_search_command_finds_installed_module_when_not_in_registry(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.fetch_all_indexes", lambda: [("official", {"modules": []})] ) @@ -688,7 +817,7 @@ class _Entry: assert "installed" in result.stdout -def test_search_command_reports_no_results_with_query_context(monkeypatch) -> None: +def test_search_command_reports_no_results_with_query_context(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.fetch_all_indexes", lambda: [("official", {"modules": []})] ) @@ -700,7 +829,7 @@ def test_search_command_reports_no_results_with_query_context(monkeypatch) -> No assert "No modules found for query 'does-not-exist'" in result.stdout -def test_list_command_sorts_modules_alphabetically(monkeypatch) -> None: +def test_list_command_sorts_modules_alphabetically(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [ @@ -729,7 +858,7 @@ def test_list_command_sorts_modules_alphabetically(monkeypatch) -> None: assert result.stdout.index("alpha") < result.stdout.index("zeta") -def test_enable_command_message_sorts_module_ids(monkeypatch) -> None: +def test_enable_command_message_sorts_module_ids(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.is_non_interactive", lambda: False) monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", @@ -754,46 +883,32 @@ def test_enable_command_message_sorts_module_ids(monkeypatch) -> None: assert "alpha, zeta" in result.stdout -def test_list_command_shows_version_state_and_trust(monkeypatch) -> None: +def test_list_command_shows_version_state_and_trust(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", - lambda: [ - { - "id": "init", - "version": "0.1.0", - "enabled": True, - "source": "builtin", - "official": True, - "publisher": "nold-ai", - }, - { - "id": "backlog", - "version": "0.2.0", - "enabled": False, - "source": "marketplace", - "official": False, - "publisher": "community-dev", - }, - ], + _version_state_trust_rows, ) result = runner.invoke(app, ["list"]) assert result.exit_code == 0 - assert "Trust" in result.stdout - assert "Publisher" in result.stdout - assert "init" in result.stdout - assert "0.1.0" in result.stdout - assert "enabled" in result.stdout - assert "official" in result.stdout - assert "backlog" in result.stdout - assert "disabled" in result.stdout - assert "community" in result.stdout - assert "nold-ai" in result.stdout - assert "community-dev" in result.stdout - - -def test_list_command_marketplace_option_shows_registry_modules(monkeypatch) -> None: + expected_values = ( + "Trust", + "Publisher", + "init", + "0.1.0", + "enabled", + "official", + "backlog", + "disabled", + "community", + "nold-ai", + "community-dev", + ) + assert all(value in result.stdout for value in expected_values) + + +def test_list_command_marketplace_option_shows_registry_modules(monkeypatch: pytest.MonkeyPatch) -> None: """specfact module list --marketplace shows modules from the registry index.""" monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.fetch_registry_index", @@ -815,7 +930,7 @@ def test_list_command_marketplace_option_shows_registry_modules(monkeypatch) -> assert "specfact module install" in result.stdout -def test_list_command_marketplace_option_offline_shows_warning(monkeypatch) -> None: +def test_list_command_marketplace_option_offline_shows_warning(monkeypatch: pytest.MonkeyPatch) -> None: """specfact module list --marketplace when registry unavailable shows friendly message.""" monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.fetch_registry_index", lambda **_: None) monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.get_modules_with_state", list) @@ -826,7 +941,7 @@ def test_list_command_marketplace_option_offline_shows_warning(monkeypatch) -> N assert "unavailable" in result.stdout.lower() or "offline" in result.stdout.lower() -def test_list_command_shows_official_label_when_marked(monkeypatch) -> None: +def test_list_command_shows_official_label_when_marked(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [ @@ -848,7 +963,7 @@ def test_list_command_shows_official_label_when_marked(monkeypatch) -> None: assert "custom" not in result.stdout -def test_list_command_show_origin_includes_origin_column(monkeypatch) -> None: +def test_list_command_show_origin_includes_origin_column(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [ @@ -883,7 +998,7 @@ def test_list_command_show_origin_includes_origin_column(monkeypatch) -> None: assert "marketplace" in result.stdout -def test_list_command_source_filter(monkeypatch) -> None: +def test_list_command_source_filter(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [ @@ -905,7 +1020,7 @@ def test_list_command_source_filter(monkeypatch) -> None: assert "init" not in result.stdout -def test_list_command_bundled_available_uses_unfiltered_installed_set(monkeypatch) -> None: +def test_list_command_bundled_available_uses_unfiltered_installed_set(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [ @@ -943,7 +1058,7 @@ def test_list_command_bundled_available_uses_unfiltered_installed_set(monkeypatc assert "All bundled modules are already installed" in result.stdout -def test_list_command_show_bundled_available_separate_section_with_hints(monkeypatch) -> None: +def test_list_command_show_bundled_available_separate_section_with_hints(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [ @@ -979,7 +1094,7 @@ def test_list_command_show_bundled_available_separate_section_with_hints(monkeyp assert "specfact module init --scope project" in result.stdout -def test_list_command_show_bundled_available_empty_when_all_installed(monkeypatch) -> None: +def test_list_command_show_bundled_available_empty_when_all_installed(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [ @@ -1008,7 +1123,7 @@ def test_list_command_show_bundled_available_empty_when_all_installed(monkeypatc assert "All bundled modules are already installed" in result.stdout -def test_list_command_without_flag_shows_hint_when_bundled_available(monkeypatch) -> None: +def test_list_command_without_flag_shows_hint_when_bundled_available(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [ @@ -1036,7 +1151,7 @@ def test_list_command_without_flag_shows_hint_when_bundled_available(monkeypatch assert "--show-bundled-available" in result.stdout -def test_list_command_fetches_module_state_once(monkeypatch) -> None: +def test_list_command_fetches_module_state_once(monkeypatch: pytest.MonkeyPatch) -> None: calls = {"count": 0} def _get_modules_with_state() -> list[dict[str, object]]: @@ -1068,39 +1183,13 @@ def _get_modules_with_state() -> list[dict[str, object]]: assert calls["count"] == 1 -def test_show_command_displays_module_details(monkeypatch) -> None: - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", - lambda: [ - { - "id": "bundle-mapper", - "version": "0.1.0", - "enabled": True, - "source": "custom", - "official": True, - "publisher": "nold-ai", - } - ], - ) - - class _Meta: - name = "bundle-mapper" - description = "Maps backlog items to modules using confidence heuristics" - license = "Apache-2.0" - tier = "community" - commands = ["backlog"] - core_compatibility = ">=0.28.0,<1.0.0" - - class publisher: # noqa: N801 - attributes = {"url": "https://github.com/nold-ai/specfact-cli-modules"} - - class _Entry: - metadata = _Meta() - - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.discover_all_modules", - lambda: [_Entry()], +def test_show_command_displays_module_details(monkeypatch: pytest.MonkeyPatch) -> None: + metadata = _module_registry_metadata( + name="bundle-mapper", + commands=["backlog"], ) + metadata.description = "Maps backlog items to modules using confidence heuristics" + _patch_show_module(monkeypatch, metadata, source="custom") result = runner.invoke(app, ["show", "bundle-mapper"]) @@ -1114,7 +1203,7 @@ class _Entry: assert "official" in result.stdout -def test_show_command_uses_command_help_keys_when_commands_missing(monkeypatch) -> None: +def test_show_command_uses_command_help_keys_when_commands_missing(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [ @@ -1154,54 +1243,11 @@ class _Entry: assert "show" in result.stdout -def test_show_command_derives_full_command_paths_with_subcommands(monkeypatch) -> None: - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", - lambda: [ - { - "id": "module-registry", - "version": "0.35.0", - "enabled": True, - "source": "builtin", - "official": True, - "publisher": "nold-ai", - } - ], +def test_show_command_derives_full_command_paths_with_subcommands(monkeypatch: pytest.MonkeyPatch) -> None: + _patch_show_module( + monkeypatch, + _module_registry_metadata(commands=["module"], command_help={"module": "Manage modules"}), ) - - class _Meta: - name = "module-registry" - description = "Manage modules" - license = "Apache-2.0" - tier = "community" - commands = ["module"] - command_help = {"module": "Manage modules"} - core_compatibility = ">=0.28.0,<1.0.0" - - class publisher: # noqa: N801 - attributes = {"url": "https://github.com/nold-ai/specfact-cli-modules"} - - class _Entry: - metadata = _Meta() - - monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.discover_all_modules", lambda: [_Entry()]) - - class _CmdInfo: - def __init__(self, name: str, help_text: str) -> None: - self.name = name - self.help = help_text - self.callback = None - - class _GroupInfo: - def __init__(self, name: str, typer_instance: object) -> None: - self.name = name - self.typer_instance = typer_instance - - class _FakeTyper: - def __init__(self, commands: list[tuple[str, str]], groups: list[object]) -> None: - self.registered_commands = [_CmdInfo(name, help_text) for name, help_text in commands] - self.registered_groups = groups - delta_app = _FakeTyper([("status", "Show delta status")], []) root_app = _FakeTyper([("list", "List modules"), ("show", "Show module details")], [_GroupInfo("delta", delta_app)]) @@ -1218,7 +1264,7 @@ def __init__(self, commands: list[tuple[str, str]], groups: list[object]) -> Non assert "module delta status - Show delta status" in result.stdout -def test_show_command_fails_for_unknown_module(monkeypatch) -> None: +def test_show_command_fails_for_unknown_module(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.get_modules_with_state", list) result = runner.invoke(app, ["show", "missing-module"]) @@ -1227,7 +1273,7 @@ def test_show_command_fails_for_unknown_module(monkeypatch) -> None: assert "is not installed" in result.stdout -def test_upgrade_command(monkeypatch, tmp_path: Path) -> None: +def test_upgrade_command(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: captured: dict[str, bool | None] = {"reinstall": None} def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): @@ -1251,7 +1297,7 @@ def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwa assert "Upgraded" in result.stdout -def test_upgrade_without_module_name_upgrades_all_marketplace(monkeypatch, tmp_path: Path) -> None: +def test_upgrade_without_module_name_upgrades_all_marketplace(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: installed: list[str] = [] reinstall_flags: list[bool] = [] @@ -1278,10 +1324,12 @@ def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwa assert "Upgraded" in result.stdout -def test_upgrade_without_module_name_reports_one_line_per_module_with_versions(monkeypatch, tmp_path: Path) -> None: +def test_upgrade_without_module_name_reports_one_line_per_module_with_versions( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: installed: list[str] = [] - def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _install(module_id: str, options: InstallModuleOptions, **_kwargs): installed.append(module_id) module_dir = tmp_path / module_id.split("/")[-1] module_dir.mkdir(parents=True, exist_ok=True) @@ -1310,7 +1358,7 @@ def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwa assert "nold-ai/specfact-project: 0.4.0 -> 0.5.0" in result.stdout -def test_upgrade_rejects_non_marketplace_source(monkeypatch) -> None: +def test_upgrade_rejects_non_marketplace_source(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", lambda: [{"id": "bundle-mapper", "version": "0.1.0", "enabled": True, "source": "custom"}], @@ -1322,11 +1370,11 @@ def test_upgrade_rejects_non_marketplace_source(monkeypatch) -> None: assert "marketplace modules" in result.stdout and "upgradeable" in result.stdout -def test_upgrade_rejects_multi_segment_module_id(monkeypatch, tmp_path: Path) -> None: +def test_upgrade_rejects_multi_segment_module_id(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: """Malformed owner/repo/extra must not resolve via last-segment fallback to a different module.""" installed: list[str] = [] - def _install(module_id: str, options: InstallModuleOptions | None = None, **_kwargs): + def _install(module_id: str, options: InstallModuleOptions, **_kwargs): installed.append(module_id) return tmp_path / module_id.split("/")[-1] @@ -1363,7 +1411,7 @@ def test_full_marketplace_module_id_for_install_rejects_multi_segment_path() -> _full_marketplace_module_id_for_install("foo/bar/backlog") -def test_enable_command_updates_state_with_dependency_checks(monkeypatch) -> None: +def test_enable_command_updates_state_with_dependency_checks(monkeypatch: pytest.MonkeyPatch) -> None: captured = {"enable_ids": None, "disable_ids": None, "force": None} def _apply(*, enable_ids, disable_ids, force): @@ -1383,7 +1431,7 @@ def _apply(*, enable_ids, disable_ids, force): assert "Enabled" in result.stdout -def test_disable_command_respects_force_cascade(monkeypatch) -> None: +def test_disable_command_respects_force_cascade(monkeypatch: pytest.MonkeyPatch) -> None: captured = {"enable_ids": None, "disable_ids": None, "force": None} def _apply(*, enable_ids, disable_ids, force): @@ -1403,7 +1451,7 @@ def _apply(*, enable_ids, disable_ids, force): assert "Disabled" in result.stdout -def test_enable_command_interactive_mode_selection(monkeypatch) -> None: +def test_enable_command_interactive_mode_selection(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.is_non_interactive", lambda: False) monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", @@ -1440,7 +1488,7 @@ def _apply(*, enable_ids, disable_ids, force): assert captured["enable_ids"] == ["backlog"] -def test_disable_command_non_interactive_requires_module_id(monkeypatch) -> None: +def test_disable_command_non_interactive_requires_module_id(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.is_non_interactive", lambda: True) result = runner.invoke(app, ["disable"]) @@ -1449,7 +1497,7 @@ def test_disable_command_non_interactive_requires_module_id(monkeypatch) -> None assert "Non-interactive mode requires explicit module id value" in result.stdout -def test_module_init_bootstraps_user_modules(monkeypatch) -> None: +def test_module_init_bootstraps_user_modules(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.sync_bundled_modules_to_user_root", lambda **_kwargs: 2, @@ -1462,7 +1510,7 @@ def test_module_init_bootstraps_user_modules(monkeypatch) -> None: assert str(USER_MODULES_ROOT) in result.stdout or "user-modules" in result.stdout -def test_module_init_project_scope_defaults_to_cwd_repo(monkeypatch, tmp_path: Path) -> None: +def test_module_init_project_scope_defaults_to_cwd_repo(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: monkeypatch.chdir(tmp_path) captured: dict[str, Path | None] = {"target_root": None} @@ -1480,7 +1528,7 @@ def _sync(target_root=None, **_kwargs): assert str(tmp_path / ".specfact" / "modules") in result.stdout.replace("\n", "") -def test_module_init_project_scope_supports_explicit_repo(monkeypatch, tmp_path: Path) -> None: +def test_module_init_project_scope_supports_explicit_repo(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: explicit_repo = tmp_path / "customer-a" explicit_repo.mkdir(parents=True) captured: dict[str, Path | None] = {"target_root": None} diff --git a/tests/unit/registry/test_module_discovery.py b/tests/unit/registry/test_module_discovery.py index aa2483c4..62d3cea7 100644 --- a/tests/unit/registry/test_module_discovery.py +++ b/tests/unit/registry/test_module_discovery.py @@ -172,7 +172,9 @@ def test_project_shadow_warning_is_actionable_and_emitted_once(tmp_path: Path, m assert len(warnings) == 1 assert "takes precedence over user-scoped module" in warnings[0] assert "specfact module list --show-origin" in warnings[0] - assert "specfact module uninstall backlog-core --scope user" in warnings[0] + assert "remains installed and available outside this workspace" in warnings[0] + assert "No action is required" in warnings[0] + assert "module uninstall" not in warnings[0] def test_discover_all_modules_with_explicit_user_root_preserves_project_scope( From 1b4452cb791c8a977549a05993733bd5d412f99a Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sat, 29 Aug 2026 23:33:51 +0200 Subject: [PATCH 02/10] test(review): capture module scope feedback regressions --- docs/module-system/installing-modules.md | 2 +- docs/module-system/module-marketplace.md | 2 +- docs/reference/commands.generated.json | 3 ++ docs/reference/commands.generated.md | 2 +- llms.txt | 2 +- openspec/CHANGE_ORDER.md | 5 +- .../TDD_EVIDENCE.md | 42 ++++++++++++++++ .../proposal.md | 4 +- .../requirements-evidence.yaml | 22 ++++++-- .../requirements-proof/review-evidence.json | 4 +- .../specs/module-scope-diagnostics/spec.md | 15 ++++-- .../tasks.md | 32 ++++++++++++ .../modules/module_registry/test_commands.py | 50 +++++++++++++++++-- tests/unit/registry/test_module_discovery.py | 25 +++++++++- 14 files changed, 189 insertions(+), 21 deletions(-) create mode 100644 openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md create mode 100644 openspec/changes/module-scope-02-preserve-user-installs/tasks.md diff --git a/docs/module-system/installing-modules.md b/docs/module-system/installing-modules.md index 5dbe4815..e3b145a8 100644 --- a/docs/module-system/installing-modules.md +++ b/docs/module-system/installing-modules.md @@ -121,7 +121,7 @@ Default columns: With `--show-origin`, an additional `Origin` column is shown (`built-in`, `project`, `user`, `marketplace`, `custom`). -`module doctor` keeps discovery metadata-only and reports effective vs shadowed duplicate copies, exact manifest versions, paths, enabled state, configured development source roots, and recovery commands. Use it when project-scoped modules under `/.specfact/modules` and user-scoped modules under `~/.specfact/modules` disagree. +`module doctor` keeps discovery metadata-only and reports effective vs shadowed duplicate copies, exact manifest versions, paths, enabled state, configured development source roots, and non-destructive scope guidance. Normal shadowing does not require uninstalling the lower-priority copy. Use it when project-scoped modules under `/.specfact/modules` and user-scoped modules under `~/.specfact/modules` disagree. ## Show Detailed Module Info diff --git a/docs/module-system/module-marketplace.md b/docs/module-system/module-marketplace.md index 90ba3414..5a8adb97 100644 --- a/docs/module-system/module-marketplace.md +++ b/docs/module-system/module-marketplace.md @@ -56,7 +56,7 @@ specfact module list --show-origin specfact module doctor nold-ai/specfact-codebase ``` -`module doctor` additionally reports shadowed duplicate copies, exact manifest versions, paths, enabled state, configured development source roots, and recovery commands. +`module doctor` additionally reports shadowed duplicate copies, exact manifest versions, paths, enabled state, configured development source roots, and non-destructive scope guidance. Normal shadowing does not require uninstalling the lower-priority copy. ## Security Model diff --git a/docs/reference/commands.generated.json b/docs/reference/commands.generated.json index cc685a7e..8004cf75 100644 --- a/docs/reference/commands.generated.json +++ b/docs/reference/commands.generated.json @@ -1000,11 +1000,13 @@ "hidden": false, "install_prerequisite": "specfact module install nold-ai/specfact-code-review", "options": [ + "--base-ref", "--bug-hunt", "--enforcement", "--exclude-tests", "--fix", "--focus", + "--head-ref", "--include-noise", "--include-tests", "--instructions", @@ -1015,6 +1017,7 @@ "--no-tests", "--out", "--path", + "--pr-context-file", "--preview-fixes", "--requirements-evidence", "--scope", diff --git a/docs/reference/commands.generated.md b/docs/reference/commands.generated.md index b85211fe..228d7f8b 100644 --- a/docs/reference/commands.generated.md +++ b/docs/reference/commands.generated.md @@ -59,7 +59,7 @@ This file is generated from the current CLI command tree. Do not edit by hand. | `specfact code review rules init` | nold-ai/specfact-code-review | --ide; args: - | - | | | `specfact code review rules show` | nold-ai/specfact-code-review | -; args: - | - | | | `specfact code review rules update` | nold-ai/specfact-code-review | --ide; args: - | - | | -| `specfact code review run` | nold-ai/specfact-code-review | --bug-hunt, --enforcement, --exclude-tests, --fix, --focus, --include-noise, --include-tests, --instructions, --interactive, --json, --level, --mode, --no-tests, --out, --path, --preview-fixes, --requirements-evidence, --scope, --score-only, --suppress-noise, --with-mutation; args: - | - | | +| `specfact code review run` | nold-ai/specfact-code-review | --base-ref, --bug-hunt, --enforcement, --exclude-tests, --fix, --focus, --head-ref, --include-noise, --include-tests, --instructions, --interactive, --json, --level, --mode, --no-tests, --out, --path, --pr-context-file, --preview-fixes, --requirements-evidence, --scope, --score-only, --suppress-noise, --with-mutation; args: - | - | | | `specfact code validate` | nold-ai/specfact-codebase | -; args: - | sidecar | | | `specfact code validate sidecar` | nold-ai/specfact-codebase | -; args: - | init, run | | | `specfact code validate sidecar init` | nold-ai/specfact-codebase | -; args: - | - | | diff --git a/llms.txt b/llms.txt index 5ebbfd9d..a8f4f4d0 100644 --- a/llms.txt +++ b/llms.txt @@ -61,7 +61,7 @@ This file is generated from the current CLI command tree. Do not edit by hand. | `specfact code review rules init` | nold-ai/specfact-code-review | --ide; args: - | - | | | `specfact code review rules show` | nold-ai/specfact-code-review | -; args: - | - | | | `specfact code review rules update` | nold-ai/specfact-code-review | --ide; args: - | - | | -| `specfact code review run` | nold-ai/specfact-code-review | --bug-hunt, --enforcement, --exclude-tests, --fix, --focus, --include-noise, --include-tests, --instructions, --interactive, --json, --level, --mode, --no-tests, --out, --path, --preview-fixes, --requirements-evidence, --scope, --score-only, --suppress-noise, --with-mutation; args: - | - | | +| `specfact code review run` | nold-ai/specfact-code-review | --base-ref, --bug-hunt, --enforcement, --exclude-tests, --fix, --focus, --head-ref, --include-noise, --include-tests, --instructions, --interactive, --json, --level, --mode, --no-tests, --out, --path, --pr-context-file, --preview-fixes, --requirements-evidence, --scope, --score-only, --suppress-noise, --with-mutation; args: - | - | | | `specfact code validate` | nold-ai/specfact-codebase | -; args: - | sidecar | | | `specfact code validate sidecar` | nold-ai/specfact-codebase | -; args: - | init, run | | | `specfact code validate sidecar init` | nold-ai/specfact-codebase | -; args: - | - | | diff --git a/openspec/CHANGE_ORDER.md b/openspec/CHANGE_ORDER.md index ffe8928d..bafdedd6 100644 --- a/openspec/CHANGE_ORDER.md +++ b/openspec/CHANGE_ORDER.md @@ -8,7 +8,7 @@ active changes should be implemented. | Bucket | Count | Location | |---|---:|---| -| **Active** | 20 | [`openspec/changes/`](changes/) | +| **Active** | 21 | [`openspec/changes/`](changes/) | | **Parked** | 21 | [`openspec/parking-lot/`](parking-lot/) | | **Archived** | 115 | [`openspec/changes/archive/`](changes/archive/) | @@ -38,7 +38,7 @@ brownfield delivery. The active roadmap should make that thesis stronger: ## Active tracks -The 20 active changes group into three product tracks plus one reliability lane. +The 21 active changes group into three product tracks plus one reliability lane. Tracks can run in parallel; within a track, follow the order column. ### Track A - Validation Evidence Spine @@ -98,6 +98,7 @@ These changes make the CLI itself trustworthy enough to be the validation tool. | Order | Change | Issue | Positioning | Blocked by | |---:|---|---|---|---| +| 0 | `module-scope-02-preserve-user-installs` | [#699](https://github.com/nold-ai/specfact-cli/issues/699) | Preserve user-scoped modules when project copies shadow them; remove destructive discovery/doctor guidance | none; paired modules [#452](https://github.com/nold-ai/specfact-cli-modules/issues/452) is coordinated but independently mergeable | | 1 | `cli-val-03-misuse-safety-proof` | [#281](https://github.com/nold-ai/specfact-cli/issues/281) | Misuse safety proof for user-facing commands | - | | 2 | `cli-val-04-acceptance-test-runner` | [#282](https://github.com/nold-ai/specfact-cli/issues/282) | Acceptance-test runner for CLI behavior proof | cli-val-03 | | 3 | `cli-val-05-ci-integration` | [#643](https://github.com/nold-ai/specfact-cli/issues/643) | Fail-closed documentation accountability and CI validation enforcement | cli-val-02, cli-val-03, cli-val-04 | diff --git a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md new file mode 100644 index 00000000..0162ce21 --- /dev/null +++ b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md @@ -0,0 +1,42 @@ +# TDD Evidence + +## Failing Before + +- `hatch run pytest tests/unit/registry/test_module_discovery.py::test_project_shadow_warning_is_actionable_and_emitted_once tests/unit/modules/module_registry/test_commands.py::test_doctor_reports_effective_and_shadowed_duplicate_modules -q` + - Result: FAIL before production edits (`2 failed`). + - Discovery still recommended `specfact module uninstall backlog-core --scope user`, and doctor still printed `Recovery: specfact module uninstall nold-ai/specfact-codebase --scope user` instead of preservation/no-action guidance. +- Retained CI red proof: Requirements Evidence run `33274750805` at signed source commit `b5ad2ea0d5e0ee062906e0c7b2f156330ea1a39f` executed the same two selectors and produced a bound `observed_maturity: red` artifact with no reconciliation findings. The workflow's overall failure is expected at this checkpoint and requires a later final implementation commit. + +## Passing After + +- `hatch run pytest tests/unit/registry/test_module_discovery.py::test_project_shadow_warning_is_actionable_and_emitted_once tests/unit/modules/module_registry/test_commands.py::test_doctor_reports_effective_and_shadowed_duplicate_modules -q` + - Result: PASS (`2 passed`). +- `hatch run pytest tests/unit/registry/test_module_discovery.py tests/unit/modules/module_registry/test_commands.py -q` + - Initial implementation result: PASS (`67 passed`). + - Discovery precedence, duplicate reporting, doctor output, and explicit uninstall command coverage remain green. + +## Review Follow-up + +- Review-driven tests were added before the follow-up production edit for actual effective-source guidance, qualified availability, and user-only discovery outside the shadowing project. +- Initial focused run: FAIL (`3 failed, 1 passed`). The current doctor always named project scope and both diagnostics made an unconditional availability claim; the user-only preservation scenario already passed. +- Passing focused run after the production edit: PASS (`4 passed`). +- Related discovery/doctor files after the review fixes: PASS (`69 passed`). + +## Quality Gates + +- `hatch run format`: PASS (942 files unchanged). +- `hatch run type-check`: PASS (0 errors; 1,657 existing warnings). +- `hatch run lint`: PASS. +- `hatch run yaml-lint`: exit code 0; it reports only pre-existing line-length/blank-line findings in untouched Requirements R07/R08 evidence. +- `hatch run contract-test` and `hatch run contract-test-contracts`: PASS using cached results after the full smart-test had refreshed hashes; both report no further modified contract inputs. The focused contract-sensitive discovery/doctor files pass, and the independent full suite exercised them. +- Schema-v2 Requirements evidence maps all changed scenarios to exact pytest selectors; the staged repository hook is the delivery gate. +- Product-owner review evidence is bound to mapping digest `sha256:f72c69a029b014b04629f58098dd866f76999150a754c95fe96ac7a0be401f99` and core issue #699 for the required test-authored maturity gate. The executable plan contains the two original behavior regressions plus the two review-follow-up scenarios; the unchanged development-root visibility test remains in the related regression suite but is not a red-proof selector. +- The built-in module-registry payload change advances the module-registry package to `0.1.34` with refreshed integrity metadata, advances all four canonical core version sources to `0.55.3`, and adds the matching changelog entry, as required by the release-integrity gates. +- `uv lock` refreshes the frozen project record from core `0.55.2` to `0.55.3`; `uv sync --locked --all-extras` then passes locally, resolving the first PR CI setup failures caused by the stale lock. +- Core CI's immutable module fixture is authoritative for generated command inventory. A local run against the newer paired modules checkout exposed three later Code Review PR-range options, but those options are intentionally absent from this core patch's generated artifacts so the frozen-fixture docs check remains reproducible. +- `hatch run bandit-scan`: PASS (no medium/high findings). +- Semgrep and its baseline gate: PASS (0 current findings, 0 baseline findings). +- Earlier local `hatch run smart-test` / `hatch run test` runs recorded `3029 passed, 12 skipped, 17 failed` before restoring the immutable fixture and refreshing the changed module signature. +- Final signed-head PR Orchestrator run `33275273173` executed `smart-test-full`: PASS (`3050 passed, 8 skipped`) on Python 3.12. Its Python 3.11 compatibility job also passed. +- The exact immutable-fixture full-enforcement Code Review used by CI passes locally with score 115 and zero findings after resolving all clean-code and type-safety warnings in the touched legacy files. The newer protected schema 1.6 capsule still reports assurance `UNKNOWN` on this macOS host because its controller supports Linux; Linux PR CI remains authoritative for that capsule. +- `git diff --check`: PASS. diff --git a/openspec/changes/module-scope-02-preserve-user-installs/proposal.md b/openspec/changes/module-scope-02-preserve-user-installs/proposal.md index 2e1622b3..991885b0 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/proposal.md +++ b/openspec/changes/module-scope-02-preserve-user-installs/proposal.md @@ -6,7 +6,7 @@ Core module discovery and `specfact module doctor` describe normal project-over- - Keep project-over-user precedence unchanged. - Replace user-scope uninstall recovery advice with non-destructive scope guidance in module discovery warnings and doctor output. -- State explicitly that the user-scoped copy remains installed and available outside the current workspace and that no action is required for normal shadowing. +- State explicitly that the user-scoped copy remains installed, normal shadowing alone does not require uninstalling it, and availability elsewhere still depends on module state and higher-priority copies. - Add regression tests that reject destructive user-scope uninstall recommendations while preserving origin diagnostics. ## Capabilities @@ -19,7 +19,7 @@ Core module discovery and `specfact module doctor` describe normal project-over- - Affected code: `src/specfact_cli/registry/module_discovery.py` and `src/specfact_cli/modules/module_registry/src/commands.py`. - Affected tests: focused module discovery and module doctor unit tests. -- Paired modules guidance: `nold-ai/specfact-cli-modules#452` corrects the repository bootstrap surfaces that trigger this behavior during review work. +- Paired modules delivery: `nold-ai/specfact-cli-modules#454` corrects the repository bootstrap surfaces that trigger this behavior during review work. - Compatibility and data impact: none. Discovery order, module state, explicit uninstall behavior, manifests, and persistent installation data remain unchanged. --- diff --git a/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml b/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml index def55705..653a7ce1 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml +++ b/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml @@ -1,7 +1,7 @@ schema_version: "2" requirements: openspec:module-scope-02-preserve-user-installs:module-scope-diagnostics:module-doctor-reports-effective-and-shadowed-module-copies: - rationale: "Module diagnostics must explain project precedence without treating valid user installations as stale." + rationale: "Module diagnostics must explain actual source precedence without treating valid user installations as stale." stakeholder_refs: - "nold-ai/specfact-cli#699" - "nold-ai/specfact-cli-modules#452" @@ -17,7 +17,7 @@ requirements: scenario_id: module-doctor-reports-effective-and-shadowed-module-copies method: test intent: "Report both copies and preserve the user-scoped installation." - observable: "Doctor states that no action is required and emits no uninstall advice." + observable: "Doctor reports both origins and paths, preserves the user copy, and emits no uninstall advice." selector: runner: pytest node_id: tests/unit/modules/module_registry/test_commands.py::test_doctor_reports_effective_and_shadowed_duplicate_modules @@ -25,7 +25,23 @@ requirements: scenario_id: module-doctor-reports-effective-and-shadowed-module-copies method: test intent: "Keep runtime precedence diagnostics non-destructive." - observable: "Discovery states that the user copy remains installed and emits no uninstall advice." + observable: "Discovery states that the user copy remains installed, qualifies availability, and emits no uninstall advice." selector: runner: pytest node_id: tests/unit/registry/test_module_discovery.py::test_project_shadow_warning_is_actionable_and_emitted_once + - case_id: MSI-CORE-003 + scenario_id: module-doctor-reports-effective-and-shadowed-module-copies + method: test + intent: "Name the source that actually shadows the user-scoped copy." + observable: "Doctor identifies built-in precedence without claiming project precedence." + selector: + runner: pytest + node_id: tests/unit/modules/module_registry/test_commands.py::test_doctor_shadowing_guidance_names_builtin_effective_source + - case_id: MSI-CORE-004 + scenario_id: module-doctor-reports-effective-and-shadowed-module-copies + method: test + intent: "Show that preservation leaves the user copy discoverable where no project copy shadows it." + observable: "Discovery selects the user-scoped copy in a workspace without the project copy." + selector: + runner: pytest + node_id: tests/unit/registry/test_module_discovery.py::test_user_module_is_discovered_outside_shadowing_project diff --git a/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json b/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json index 80c4dc14..2019aa19 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json +++ b/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json @@ -3,7 +3,7 @@ "decision": "accepted", "reviewer_id": "djm81", "reviewer_role": "product-owner", - "recorded_at": "2026-08-29T22:20:04+02:00", + "recorded_at": "2026-08-29T23:28:00+02:00", "reference": "https://github.com/nold-ai/specfact-cli/issues/699", - "mapping_digest": "sha256:d25cd13f7e7fb1e327e2d9ba6c38eab85f5ba7ae870bd4a61efc13e05446f8f4" + "mapping_digest": "sha256:f72c69a029b014b04629f58098dd866f76999150a754c95fe96ac7a0be401f99" } diff --git a/openspec/changes/module-scope-02-preserve-user-installs/specs/module-scope-diagnostics/spec.md b/openspec/changes/module-scope-02-preserve-user-installs/specs/module-scope-diagnostics/spec.md index c36b63e5..6a234cc8 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/specs/module-scope-diagnostics/spec.md +++ b/openspec/changes/module-scope-02-preserve-user-installs/specs/module-scope-diagnostics/spec.md @@ -11,8 +11,9 @@ The system SHALL provide module-scope diagnostics that report module origin, ver - **THEN** the output identifies the project copy as effective - **AND** the output identifies the user copy as shadowed - **AND** the output shows both versions and paths -- **AND** the output states that the user-scoped copy remains installed and available outside the current workspace -- **AND** the output states that no action is required for normal shadowing +- **AND** the output states that the user-scoped copy remains installed +- **AND** the output states that normal shadowing alone does not require uninstalling it +- **AND** any claim about use outside the current workspace accounts for the module's enabled state and other higher-priority copies - **AND** the output does not recommend uninstalling the user-scoped copy #### Scenario: Runtime discovery reports project-over-user precedence @@ -20,9 +21,17 @@ The system SHALL provide module-scope diagnostics that report module origin, ver - **GIVEN** a module id exists in project scope and user scope - **WHEN** runtime discovery selects the project-scoped copy - **THEN** the diagnostic identifies project scope as effective in the current workspace -- **AND** it states that the user-scoped copy remains installed and available outside the workspace +- **AND** it states that the user-scoped copy remains installed +- **AND** it does not claim that the user copy is active outside the workspace without accounting for module state and other higher-priority copies - **AND** it does not recommend uninstalling the user-scoped copy +#### Scenario: Doctor identifies the actual effective source + +- **GIVEN** a user-scoped module is shadowed by a higher-priority copy +- **WHEN** the user runs `specfact module doctor ` +- **THEN** the guidance identifies the actual effective source +- **AND** it does not describe built-in, marketplace, or custom shadowing as project precedence + #### Scenario: Development source roots are disclosed - **GIVEN** development source root environment variables are configured diff --git a/openspec/changes/module-scope-02-preserve-user-installs/tasks.md b/openspec/changes/module-scope-02-preserve-user-installs/tasks.md new file mode 100644 index 00000000..dbddb4a5 --- /dev/null +++ b/openspec/changes/module-scope-02-preserve-user-installs/tasks.md @@ -0,0 +1,32 @@ +## 1. Governance and Scope + +- [x] 1.1 Create bug issue #699 with Bug type, labels, assignee, parent Feature #353, project assignment, In Progress status, and explicit no-blocker metadata. +- [x] 1.2 Cross-link paired modules bug nold-ai/specfact-cli-modules#452 and confirm the archived diagnostics change is the capability authority. +- [x] 1.3 Add and strictly validate the paired OpenSpec change before behavior edits. +- [x] 1.4 Keep the internal wiki source mirror aligned with both active public changes. + +## 2. Tests Before Implementation + +- [x] 2.1 Change the discovery-warning expectation to require preservation/no-action guidance and reject user-scope uninstall advice. +- [x] 2.2 Change the doctor expectation to require preservation/no-action guidance and reject user-scope uninstall advice. +- [x] 2.3 Run the focused tests and record failing-before evidence before production edits. + +## 3. Implementation + +- [x] 3.1 Replace destructive discovery warning text with non-destructive workspace precedence guidance. +- [x] 3.2 Replace doctor uninstall recovery output with non-destructive shadowing guidance. +- [x] 3.3 Keep discovery precedence, installed module state, and explicit uninstall behavior unchanged. + +## 4. Evidence and Delivery + +- [x] 4.1 Run focused passing tests and record passing-after evidence. +- [x] 4.2 Run format, type-check, lint, yaml-lint, contract, smart-test, full test, independent static analysis, and applicable signature gates; document reproducible `origin/dev` baseline failures. +- [x] 4.3 Run SpecFact changed-scope review, resolve every finding, and record fresh JSON evidence, including the macOS protected-capsule limitation. +- [x] 4.4 Commit with a signed Conventional Commit, push the bugfix branch, and open PR #700 to `dev` cross-linked to both issues and paired modules PR #454. + +## 5. Review Follow-up + +- [x] 5.1 Add red-first coverage for actual effective-source guidance and user-only discovery outside a shadowing project. +- [x] 5.2 Qualify availability claims using module state and other higher-priority copies. +- [x] 5.3 Align module-system documentation, paired PR references, and the active-change count with the delivered behavior. +- [ ] 5.4 Re-run requirements evidence, review, signatures, and CI on the review-fix head; resolve verified review threads. diff --git a/tests/unit/modules/module_registry/test_commands.py b/tests/unit/modules/module_registry/test_commands.py index eb7414b1..dd776642 100644 --- a/tests/unit/modules/module_registry/test_commands.py +++ b/tests/unit/modules/module_registry/test_commands.py @@ -264,8 +264,15 @@ def test_doctor_reports_effective_and_shadowed_duplicate_modules( lambda _repo: entries, ) monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.read_modules_state", dict) + monkeypatch.setattr( + "specfact_cli.runtime.get_console_config", + lambda: {"force_terminal": False, "no_color": True, "width": 500}, + ) - result = runner.invoke(app, ["doctor", "nold-ai/specfact-codebase", "--repo", str(tmp_path / "repo")]) + result = runner.invoke( + app, + ["doctor", "nold-ai/specfact-codebase", "--repo", str(tmp_path / "repo")], + ) assert result.exit_code == 0 assert "effective" in result.stdout @@ -273,8 +280,45 @@ def test_doctor_reports_effective_and_shadowed_duplicate_modules( assert "0.41.0" in result.stdout assert "0.40.0" in result.stdout normalized_output = " ".join(result.stdout.split()) - assert "remains installed and available outside this workspace" in normalized_output - assert "No action is required" in normalized_output + assert "project" in normalized_output + assert "user" in normalized_output + assert str(project_dir) in normalized_output + assert str(user_dir) in normalized_output + assert "remains installed" in normalized_output + assert "No uninstall is required due to normal shadowing" in normalized_output + assert "module uninstall" not in normalized_output + + +def test_doctor_shadowing_guidance_names_builtin_effective_source( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + builtin_dir = tmp_path / "builtin" / "specfact-codebase" + user_dir = tmp_path / "user-modules" / "specfact-codebase" + entries = [ + DiscoveredModule( + builtin_dir, + ModulePackageMetadata(name=CODEBASE_MODULE_ID, version="0.41.0", commands=["code"]), + "builtin", + ), + DiscoveredModule( + user_dir, + ModulePackageMetadata(name=CODEBASE_MODULE_ID, version="0.40.0", commands=["code"]), + "user", + ), + ] + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.discover_all_modules_for_project_with_shadowed", + lambda _repo: entries, + ) + monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.read_modules_state", dict) + + result = runner.invoke(app, ["doctor", CODEBASE_MODULE_ID, "--repo", str(tmp_path / "repo")]) + + normalized_output = " ".join(result.stdout.split()) + assert result.exit_code == 0 + assert "Built-in scope takes precedence" in normalized_output + assert "Project scope takes precedence" not in normalized_output + assert "remains installed" in normalized_output assert "module uninstall" not in normalized_output diff --git a/tests/unit/registry/test_module_discovery.py b/tests/unit/registry/test_module_discovery.py index 62d3cea7..bb5e500a 100644 --- a/tests/unit/registry/test_module_discovery.py +++ b/tests/unit/registry/test_module_discovery.py @@ -172,11 +172,32 @@ def test_project_shadow_warning_is_actionable_and_emitted_once(tmp_path: Path, m assert len(warnings) == 1 assert "takes precedence over user-scoped module" in warnings[0] assert "specfact module list --show-origin" in warnings[0] - assert "remains installed and available outside this workspace" in warnings[0] - assert "No action is required" in warnings[0] + assert "remains installed" in warnings[0] + assert "availability outside this workspace depends on module state" in warnings[0] + assert "available outside this workspace" not in warnings[0] assert "module uninstall" not in warnings[0] +def test_user_module_is_discovered_outside_shadowing_project(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A preserved user copy is discoverable where no project copy shadows it.""" + repo_root = tmp_path / "other-repo" + builtin_root = tmp_path / "builtin" + user_root = tmp_path / "user-modules" + repo_root.mkdir() + _write_manifest(builtin_root, "init") + _write_manifest(user_root, "backlog-core") + monkeypatch.chdir(repo_root) + + discovered = discover_all_modules( + builtin_root=builtin_root, + user_root=user_root, + include_legacy_roots=False, + ) + + backlog = next(entry for entry in discovered if entry.metadata.name == "backlog-core") + assert backlog.source == "user" + + def test_discover_all_modules_with_explicit_user_root_preserves_project_scope( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From d14ec5c19aec820062e8c4684a6c695b405bb1ac Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sat, 29 Aug 2026 23:38:28 +0200 Subject: [PATCH 03/10] test(review): bind failing module scope cases --- .../TDD_EVIDENCE.md | 4 ++-- .../requirements-evidence.yaml | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md index 0162ce21..e0c6e78f 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md +++ b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md @@ -18,7 +18,7 @@ ## Review Follow-up - Review-driven tests were added before the follow-up production edit for actual effective-source guidance, qualified availability, and user-only discovery outside the shadowing project. -- Initial focused run: FAIL (`3 failed, 1 passed`). The current doctor always named project scope and both diagnostics made an unconditional availability claim; the user-only preservation scenario already passed. +- Initial focused run: FAIL (`3 failed, 1 passed`). The current doctor always named project scope and both diagnostics made an unconditional availability claim. The user-only preservation scenario already passed, so it remains supplementary regression coverage rather than a retained red-proof selector. - Passing focused run after the production edit: PASS (`4 passed`). - Related discovery/doctor files after the review fixes: PASS (`69 passed`). @@ -30,7 +30,7 @@ - `hatch run yaml-lint`: exit code 0; it reports only pre-existing line-length/blank-line findings in untouched Requirements R07/R08 evidence. - `hatch run contract-test` and `hatch run contract-test-contracts`: PASS using cached results after the full smart-test had refreshed hashes; both report no further modified contract inputs. The focused contract-sensitive discovery/doctor files pass, and the independent full suite exercised them. - Schema-v2 Requirements evidence maps all changed scenarios to exact pytest selectors; the staged repository hook is the delivery gate. -- Product-owner review evidence is bound to mapping digest `sha256:f72c69a029b014b04629f58098dd866f76999150a754c95fe96ac7a0be401f99` and core issue #699 for the required test-authored maturity gate. The executable plan contains the two original behavior regressions plus the two review-follow-up scenarios; the unchanged development-root visibility test remains in the related regression suite but is not a red-proof selector. +- Product-owner review evidence is bound to the current requirements mapping and core issue #699 for the required test-authored maturity gate. The executable plan contains the two original behavior regressions plus the failing effective-source review scenario; the already-passing user-only preservation scenario remains supplementary regression coverage. - The built-in module-registry payload change advances the module-registry package to `0.1.34` with refreshed integrity metadata, advances all four canonical core version sources to `0.55.3`, and adds the matching changelog entry, as required by the release-integrity gates. - `uv lock` refreshes the frozen project record from core `0.55.2` to `0.55.3`; `uv sync --locked --all-extras` then passes locally, resolving the first PR CI setup failures caused by the stale lock. - Core CI's immutable module fixture is authoritative for generated command inventory. A local run against the newer paired modules checkout exposed three later Code Review PR-range options, but those options are intentionally absent from this core patch's generated artifacts so the frozen-fixture docs check remains reproducible. diff --git a/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml b/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml index 653a7ce1..5a958ef3 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml +++ b/openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml @@ -37,11 +37,3 @@ requirements: selector: runner: pytest node_id: tests/unit/modules/module_registry/test_commands.py::test_doctor_shadowing_guidance_names_builtin_effective_source - - case_id: MSI-CORE-004 - scenario_id: module-doctor-reports-effective-and-shadowed-module-copies - method: test - intent: "Show that preservation leaves the user copy discoverable where no project copy shadows it." - observable: "Discovery selects the user-scoped copy in a workspace without the project copy." - selector: - runner: pytest - node_id: tests/unit/registry/test_module_discovery.py::test_user_module_is_discovered_outside_shadowing_project From b897976ed994a708a7ad1984839090b2f858e00c Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sat, 29 Aug 2026 23:40:55 +0200 Subject: [PATCH 04/10] test(review): accept corrected proof mapping --- .../module-scope-02-preserve-user-installs/TDD_EVIDENCE.md | 2 +- .../requirements-proof/review-evidence.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md index e0c6e78f..12d7cbb5 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md +++ b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md @@ -30,7 +30,7 @@ - `hatch run yaml-lint`: exit code 0; it reports only pre-existing line-length/blank-line findings in untouched Requirements R07/R08 evidence. - `hatch run contract-test` and `hatch run contract-test-contracts`: PASS using cached results after the full smart-test had refreshed hashes; both report no further modified contract inputs. The focused contract-sensitive discovery/doctor files pass, and the independent full suite exercised them. - Schema-v2 Requirements evidence maps all changed scenarios to exact pytest selectors; the staged repository hook is the delivery gate. -- Product-owner review evidence is bound to the current requirements mapping and core issue #699 for the required test-authored maturity gate. The executable plan contains the two original behavior regressions plus the failing effective-source review scenario; the already-passing user-only preservation scenario remains supplementary regression coverage. +- Product-owner review evidence is bound to mapping digest `sha256:fc0ff2c618b508f00943c66a987a14edf5175c730e9b26ac146785aa2045fe68` and core issue #699 for the required test-authored maturity gate. The executable plan contains the two original behavior regressions plus the failing effective-source review scenario; the already-passing user-only preservation scenario remains supplementary regression coverage. - The built-in module-registry payload change advances the module-registry package to `0.1.34` with refreshed integrity metadata, advances all four canonical core version sources to `0.55.3`, and adds the matching changelog entry, as required by the release-integrity gates. - `uv lock` refreshes the frozen project record from core `0.55.2` to `0.55.3`; `uv sync --locked --all-extras` then passes locally, resolving the first PR CI setup failures caused by the stale lock. - Core CI's immutable module fixture is authoritative for generated command inventory. A local run against the newer paired modules checkout exposed three later Code Review PR-range options, but those options are intentionally absent from this core patch's generated artifacts so the frozen-fixture docs check remains reproducible. diff --git a/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json b/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json index 2019aa19..7fbe2117 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json +++ b/openspec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json @@ -3,7 +3,7 @@ "decision": "accepted", "reviewer_id": "djm81", "reviewer_role": "product-owner", - "recorded_at": "2026-08-29T23:28:00+02:00", + "recorded_at": "2026-08-29T23:41:00+02:00", "reference": "https://github.com/nold-ai/specfact-cli/issues/699", - "mapping_digest": "sha256:f72c69a029b014b04629f58098dd866f76999150a754c95fe96ac7a0be401f99" + "mapping_digest": "sha256:fc0ff2c618b508f00943c66a987a14edf5175c730e9b26ac146785aa2045fe68" } From daf05baa9303ef914f5659eafe940146d311af25 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sat, 29 Aug 2026 23:50:05 +0200 Subject: [PATCH 05/10] test(review): simplify preserved-install diagnostics proof --- .../modules/module_registry/test_commands.py | 114 +++++++++--------- 1 file changed, 55 insertions(+), 59 deletions(-) diff --git a/tests/unit/modules/module_registry/test_commands.py b/tests/unit/modules/module_registry/test_commands.py index dd776642..6f2f6b59 100644 --- a/tests/unit/modules/module_registry/test_commands.py +++ b/tests/unit/modules/module_registry/test_commands.py @@ -5,7 +5,7 @@ from pathlib import Path import pytest -from typer.testing import CliRunner +from typer.testing import CliRunner, Result from specfact_cli.models.module_package import ModulePackageMetadata, PublisherInfo from specfact_cli.modules.module_registry.src.commands import app @@ -150,6 +150,53 @@ def _patch_show_module( ) +def _doctor_entry(path: Path, version: str, source: str) -> DiscoveredModule: + """Build a codebase module entry for doctor diagnostics.""" + return DiscoveredModule( + path, + ModulePackageMetadata(name=CODEBASE_MODULE_ID, version=version, commands=["code"]), + source, + ) + + +def _invoke_doctor( + monkeypatch: pytest.MonkeyPatch, + repo: Path, + entries: list[DiscoveredModule], +) -> Result: + """Invoke doctor with deterministic discovery and unwrapped output.""" + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.discover_all_modules_for_project_with_shadowed", + lambda _repo: entries, + ) + monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.read_modules_state", dict) + monkeypatch.setattr( + "specfact_cli.runtime.get_console_config", + lambda: {"force_terminal": False, "no_color": True, "width": 500}, + ) + return runner.invoke(app, ["doctor", CODEBASE_MODULE_ID, "--repo", str(repo)]) + + +def _assert_duplicate_doctor_output(result: Result, project_dir: Path, user_dir: Path) -> None: + """Assert complete, non-destructive duplicate diagnostics.""" + normalized_output = " ".join(result.stdout.split()) + expected_fragments = ( + "effective", + "shadowed", + "0.41.0", + "0.40.0", + "project", + "user", + str(project_dir), + str(user_dir), + "remains installed", + "No uninstall is required due to normal shadowing", + ) + assert result.exit_code == 0 + assert all(fragment in normalized_output for fragment in expected_fragments) + assert "module uninstall" not in normalized_output + + class _CommandInfo: def __init__(self, name: str, help_text: str) -> None: self.name = name @@ -244,49 +291,12 @@ def test_doctor_reports_effective_and_shadowed_duplicate_modules( ) -> None: project_dir = tmp_path / "repo" / ".specfact" / "modules" / "specfact-codebase" user_dir = tmp_path / "user-modules" / "specfact-codebase" - project_dir.mkdir(parents=True) - user_dir.mkdir(parents=True) entries = [ - DiscoveredModule( - project_dir, - ModulePackageMetadata(name="nold-ai/specfact-codebase", version="0.41.0", commands=["code"]), - "project", - ), - DiscoveredModule( - user_dir, - ModulePackageMetadata(name="nold-ai/specfact-codebase", version="0.40.0", commands=["code"]), - "user", - ), + _doctor_entry(project_dir, "0.41.0", "project"), + _doctor_entry(user_dir, "0.40.0", "user"), ] - - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.discover_all_modules_for_project_with_shadowed", - lambda _repo: entries, - ) - monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.read_modules_state", dict) - monkeypatch.setattr( - "specfact_cli.runtime.get_console_config", - lambda: {"force_terminal": False, "no_color": True, "width": 500}, - ) - - result = runner.invoke( - app, - ["doctor", "nold-ai/specfact-codebase", "--repo", str(tmp_path / "repo")], - ) - - assert result.exit_code == 0 - assert "effective" in result.stdout - assert "shadowed" in result.stdout - assert "0.41.0" in result.stdout - assert "0.40.0" in result.stdout - normalized_output = " ".join(result.stdout.split()) - assert "project" in normalized_output - assert "user" in normalized_output - assert str(project_dir) in normalized_output - assert str(user_dir) in normalized_output - assert "remains installed" in normalized_output - assert "No uninstall is required due to normal shadowing" in normalized_output - assert "module uninstall" not in normalized_output + result = _invoke_doctor(monkeypatch, tmp_path / "repo", entries) + _assert_duplicate_doctor_output(result, project_dir, user_dir) def test_doctor_shadowing_guidance_names_builtin_effective_source( @@ -295,24 +305,10 @@ def test_doctor_shadowing_guidance_names_builtin_effective_source( builtin_dir = tmp_path / "builtin" / "specfact-codebase" user_dir = tmp_path / "user-modules" / "specfact-codebase" entries = [ - DiscoveredModule( - builtin_dir, - ModulePackageMetadata(name=CODEBASE_MODULE_ID, version="0.41.0", commands=["code"]), - "builtin", - ), - DiscoveredModule( - user_dir, - ModulePackageMetadata(name=CODEBASE_MODULE_ID, version="0.40.0", commands=["code"]), - "user", - ), + _doctor_entry(builtin_dir, "0.41.0", "builtin"), + _doctor_entry(user_dir, "0.40.0", "user"), ] - monkeypatch.setattr( - "specfact_cli.modules.module_registry.src.commands.discover_all_modules_for_project_with_shadowed", - lambda _repo: entries, - ) - monkeypatch.setattr("specfact_cli.modules.module_registry.src.commands.read_modules_state", dict) - - result = runner.invoke(app, ["doctor", CODEBASE_MODULE_ID, "--repo", str(tmp_path / "repo")]) + result = _invoke_doctor(monkeypatch, tmp_path / "repo", entries) normalized_output = " ".join(result.stdout.split()) assert result.exit_code == 0 From 31ec674542fece6b4a9ee2e4af6e8391ef4d8617 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sat, 29 Aug 2026 23:51:55 +0200 Subject: [PATCH 06/10] fix(modules): preserve user installs in shadow diagnostics --- CHANGELOG.md | 10 ++ .../TDD_EVIDENCE.md | 3 +- pyproject.toml | 2 +- setup.py | 2 +- src/__init__.py | 2 +- src/specfact_cli/__init__.py | 2 +- .../module_registry/module-package.yaml | 6 +- .../modules/module_registry/src/commands.py | 130 ++++++++---------- src/specfact_cli/registry/module_discovery.py | 7 +- uv.lock | 2 +- 10 files changed, 80 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e329597..7038904d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,16 @@ All notable changes to this project will be documented in this file. --- +## [0.55.3] - 2026-08-29 + +### Fixed + +- **Module scope diagnostics:** preserve valid user-scoped module installations + when a project-local copy takes precedence, and replace routine uninstall + advice with non-destructive origin guidance. + +--- + ## [0.55.2] - 2026-08-27 ### Security diff --git a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md index 12d7cbb5..86a6ca9f 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md +++ b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md @@ -19,13 +19,14 @@ - Review-driven tests were added before the follow-up production edit for actual effective-source guidance, qualified availability, and user-only discovery outside the shadowing project. - Initial focused run: FAIL (`3 failed, 1 passed`). The current doctor always named project scope and both diagnostics made an unconditional availability claim. The user-only preservation scenario already passed, so it remains supplementary regression coverage rather than a retained red-proof selector. +- Retained review red proof: Requirements Evidence run `33277091672` at signed source commit `daf05baa9303ef914f5659eafe940146d311af25` executed all three mapped selectors using the final reviewed test bytes and produced a bound `observed_maturity: red` artifact with no reconciliation findings. - Passing focused run after the production edit: PASS (`4 passed`). - Related discovery/doctor files after the review fixes: PASS (`69 passed`). ## Quality Gates - `hatch run format`: PASS (942 files unchanged). -- `hatch run type-check`: PASS (0 errors; 1,657 existing warnings). +- `hatch run type-check`: PASS (0 errors; 1,531 existing warnings). - `hatch run lint`: PASS. - `hatch run yaml-lint`: exit code 0; it reports only pre-existing line-length/blank-line findings in untouched Requirements R07/R08 evidence. - `hatch run contract-test` and `hatch run contract-test-contracts`: PASS using cached results after the full smart-test had refreshed hashes; both report no further modified contract inputs. The focused contract-sensitive discovery/doctor files pass, and the independent full suite exercised them. diff --git a/pyproject.toml b/pyproject.toml index 57a51912..c5d800bb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "specfact-cli" -version = "0.55.2" +version = "0.55.3" description = "AI-bloat defense CLI for Python teams. Run deterministic code review, cleanup forecasts, and spec/contract evidence for AI-assisted and brownfield delivery." readme = "README.md" requires-python = ">=3.11" diff --git a/setup.py b/setup.py index 83b55366..01614903 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ if __name__ == "__main__": _setup = setup( name="specfact-cli", - version="0.55.2", + version="0.55.3", description=( "AI-bloat defense CLI for Python teams. Run deterministic code review, cleanup forecasts, " "and spec/contract evidence for AI-assisted and brownfield delivery." diff --git a/src/__init__.py b/src/__init__.py index 4b0c1e30..3c40fece 100644 --- a/src/__init__.py +++ b/src/__init__.py @@ -3,4 +3,4 @@ """ # Package version: keep in sync with pyproject.toml, setup.py, src/specfact_cli/__init__.py -__version__ = "0.55.2" +__version__ = "0.55.3" diff --git a/src/specfact_cli/__init__.py b/src/specfact_cli/__init__.py index 35ceebd6..3bf64bc1 100644 --- a/src/specfact_cli/__init__.py +++ b/src/specfact_cli/__init__.py @@ -76,6 +76,6 @@ def _install_progressive_disclosure() -> None: # keeps missing-command and missing-parameter UX consistent outside the root CLI too. _install_progressive_disclosure() -__version__ = "0.55.2" +__version__ = "0.55.3" __all__ = ["__version__"] diff --git a/src/specfact_cli/modules/module_registry/module-package.yaml b/src/specfact_cli/modules/module_registry/module-package.yaml index 6f06187e..934e3f77 100644 --- a/src/specfact_cli/modules/module_registry/module-package.yaml +++ b/src/specfact_cli/modules/module_registry/module-package.yaml @@ -1,5 +1,5 @@ name: module-registry -version: 0.1.33 +version: 0.1.35 commands: - module category: core @@ -17,5 +17,5 @@ publisher: description: 'Manage modules: search, list, show, install, and upgrade.' license: Apache-2.0 integrity: - checksum: sha256:42672d1ec701d5854f39434bcd1b80b20cdad36aeb04574334ad8227ca6e8422 - signature: RZDbDR7X3eutmY113HXoqFEPz54wWBwcVobahSa9430lQTkyf10d51kzKWx/x65Ds2wkG1w5DoJ4WzEC3NS1AA== + checksum: sha256:2e5c27cfe497342356c4bfd9c261cc0a1a4bff7964bdfaab77d2a010292d450f + signature: iUE9y1OX2xpTiOPrGyPoVI9u8fPCwunn0Ua5tWPiz55r4K1VnE1YI9nNvpAL4fO/R0QqhzFlnoRDcopI4Af/CA== diff --git a/src/specfact_cli/modules/module_registry/src/commands.py b/src/specfact_cli/modules/module_registry/src/commands.py index f05aecd3..01c48aca 100644 --- a/src/specfact_cli/modules/module_registry/src/commands.py +++ b/src/specfact_cli/modules/module_registry/src/commands.py @@ -83,38 +83,6 @@ def _module_upgrade_status(description: str) -> Iterator[None]: yield -def _init_scope_nonempty(scope: str) -> bool: - return bool(scope) - - -def _strip_nonempty(s: str) -> bool: - return bool(s.strip()) - - -def _module_name_arg_nonempty(module_name: str) -> bool: - return _strip_nonempty(module_name) - - -def _alias_name_nonempty(alias_name: str) -> bool: - return _strip_nonempty(alias_name) - - -def _command_name_nonempty(command_name: str) -> bool: - return _strip_nonempty(command_name) - - -def _url_nonempty(url: str) -> bool: - return url.strip() != "" - - -def _registry_id_nonempty(registry_id: str) -> bool: - return _strip_nonempty(registry_id) - - -def _search_query_nonempty(query: str) -> bool: - return _strip_nonempty(query) - - def _module_id_optional_nonempty(module_id: str | None) -> bool: return module_id is None or module_id.strip() != "" @@ -129,14 +97,6 @@ def _upgrade_module_names_valid(module_names: list[str] | None) -> bool: return all(m.strip() != "" for m in module_names) -def _install_module_ids_nonempty(module_ids: list[str]) -> bool: - return bool(module_ids) and all(m.strip() != "" for m in module_ids) - - -def _uninstall_module_names_nonempty(module_names: list[str]) -> bool: - return bool(module_names) and all(m.strip() != "" for m in module_names) - - def _publisher_url_from_metadata(metadata: object | None) -> str: if not metadata: return "n/a" @@ -238,35 +198,34 @@ def _enable_if_disabled(module_id: str, base_path: Path | None = None) -> bool: def _install_skip_if_already_satisfied( - scope_normalized: str, requested_name: str, - target_root: Path, - repo: Path | None, - reinstall: bool, - discovered_by_name: dict[str, Any], + params: _InstallOneParams, ) -> bool: - installed_dir = target_root / requested_name - if (installed_dir / "module-package.yaml").exists() and not reinstall: + installed_dir = params.target_root / requested_name + if (installed_dir / "module-package.yaml").exists() and not params.reinstall: module_id = _read_installed_manifest_id(installed_dir, requested_name) - enabled = _enable_if_disabled(module_id, base_path=repo if scope_normalized == "project" else None) + enabled = _enable_if_disabled( + module_id, + base_path=params.repo if params.scope_normalized == "project" else None, + ) if enabled: console.print( - f"[yellow]Module '{module_id}' is already installed in {target_root}; " + f"[yellow]Module '{module_id}' is already installed in {params.target_root}; " "enabled it in module state.[/yellow]" ) else: - console.print(f"[yellow]Module '{module_id}' is already installed in {target_root}.[/yellow]") + console.print(f"[yellow]Module '{module_id}' is already installed in {params.target_root}.[/yellow]") return True skip_sources = {"builtin", "project", "user", "custom"} - if scope_normalized == "project": + if params.scope_normalized == "project": skip_sources.discard("user") - if scope_normalized == "user": + if params.scope_normalized == "user": skip_sources.discard("project") - existing = discovered_by_name.get(requested_name) + existing = params.discovered_by_name.get(requested_name) if existing is not None and existing.source in skip_sources: enabled = _enable_if_disabled( existing.metadata.name, - base_path=repo if scope_normalized == "project" else None, + base_path=params.repo if params.scope_normalized == "project" else None, ) state_hint = " Enabled it in module state." if enabled else "" console.print( @@ -307,7 +266,7 @@ def _try_install_bundled_module( @app.command(name="init") @beartype -@require(_init_scope_nonempty, "scope must not be empty") +@require(lambda scope: bool(cast(str, scope).strip()), "scope must not be empty") def init_modules( scope: str = typer.Option("user", "--scope", help="Bootstrap scope: user or project"), repo: Path | None = typer.Option(None, "--repo", help="Repository path for project scope (default: current dir)"), @@ -361,12 +320,8 @@ def _install_one(module_id: str, params: _InstallOneParams) -> bool: """Install a single module; return True on success, False if skipped/already installed.""" normalized, requested_name = _normalize_install_module_id(module_id) if _install_skip_if_already_satisfied( - params.scope_normalized, requested_name, - params.target_root, - params.repo, - params.reinstall, - params.discovered_by_name, + params, ): return True if _try_install_bundled_module( @@ -492,7 +447,10 @@ def _install_impl(module_ids: list[str], **kwargs: Any) -> None: @app.command() -@require(_install_module_ids_nonempty, "at least one non-blank module id is required") +@require( + lambda module_ids: bool(module_ids) and all(module_id.strip() for module_id in cast(list[str], module_ids)), + "at least one non-blank module id is required", +) @beartype def install( module_ids: Annotated[ @@ -634,7 +592,12 @@ def _uninstall_marketplace_default(normalized: str) -> None: @app.command() -@require(_uninstall_module_names_nonempty, "at least one non-blank module name is required") +@require( + lambda module_names: ( + bool(module_names) and all(module_name.strip() for module_name in cast(list[str], module_names)) + ), + "at least one non-blank module name is required", +) @beartype def uninstall( module_names: Annotated[ @@ -662,8 +625,8 @@ def uninstall( @alias_app.command(name="create") @beartype -@require(_alias_name_nonempty, "alias_name must not be empty") -@require(_command_name_nonempty, "command_name must not be empty") +@require(lambda alias_name: bool(cast(str, alias_name).strip()), "alias_name must not be empty") +@require(lambda command_name: bool(cast(str, command_name).strip()), "command_name must not be empty") def alias_create( alias_name: str = typer.Argument(..., help="Alias (command name) to map"), command_name: str = typer.Argument(..., help="Command name to invoke (e.g. backlog, module)"), @@ -697,7 +660,7 @@ def alias_list() -> None: @alias_app.command(name="remove") @beartype -@require(_alias_name_nonempty, "alias_name must not be empty") +@require(lambda alias_name: bool(cast(str, alias_name).strip()), "alias_name must not be empty") def alias_remove( alias_name: str = typer.Argument(..., help="Alias to remove"), ) -> None: @@ -712,7 +675,7 @@ def alias_remove( @app.command(name="add-registry") @beartype -@require(_url_nonempty, "url must not be empty") +@require(lambda url: bool(cast(str, url).strip()), "url must not be empty") def add_registry_cmd( url: str = typer.Argument(..., help="Registry index URL (e.g. https://company.com/index.json)"), id: str | None = typer.Option(None, "--id", help="Registry id (default: derived from URL)"), @@ -758,13 +721,14 @@ def list_registries_cmd() -> None: @app.command(name="remove-registry") @beartype -@require(_registry_id_nonempty, "registry_id must not be empty") +@require(lambda registry_id: bool(cast(str, registry_id).strip()), "registry_id must not be empty") def remove_registry_cmd( registry_id: str = typer.Argument(..., help="Registry id to remove"), ) -> None: """Remove a custom registry from the config.""" - remove_registry(registry_id.strip()) - console.print(f"[green]Removed registry[/green] {registry_id!r}") + normalized_registry_id = registry_id.strip() + remove_registry(normalized_registry_id) + console.print(f"[green]Removed registry[/green] {normalized_registry_id!r}") @app.command() @@ -840,7 +804,7 @@ def disable( @app.command() @beartype -@require(_search_query_nonempty, "query must not be empty") +@require(lambda query: bool(cast(str, query).strip()), "query must not be empty") def search(query: str = typer.Argument(..., help="Search query")) -> None: """Search marketplace and installed modules by id/description/tags.""" query_l = query.lower().strip() @@ -1119,11 +1083,29 @@ def _doctor_status( return "effective" -def _print_doctor_recovery(entries: list[tuple[DiscoveredModule, str]]) -> None: +def _print_doctor_shadowing_guidance(entries: list[tuple[DiscoveredModule, str]]) -> None: + effective_by_module_id = {entry.metadata.name: (entry, status) for entry, status in entries if status != "shadowed"} for entry, status in entries: if status != "shadowed" or entry.source != "user": continue - console.print(f"[yellow]Recovery:[/yellow] specfact module uninstall {entry.metadata.name} --scope user") + effective_entry, effective_status = effective_by_module_id[entry.metadata.name] + source_label = { + "builtin": "Built-in", + "marketplace": "Marketplace", + "project": "Project", + "custom": "Custom", + "user": "User", + }.get(effective_entry.source, effective_entry.source.replace("_", "-").capitalize()) + availability = ( + "The module is disabled in module state; enable it before use outside this workspace." + if effective_status == "disabled" + else "Availability outside this workspace depends on module state and other higher-priority copies." + ) + console.print( + f"[blue]Info:[/blue] {source_label} scope takes precedence over the user-scoped copy of " + f"{entry.metadata.name}. The user copy remains installed. {availability} " + "No uninstall is required due to normal shadowing." + ) @app.command(name="doctor") @@ -1166,7 +1148,7 @@ def doctor( str(entry.package_dir), ) console.print(table) - _print_doctor_recovery(rows) + _print_doctor_shadowing_guidance(rows) dev_roots = _doctor_dev_roots() if not dev_roots: @@ -1278,7 +1260,7 @@ def _build_module_details_table(module_name: str, module_row: dict[str, Any], me @app.command() @beartype -@require(_module_name_arg_nonempty, "module_name must not be empty") +@require(lambda module_name: bool(cast(str, module_name).strip()), "module_name must not be empty") def show(module_name: str = typer.Argument(..., help="Installed module name")) -> None: """Show detailed metadata for an installed module.""" modules = get_modules_with_state() diff --git a/src/specfact_cli/registry/module_discovery.py b/src/specfact_cli/registry/module_discovery.py index ff3e859a..31a80538 100644 --- a/src/specfact_cli/registry/module_discovery.py +++ b/src/specfact_cli/registry/module_discovery.py @@ -158,9 +158,10 @@ def _maybe_warn_user_shadowed_by_project( _SHADOW_HINT_KEYS.add(warning_key) print_warning( f"Module '{module_name}' from project scope ({existing.package_dir}) takes precedence over " - f"user-scoped module ({package_dir}) in this workspace. The user copy is ignored here. " - f"Inspect origins with `specfact module list --show-origin`; if stale, clean user scope " - f"with `specfact module uninstall {module_name} --scope user`." + f"user-scoped module ({package_dir}) in this workspace. The user copy remains installed; " + "availability outside this workspace depends on module state and other higher-priority copies. " + "No uninstall is required due to normal shadowing. Inspect origins with " + "`specfact module list --show-origin`." ) diff --git a/uv.lock b/uv.lock index c9c37dd7..693a3e5e 100644 --- a/uv.lock +++ b/uv.lock @@ -2771,7 +2771,7 @@ wheels = [ [[package]] name = "specfact-cli" -version = "0.55.2" +version = "0.55.3" source = { editable = "." } dependencies = [ { name = "azure-identity" }, From 9521ca662945dfb9d32ecdd767afd1db8b9d77fb Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sat, 29 Aug 2026 23:56:13 +0200 Subject: [PATCH 07/10] docs(cli): align command inventory with frozen fixture --- docs/reference/commands.generated.json | 3 --- docs/reference/commands.generated.md | 2 +- llms.txt | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/docs/reference/commands.generated.json b/docs/reference/commands.generated.json index 8004cf75..cc685a7e 100644 --- a/docs/reference/commands.generated.json +++ b/docs/reference/commands.generated.json @@ -1000,13 +1000,11 @@ "hidden": false, "install_prerequisite": "specfact module install nold-ai/specfact-code-review", "options": [ - "--base-ref", "--bug-hunt", "--enforcement", "--exclude-tests", "--fix", "--focus", - "--head-ref", "--include-noise", "--include-tests", "--instructions", @@ -1017,7 +1015,6 @@ "--no-tests", "--out", "--path", - "--pr-context-file", "--preview-fixes", "--requirements-evidence", "--scope", diff --git a/docs/reference/commands.generated.md b/docs/reference/commands.generated.md index 228d7f8b..b85211fe 100644 --- a/docs/reference/commands.generated.md +++ b/docs/reference/commands.generated.md @@ -59,7 +59,7 @@ This file is generated from the current CLI command tree. Do not edit by hand. | `specfact code review rules init` | nold-ai/specfact-code-review | --ide; args: - | - | | | `specfact code review rules show` | nold-ai/specfact-code-review | -; args: - | - | | | `specfact code review rules update` | nold-ai/specfact-code-review | --ide; args: - | - | | -| `specfact code review run` | nold-ai/specfact-code-review | --base-ref, --bug-hunt, --enforcement, --exclude-tests, --fix, --focus, --head-ref, --include-noise, --include-tests, --instructions, --interactive, --json, --level, --mode, --no-tests, --out, --path, --pr-context-file, --preview-fixes, --requirements-evidence, --scope, --score-only, --suppress-noise, --with-mutation; args: - | - | | +| `specfact code review run` | nold-ai/specfact-code-review | --bug-hunt, --enforcement, --exclude-tests, --fix, --focus, --include-noise, --include-tests, --instructions, --interactive, --json, --level, --mode, --no-tests, --out, --path, --preview-fixes, --requirements-evidence, --scope, --score-only, --suppress-noise, --with-mutation; args: - | - | | | `specfact code validate` | nold-ai/specfact-codebase | -; args: - | sidecar | | | `specfact code validate sidecar` | nold-ai/specfact-codebase | -; args: - | init, run | | | `specfact code validate sidecar init` | nold-ai/specfact-codebase | -; args: - | - | | diff --git a/llms.txt b/llms.txt index a8f4f4d0..5ebbfd9d 100644 --- a/llms.txt +++ b/llms.txt @@ -61,7 +61,7 @@ This file is generated from the current CLI command tree. Do not edit by hand. | `specfact code review rules init` | nold-ai/specfact-code-review | --ide; args: - | - | | | `specfact code review rules show` | nold-ai/specfact-code-review | -; args: - | - | | | `specfact code review rules update` | nold-ai/specfact-code-review | --ide; args: - | - | | -| `specfact code review run` | nold-ai/specfact-code-review | --base-ref, --bug-hunt, --enforcement, --exclude-tests, --fix, --focus, --head-ref, --include-noise, --include-tests, --instructions, --interactive, --json, --level, --mode, --no-tests, --out, --path, --pr-context-file, --preview-fixes, --requirements-evidence, --scope, --score-only, --suppress-noise, --with-mutation; args: - | - | | +| `specfact code review run` | nold-ai/specfact-code-review | --bug-hunt, --enforcement, --exclude-tests, --fix, --focus, --include-noise, --include-tests, --instructions, --interactive, --json, --level, --mode, --no-tests, --out, --path, --preview-fixes, --requirements-evidence, --scope, --score-only, --suppress-noise, --with-mutation; args: - | - | | | `specfact code validate` | nold-ai/specfact-codebase | -; args: - | sidecar | | | `specfact code validate sidecar` | nold-ai/specfact-codebase | -; args: - | init, run | | | `specfact code validate sidecar init` | nold-ai/specfact-codebase | -; args: - | - | | From 123179ccb4b9d8523a8b3a7323010f6d6827a5ee Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sun, 30 Aug 2026 00:13:16 +0200 Subject: [PATCH 08/10] docs(review): address PR feedback evidence --- CHANGELOG.md | 3 + .../TDD_EVIDENCE.md | 79 +++++++++++++++---- .../proposal.md | 39 ++++++--- 3 files changed, 95 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7038904d..168d5280 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ All notable changes to this project will be documented in this file. - **Module scope diagnostics:** preserve valid user-scoped module installations when a project-local copy takes precedence, and replace routine uninstall advice with non-destructive origin guidance. +- **Module registry package:** advance the bundled `module-registry` package to + `0.1.35` and refresh its manifest integrity metadata for the updated + diagnostics. --- diff --git a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md index 86a6ca9f..bdffc9b4 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md +++ b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md @@ -4,8 +4,15 @@ - `hatch run pytest tests/unit/registry/test_module_discovery.py::test_project_shadow_warning_is_actionable_and_emitted_once tests/unit/modules/module_registry/test_commands.py::test_doctor_reports_effective_and_shadowed_duplicate_modules -q` - Result: FAIL before production edits (`2 failed`). - - Discovery still recommended `specfact module uninstall backlog-core --scope user`, and doctor still printed `Recovery: specfact module uninstall nold-ai/specfact-codebase --scope user` instead of preservation/no-action guidance. -- Retained CI red proof: Requirements Evidence run `33274750805` at signed source commit `b5ad2ea0d5e0ee062906e0c7b2f156330ea1a39f` executed the same two selectors and produced a bound `observed_maturity: red` artifact with no reconciliation findings. The workflow's overall failure is expected at this checkpoint and requires a later final implementation commit. + - Discovery still recommended `specfact module uninstall backlog-core --scope + user`, and doctor still printed `Recovery: specfact module uninstall + nold-ai/specfact-codebase --scope user` instead of preservation/no-action + guidance. +- Retained CI red proof: Requirements Evidence run `33274750805` at signed + source commit `b5ad2ea0d5e0ee062906e0c7b2f156330ea1a39f` executed the same two + selectors and produced a bound `observed_maturity: red` artifact with no + reconciliation findings. The workflow's overall failure is expected at this + checkpoint and requires a later final implementation commit. ## Passing After @@ -17,9 +24,17 @@ ## Review Follow-up -- Review-driven tests were added before the follow-up production edit for actual effective-source guidance, qualified availability, and user-only discovery outside the shadowing project. -- Initial focused run: FAIL (`3 failed, 1 passed`). The current doctor always named project scope and both diagnostics made an unconditional availability claim. The user-only preservation scenario already passed, so it remains supplementary regression coverage rather than a retained red-proof selector. -- Retained review red proof: Requirements Evidence run `33277091672` at signed source commit `daf05baa9303ef914f5659eafe940146d311af25` executed all three mapped selectors using the final reviewed test bytes and produced a bound `observed_maturity: red` artifact with no reconciliation findings. +- Review-driven tests were added before the follow-up production edit for + actual effective-source guidance, qualified availability, and user-only + discovery outside the shadowing project. +- Initial focused run: FAIL (`3 failed, 1 passed`). The current doctor always + named project scope and both diagnostics made an unconditional availability + claim. The user-only preservation scenario already passed, so it remains + supplementary regression coverage rather than a retained red-proof selector. +- Retained review red proof: Requirements Evidence run `33277091672` at signed + source commit `daf05baa9303ef914f5659eafe940146d311af25` executed all three + mapped selectors using the final reviewed test bytes and produced a bound + `observed_maturity: red` artifact with no reconciliation findings. - Passing focused run after the production edit: PASS (`4 passed`). - Related discovery/doctor files after the review fixes: PASS (`69 passed`). @@ -28,16 +43,50 @@ - `hatch run format`: PASS (942 files unchanged). - `hatch run type-check`: PASS (0 errors; 1,531 existing warnings). - `hatch run lint`: PASS. -- `hatch run yaml-lint`: exit code 0; it reports only pre-existing line-length/blank-line findings in untouched Requirements R07/R08 evidence. -- `hatch run contract-test` and `hatch run contract-test-contracts`: PASS using cached results after the full smart-test had refreshed hashes; both report no further modified contract inputs. The focused contract-sensitive discovery/doctor files pass, and the independent full suite exercised them. -- Schema-v2 Requirements evidence maps all changed scenarios to exact pytest selectors; the staged repository hook is the delivery gate. -- Product-owner review evidence is bound to mapping digest `sha256:fc0ff2c618b508f00943c66a987a14edf5175c730e9b26ac146785aa2045fe68` and core issue #699 for the required test-authored maturity gate. The executable plan contains the two original behavior regressions plus the failing effective-source review scenario; the already-passing user-only preservation scenario remains supplementary regression coverage. -- The built-in module-registry payload change advances the module-registry package to `0.1.34` with refreshed integrity metadata, advances all four canonical core version sources to `0.55.3`, and adds the matching changelog entry, as required by the release-integrity gates. -- `uv lock` refreshes the frozen project record from core `0.55.2` to `0.55.3`; `uv sync --locked --all-extras` then passes locally, resolving the first PR CI setup failures caused by the stale lock. -- Core CI's immutable module fixture is authoritative for generated command inventory. A local run against the newer paired modules checkout exposed three later Code Review PR-range options, but those options are intentionally absent from this core patch's generated artifacts so the frozen-fixture docs check remains reproducible. +- `openspec validate module-scope-02-preserve-user-installs --strict`: PASS. +- `hatch run yaml-lint`: exit code 0; it reports only pre-existing + line-length/blank-line findings in untouched Requirements R07/R08 evidence. +- `hatch run contract-test` and `hatch run contract-test-contracts`: PASS using + cached results after the full smart-test had refreshed hashes; both report no + further modified contract inputs. The focused contract-sensitive + discovery/doctor files pass, and the independent full suite exercised them. +- Schema-v2 Requirements evidence maps all behavior-changing scenarios to exact + pytest selectors; the staged repository hook is the delivery gate. The + unchanged development-source-root disclosure remains covered by + `tests/unit/modules/module_registry/test_commands.py::test_doctor_reports_configured_development_source_roots` + and is intentionally excluded from the red-proof mapping because it was + already green before this fix. +- Product-owner review evidence is bound to mapping digest + `sha256:fc0ff2c618b508f00943c66a987a14edf5175c730e9b26ac146785aa2045fe68` + and core issue #699 for the required test-authored maturity gate. The + executable plan contains the two original behavior regressions plus the + failing effective-source review scenario; the already-passing user-only + preservation scenario remains supplementary regression coverage. +- The built-in module-registry payload change advances the module-registry + package to `0.1.35` with refreshed integrity metadata, advances all four + canonical core version sources to `0.55.3`, and adds the matching changelog + entry. The PR's `Verify Module Signatures` job passes for that delivered + payload. +- `uv lock` refreshes the frozen project record from core `0.55.2` to `0.55.3`; + `uv sync --locked --all-extras` then passes locally, resolving the first PR CI + setup failures caused by the stale lock. +- Core CI's immutable module fixture is authoritative for generated command + inventory. A local run against the newer paired modules checkout exposed + three later Code Review PR-range options, but those options are intentionally + absent from this core patch's generated artifacts so the frozen-fixture docs + check remains reproducible. - `hatch run bandit-scan`: PASS (no medium/high findings). - Semgrep and its baseline gate: PASS (0 current findings, 0 baseline findings). -- Earlier local `hatch run smart-test` / `hatch run test` runs recorded `3029 passed, 12 skipped, 17 failed` before restoring the immutable fixture and refreshing the changed module signature. -- Final signed-head PR Orchestrator run `33275273173` executed `smart-test-full`: PASS (`3050 passed, 8 skipped`) on Python 3.12. Its Python 3.11 compatibility job also passed. -- The exact immutable-fixture full-enforcement Code Review used by CI passes locally with score 115 and zero findings after resolving all clean-code and type-safety warnings in the touched legacy files. The newer protected schema 1.6 capsule still reports assurance `UNKNOWN` on this macOS host because its controller supports Linux; Linux PR CI remains authoritative for that capsule. +- Earlier local `hatch run smart-test` / `hatch run test` runs recorded + `3029 passed, 12 skipped, 17 failed` before restoring the immutable fixture + and refreshing the changed module signature. +- Final signed-head PR Orchestrator run `33275273173` executed + `smart-test-full`: PASS (`3050 passed, 8 skipped`) on Python 3.12. Its Python + 3.11 compatibility job also passed. +- The exact immutable-fixture full-enforcement Code Review used by CI passes + locally with score 115 and zero findings after resolving all clean-code and + type-safety warnings in the touched legacy files. The newer protected schema + 1.6 capsule still reports assurance `UNKNOWN` on this macOS host because its + controller supports Linux; Linux PR CI remains authoritative for that + capsule. - `git diff --check`: PASS. diff --git a/openspec/changes/module-scope-02-preserve-user-installs/proposal.md b/openspec/changes/module-scope-02-preserve-user-installs/proposal.md index 991885b0..9bdeb336 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/proposal.md +++ b/openspec/changes/module-scope-02-preserve-user-installs/proposal.md @@ -1,26 +1,40 @@ ## Why -Core module discovery and `specfact module doctor` describe normal project-over-user shadowing as stale state and recommend uninstalling the user-scoped copy. Review/bootstrap workflows surface and follow that recommendation, repeatedly removing `specfact-codebase` and `specfact-code-review` from the user scope even though those installations are still needed in other repositories. +Core module discovery and `specfact module doctor` describe normal +project-over-user shadowing as stale state and recommend uninstalling the +user-scoped copy. Review/bootstrap workflows surface and follow that +recommendation, repeatedly removing `specfact-codebase` and +`specfact-code-review` from the user scope even though those installations are +still needed in other repositories. ## What Changes - Keep project-over-user precedence unchanged. -- Replace user-scope uninstall recovery advice with non-destructive scope guidance in module discovery warnings and doctor output. -- State explicitly that the user-scoped copy remains installed, normal shadowing alone does not require uninstalling it, and availability elsewhere still depends on module state and higher-priority copies. -- Add regression tests that reject destructive user-scope uninstall recommendations while preserving origin diagnostics. +- Replace user-scope uninstall recovery advice with non-destructive scope + guidance in module discovery warnings and doctor output. +- State explicitly that the user-scoped copy remains installed, normal + shadowing alone does not require uninstalling it, and availability elsewhere + still depends on module state and higher-priority copies. +- Add regression tests that reject destructive user-scope uninstall + recommendations while preserving origin diagnostics. ## Capabilities ### Modified Capabilities -- `module-scope-diagnostics`: Discovery and doctor diagnostics report shadowing without treating a valid user installation as cleanup residue. +- `module-scope-diagnostics`: Discovery and doctor diagnostics report shadowing + without treating a valid user installation as cleanup residue. ## Impact -- Affected code: `src/specfact_cli/registry/module_discovery.py` and `src/specfact_cli/modules/module_registry/src/commands.py`. +- Affected code: `src/specfact_cli/registry/module_discovery.py` and + `src/specfact_cli/modules/module_registry/src/commands.py`. - Affected tests: focused module discovery and module doctor unit tests. -- Paired modules delivery: `nold-ai/specfact-cli-modules#454` corrects the repository bootstrap surfaces that trigger this behavior during review work. -- Compatibility and data impact: none. Discovery order, module state, explicit uninstall behavior, manifests, and persistent installation data remain unchanged. +- Paired modules delivery: `nold-ai/specfact-cli-modules#454` corrects the + repository bootstrap surfaces that trigger this behavior during review work. +- Compatibility and data impact: none. Discovery order, module state, explicit + uninstall behavior, manifests, and persistent installation data remain + unchanged. --- @@ -30,9 +44,12 @@ Core module discovery and `specfact module doctor` describe normal project-over- - **Parent Feature**: [#353](https://github.com/nold-ai/specfact-cli/issues/353) - **Parent Epic**: [#194](https://github.com/nold-ai/specfact-cli/issues/194) - **Bug Issue**: [#699](https://github.com/nold-ai/specfact-cli/issues/699) -- **Paired Modules Bug**: [nold-ai/specfact-cli-modules#452](https://github.com/nold-ai/specfact-cli-modules/issues/452) -- **Issue Relationships**: `#699` is a sub-issue of Feature `#353`; Feature `#353` is a sub-issue of Epic `#194`. +- **Paired Modules Bug**: + [nold-ai/specfact-cli-modules#452](https://github.com/nold-ai/specfact-cli-modules/issues/452) +- **Issue Relationships**: `#699` is a sub-issue of Feature `#353`; Feature + `#353` is a sub-issue of Epic `#194`. - **Blocked By**: none - **Repository**: nold-ai/specfact-cli -- **Last Synced Status**: issue type, labels, assignee, parent, project assignment, In Progress status, and blocker metadata verified on 2026-08-29 +- **Last Synced Status**: issue type, labels, assignee, parent, project + assignment, In Progress status, and blocker metadata verified on 2026-08-29 - **Sanitized**: false From 37138693df6dfa1d9549c78f9f3f00e1f3170c89 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:14:02 +0000 Subject: [PATCH 09/10] chore(modules): manual approval-workflow sign changed modules --- src/specfact_cli/modules/module_registry/module-package.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/specfact_cli/modules/module_registry/module-package.yaml b/src/specfact_cli/modules/module_registry/module-package.yaml index 934e3f77..365f8974 100644 --- a/src/specfact_cli/modules/module_registry/module-package.yaml +++ b/src/specfact_cli/modules/module_registry/module-package.yaml @@ -17,5 +17,5 @@ publisher: description: 'Manage modules: search, list, show, install, and upgrade.' license: Apache-2.0 integrity: - checksum: sha256:2e5c27cfe497342356c4bfd9c261cc0a1a4bff7964bdfaab77d2a010292d450f - signature: iUE9y1OX2xpTiOPrGyPoVI9u8fPCwunn0Ua5tWPiz55r4K1VnE1YI9nNvpAL4fO/R0QqhzFlnoRDcopI4Af/CA== + checksum: sha256:c206f79f4c7955bf174e3d95c9cafbba282956f266271ab820bec85e1b22b472 + signature: AmTNHQEiZE1HgwcnUaekIeuuer4yc49JOiIkmj3eOd8t/0mwTKfQzoayQYPljbfBaPxrmfsh+R4s0IhKSPHtBw== From cef94ed79b625048e18bfb8e73aa88476643aab4 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sun, 30 Aug 2026 00:25:53 +0200 Subject: [PATCH 10/10] docs(openspec): close PR review follow-up --- .../TDD_EVIDENCE.md | 14 ++++++++++++++ .../tasks.md | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md index bdffc9b4..369a1ae0 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md +++ b/openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md @@ -90,3 +90,17 @@ controller supports Linux; Linux PR CI remains authoritative for that capsule. - `git diff --check`: PASS. + +## Final Review Closeout + +- Signed review-fix payload head + `37138693df6dfa1d9549c78f9f3f00e1f3170c89` passed Requirements Evidence run + `33278069038` and PR Orchestrator run `33278069078`. +- The final orchestrator recorded `3052 passed, 8 skipped` on Python 3.12 and + `3006 passed, 7 skipped` on Python 3.11. Strict local module-signature + verification also passed for the signed `module-registry` `0.1.35` payload. +- Docs Review run `33278069066`, Module Signature Hardening run `33278069032`, + and SpecFact CLI Validation run `33278069156` passed on the same signed head. +- The final GitHub review-thread audit found 20 inline threads, all resolved, + after verifying every requested production, test, documentation, and evidence + correction. diff --git a/openspec/changes/module-scope-02-preserve-user-installs/tasks.md b/openspec/changes/module-scope-02-preserve-user-installs/tasks.md index dbddb4a5..fd0ffece 100644 --- a/openspec/changes/module-scope-02-preserve-user-installs/tasks.md +++ b/openspec/changes/module-scope-02-preserve-user-installs/tasks.md @@ -29,4 +29,4 @@ - [x] 5.1 Add red-first coverage for actual effective-source guidance and user-only discovery outside a shadowing project. - [x] 5.2 Qualify availability claims using module state and other higher-priority copies. - [x] 5.3 Align module-system documentation, paired PR references, and the active-change count with the delivered behavior. -- [ ] 5.4 Re-run requirements evidence, review, signatures, and CI on the review-fix head; resolve verified review threads. +- [x] 5.4 Re-run requirements evidence, review, signatures, and CI on the review-fix head; resolve verified review threads.