Skip to content

fix(skill): preserve custom installed instructions - #96

Open
NianJiuZst wants to merge 2 commits into
Tencent:mainfrom
NianJiuZst:codex/preserve-custom-skill-source
Open

fix(skill): preserve custom installed instructions#96
NianJiuZst wants to merge 2 commits into
Tencent:mainfrom
NianJiuZst:codex/preserve-custom-skill-source

Conversation

@NianJiuZst

Copy link
Copy Markdown
Contributor

Summary

  • record whether an installed SKILL.md came from the bundled source or --source
  • hash managed bundled content so automatic sync only updates an untouched managed copy
  • preserve custom, manually edited, and unknown marker-less installations
  • expose preserved installations through daemon logs and bsk doctor

Why

bsk install-skill --source ... installs user-owned instructions, but daemon startup currently rewrites every differing SKILL.md with the bundled copy. Starting a session can therefore silently destroy custom instructions.

The sidecar marker is written fail-safe: custom ownership is recorded before content replacement, and bundled markers include the exact installed-content hash. A marker-less file is only adopted automatically when it exactly matches the current bundle.

Tests

  • cargo test -p bsk --locked
  • cargo clippy -p bsk --all-targets --locked -- -D warnings

@NianJiuZst
NianJiuZst marked this pull request as ready for review August 17, 2026 07:20
@iuyo5678

Copy link
Copy Markdown
Collaborator

A very good PR, but we need to fix the bug where users who haven't upgraded to the latest version will never be able to upgrade automatically again, the cleanest solution is to prepare a historical fingerprint for the "no marker" case

Before .bsk-source markers existed, any user who installed via the
bundled bsk CLI had a plain SKILL.md on disk. The new marker-aware
sync then refused to overwrite that file as long as its content
differed from the current bundle, so users were silently frozen on
their first-ever install until they manually reran bsk install-skill.

Recognise legacy bundled installs by hashing their on-disk content and
comparing against the SHA-256 digests of every SKILL.md that bsk ever
shipped. When the on-disk content matches the current bundle, stamp a
bundled marker and report UpToDate. When it matches a historical
fingerprint, atomically rewrite to the current bundle, stamp a fresh
bundled marker, and report Updated. Anything else stays Preserved as
before.
@NianJiuZst
NianJiuZst force-pushed the codex/preserve-custom-skill-source branch from a3eaabc to 25af5c5 Compare August 20, 2026 05:53

@NianJiuZst NianJiuZst left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the catch. You're right — anyone who installed before .bsk-source markers existed would be frozen on their first-ever bundled SKILL.md because the new marker-aware sync refused to overwrite a differing file. Now fixed.

What changed in this update:

  • Added HISTORICAL_BUNDLED_FINGERPRINTS: &[&str] in crates/bsk-cli/src/skill_install/sync.rs containing the SHA-256 hex digests of every SKILL.md ever shipped in this repo (initial public release through feat(session): support unfocused agent windows --no-focus #87, i.e. the same content as the current bundled SKILL.md).
  • Extended the marker-less branch of sync_one: when there's no marker, hash the on-disk content and adopt as managed if it matches either the current bundle or any historical fingerprint. Matching the current bundle stamps a bundled marker and reports UpToDate; matching a historical fingerprint atomically rewrites to the current bundle, stamps a fresh bundled marker, and reports Updated. Anything else still goes to Preserved (unrecognised manual edits remain untouched).
  • Refactored sync_one to share the atomic-replace + marker-refresh path between stale-managed upgrades and legacy-bundled adoptions (extracted into a small atomic_replace closure).
  • Updated SyncReport::preserved's docstring accordingly.
  • New unit test sync_adopts_markerless_legacy_bundle_and_upgrades_it writes the real initial-release SKILL.md content (sha256 28d39215...d4a3e1bc5a, exactly matching the list entry) without a marker, syncs against a new bundle, and asserts the file was upgraded and a fresh bundled marker was stamped. The existing sync_preserves_custom_edited_and_unknown_skills, sync_adopts_markerless_current_bundle_then_updates_it, sync_updates_outdated_skill, etc. all still pass.

New head: 25af5c5 (rebased onto current main@6b78211).

Verified locally after the rebase:

  • cargo test -p bsk --locked --lib skill_install::sync — all 7 tests pass, including the new one.
  • cargo clippy -p bsk --all-targets --locked -- -D warnings — clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants