fix(install): don't destroy local changes on sync, and manage hooks/ - #111
fix(install): don't destroy local changes on sync, and manage hooks/#111MajorLift wants to merge 3 commits into
Conversation
tools/install copies source over the installed tree with `rm -rf` followed by `cp -R`, so an edit made in the installed copy is destroyed on the next sync with no warning and no record. Comparing source against the install cannot tell the two cases apart, because both make them differ: the install was edited locally, or the source moved ahead. A guard that refuses on any difference breaks every routine update, which is a good way to get itself disabled. So each install now records a content hash of what it wrote, and a later run compares the install against that record. Matching means nothing local happened, and the copy proceeds no matter how far source has moved. Bundles with no record predate the manifest and fall back to comparing against source. That is conservative — it refuses a legitimate update once, until a manifest exists — on the grounds that a one-time re-run beats a silent delete. Also adds hooks/ to BUNDLE_DIRS and to the directories tools/install copies. It was in neither, so a skill shipping a hook had it installed by hand and tracked nowhere. That is how the evidence skill's wired PreToolUse gate — the one control that fires on every publish — came to have no source of record. The existing lint-skill-entry test asserts these two lists agree, and caught the omission when only one was updated. Controls, against a temp target and temp HOME: clean install exit 0 re-install, nothing touched exit 0 source moves ahead exit 0, change propagates local edit to the install exit 1, REFUSED, sha unchanged same with --force exit 0, edit discarded `node --test test/*.test.mjs` 61/61.
`description` was a plain scalar containing ": ", which YAML reads as a nested mapping, so the frontmatter did not parse. Any tool reading source frontmatter with a YAML parser fails on it; tools/install masked the defect by folding the value to a block scalar on the way out, so the installed copy parsed and the source did not. Folded to a block scalar at rest. The description text is unchanged.
`hooks/` was added here to `BUNDLE_DIRS` and to the installer's copy list, but `jongsun/add/pr-validate-falsifying-test` already does both in `edf65a3`, along with `tools/check-bundle-contract.sh` to assert the two lists agree. Two branches making the same change is how one of them ends up reverted by a merge resolution nobody reads. So this branch keeps only the part that is its own: refusing to overwrite an installed copy that has local changes. The guard iterates whatever `BUNDLE_DIRS` holds, so it covers `hooks/` for free once that branch lands. Dropping it also removes a defect this branch would otherwise carry into main. `check-bundle-contract.sh` reads the array with a single-line `sed`, and the multi-line form here made it fail to PARSE rather than fail a comparison — it exited 2 reporting that it could not read the declaration, which is a check that has stopped checking rather than one that found something. Controls re-run after narrowing, since the guard now iterates a different set: clean install 0, no-op re-install 0, source moves ahead 0 and propagates, local edit 1 with the sha unchanged, `--force` 0 and discards. 61/61.
|
Narrowed in Why
So this keeps only what is its own: refusing to overwrite an installed copy that has local changes. The guard iterates whatever It was also carrying a defect into main
Found by merging both into the trial branch and running the contract check there, which is the only place they currently coexist. The merge confirmed the concern was realRe-merging the narrowed branch into trial silently removed That is exactly the failure the contract check was written for, and exactly the failure the "two branches, one change" note above is about. Resolved on trial by keeping Controls re-run after narrowingThe guard now iterates a different set of directories, so the previous run doesn't transfer:
|
1 participant
What
tools/installcopies source over the installed tree withrm -rfthencp -R. An edit made in the installed copy is destroyed on the next sync, with no warning and no record.Why the obvious guard doesn't work
Comparing source against the install cannot tell two cases apart, because both make them differ:
A guard that refuses on any difference breaks every routine update, which is a good way to get itself disabled.
What this does instead
Each install records a content hash of what it wrote, to
.mms-install-manifestin the installed skill dir. A later run compares the install against that record. Matching means nothing local happened, so the copy proceeds no matter how far source has moved.Bundles with no record predate the manifest and fall back to comparing against source — conservative, refusing a legitimate update once until a manifest exists, on the grounds that a one-time re-run beats a silent delete.
--forcerestores the previous behaviour.Also:
hooks/was unmanagedhooks/was in neitherBUNDLE_DIRSnor the directoriestools/installcopies. A skill shipping a hook had it installed by hand and tracked nowhere — which is how the evidence skill's wiredPreToolUsegate, the one control that fires on every publish, came to have no source of record.The existing
lint-skill-entrytest asserts those two lists agree, and caught the omission when only one was updated.Verification
Against a temp target and temp
HOME:REFUSED, sha unchanged--forcenode --test test/*.test.mjs— 61/61.Unrelated fix carried along
domains/performance/skills/performance/skill.mdhad adescriptionthat was a plain YAML scalar containing": ", so the frontmatter did not parse.tools/installmasked it by folding the value to a block scalar on the way out, so the installed copy parsed and the source did not. Folded at rest; text unchanged.