Skip to content

feat(skills): add pr-review skill; copy verbatim when sources scan clean - #134

Merged
mattnorris merged 2 commits into
mainfrom
skills-sync
Jul 31, 2026
Merged

mattnorris merged 2 commits into
mainfrom
skills-sync

Conversation

@mattnorris

@mattnorris mattnorris commented Jul 31, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Two commits.

feat(skills) — adds skills/ess/pr-review (SKILL.md, 6 reference docs, 9 scripts) and brings the four existing skills back to parity with their source tree:

  • pr-address-comments now points at the pr-review skill instead of a stale /review-pr command, a reference that resolves now that pr-review ships here.
  • summarize-change-log example corrected to langsmith-client deploy docker.
  • summarize-change-log _REPO_ROOT was parents[5], carried over from an earlier .cursor/skills/mn/ path one level deeper. At skills/ess/<skill>/scripts/ the repo root is parents[4], so the old value resolved above the repo. Latent rather than breaking, since the tests never exercise that lookup.

Also replaces the 13-line appendix stub in LICENSE, at the root and in every skill, with the full Apache-2.0 text that §4(a) requires when redistributing, and moves copyright attribution to a root NOTICE per §4(d).

perf(essentials-sync) — Phase B handed every sync to the primary agent, including sources the scanners had already cleared. With no jargon to generalize the agent paraphrases working prose instead: on this very skill directory it rewrote all 16 files and 187 lines, replacing a live sibling-skill reference and a named tool with vague descriptions, and swapping one table row for unrelated content.

Now, when --no-extract is set and the source pre-scan is empty, the tree is copied verbatim. The copy still faces the deterministic scanners and the adversarial reviewer, and escalates to the agent if either raises anything critical, so the safety bar is unchanged — the reviewer was always a detector; only the primary agent rewrote, and it did so unconditionally. Same run before and after: 11min with 187 changed lines, versus 27s with zero drift.

The copy honors .gitignore and the scanners' skip list. That is a safety property rather than an optimization: the scanners never read gitignored paths, so copying them would ship content nothing vouched for, .env being the obvious hazard. Files copied but unreadable by the text scanners (binaries, anything past the size cap) are reported at the end of a run, and target-only files such as LICENSE/NOTICE are left alone so a re-sync cannot clobber them. --no-fast-copy forces the agent path.

Test plan

  • diff -rq between the source tree and skills/ess/ reports only the intentional target-only LICENSE files
  • 24 skill tests pass (test_report.py 14, test_validate_summary.py 10); all pr-review shell scripts pass bash -n
  • Executable bits preserved through the copy and recorded as 100755 in git
  • Deterministic scanners report zero findings over skills/ess/
  • No __pycache__ or .pyc committed
  • All five LICENSE files byte-identical to canonical Apache-2.0 (md5 3b83ef96387f14655fc854ddc3c6bd57)
  • essentials-sync suite 21 -> 33 tests; tsc --noEmit clean
  • New tests cover the copy refusing gitignored files, preserving exec bits, leaving target-only files alone, reporting unscannable files and symlinks, plus all five fast-path eligibility branches
  • End-to-end --dry-run on pr-review produces output byte-identical to the source with matching exec bits

Add skills/ess/pr-review (SKILL.md, 6 reference docs, 9 scripts) and bring the
four existing skills back to parity with their source tree:

- pr-address-comments now points at the pr-review skill rather than a stale
  /review-pr command, a reference that resolves now that pr-review ships here
- summarize-change-log: correct the example to `langsmith-client deploy docker`
- summarize-change-log: _REPO_ROOT was parents[5], carried over from an earlier
  .cursor/skills/mn/ path one level deeper. At skills/ess/<skill>/scripts/ the
  repo root is parents[4], so the old value resolved above the repo. Latent
  rather than breaking, since the tests never exercise that lookup.

Also replace the 13-line appendix stub in LICENSE, at the root and in every
skill, with the full Apache-2.0 text that section 4(a) requires when
redistributing, and move copyright attribution to a root NOTICE per 4(d).
Phase B handed every sync to the primary agent, including sources the scanners
had already cleared. With no jargon to generalize the agent paraphrases working
prose instead: on a 16-file skill directory it rewrote all 16 files and 187
lines, replacing a live sibling-skill reference and a named tool with vague
descriptions, and swapping one table row for unrelated content.

Skip the agent when --no-extract is set and the source pre-scan is empty, and
copy the tree instead. The copy still faces the deterministic scanners and the
adversarial reviewer, and escalates to the agent if either raises anything
critical, so the safety bar is unchanged. The reviewer was always a detector;
only the primary agent rewrote, and it did so unconditionally. Same run before
and after: 11min with 187 changed lines, versus 27s with zero drift.

The copy honors .gitignore and the scanners' skip list, which is a safety
property rather than an optimization: the scanners never read gitignored paths,
so copying them would ship content nothing vouched for, .env being the obvious
hazard. Files copied but unreadable by the text scanners, binaries and anything
past the size cap, are reported at the end of the run. Target-only files such as
LICENSE and NOTICE are left alone so a re-sync cannot clobber them.

Add --no-fast-copy to force the agent path.
@mattnorris
mattnorris merged commit 4f50765 into main Jul 31, 2026
2 checks passed
@mattnorris
mattnorris deleted the skills-sync branch July 31, 2026 19:20
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.

1 participant