feat(skills): vendor shared skills read-only with explicit Pi overlays - #271
Open
Alan-TheGentleman wants to merge 11 commits into
Open
feat(skills): vendor shared skills read-only with explicit Pi overlays#271Alan-TheGentleman wants to merge 11 commits into
Alan-TheGentleman wants to merge 11 commits into
Conversation
…n anchored overlay
Add scripts/build-skill-overlays.mjs (--write/--check), which produces
skills/<name>/SKILL.md by applying a hand-authored, ordered overlay.md of
anchored [anchor]/[replace] blocks on top of a vendored provider body
(skills/_vendor/<name>/SKILL.md). Two failures fail loudly rather than
silently: a hand-edited vendored file is caught by a digest drift gate
against skills/_vendor/manifest.json, and an overlay anchor that no longer
matches the vendored body exactly once (upstream reworded or dropped it)
fails with an actionable message pointing at the overlay, never the
vendored file.
The admitted-skill set starts empty (manifest.skills: {}); each candidate
is admitted only after its own per-file portability comparison, added one
at a time in the commits that follow.
Per-file portability comparison against gentle-ai's comment-writer skill
found exactly one deliberate Pi delta: an added sentence in the "Match
target context language" rule ("Do not use the active persona as the
source of truth for public comments"), no repository identity encoded.
Admits the vendored body verbatim with one anchored overlay block
restoring that sentence.
… copy Per-file portability comparison found gentle-pi's checked-in copy was missing content gentle-ai already had: three Work Unit Checklist items (focused test command, runtime harness, rollback boundary) and the "implementation evidence MUST include" paragraph. No repository identity encoded, and this repo already requires the same evidence independently via skills/sdd-apply's Work Unit Evidence hard gate, so gentle-pi was simply stale. Admits the vendored (fuller) body verbatim -- zero overlay blocks, since Pi has nothing project-specific to preserve on top of it.
Per-file portability comparison against gentle-ai's branch-pr skill found only the frontmatter name differs (unprefixed "branch-pr" upstream vs the prefixed "gentle-ai-branch-pr" gentle-pi's own loader already requires, per tests/skill-collision-prefixes.test.ts) -- no repository identity encoded in the workflow, PR body, or commit-convention content. Admits the vendored body with one anchored overlay block re-applying the existing collision-prefix fix.
Same result and rationale as branch-pr: only the frontmatter name differs (unprefixed upstream vs gentle-pi's required "gentle-ai-chained-pr"), no repository identity encoded elsewhere. Admits the vendored body with one anchored overlay block re-applying the collision-prefix fix.
Per-file portability comparison found this candidate byte-identical to gentle-pi's checked-in copy -- no delta, no repository identity encoded. Admits the vendored body verbatim with zero overlay blocks, completing the five first-tier admissions (task 6.4).
Per-file portability comparison against gentle-ai's skill-improver skill found seven bounded, nameable deltas (17 of 54 lines): the collision-prefix name fix, plus gentle-pi's simplified skill-style-guide fallback chain, .atl/skill-registry.md availability wording, and the /skill-registry:refresh slash-command convention in place of gentle-ai's own "gentle-ai skill-registry refresh" CLI verb. None encode repository identity, and every delta is a self-contained sentence/bullet -- genuinely overlay-representable, unlike judgment-day/skill-creator (see their rejection in scripts/build-skill-overlays.mjs REJECTED_CANDIDATES). Admits the vendored body with those seven anchored overlay blocks.
Per-file portability comparison found this second-tier candidate byte-identical to gentle-pi's checked-in copy -- no delta, no repository identity encoded. Admits the vendored body verbatim with zero overlay blocks, completing task 6.5's two passing admissions (skill-improver and skill-registry); judgment-day and skill-creator failed the comparison and stay out (documented in scripts/build-skill-overlays.mjs REJECTED_CANDIDATES).
Add the offline node scripts/build-skill-overlays.mjs --check step to the per-PR gate, alongside the existing mirror/lock and baselines checks.
Tick tasks 6.1-6.8 in openspec/changes/consume-gentle-ai-release-artifacts/tasks.md.
Append the P3b section to apply-progress.md: per-skill portability comparison results (5/5 first-tier admitted, 2/4 second-tier admitted, judgment-day and skill-creator rejected as too extensive for an anchored overlay), the overlay mechanism, TDD/Work Unit evidence, deviations, and rollback boundary.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Sixth slice of the consumer chain, stacked on #270.
The comparison is the gate, not the allowlist
Nine candidates were compared file by file. Seven admitted, two rejected.
Admitted first tier, all five:
comment-writer(one persona-safety sentence as the Pi delta),work-unit-commits(Pi was stale — gentle-ai already had content Pi lacked, so this catches Pi up),branch-prandchained-pr(only the pre-existing collision-prefixname:field differs),cognitive-doc-design(byte-identical).Admitted second tier, two of four:
skill-improver(seven bounded, nameable deltas) andskill-registry(byte-identical).Rejected:
judgment-dayandskill-creator. Both diverged into near-total rewrites. Expressing that as an overlay would need a single anchor spanning almost the entire body, which is functionally the full-fork shape the overlay design exists to avoid. No repository name leaks in either — they are simply too far apart to pretend otherwise.issue-creationis excluded entirely: no vendoring, no gate, no comparison. The two copies describe different repositories by design. That is not drift to reconcile.The failure this actually prevents
A hand-edited vendored file is caught by the drift gate. But the one that matters under a fast provider cadence is the stale anchor: upstream rewords a section, the Pi delta's anchor no longer matches, and the delta silently disappears from the skill agents load.
That fails loudly with
edit the overlay, not the vendored file: skills/_vendor/<name>/overlay.md. A missing anchor and an ambiguous one fail identically, because the remediation is the same.One deviation you should know about
The release archive contains no skill files. That is by design — the first published archive carries contracts, snapshot and docs only, per the decision to keep the frozen surface small.
So the vendored bodies here were seeded as manually cross-checked byte-for-byte copies from the gentle-ai checkout, not from a live sync run. Re-syncing an admitted skill needs a human re-copy until a git-based skill sync is wired. Flagged as a follow-up rather than presented as automated.
Tests
15 new, all green.
--checkclean across all seven skills. Full suite 1117 pass, 1 fail — the environmental no-network failure documented since #267.Rollback
Each admission is its own commit, and the generator iterates the manifest rather than a hardcoded list, so reverting one removes only that skill. Reverting the whole PR restores every
SKILL.mdand deletes the generator, vendor tree and tests. Nothing from the earlier slices is touched.