fix(skills): restore upstream content and verify full trees - #12
Conversation
3metaJun
left a comment
There was a problem hiding this comment.
Reviewed last in the #10 → #11 → #12 order. Verified locally on Windows: npm test passes with 102 passing tests and the one expected Unix-only skip — now including the new skill-baseline --check gate in the default test script.
The baseline machinery holds up under its own mutation tests: inside() rejects path escapes, absolute paths, and symlinks component-by-component; refreshes refuse to bless deleted source-mapped targets; dirty or wrongly pinned source checkouts are rejected; stale move/omission rules error out; and the skill inventory must be exactly covered by the declared sources. Writing the manifest atomically under the existing upstream-sync lock is the right reuse, and CI checking out both upstreams at pinned full-SHA refs closes the loop. The docs are honest about the limit that matters: a matching digest proves no drift, not semantic equivalence or workflow success.
Non-blocking nits:
checkLocalBaselinebuilds the untracked-file set only fromskills/, so a brand-new untracked file under a moved target root such astools/meta-mode/is never flagged — tracked moved files are drift-checked, but untracked additions there are invisible. Consider walking the moved target roots too.- The
check-upstream.mjschange (canonical-manifest problems print in non-strict runs but only fail under--strict) is not a weakening in CI sincevalidate.ymlkeeps--strict, but it reads like one at first glance; a sentence in the commit message on why non-strict runs should warn-and-continue would help future readers. - First-run UX: plain
npm run skill-baselinewith no sources and no mode flag throws "Missing source checkout for pstack" rather than pointing at--help. The help text does document the requirement, so this is minor.
Merge note: landing last is load-bearing here — the manifest records the complete tree, so it must be refreshed against the combined result of #10 + #11 (rebase, re-review the changed skill bodies, then re-run with --write), as described in the PR description and docs/skill-integrity.md.
[gpt-6] RESPONDING ON BEHALF OF 3metajunVerified all three review notes against the checker and refresh workflow.
The branch is rebased after #10 and #11. Both test registrations are retained, and the final complete-tree baseline is refreshed against their combined instructions and the latest history helper. Focused tests also cover file-only move boundaries, dependency exclusion, symbolic links, and path escapes. |
65074fb to
b7c0e26
Compare
Several skill bodies had lost upstream rules and examples, and integrity checks covered only 47 entry documents. This restores the three Matt Pocock workflows and design references from their pinned originals, restores the full TypeScript examples with validated corrections, and adds the missing Notes recipes, mechanics reference, and optional interaction template. Harness-specific paths and available tools remain explicit adaptations.
A new manifest covers all 50 skill trees and their moved files: 134 targets from both pinned sources. Every source file maps to a target or a documented omission; only three upstream UI metadata files are omitted. The review tool prints source-to-adaptation diffs and updates the baseline only with
--write. CI checks both source checkouts, while artifact synchronization retains its existing transaction behavior.Validation: 102 tests passed with one expected Windows skip, including mutation checks for missing references, local additions, dirty or incorrect sources, and invalid mappings. Both upstream checks and package verification passed. Independent review found no blockers. Real pi and OpenCode runs loaded the restored diagnosis skill, reproduced an asynchronous cache failure, and identified its cause in disposable projects. Claude authentication and a local Codex Windows sandbox error blocked those two live runs.
Merge this after #10 and #11. Rebase and review the final source diff before refreshing the complete baseline so their intended skill edits are included. A matching digest proves unchanged content, not semantic equivalence or workflow success. npm publication remains deferred.
Agent: GPT-6 via Codex