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
+775
−364
Merged
Changes from 1 commit
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
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 |
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 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 | ||
|
|
||
| <!-- 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 | ||
31 changes: 31 additions & 0 deletions
31
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,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 | ||
|
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 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 | ||
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-29T22:20:04+02:00", | ||
| "reference": "https://github.com/nold-ai/specfact-cli/issues/699", | ||
| "mapping_digest": "sha256:d25cd13f7e7fb1e327e2d9ba6c38eab85f5ba7ae870bd4a61efc13e05446f8f4" | ||
| } |
30 changes: 30 additions & 0 deletions
30
...s/module-scope-02-preserve-user-installs/specs/module-scope-diagnostics/spec.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,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 <module-id>` | ||
| - **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 | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| #### 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 | ||
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.