-
Notifications
You must be signed in to change notification settings - Fork 1
fix: safe VS Code settings merge and project artifact writes (#490) #496
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 all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
4e98197
fix: safe merge for VS Code settings.json on init ide (profile-04)
djm81 ed16399
fix(profile-04): satisfy review gate, pin setuptools for semgrep
djm81 3a76aac
ci: run safe-write verifier in PR orchestrator lint job
djm81 75cf864
fix(profile-04): address CodeRabbit review (docs, guard, contracts, tβ¦
djm81 13d12d6
fix(profile-04): JSON5 settings, repo containment, review follow-ups
djm81 002bacd
docs(profile-04): tasks pre-flight + full pytest; narrow icontract enβ¦
djm81 d88b873
fix: clear specfact code review on safe-write modules
djm81 05c2fb0
docs(profile-04): record hatch test --cover -v in TDD_EVIDENCE
djm81 91c9e22
fix: reduce KISS blockers (bridge sync contexts, tools, partial adaptβ¦
djm81 5d4e1c9
Fix review findings and sign modules
djm81 b6867c6
fix(tests): register dynamic check_doc_frontmatter module; align _updβ¦
djm81 337a6bd
fix(tests): align install_module mocks with InstallModuleOptions; regβ¦
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
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
70 changes: 70 additions & 0 deletions
70
openspec/changes/profile-04-safe-project-artifact-writes/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,70 @@ | ||
| # TDD evidence β profile-04-safe-project-artifact-writes | ||
|
|
||
| ## Failing-first (targeted) | ||
|
|
||
| - **When**: 2026-04-12 (Europe/Berlin) | ||
| - **Command**: | ||
|
|
||
| ```bash | ||
| cd ../specfact-cli-worktrees/bugfix/profile-04-safe-project-artifact-writes | ||
| hatch run pytest \ | ||
| tests/unit/utils/test_project_artifact_write.py \ | ||
| tests/unit/utils/test_ide_setup.py \ | ||
| tests/unit/scripts/test_verify_safe_project_writes.py \ | ||
| tests/unit/modules/init/test_init_ide_prompt_selection.py \ | ||
| -q | ||
| ``` | ||
|
|
||
| - **Note**: New scenarios (`malformed_json_raises`, `preserves_unrelated_keys`, verify script) were added | ||
| before the safe-merge implementation; prior behavior treated invalid JSON as `{}` and could destroy user | ||
| settings (issue #487). | ||
|
|
||
| ## Passing-after (targeted + e2e) | ||
|
|
||
| - **When**: 2026-04-12 | ||
| - **Commands**: | ||
|
|
||
| ```bash | ||
| hatch run pytest tests/unit/utils/test_project_artifact_write.py \ | ||
| tests/unit/utils/test_ide_setup.py tests/unit/scripts/test_verify_safe_project_writes.py \ | ||
| tests/unit/modules/init/test_init_ide_prompt_selection.py tests/e2e/test_init_command.py -q | ||
| hatch run format && hatch run type-check && hatch run lint | ||
| hatch run contract-test | ||
| hatch run smart-test | ||
| hatch test --cover -v | ||
| ``` | ||
|
|
||
| - **Full suite + coverage (`tasks.md` 4.3)**: same worktree; `hatch test --cover -v` β **exit 0**. Pytest summary: | ||
| `2450 passed, 9 skipped in 358.61s (0:05:58)`. Coverage footer (pytest-cov): `TOTAL ... 62%` on the combined | ||
| `src/` + `tools/` table (see run log for per-file lines). | ||
|
|
||
| - **Module signatures**: run | ||
| `hatch run ./scripts/verify-modules-signature.py --require-signature` β pass without bumping | ||
| `src/specfact_cli/modules/init/module-package.yaml` (init UX errors are raised from `ide_setup` so the init | ||
| module payload checksum is unchanged). | ||
|
|
||
| ## Code review gate | ||
|
|
||
| - **Pass (2026-04-12)**: after `hatch run specfact module install nold-ai/specfact-codebase` and | ||
| `hatch run specfact module install nold-ai/specfact-code-review` (user scope), run: | ||
|
|
||
| ```bash | ||
| hatch run specfact code review run --json --out .specfact/code-review.json \ | ||
| src/specfact_cli/utils/project_artifact_write.py \ | ||
| src/specfact_cli/utils/ide_setup.py \ | ||
| scripts/verify_safe_project_writes.py | ||
| ``` | ||
|
|
||
| - Report: `.specfact/code-review.json` (exit 0, no blocking findings after merge-helper refactor and setuptools | ||
| pin). | ||
|
|
||
| ## OpenSpec strict validation | ||
|
|
||
| - **Pass (2026-04-12)**: | ||
| `openspec validate profile-04-safe-project-artifact-writes --strict` β exit 0 (recorded at sign-off per | ||
| `tasks.md` 4.7). | ||
|
|
||
| ## Worktree cleanup (post-merge on developer machine) | ||
|
|
||
| - Remove worktree, delete branch, prune β see `tasks.md` section 5 (not executed in this implementation | ||
| session). | ||
65 changes: 47 additions & 18 deletions
65
openspec/changes/profile-04-safe-project-artifact-writes/tasks.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 |
|---|---|---|
| @@ -1,35 +1,64 @@ | ||
| ## 1. Branch, coordination, and issue sync | ||
|
|
||
| - [ ] 1.1 Create `bugfix/profile-04-safe-project-artifact-writes` in a dedicated worktree from `origin/dev` and bootstrap Hatch in that worktree. | ||
| - [ ] 1.2 Sync the change proposal to GitHub under parent feature `#365`, link bug `#487`, and update `proposal.md` Source Tracking with issue metadata. | ||
| - [ ] 1.3 Confirm the paired modules-side change `project-runtime-01-safe-artifact-write-policy` is available and note the dependency in both PR descriptions/change evidence. | ||
| - [x] 1.1 Create `bugfix/profile-04-safe-project-artifact-writes` in a dedicated worktree from | ||
| `origin/dev`; run `hatch env create`, then pre-flight status checks `hatch run smart-test-status` and | ||
| `hatch run contract-test-status`. | ||
| - [ ] 1.2 Sync the change proposal to GitHub under parent feature `#365`, link bug `#487`, and update | ||
| `proposal.md` Source Tracking with issue metadata. *(human / PR author)* | ||
| - [ ] 1.3 Confirm the paired modules-side change `project-runtime-01-safe-artifact-write-policy` is | ||
| available and note the dependency in both PR descriptions/change evidence. *(human)* | ||
|
|
||
| ## 2. Specs, regression fixtures, and failing evidence | ||
|
|
||
| - [ ] 2.1 Add or update regression fixtures for existing user-owned project artifacts such as `.vscode/settings.json` with unrelated custom settings. | ||
| - [ ] 2.2 Write tests from the new scenarios covering partial ownership, malformed settings fail-safe behavior, backup creation, and preservation of unrelated settings. | ||
| - [ ] 2.3 Write tests for the CI/static unsafe-write gate so direct writes to protected project artifacts are rejected unless routed through the sanctioned helper. | ||
| - [ ] 2.4 Run the targeted tests before implementation, capture the failing results, and record commands/timestamps in `openspec/changes/profile-04-safe-project-artifact-writes/TDD_EVIDENCE.md`. | ||
| - [x] 2.1 Add or update regression fixtures for existing user-owned project artifacts such as | ||
| `.vscode/settings.json` with unrelated custom settings. *(pytest tmp_path fixtures in | ||
| `test_project_artifact_write.py`)* | ||
| - [x] 2.2 Write tests from the new scenarios covering partial ownership, malformed settings fail-safe | ||
| behavior, backup creation, and preservation of unrelated settings. | ||
| - [x] 2.3 Write tests for the CI/static unsafe-write gate so direct writes to protected project | ||
| artifacts are rejected unless routed through the sanctioned helper. | ||
| - [x] 2.4 Run the targeted tests before implementation, capture the failing results, and record | ||
| commands/timestamps in `openspec/changes/profile-04-safe-project-artifact-writes/TDD_EVIDENCE.md`. | ||
|
|
||
| ## 3. Core safe-write implementation | ||
|
|
||
| - [ ] 3.1 Implement the core safe-write helper and ownership model with `@beartype` and `@icontract` on public APIs. | ||
| - [ ] 3.2 Route `src/specfact_cli/utils/ide_setup.py` settings mutation through the helper so `.vscode/settings.json` preserves unrelated user-managed settings and strips only SpecFact-managed entries when needed. | ||
| - [ ] 3.3 Route applicable init/setup artifact copy flows through the helper or explicit safe modes, including fail-safe handling for malformed structured files and backup creation for explicit replacement. | ||
| - [ ] 3.4 Implement the CI/static guard for protected user-project artifacts in init/setup code paths and integrate it into the relevant local/CI quality workflow. | ||
| - [x] 3.1 Implement the core safe-write helper and ownership model with `@beartype` and `@icontract` | ||
| on public APIs. | ||
| - [x] 3.2 Route `src/specfact_cli/utils/ide_setup.py` settings mutation through the helper so | ||
| `.vscode/settings.json` preserves unrelated user-managed settings and strips only SpecFact-managed | ||
| entries when needed. | ||
| - [x] 3.3 Route applicable init/setup artifact copy flows through the helper or explicit safe modes, | ||
| including fail-safe handling for malformed structured files and backup creation for explicit | ||
| replacement. *(VS Code settings path; `init ide` surfaces errors + `--force` + backup)* | ||
| - [x] 3.4 Implement the CI/static guard for protected user-project artifacts in init/setup code paths | ||
| and integrate it into the relevant local/CI quality workflow. *(`scripts/verify_safe_project_writes.py` | ||
| + `hatch run lint`)* | ||
|
|
||
| ## 4. Verification, docs, and cross-repo handoff | ||
|
|
||
| - [ ] 4.1 Re-run the targeted tests and any broader init/setup regression coverage, capture passing results, and update `TDD_EVIDENCE.md`. | ||
| - [ ] 4.2 Research and update affected docs (`README.md`, installation/quickstart/init references) to document preservation guarantees, backup behavior, and explicit replacement semantics. | ||
| - [ ] 4.3 Run quality gates: `hatch run format`, `hatch run type-check`, `hatch run lint`, `hatch run yaml-lint`, `hatch run contract-test`, and `hatch run smart-test`. | ||
| - [ ] 4.4 Run `hatch run ./scripts/verify-modules-signature.py --require-signature`; if any bundled module manifests changed, bump versions, re-sign as required, and re-run verification. | ||
| - [ ] 4.5 Ensure `.specfact/code-review.json` is fresh, remediate all findings, and record the final review command/timestamp in `TDD_EVIDENCE.md` or PR notes. | ||
| - [ ] 4.6 Apply the appropriate version/changelog update for a bugfix release if implementation changes user-facing behavior, then open a PR to `dev` referencing the paired modules change. | ||
| - [x] 4.1 Re-run the targeted tests and any broader init/setup regression coverage, capture passing | ||
| results, and update `TDD_EVIDENCE.md`. | ||
| - [x] 4.2 Research and update affected docs (`README.md`, installation/quickstart/init references) to | ||
| document preservation guarantees, backup behavior, and explicit replacement semantics. | ||
| *(installation.md + version pins in README / samples)* | ||
| - [x] 4.3 Run quality gates: `hatch run format`, `hatch run type-check`, `hatch run lint`, | ||
| `hatch run yaml-lint`, `hatch run contract-test`, `hatch run smart-test`, and `hatch test --cover -v`. | ||
| - [x] 4.4 Run `hatch run ./scripts/verify-modules-signature.py --require-signature`; if any bundled | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| module manifests changed, bump versions, re-sign as required, and re-run verification. *(no | ||
| `modules/init` payload change β error UX handled in `ide_setup` to avoid re-signing)* | ||
| - [x] 4.5 Ensure `.specfact/code-review.json` is fresh, remediate all findings, and record the final | ||
| review command/timestamp in `TDD_EVIDENCE.md` or PR notes. | ||
| - [x] 4.6 Apply the appropriate version/changelog update for a bugfix release if implementation changes | ||
| user-facing behavior, then open a PR to `dev` referencing the paired modules change. | ||
| *(version/changelog done; PR human)* | ||
| - [x] 4.7 Run strict OpenSpec validation before sign-off: | ||
| `openspec validate profile-04-safe-project-artifact-writes --strict`; fix any validation errors until | ||
| it passes; record the successful run command and timestamp in `TDD_EVIDENCE.md` (or PR notes). | ||
|
|
||
| ## 5. Worktree cleanup | ||
|
|
||
| - [ ] 5.1 Remove the worktree used for this change (for example `git worktree remove ../specfact-cli-worktrees/bugfix/profile-04-safe-project-artifact-writes`). | ||
| - [ ] 5.1 Remove the worktree used for this change (for example | ||
| `git worktree remove ../specfact-cli-worktrees/bugfix/profile-04-safe-project-artifact-writes`). | ||
| - [ ] 5.2 Delete the local branch after merge (`git branch -d bugfix/profile-04-safe-project-artifact-writes`). | ||
| - [ ] 5.3 Prune stale worktree metadata (`git worktree prune`). | ||
| - [ ] 5.4 Record cleanup completion in `TDD_EVIDENCE.md` alongside the 4.x verification notes. | ||
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.