diff --git a/CHANGELOG.md b/CHANGELOG.md index 916a9248..94096f54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ All notable changes to this project will be documented in this file. **Important:** Changes need to be documented below this block as this is the header section. Each section should be separated by a horizontal rule. Newer changelog entries need to be added on top of prior ones to keep the history chronological with most recent changes first. +--- + +## [0.37.4] - 2026-02-25 + +### Changed + +- `specfact backlog map-fields` GitHub flow now treats ProjectV2 as optional and clears stale `provider_fields.github_project_v2` when ProjectV2 input is intentionally left blank. +- GitHub module discovery shadow behavior now emits a single actionable project-over-user precedence hint per process instead of repeating raw warning lines across repeated discovery calls. +- Registry diagnostic messages that are operational/debug in nature were moved from normal `INFO/WARNING` output to `DEBUG` where appropriate to reduce noisy default command output. + +### Fixed + +- Fixed `specfact backlog add` GitHub issue-type mapping precedence so valid `settings.github_issue_types.type_ids` is used when `settings.provider_fields.github_issue_types` is present but empty. +- Fixed stale GitHub ProjectV2 IDs continuing to trigger type-field update attempts after optional map-fields flows by explicitly clearing old ProjectV2 settings in blank-ProjectV2 reconfiguration. +- Reduced duplicate discovery work in `specfact module list` by avoiding repeated module-state fetches within the same command run. + --- ## [0.37.3] - 2026-02-24 diff --git a/docs/reference/commands.md b/docs/reference/commands.md index cbf206dd..8c7b3e1b 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -4268,6 +4268,12 @@ specfact backlog map-fields [OPTIONS] - `--ado-base-url` - Azure DevOps base URL (optional, defaults to `https://dev.azure.com`) - `--reset` - Reset custom field mapping to defaults (deletes `ado_custom.yaml` and restores default mappings) +**GitHub Notes:** + +- In GitHub mode, repository issue-type IDs are the primary mapping source for automatic issue Type updates. +- GitHub ProjectV2 metadata is optional. Leaving ProjectV2 input blank keeps repository issue-type mapping enabled. +- If ProjectV2 was configured previously and you rerun mapping with blank ProjectV2 input, stale `github_project_v2` mapping is cleared to avoid invalid ProjectV2 update attempts during `backlog add`. + **Token Resolution Priority:** 1. Explicit `--ado-token` parameter diff --git a/modules/backlog-core/module-package.yaml b/modules/backlog-core/module-package.yaml index 5e49ab37..9da20259 100644 --- a/modules/backlog-core/module-package.yaml +++ b/modules/backlog-core/module-package.yaml @@ -1,5 +1,5 @@ name: backlog-core -version: 0.1.2 +version: 0.1.3 commands: - backlog command_help: @@ -22,8 +22,8 @@ publisher: url: https://github.com/nold-ai/specfact-cli-modules email: oss@nold.ai integrity: - checksum: sha256:fce946fdc3a423b448bb85925df64687ed736c36dc5485e1e64531ae3dd75fe8 - signature: dgEEokYsULOxQRWE6WjRFkxCVGQ2Oo8kSmKg0qxfMauNE8hsRL9GY9vDrvn/ppPfX870BwgB+I7XxnvCZnwWBw== + checksum: sha256:12839ef2ee3a2eb6fd9901abc86b8837e6afb89ca372b6b3a0df1b4ed7c66279 + signature: rGstdKKFPad8oJ66Lse3UUD/cYx5u7HMg+ZR12mL+QNetamMeYk8QCdiryh/RAy3JIrtNW5zVY47OVuxzhSCAg== dependencies: [] description: Provide advanced backlog analysis and readiness capabilities. license: Apache-2.0 diff --git a/modules/backlog-core/src/backlog_core/commands/add.py b/modules/backlog-core/src/backlog_core/commands/add.py index 0cf71ee3..da3c7526 100644 --- a/modules/backlog-core/src/backlog_core/commands/add.py +++ b/modules/backlog-core/src/backlog_core/commands/add.py @@ -286,13 +286,19 @@ def _extract_github_project_v2(source: dict[str, Any]) -> dict[str, Any]: return {} def _extract_github_issue_types(source: dict[str, Any]) -> dict[str, Any]: + def _has_type_id_mapping(candidate: dict[str, Any]) -> bool: + raw_type_ids = candidate.get("type_ids") + if not isinstance(raw_type_ids, dict): + return False + return any(str(value).strip() for value in raw_type_ids.values()) + provider_fields = source.get("provider_fields") if isinstance(provider_fields, dict): candidate = provider_fields.get("github_issue_types") - if isinstance(candidate, dict): + if isinstance(candidate, dict) and _has_type_id_mapping(candidate): return dict(candidate) fallback = source.get("github_issue_types") - if isinstance(fallback, dict): + if isinstance(fallback, dict) and _has_type_id_mapping(fallback): return dict(fallback) return {} diff --git a/modules/backlog-core/tests/unit/test_add_command.py b/modules/backlog-core/tests/unit/test_add_command.py index fc7e660b..3f958870 100644 --- a/modules/backlog-core/tests/unit/test_add_command.py +++ b/modules/backlog-core/tests/unit/test_add_command.py @@ -798,3 +798,63 @@ def test_backlog_add_warns_when_github_issue_type_mapping_missing(monkeypatch) - assert result.exit_code == 0 assert "repository issue-type mapping is not configured" in result.stdout + + +def test_backlog_add_prefers_root_issue_type_ids_when_provider_fields_issue_types_empty( + monkeypatch, tmp_path: Path +) -> None: + """backlog add should use root github_issue_types when provider_fields copy is empty.""" + from specfact_cli.adapters.registry import AdapterRegistry + + spec_dir = tmp_path / ".specfact" + spec_dir.mkdir(parents=True, exist_ok=True) + (spec_dir / "backlog-config.yaml").write_text( + """ +backlog_config: + providers: + github: + adapter: github + project_id: nold-ai/specfact-demo-repo + settings: + provider_fields: + github_issue_types: + type_ids: {} + github_issue_types: + type_ids: + task: IT_TASK_SPEC +""".strip(), + encoding="utf-8", + ) + + created_payloads: list[dict] = [] + adapter = _FakeAdapter(items=[], relationships=[], created=created_payloads) + monkeypatch.setattr(AdapterRegistry, "get_adapter", lambda _adapter: adapter) + + result = runner.invoke( + backlog_app, + [ + "add", + "--project-id", + "nold-ai/specfact-demo-repo", + "--adapter", + "github", + "--type", + "task", + "--title", + "Implement task", + "--body", + "Body", + "--non-interactive", + "--repo-path", + str(tmp_path), + ], + ) + + assert result.exit_code == 0 + assert created_payloads + provider_fields = created_payloads[0].get("provider_fields") + assert isinstance(provider_fields, dict) + github_issue_types = provider_fields.get("github_issue_types") + assert isinstance(github_issue_types, dict) + assert github_issue_types.get("type_ids", {}).get("task") == "IT_TASK_SPEC" + assert "repository issue-type mapping is not configured" not in result.stdout diff --git a/openspec/changes/backlog-core-05-user-modules-bootstrap/CHANGE_VALIDATION.md b/openspec/changes/backlog-core-05-user-modules-bootstrap/CHANGE_VALIDATION.md index 572aaaa5..3a67cd3b 100644 --- a/openspec/changes/backlog-core-05-user-modules-bootstrap/CHANGE_VALIDATION.md +++ b/openspec/changes/backlog-core-05-user-modules-bootstrap/CHANGE_VALIDATION.md @@ -10,3 +10,30 @@ - Notes: - Status/instructions confirmed spec-driven schema and artifact completeness. - Validation emitted non-blocking schema warnings from `openspec/config.yaml` rule format, but strict change validation succeeded. + +## Refresh (2026-02-25) + +- Validation command: + - `openspec validate backlog-core-05-user-modules-bootstrap --strict` +- Validation result: + - `Change 'backlog-core-05-user-modules-bootstrap' is valid` +- Notes: + - OpenSpec telemetry flush attempted network access and failed in this environment (`edge.openspec.dev` DNS), which did not affect strict validation outcome. + +## Refresh (2026-02-25, map-fields optional ProjectV2) + +- Validation command: + - `openspec validate backlog-core-05-user-modules-bootstrap --strict` +- Validation result: + - `Change 'backlog-core-05-user-modules-bootstrap' is valid` +- Notes: + - OpenSpec telemetry network flush warnings were non-blocking and unrelated to change validity. + +## Refresh (2026-02-25, stale ProjectV2 cleanup + add precedence) + +- Validation command: + - `openspec validate backlog-core-05-user-modules-bootstrap --strict` +- Validation result: + - `Change 'backlog-core-05-user-modules-bootstrap' is valid` +- Notes: + - OpenSpec telemetry network flush warnings remained non-blocking (`edge.openspec.dev` DNS in this environment). diff --git a/openspec/changes/backlog-core-05-user-modules-bootstrap/TDD_EVIDENCE.md b/openspec/changes/backlog-core-05-user-modules-bootstrap/TDD_EVIDENCE.md index db6eaf55..f635603e 100644 --- a/openspec/changes/backlog-core-05-user-modules-bootstrap/TDD_EVIDENCE.md +++ b/openspec/changes/backlog-core-05-user-modules-bootstrap/TDD_EVIDENCE.md @@ -204,3 +204,60 @@ - Full signing-artifacts suite: `15 passed`. - Changed-only automation bumped and re-signed changed bundled manifest (`backlog`), restoring runtime integrity sync. - Regression safety suites after module re-sign: `95 passed, 1 skipped`. + +## Follow-up failing run (project-over-user shadow warning noise + duplicate list state fetch) + +- Timestamp: 2026-02-25T09:17:24Z +- Command(s): `hatch test -- tests/unit/registry/test_module_discovery.py tests/unit/modules/module_registry/test_commands.py -q` +- Failure summary: + - `test_project_shadow_warning_is_actionable_and_emitted_once` failed because `module_discovery` did not provide one-time actionable user guidance for project-over-user module shadowing. + - `test_list_command_fetches_module_state_once` failed because `specfact module list` called `get_modules_with_state()` twice. + - Additional command-suite failures were environment-local and unrelated to this behavior change. + +## Follow-up passing run (shadow warning UX + debug/log-level cleanup) + +- Timestamp: 2026-02-25T09:19:03Z +- Command(s): + - `python -m pytest tests/unit/registry/test_module_discovery.py -k "project_scope_takes_priority_over_user or project_shadow_warning_is_actionable_and_emitted_once" -q` + - `python -m pytest tests/unit/modules/module_registry/test_commands.py -k "list_command_fetches_module_state_once" -q` +- Result summary: + - Project-over-user precedence warning behavior tests: `2 passed`. + - Module list duplicate state fetch regression test: `1 passed`. + - Shadow guidance is now actionable and emitted once per process; duplicate discovery noise is reduced by avoiding repeated module-state fetch in `module list`. + +## Follow-up failing run (GitHub ProjectV2 optionality for map-fields) + +- Timestamp: 2026-02-25T08:30:21Z +- Command(s): `python -m pytest tests/unit/commands/test_backlog_commands.py -k "allows_blank_project_v2" -q` +- Failure summary: + - `test_map_fields_github_provider_allows_blank_project_v2` failed because blank ProjectV2 input still triggered GraphQL ProjectV2 resolution and command exit with error. + +## Follow-up passing run (GitHub ProjectV2 optionality for map-fields) + +- Timestamp: 2026-02-25T08:30:21Z +- Command(s): + - `python -m pytest tests/unit/commands/test_backlog_commands.py -k "allows_blank_project_v2 or persists_backlog_config or fails_when_issue_types_unavailable" -q` +- Result summary: + - Targeted GitHub map-fields coverage: `3 passed`. + - Blank ProjectV2 input now skips optional ProjectV2 mapping and still persists repository issue-type IDs. + +## Follow-up failing run (stale ProjectV2 and issue-type precedence) + +- Timestamp: 2026-02-25T08:36:01Z +- Command(s): + - `python -m pytest tests/unit/commands/test_backlog_commands.py -k "blank_project_v2_clears_stale_project_mapping" -q` + - `python -m pytest modules/backlog-core/tests/unit/test_add_command.py -k "prefers_root_issue_type_ids_when_provider_fields_issue_types_empty" -q` +- Failure summary: + - `test_map_fields_blank_project_v2_clears_stale_project_mapping` failed because blank ProjectV2 input did not clear stale `provider_fields.github_project_v2`. + - `test_backlog_add_prefers_root_issue_type_ids_when_provider_fields_issue_types_empty` failed because `backlog add` preferred empty `provider_fields.github_issue_types` over populated root `github_issue_types`. + +## Follow-up passing run (stale ProjectV2 cleanup + issue-type precedence) + +- Timestamp: 2026-02-25T08:36:01Z +- Command(s): + - `python -m pytest tests/unit/commands/test_backlog_commands.py -k "allows_blank_project_v2 or blank_project_v2_clears_stale_project_mapping or persists_backlog_config or fails_when_issue_types_unavailable" -q` + - `python -m pytest modules/backlog-core/tests/unit/test_add_command.py -k "prefers_root_issue_type_ids_when_provider_fields_issue_types_empty or forwards_github_project_v2_from_backlog_config or warns_when_github_issue_type_mapping_missing" -q` +- Result summary: + - GitHub map-fields optional ProjectV2 and stale-cleanup coverage: `4 passed`. + - Backlog-core add mapping-precedence coverage: `3 passed`. + - Blank ProjectV2 flow now clears stale ProjectV2 mapping and `backlog add` now uses valid root `github_issue_types.type_ids` when provider-fields copy is empty. diff --git a/openspec/changes/backlog-core-05-user-modules-bootstrap/proposal.md b/openspec/changes/backlog-core-05-user-modules-bootstrap/proposal.md index 786ee64a..692682c4 100644 --- a/openspec/changes/backlog-core-05-user-modules-bootstrap/proposal.md +++ b/openspec/changes/backlog-core-05-user-modules-bootstrap/proposal.md @@ -6,6 +6,8 @@ + + `specfact backlog add` is still missing in installed-runtime contexts when command discovery depends on repository-local `modules/` folders. This makes behavior vary by working directory and machine. For production usage, shipped modules and their resources should be managed as user-level artifacts. We need a reliable path where `specfact module init` prepares a per-user module root (not repo-local) so command availability is stable. @@ -16,6 +18,8 @@ For production usage, shipped modules and their resources should be managed as u + + - **MODIFY**: Add a canonical user module root at `/.specfact/modules` for installed module artifacts. - **MODIFY**: Ensure discovery and installer flows prefer `/.specfact/modules` and stop treating workspace `./modules` as an automatic discovery root. - **MODIFY**: Add workspace-local module discovery only under `/.specfact/modules` to avoid claiming ownership of non-SpecFact repository paths. @@ -54,4 +58,4 @@ For production usage, shipped modules and their resources should be managed as u - **Issue URL**: - **Last Synced Status**: proposed - **Sanitized**: false - \ No newline at end of file + \ No newline at end of file diff --git a/openspec/changes/backlog-core-05-user-modules-bootstrap/specs/backlog-map-fields/spec.md b/openspec/changes/backlog-core-05-user-modules-bootstrap/specs/backlog-map-fields/spec.md index 4b0214ee..b2d866df 100644 --- a/openspec/changes/backlog-core-05-user-modules-bootstrap/specs/backlog-map-fields/spec.md +++ b/openspec/changes/backlog-core-05-user-modules-bootstrap/specs/backlog-map-fields/spec.md @@ -18,3 +18,19 @@ The system SHALL verify auth context and discover provider fields/metadata befor - **WHEN** `specfact backlog map-fields` persists GitHub settings - **THEN** `.specfact/backlog-config.yaml` includes `backlog_config.providers.github.settings.github_issue_types.type_ids` - **AND** subsequent `specfact backlog add` can consume those IDs for issue-type updates. + +#### Scenario: GitHub ProjectV2 mapping is optional + +- **GIVEN** GitHub repository issue types are successfully discovered +- **AND** the user leaves GitHub ProjectV2 input empty +- **WHEN** `specfact backlog map-fields` runs +- **THEN** the command succeeds and persists repository issue-type IDs +- **AND** ProjectV2 field mapping is skipped without a hard failure. + +#### Scenario: Blank ProjectV2 input clears stale ProjectV2 mapping + +- **GIVEN** existing `backlog-config` contains stale `provider_fields.github_project_v2` values +- **AND** GitHub repository issue types are successfully discovered +- **WHEN** `specfact backlog map-fields` runs with blank ProjectV2 input +- **THEN** stale `provider_fields.github_project_v2` configuration is cleared +- **AND** subsequent `specfact backlog add` does not attempt ProjectV2 type-field updates from stale IDs. diff --git a/openspec/changes/backlog-core-05-user-modules-bootstrap/specs/user-module-root/spec.md b/openspec/changes/backlog-core-05-user-modules-bootstrap/specs/user-module-root/spec.md index a9bf3b10..02a689cb 100644 --- a/openspec/changes/backlog-core-05-user-modules-bootstrap/specs/user-module-root/spec.md +++ b/openspec/changes/backlog-core-05-user-modules-bootstrap/specs/user-module-root/spec.md @@ -83,6 +83,14 @@ Workspace project modules SHALL take precedence over user-scope modules. - **THEN** the discovered module source for `` resolves to project scope - **AND** command behavior uses project module artifacts for that repo context. +#### Scenario: Shadow guidance is actionable and emitted once per process + +- **GIVEN** `/.specfact/modules/` exists +- **AND** `/.specfact/modules/` exists +- **WHEN** module discovery runs repeatedly in the same process +- **THEN** CLI emits at most one user-facing warning that project scope takes precedence +- **AND** the warning includes actionable guidance to inspect origins and optionally clean a stale user-scope module copy. + ### Requirement: Startup Module Freshness Guidance Startup checks SHALL provide module freshness guidance for bundled modules across project and user scopes. diff --git a/pyproject.toml b/pyproject.toml index 408083e4..a3dae7e3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "specfact-cli" -version = "0.37.3" +version = "0.37.4" description = "The swiss knife CLI for agile DevOps teams. Keep backlog, specs, tests, and code in sync with validation and contract enforcement for new projects and long-lived codebases." readme = "README.md" requires-python = ">=3.11" diff --git a/setup.py b/setup.py index 8eb249c0..4be44b82 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ if __name__ == "__main__": _setup = setup( name="specfact-cli", - version="0.37.3", + version="0.37.4", description=( "The swiss knife CLI for agile DevOps teams. Keep backlog, specs, tests, and code in sync with " "validation and contract enforcement for new projects and long-lived codebases." diff --git a/src/specfact_cli/__init__.py b/src/specfact_cli/__init__.py index 75922b3e..f5f90200 100644 --- a/src/specfact_cli/__init__.py +++ b/src/specfact_cli/__init__.py @@ -8,6 +8,6 @@ - Supporting agile ceremonies and team workflows """ -__version__ = "0.37.3" +__version__ = "0.37.4" __all__ = ["__version__"] diff --git a/src/specfact_cli/modules/backlog/module-package.yaml b/src/specfact_cli/modules/backlog/module-package.yaml index e1fd3b15..8c881e51 100644 --- a/src/specfact_cli/modules/backlog/module-package.yaml +++ b/src/specfact_cli/modules/backlog/module-package.yaml @@ -1,5 +1,5 @@ name: backlog -version: 0.1.0 +version: 0.1.2 commands: - backlog command_help: @@ -28,5 +28,5 @@ publisher: description: Manage backlog ceremonies, refinement, and dependency insights. license: Apache-2.0 integrity: - checksum: sha256:1ab19fd9dd206ea5bb12af2f785fda4b56c5e3324902154f31ca24cc88334b4f - signature: UDxeFiE6DafWglCWREHuSZz4ezApHwygKI4wqkHiFIcHFk5CRqnH9KPUdTVyB+Agiys4k3kKNfGb/0Ljit//DA== + checksum: sha256:ef3febe90c62fa24b3da6b6b2f8a7b605c5b544db01aac10e5cfacc807bcca26 + signature: 2b/UNbFlgRQiwJ+LrEQKyoYUi8lxeLGc4hrcTUdCinU9yXa1h6GvYpRkAkzRW8kxNNNsetphuf7hLZ3II3bNDQ== diff --git a/src/specfact_cli/modules/backlog/src/commands.py b/src/specfact_cli/modules/backlog/src/commands.py index 8670fb8d..3e18eea1 100644 --- a/src/specfact_cli/modules/backlog/src/commands.py +++ b/src/specfact_cli/modules/backlog/src/commands.py @@ -4224,6 +4224,12 @@ def _github_graphql(query: str, variables: dict[str, Any]) -> dict[str, Any]: if key and value: cli_option_map[key] = value + issue_type_id_map = { + issue_type: repo_issue_types.get(issue_type, "") + for issue_type in ["epic", "feature", "story", "task", "bug"] + if repo_issue_types.get(issue_type) + } + # Fast-path for fully specified non-interactive invocations. if project_ref and (github_type_field_id or "").strip() and cli_option_map: github_custom_mapping_file = _persist_github_custom_mapping_file(repo_issue_types) @@ -4247,6 +4253,33 @@ def _github_graphql(query: str, variables: dict[str, Any]) -> dict[str, Any]: console.print(f"[green]Custom mapping:[/green] {github_custom_mapping_file}") return + if not project_ref: + if cli_option_map or (github_type_field_id or "").strip(): + console.print( + "[yellow]⚠[/yellow] GitHub ProjectV2 Type options/field-id were provided, but no ProjectV2 " + "number/ID was set. Skipping ProjectV2 mapping." + ) + github_custom_mapping_file = _persist_github_custom_mapping_file(repo_issue_types) + initial_settings_update: dict[str, Any] = { + "github_issue_types": {"type_ids": issue_type_id_map}, + # Clear stale ProjectV2 mapping when user explicitly skips ProjectV2 input. + "provider_fields": {"github_project_v2": None}, + "field_mapping_file": ".specfact/templates/backlog/field_mappings/github_custom.yaml", + } + config_path = _upsert_backlog_provider_settings( + "github", + initial_settings_update, + project_id=project_context, + adapter="github", + ) + console.print(f"[green]✓[/green] GitHub mapping saved to {config_path}") + console.print(f"[green]Custom mapping:[/green] {github_custom_mapping_file}") + console.print( + "[dim]ProjectV2 Type field mapping skipped; repository issue types were captured " + "(ProjectV2 is optional).[/dim]" + ) + return + project_id = "" project_title = "" fields_nodes: list[dict[str, Any]] = [] @@ -4445,12 +4478,6 @@ def _field_options(field: dict[str, Any]) -> set[str]: if option_name and option_name in option_name_to_id: option_map[issue_type] = option_name_to_id[option_name] - issue_type_id_map = { - issue_type: repo_issue_types.get(issue_type, "") - for issue_type in ["epic", "feature", "story", "task", "bug"] - if repo_issue_types.get(issue_type) - } - settings_update: dict[str, Any] = {} if issue_type_id_map: settings_update["github_issue_types"] = {"type_ids": issue_type_id_map} diff --git a/src/specfact_cli/modules/module_registry/module-package.yaml b/src/specfact_cli/modules/module_registry/module-package.yaml index 6d764408..0cc72b1a 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.0 +version: 0.1.3 commands: - module command_help: @@ -15,5 +15,5 @@ publisher: description: 'Manage modules: search, list, show, install, and upgrade.' license: Apache-2.0 integrity: - checksum: sha256:933aeb8adb353565167a3a63dfda9dd62c15ce4f5574a85198c8007e992780e3 - signature: 8bn3S2Unl2sLaGIF7f0MlLkH4AoplU5h2VY/uDoJBwmO3rl3xweYXuoJftPKmFASBUWcVxxoK+YoGvOzY6fpDA== + checksum: sha256:92a3673edb39974791afcc357a1e931b6e4152d1ad7af0e4a4d722c29431b90d + signature: eynkV+88hx9PPGlOnB5rwWHkiEtgFe8wNIU+5I0C8PZR+WWoRIcTcvTe5TYYAyiHi6JmJZZLvrDsTIpe3K3VDQ== diff --git a/src/specfact_cli/modules/module_registry/src/commands.py b/src/specfact_cli/modules/module_registry/src/commands.py index 244e61c2..7ad0959e 100644 --- a/src/specfact_cli/modules/module_registry/src/commands.py +++ b/src/specfact_cli/modules/module_registry/src/commands.py @@ -509,13 +509,14 @@ def list_modules( ), ) -> None: """List installed modules with trust labels and optional origin details.""" - modules = get_modules_with_state() + all_modules = get_modules_with_state() + modules = all_modules if source: modules = [m for m in modules if str(m.get("source", "")) == source] render_modules_table(console, modules, show_origin=show_origin) bundled = get_bundled_module_metadata() - installed_ids = {str(module.get("id", "")).strip() for module in get_modules_with_state()} + installed_ids = {str(module.get("id", "")).strip() for module in all_modules} available = [meta for name, meta in bundled.items() if name not in installed_ids] if not show_bundled_available: if available: diff --git a/src/specfact_cli/registry/marketplace_client.py b/src/specfact_cli/registry/marketplace_client.py index 97c9a63b..9572ed87 100644 --- a/src/specfact_cli/registry/marketplace_client.py +++ b/src/specfact_cli/registry/marketplace_client.py @@ -96,5 +96,5 @@ def download_module( file_name = Path(parsed.path).name or f"{module_id.replace('/', '-')}.tar.gz" target_path = target_dir / file_name target_path.write_bytes(content) - logger.info("Downloaded module '%s' to '%s'", module_id, target_path) + logger.debug("Downloaded module '%s' to '%s'", module_id, target_path) return target_path diff --git a/src/specfact_cli/registry/module_discovery.py b/src/specfact_cli/registry/module_discovery.py index 55d8380a..9eea3721 100644 --- a/src/specfact_cli/registry/module_discovery.py +++ b/src/specfact_cli/registry/module_discovery.py @@ -10,11 +10,13 @@ from specfact_cli.common import get_bridge_logger from specfact_cli.models.module_package import ModulePackageMetadata +from specfact_cli.utils.prompts import print_warning USER_MODULES_ROOT = Path.home() / ".specfact" / "modules" MARKETPLACE_MODULES_ROOT = Path.home() / ".specfact" / "marketplace-modules" CUSTOM_MODULES_ROOT = Path.home() / ".specfact" / "custom-modules" +_SHADOW_HINT_KEYS: set[tuple[str, str, str, str]] = set() @dataclass(frozen=True) @@ -45,7 +47,7 @@ def discover_all_modules( logger = get_bridge_logger(__name__) discovered: list[DiscoveredModule] = [] - seen_names: set[str] = set() + seen_by_name: dict[str, DiscoveredModule] = {} effective_builtin_root = builtin_root or get_modules_root() effective_project_root = get_workspace_modules_root() @@ -87,23 +89,40 @@ def discover_all_modules( entries = discover_package_metadata(root, source=source) for package_dir, metadata in entries: module_name = metadata.name - if module_name in seen_names: + if module_name in seen_by_name: + existing = seen_by_name[module_name] + if source == "user" and existing.source == "project": + warning_key = ( + module_name, + existing.source, + source, + str(existing.package_dir.resolve()), + ) + if warning_key not in _SHADOW_HINT_KEYS: + _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`." + ) if source in {"user", "marketplace", "custom"}: - logger.warning( - "Module '%s' from %s at '%s' is shadowed by higher-priority source.", + logger.debug( + "Module '%s' from %s at '%s' is shadowed by higher-priority source %s at '%s'.", module_name, source, package_dir, + existing.source, + existing.package_dir, ) continue - seen_names.add(module_name) - discovered.append( - DiscoveredModule( - package_dir=package_dir, - metadata=metadata, - source=source, - ) + entry = DiscoveredModule( + package_dir=package_dir, + metadata=metadata, + source=source, ) + seen_by_name[module_name] = entry + discovered.append(entry) return discovered diff --git a/src/specfact_cli/registry/module_installer.py b/src/specfact_cli/registry/module_installer.py index 64520d1c..24332ac3 100644 --- a/src/specfact_cli/registry/module_installer.py +++ b/src/specfact_cli/registry/module_installer.py @@ -409,7 +409,7 @@ def install_module( manifest_path = final_path / "module-package.yaml" if manifest_path.exists() and not reinstall: - logger.info("Module already installed (%s)", module_name) + logger.debug("Module already installed (%s)", module_name) return final_path archive_path = download_module(module_id, version=version) @@ -481,7 +481,7 @@ def install_module( shutil.rmtree(staged_path) raise - logger.info("Installed marketplace module '%s' to '%s'", module_id, final_path) + logger.debug("Installed marketplace module '%s' to '%s'", module_id, final_path) return final_path @@ -519,7 +519,7 @@ def uninstall_module( if not module_path.exists(): continue shutil.rmtree(module_path) - logger.info("Uninstalled module '%s' from '%s'", module_name, root) + logger.debug("Uninstalled module '%s' from '%s'", module_name, root) return roots_str = ", ".join(str(root) for root in candidate_roots) diff --git a/src/specfact_cli/registry/module_lifecycle.py b/src/specfact_cli/registry/module_lifecycle.py index 33c9f9a8..f70e2d38 100644 --- a/src/specfact_cli/registry/module_lifecycle.py +++ b/src/specfact_cli/registry/module_lifecycle.py @@ -15,7 +15,6 @@ discover_all_package_metadata, expand_disable_with_dependents, expand_enable_with_dependencies, - get_discovered_modules_for_state, merge_module_state, validate_disable_safe, validate_enable_safe, @@ -34,24 +33,24 @@ def get_modules_with_state( disable_ids: list[str] | None = None, ) -> list[dict[str, Any]]: """Return discovered modules with version, enabled state, source, and trust metadata.""" - modules_list = get_discovered_modules_for_state(enable_ids=enable_ids or [], disable_ids=disable_ids or []) discovered = discover_all_modules() - source_map = {entry.metadata.name: entry.source for entry in discovered} - official_map = { - entry.metadata.name: bool( - entry.metadata.publisher and entry.metadata.publisher.name.strip().lower() == "nold-ai" + discovered_list: list[tuple[str, str]] = [(entry.metadata.name, entry.metadata.version) for entry in discovered] + state = read_modules_state() + enabled_map = merge_module_state(discovered_list, state, enable_ids or [], disable_ids or []) + + modules_list: list[dict[str, Any]] = [] + for entry in discovered: + publisher_name = entry.metadata.publisher.name if entry.metadata.publisher else "unknown" + modules_list.append( + { + "id": entry.metadata.name, + "version": entry.metadata.version, + "enabled": enabled_map.get(entry.metadata.name, True), + "source": entry.source, + "official": bool(publisher_name.strip().lower() == "nold-ai"), + "publisher": publisher_name, + } ) - for entry in discovered - } - publisher_map = { - entry.metadata.name: entry.metadata.publisher.name if entry.metadata.publisher else "unknown" - for entry in discovered - } - for item in modules_list: - module_id = str(item.get("id", "")) - item["source"] = source_map.get(module_id, "unknown") - item["official"] = official_map.get(module_id, False) - item["publisher"] = publisher_map.get(module_id, "unknown") return modules_list @@ -80,7 +79,11 @@ def apply_module_state_update(*, enable_ids: list[str], disable_ids: list[str], for module_id, dependents in blocked_disable.items(): lines.append(f"Cannot disable '{module_id}': required by enabled modules: {', '.join(dependents)}") raise ValueError("\n".join(lines)) - modules_list = get_discovered_modules_for_state(enable_ids=enable_ids, disable_ids=disable_ids) + final_enabled_map = merge_module_state(discovered_list, state, enable_ids, disable_ids) + modules_list = [ + {"id": meta.name, "version": meta.version, "enabled": final_enabled_map.get(meta.name, True)} + for _package_dir, meta in packages + ] write_modules_state(modules_list) run_discovery_and_write_cache(__version__) return get_modules_with_state() diff --git a/src/specfact_cli/registry/module_packages.py b/src/specfact_cli/registry/module_packages.py index 336ae4f7..c096227d 100644 --- a/src/specfact_cli/registry/module_packages.py +++ b/src/specfact_cli/registry/module_packages.py @@ -831,7 +831,7 @@ def register_module_package_commands( f"schema version {meta.schema_version} required, current is {CURRENT_PROJECT_SCHEMA_VERSION}", ) ) - logger.warning( + logger.debug( "Module %s: Schema version %s required, but current is %s (skipped)", meta.name, meta.schema_version, @@ -841,7 +841,7 @@ def register_module_package_commands( if meta.schema_version is None: logger.debug("Module %s: No schema version declared (assuming current)", meta.name) else: - logger.info("Module %s: Schema version %s (compatible)", meta.name, meta.schema_version) + logger.debug("Module %s: Schema version %s (compatible)", meta.name, meta.schema_version) if meta.schema_extensions: try: @@ -892,7 +892,7 @@ def register_module_package_commands( elif operations: partial_modules.append((meta.name, operations)) if is_debug_mode(): - logger.warning("Module %s: ModuleIOContract partial (%s)", meta.name, ", ".join(operations)) + logger.info("Module %s: ModuleIOContract partial (%s)", meta.name, ", ".join(operations)) protocol_partial += 1 else: legacy_modules.append(meta.name) diff --git a/tests/unit/commands/test_backlog_commands.py b/tests/unit/commands/test_backlog_commands.py index 0149dab7..e047b840 100644 --- a/tests/unit/commands/test_backlog_commands.py +++ b/tests/unit/commands/test_backlog_commands.py @@ -393,6 +393,121 @@ def test_map_fields_github_provider_fails_when_issue_types_unavailable( assert result.exit_code != 0 assert "repository issue types" in result.stdout.lower() + @patch("questionary.checkbox") + @patch("specfact_cli.modules.backlog.src.commands.typer.prompt") + @patch("specfact_cli.utils.auth_tokens.get_token") + @patch("requests.post") + def test_map_fields_github_provider_allows_blank_project_v2( + self, + mock_post: MagicMock, + mock_get_token: MagicMock, + mock_prompt: MagicMock, + mock_checkbox: MagicMock, + tmp_path, + ) -> None: + """GitHub map-fields should not require ProjectV2 when repository issue types are available.""" + mock_checkbox.return_value.ask.return_value = ["github"] + mock_get_token.return_value = {"access_token": "gho_test", "token_type": "bearer"} + mock_prompt.side_effect = [ + "nold-ai/specfact-demo-repo", # owner/repo + "", # blank project ref (optional) + ] + mock_response = MagicMock() + mock_response.raise_for_status.return_value = None + mock_response.json.return_value = { + "data": { + "repository": { + "issueTypes": { + "nodes": [ + {"id": "IT_BUG", "name": "Bug"}, + {"id": "IT_TASK", "name": "Task"}, + ] + } + } + } + } + mock_post.return_value = mock_response + + import os + + cwd = Path.cwd() + try: + os.chdir(tmp_path) + result = runner.invoke(backlog_app, ["map-fields"]) + finally: + os.chdir(cwd) + + assert result.exit_code == 0 + assert "projectv2 type field mapping skipped" in result.stdout.lower() + cfg_file = tmp_path / ".specfact" / "backlog-config.yaml" + assert cfg_file.exists() + loaded = yaml.safe_load(cfg_file.read_text(encoding="utf-8")) + github_settings = loaded["backlog_config"]["providers"]["github"]["settings"] + assert github_settings["github_issue_types"]["type_ids"]["task"] == "IT_TASK" + provider_fields = github_settings.get("provider_fields", {}) + if isinstance(provider_fields, dict): + assert provider_fields.get("github_project_v2") is None + + @patch("questionary.checkbox") + @patch("specfact_cli.modules.backlog.src.commands.typer.prompt") + @patch("specfact_cli.utils.auth_tokens.get_token") + @patch("requests.post") + def test_map_fields_blank_project_v2_clears_stale_project_mapping( + self, + mock_post: MagicMock, + mock_get_token: MagicMock, + mock_prompt: MagicMock, + mock_checkbox: MagicMock, + tmp_path, + ) -> None: + """Blank ProjectV2 input should clear stale ProjectV2 provider_fields mapping.""" + spec_dir = tmp_path / ".specfact" + spec_dir.mkdir(parents=True, exist_ok=True) + (spec_dir / "backlog-config.yaml").write_text( + """ +backlog_config: + providers: + github: + adapter: github + project_id: nold-ai/specfact-demo-repo + settings: + provider_fields: + github_project_v2: + project_id: PVT_project_id + type_field_id: PVT_type_field + type_option_ids: + task: PVT_option_task +""".strip(), + encoding="utf-8", + ) + mock_checkbox.return_value.ask.return_value = ["github"] + mock_get_token.return_value = {"access_token": "gho_test", "token_type": "bearer"} + mock_prompt.side_effect = [ + "nold-ai/specfact-demo-repo", + "", + ] + mock_response = MagicMock() + mock_response.raise_for_status.return_value = None + mock_response.json.return_value = { + "data": {"repository": {"issueTypes": {"nodes": [{"id": "IT_TASK", "name": "Task"}]}}} + } + mock_post.return_value = mock_response + + import os + + cwd = Path.cwd() + try: + os.chdir(tmp_path) + result = runner.invoke(backlog_app, ["map-fields"]) + finally: + os.chdir(cwd) + + assert result.exit_code == 0 + loaded = yaml.safe_load((spec_dir / "backlog-config.yaml").read_text(encoding="utf-8")) + github_settings = loaded["backlog_config"]["providers"]["github"]["settings"] + provider_fields = github_settings.get("provider_fields", {}) + assert provider_fields.get("github_project_v2") is None + def test_backlog_init_config_scaffolds_default_file(self, tmp_path) -> None: """Test backlog init-config creates default backlog-config scaffold.""" import os diff --git a/tests/unit/modules/module_registry/test_commands.py b/tests/unit/modules/module_registry/test_commands.py index 99603950..91e90bf1 100644 --- a/tests/unit/modules/module_registry/test_commands.py +++ b/tests/unit/modules/module_registry/test_commands.py @@ -618,6 +618,44 @@ 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: + 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-core", + "version": "0.2.0", + "enabled": True, + "source": "project", + "official": True, + "publisher": "nold-ai", + }, + ], + ) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.get_bundled_module_metadata", + lambda: { + "init": ModulePackageMetadata(name="init", version="0.1.0", description="Core init module"), + "backlog-core": ModulePackageMetadata(name="backlog-core", version="0.2.0", description="Backlog"), + }, + raising=False, + ) + + result = runner.invoke(app, ["list", "--source", "builtin", "--show-bundled-available"]) + + assert result.exit_code == 0 + assert "Bundled Modules Available" not in result.stdout + assert "All bundled modules are already installed" in result.stdout + + def test_list_command_show_bundled_available_separate_section_with_hints(monkeypatch) -> None: monkeypatch.setattr( "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", @@ -711,6 +749,38 @@ 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: + calls = {"count": 0} + + def _get_modules_with_state() -> list[dict[str, object]]: + calls["count"] += 1 + return [ + { + "id": "init", + "version": "0.1.0", + "enabled": True, + "source": "builtin", + "official": True, + "publisher": "nold-ai", + } + ] + + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.get_modules_with_state", _get_modules_with_state + ) + monkeypatch.setattr( + "specfact_cli.modules.module_registry.src.commands.get_bundled_module_metadata", + lambda: { + "init": ModulePackageMetadata(name="init", version="0.1.0", description="Core init module"), + }, + ) + + result = runner.invoke(app, ["list"]) + + assert result.exit_code == 0 + 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", diff --git a/tests/unit/registry/test_module_discovery.py b/tests/unit/registry/test_module_discovery.py index 4c11be05..50d73aed 100644 --- a/tests/unit/registry/test_module_discovery.py +++ b/tests/unit/registry/test_module_discovery.py @@ -115,3 +115,35 @@ def test_discover_all_modules_project_scope_takes_priority_over_user(tmp_path: P sources = {entry.metadata.name: entry.source for entry in discovered} assert sources["backlog-core"] == "project" + + +def test_project_shadow_warning_is_actionable_and_emitted_once(tmp_path: Path, monkeypatch) -> None: + """Project-over-user shadow guidance should be user-facing but deduplicated per process.""" + repo_root = tmp_path / "repo" + project_root = repo_root / ".specfact" / "modules" + builtin_root = tmp_path / "builtin" + user_root = tmp_path / "user-modules" + _write_manifest(builtin_root, "init") + _write_manifest(project_root, "backlog-core") + _write_manifest(user_root, "backlog-core") + + monkeypatch.chdir(repo_root) + warnings: list[str] = [] + monkeypatch.setattr(module_discovery, "print_warning", warnings.append) + module_discovery._SHADOW_HINT_KEYS.clear() + + discover_all_modules( + builtin_root=builtin_root, + user_root=user_root, + include_legacy_roots=True, + ) + discover_all_modules( + builtin_root=builtin_root, + user_root=user_root, + include_legacy_roots=True, + ) + + 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] diff --git a/tests/unit/registry/test_module_lifecycle.py b/tests/unit/registry/test_module_lifecycle.py new file mode 100644 index 00000000..c7827325 --- /dev/null +++ b/tests/unit/registry/test_module_lifecycle.py @@ -0,0 +1,30 @@ +from pathlib import Path + +from specfact_cli.models.module_package import ModulePackageMetadata +from specfact_cli.registry import module_lifecycle + + +def test_apply_module_state_update_persists_disabled_modules(monkeypatch) -> None: + monkeypatch.setattr( + module_lifecycle, + "discover_all_package_metadata", + lambda: [(Path("/tmp/mock-module"), ModulePackageMetadata(name="mock-module", version="0.1.0"))], + ) + monkeypatch.setattr( + module_lifecycle, + "read_modules_state", + lambda: {"mock-module": {"version": "0.1.0", "enabled": True}}, + ) + monkeypatch.setattr(module_lifecycle, "run_discovery_and_write_cache", lambda _version: None) + monkeypatch.setattr(module_lifecycle, "get_modules_with_state", list) + + captured: dict[str, list[dict[str, object]]] = {"modules": []} + + def _capture_write(modules: list[dict[str, object]]) -> None: + captured["modules"] = modules + + monkeypatch.setattr(module_lifecycle, "write_modules_state", _capture_write) + + module_lifecycle.apply_module_state_update(enable_ids=[], disable_ids=["mock-module"], force=False) + + assert captured["modules"] == [{"id": "mock-module", "version": "0.1.0", "enabled": False}]