Repository navigation
fix(modules): preserve shadowed user installs #700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
b5ad2ea
test(modules): capture user-scope preservation regression
djm81 1b4452c
test(review): capture module scope feedback regressions
djm81 d14ec5c
test(review): bind failing module scope cases
djm81 b897976
test(review): accept corrected proof mapping
djm81 daf05ba
test(review): simplify preserved-install diagnostics proof
djm81 31ec674
fix(modules): preserve user installs in shadow diagnostics
djm81 9521ca6
docs(cli): align command inventory with frozen fixture
djm81 123179c
docs(review): address PR feedback evidence
djm81 3713869
chore(modules): manual approval-workflow sign changed modules
github-actions[bot] cef94ed
docs(openspec): close PR review follow-up
djm81 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2
openspec/changes/module-scope-02-preserve-user-installs/.openspec.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-08-29 |
43 changes: 43 additions & 0 deletions
43
openspec/changes/module-scope-02-preserve-user-installs/TDD_EVIDENCE.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # 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, 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,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. | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| - 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. | ||
|
djm81 marked this conversation as resolved.
Outdated
|
||
| - `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. | ||
21 changes: 21 additions & 0 deletions
21
openspec/changes/module-scope-02-preserve-user-installs/design.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
38 changes: 38 additions & 0 deletions
38
openspec/changes/module-scope-02-preserve-user-installs/proposal.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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, normal shadowing alone does not require uninstalling it, and availability elsewhere still depends on module state and higher-priority copies. | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| - 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 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. | ||
|
|
||
| --- | ||
|
|
||
| ## Source Tracking | ||
|
|
||
| <!-- source_repo: nold-ai/specfact-cli --> | ||
| - **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 | ||
39 changes: 39 additions & 0 deletions
39
openspec/changes/module-scope-02-preserve-user-installs/requirements-evidence.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| 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 actual source 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 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 | ||
|
djm81 marked this conversation as resolved.
|
||
| - 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, 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 | ||
9 changes: 9 additions & 0 deletions
9
...ec/changes/module-scope-02-preserve-user-installs/requirements-proof/review-evidence.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "schema_version": "1", | ||
| "decision": "accepted", | ||
| "reviewer_id": "djm81", | ||
| "reviewer_role": "product-owner", | ||
| "recorded_at": "2026-08-29T23:41:00+02:00", | ||
| "reference": "https://github.com/nold-ai/specfact-cli/issues/699", | ||
| "mapping_digest": "sha256:fc0ff2c618b508f00943c66a987a14edf5175c730e9b26ac146785aa2045fe68" | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.