[kartsan03]

Description

specify integration switch and specify integration use re-register installed presets for the target integration after its manifest is on disk, but they never re-hash the files those presets rewrite. When a preset overrides a core command, the rendered skill no longer matches its manifest hash. integration status then lists it as modified, and the next integration upgrade refuses without --force, although the user changed nothing. integration upgrade already handles this case with _resync_manifest_after_registration (#4696); switch and use don't call it.

When switch installs an integration that wasn't installed yet, its manifest is saved just before registration, so that path calls the helper the same way upgrade does.

use, and a switch to an integration that is already installed, reuse a manifest written earlier. The user may have edited tracked files since then, and re-hashing everything would record those edits as pristine, so a later upgrade would overwrite them without --force. These two paths now record which tracked files were unmodified before registration (_snapshot_manifest_before_registration), and the helper refreshes only those whose bytes registration changed (new unmodified_files argument). A file the user edited earlier keeps its old hash and is still reported as modified.

Both steps are best-effort like the existing helper: a failure prints a warning and doesn't abort the command, and a missing manifest is skipped.

This doesn't repair projects that already hit the bug. Their stale hash looks the same as a user edit, so use leaves it alone and upgrade still refuses. One specify integration upgrade <key> --force fixes it: the preset content is kept (upgrade re-registers presets), and later use/upgrade runs work. I checked this on a project from the main repro.

Repro with the real CLI, using the preset from the issue (overrides speckit.tasks):

specify init --here --integration claude --script sh --ignore-agent-tools
specify preset add --dev ../tasks-preset
specify integration switch codex --script sh     # or: integration install codex; preset add; integration use codex
specify integration upgrade codex --script sh

Step	main (222a973)	This PR
switch codex, then manifest hash of .agents/skills/speckit-tasks/SKILL.md	stale	matches the file
upgrade codex after switch	exit 1, "1 file(s) have been modified since installation"	exit 0
use codex, then manifest hash	stale	matches the file
upgrade codex after use	exit 1, same message	exit 0

Refs #5078

Testing

• Tested locally with uv run specify --help
• Ran existing tests with uv sync && uv run pytest
• Tested with a sample project (if applicable)

New tests, all through the CLI with a real preset:

• test_command_switch.py::test_switch_syncs_manifest_hash_for_preset_override[False|True]: new-target and installed-target switch. After the switch the hash matches the file and upgrade without --force exits 0.
• test_command_use.py::test_use_syncs_manifest_without_blessing_user_edits[None]: the same for use.
• test_command_switch.py::test_switch_installed_target_preserves_modified_manifest_hash[plan|tasks] and test_use_syncs_manifest_without_blessing_user_edits[plan|tasks]: the user edits a skill before switch/use, once one the preset doesn't touch (plan) and once one it rewrites (tasks). The edit keeps its old hash, check_modified() still reports it, and upgrade still refuses.

With the three source files reverted to main, the three sync tests fail with "manifest hash must match the preset-overridden skill (#5078)". The four edit tests pass on main too, since main never re-hashes. With only the "unmodified before registration" check removed, all four fail with "pre-existing edits must not be recorded as pristine".

Full suite on Linux, Python 3.13: 13174 passed, 435 skipped (uv sync --extra test, then pytest -n 3, 7 min). Ruff 0.15.0 (ruff check src tests) passes. I haven't run it on Windows.

AI Disclosure

• I did not use AI assistance for this contribution
• I did use AI assistance (fill in the disclosure below)

AI disclosure: Codex CLI (model: gpt-6.1-sol, reasoning effort xhigh, autonomous, offline sandbox) reproduced the three paths on main and wrote the fix and the tests. Claude Code (model: claude-opus-5-5, autonomous) reviewed and trimmed the change, reran the repro with the real CLI on main and the branch, ran the full suite and lint, and drafted this description. The commit carries Assisted-by: trailers for both.

[Copilot]

🟡 Changes recommended

Path probes outside the per-file OSError handler can abort refreshes, leaving other preset hashes stale and blocking later upgrades.

1 open issue

• Handle probe errors without aborting refresh · New

What changed in this PR

Updates Specify CLI’s integration switch and integration use to refresh manifest hashes after preset registration, addressing #5078 while keeping pre-existing user edits detectable.

Changes:

• Snapshot unmodified tracked files before reusing an installed manifest.
• Refresh hashes after registration on both switch paths and on use.
• Add CLI regression tests for preset overrides and user-edit detection.

Project tests were not rerun; verification used an isolated standard-library check.

AI disclosure: GitHub Copilot (model unknown, autonomous) generated review text and suggested fixes for the review requester.

File	Description
tests/​specify_cli/​integrations/​test_command_use.py	Tests hash refresh and preserved edit detection for use.
tests/​specify_cli/​integrations/​test_command_switch.py	Tests both switch paths and preserved edit detection.
tests/​specify_cli/​integrations/​conftest.py	Adds a reusable tasks-override preset fixture.
src/​specify_cli/​integrations/​command_use.py	Adds snapshot and selective manifest refresh.
src/​specify_cli/​integrations/​command_switch.py	Refreshes manifests after registration on both paths.
src/​specify_cli/​integrations/​_helpers.py	Adds snapshot support and selective hash synchronization.

_{Review effort: Max}

[mnriem]

Please address Copilot feedback

[kartsan03]

Addressed in 51ef812.

• The resync no longer calls check_modified() up front. Each file in unmodified_files is compared inside the per-file handler, so a probe error on one file prints a warning and the other files are still refreshed.
• test_resync_manifest_probe_error_does_not_abort_remaining_files now also runs with unmodified_files. That case fails on the previous head.
• test_resync_with_snapshot_skips_files_registration_left_unchanged covers the check that skips files registration didn't change, since record_existing() clears the recovered marker. It fails if that check is removed.

Full suite on Linux, Python 3.13: 13176 passed, 435 skipped. Ruff 0.15.0 passes.

Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous).

[Copilot]

🟡 Changes recommended

Resync can read files outside the project through unsafe manifest paths and clear recovered-file markers, allowing later teardown to delete user-owned dispatchers.

1 open issue

• Exclude invalid manifest paths from snapshot hashing · New

1 previously missed issue

In code that hasn't changed since last review

Preserve recovered marker for unchanged new-target files

src/​specify_cli/​integrations/​command_switch.py:405

If a new target's setup records a pre-existing dispatcher with recovered=True, this resync re-records the unchanged file and clears its recovered marker. A later teardown can then delete the user-owned dispatcher, contrary to design/integration.md (“Ownership and lifecycle”). Pass unmodified_files=set(manifest.files) here to enable the unchanged-bytes guard, and cover the new-target path in the recovered-file regression test.

1 resolved since last review

• Handle probe errors without aborting refresh

_{Review effort: Max}

[mnriem]

Please address Copilot feedback

[kartsan03]

Addressed in 00fc9d6.

The snapshot now keeps only keys that pass _validate_record_path(), the same path check record_existing() runs. Absolute and .. keys are left out, and so are keys under a symlinked directory, so the resync never hashes a file outside the project.

test_snapshot_skips_manifest_paths_outside_the_project[dotdot|symlinked-parent] covers both cases, and both fail on the previous head. With only an absolute/.. check, the symlinked-parent case still fails.

Full suite on Linux, Python 3.13: 13178 passed, 435 skipped. Ruff 0.15.0 passes.

Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous).

[Copilot]

🔵 Needs a closer look

Tracked files are hashed before path validation during use and installed-target switch, allowing symlinked parent paths to trigger reads outside the project.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Validate paths before hashing tracked files

src/​specify_cli/​integrations/​_helpers.py:503

During use and installed-target switch, check_modified() hashes tracked files before path validation, so a symlinked parent such as outside-link/secret.md causes a read outside the project. Validate each entry before hashing it, and extend the symlink-parent test to assert that rejected paths never reach _sha256.

AI-generated by GitHub Copilot (model: unknown; autonomous review) on behalf of the review requester.

1 resolved since last review

• Exclude invalid manifest paths from snapshot hashing

_{Review effort: Max}

[kartsan03]

Addressed both findings in 8545dfa.

• Paths are now checked before hashing. The snapshot no longer calls check_modified(). It runs _validate_record_path() on each key first, then applies the same rules one file at a time: a deleted file counts as unmodified, and an edited or unreadable one as modified. The resync also checks the path before its changed-bytes hash. test_snapshot_skips_manifest_paths_outside_the_project now records every _sha256() call and asserts the outside file is never hashed, by the snapshot or by the resync. test_snapshot_follows_check_modified_rules_one_file_at_a_time covers the in-project rules.
• The new-target switch now passes unmodified_files=set(manifest.files). Its manifest is saved just before registration, so every tracked file is unmodified at that point. test_switch_new_target_keeps_recovered_markers marks a file as recovered during setup and checks that the marker survives the switch. No integration setup records a recovered file today (shared infra records them in the speckit manifest), so the test adds one through a wrapped setup().

All four new or extended test cases fail on 00fc9d6, and each new check has a test that fails when the check is removed.

Full suite on Linux, Python 3.13: 13180 passed, 435 skipped. Ruff 0.15.0 passes.

Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous).

[Copilot]

🟡 Changes recommended

Snapshot probe or read errors are silently skipped without per-file warnings, so subsequent registration changes can leave stale hashes that block the next upgrade despite a reported success.

1 open issue

• Warn when snapshot files are excluded after probe or read errors · New

_{Review effort: Max}

[mnriem]

Please address Copilot feedback. If not applicable, explain why

[kartsan03]

Addressed in a178d2e.

The snapshot now prints a warning for each file it leaves out after a path check, probe or read error. The warning names the file and says its hash won't be refreshed after registration. The file still stays out of the refresh, so an unknown user edit isn't recorded as pristine. Paths outside the project get the same warning, which matches what the resync already prints for them.

test_snapshot_follows_check_modified_rules_one_file_at_a_time now checks that the unreadable file is named in a warning and the edited file isn't. test_snapshot_skips_manifest_paths_outside_the_project checks that the escaping key is named. All three cases fail on 8545dfa.

Full suite on Linux, Python 3.13: 13180 passed, 435 skipped. Ruff 0.15.0 passes.

Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous).

[Copilot]

🔵 Needs a closer look

Human assessment is needed to judge the regression risk from untested missing or unreadable target manifests in use and installed-target switch.

No open issues 🎉

1 resolved since last review

• Warn when snapshot files are excluded after probe or read errors

_{Review effort: Max}

[kartsan03]

Covered the case from the last review in 2d6351c: use and installed-target switch when the target's manifest is missing or unreadable.

test_use_without_readable_manifest_skips_hash_refresh and test_switch_installed_target_without_readable_manifest_skips_hash_refresh delete or corrupt codex.manifest.json, then run the command. In both cases the command exits 0, sets the default, and still registers the preset. A missing manifest stays missing and no warning is printed. A corrupt one is left as is, and a warning names it.

I also ran these tests against the merge base. Everything matches except the new warning, so these commands behave as before when the target manifest is missing or corrupt. Removing the manifest is not None check, or either except branch in the snapshot, fails at least one case.

Full suite on Linux, Python 3.13: 13184 passed, 435 skipped. Ruff 0.15.0 passes.

Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous).

[Copilot]

🔵 Needs a closer look

Manifest-hash rewrites affect protection of user edits, but uninstall fallback, init --force, and switch rollback still skip them, requiring maintainer confirmation of the data-safety design.

No open issues 🎉

_{Review effort: Max}

[kartsan03]

The review on 2d6351c has no open findings. Its summary mentions init --force, the uninstall fallback and the switch rollback, which re-run registration without refreshing manifest hashes. That code is unchanged from main and outside this PR. #5078 is about switch, and this PR also fixes use, which had the same problem.

Those three paths share one cause, so I can fix them together in a follow-up PR with the same kind of tests, if you'd like.

Drafted on behalf of @kartsan03 by Claude Code (model: claude-opus-5-5, autonomous).

[mnriem]

Thank you!