feat(adcp): sync canonical agent skills from protocol tarball#275
Merged
feat(adcp): sync canonical agent skills from protocol tarball#275
Conversation
Extends scripts/sync_schemas.py to extract protocol-managed skills from the same bundle it already uses for schemas. Reads manifest.json to enumerate skills, copies each skill tree (excluding nested schemas/ subdirs), and snapshots previous versions as <name>.previous siblings. SDK-local skills not in the manifest are left untouched. - Add sync_skills_from_bundle() with path-traversal guard, str validation, and manifest-driven copy excluding schemas/ subdirs - Refactor replace_cache_from_bundle() to accept pre-extracted bundle_root (extract_bundle() now owns the TemporaryDirectory lifecycle) - Add --no-skills flag so check-schema-drift Makefile target skips skill sync when only checking schema drift - Add skills/*.previous to .gitignore (snapshot working copies) - Add 11 tests covering all discrete behaviors Closes #274 https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}
- Strengthen path-traversal guard: reject any name containing "/" or where name != Path(name).name (covers "good/../evil" bypass) - Add encoding="utf-8" to manifest.json read_text() - Move src.is_dir() check before dst manipulation to prevent data loss when a skill is listed in manifest but absent from the bundle - Add replace_cache_from_bundle tests and path_traversal_slash test - Add check-schema-drift to Makefile .PHONY https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}
Contributor
IPR Policy Agreement RequiredThank you for your contribution! Before we can accept your pull request, you must agree to our Intellectual Property Rights Policy. By making a Contribution, you agree that:
To agree, please comment below with the exact phrase: You can read the full IPR Policy here. I have read the IPR Policy You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Contributor
Author
|
I have read the IPR Policy |
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
2 participants
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.
Summary
Closes #274
Extends
scripts/sync_schemas.pyto extract protocol-managed skills from the same bundle it already uses for schemas. After adcontextprotocol/adcp#3097 merged, the tarball'smanifest.jsonenumerates seven skills (adcp-brand,adcp-creative,adcp-governance,adcp-media-buy,adcp-si,adcp-signals,call-adcp-agent). Without this sync, coding agents using the Python SDK see no skill content and fall back to training-data hallucinations of AdCP shapes.sync_skills_from_bundle(bundle_root, skills_dir)— readsmanifest.json, copies each listed skill directory (excluding embeddedschemas/subdirs), snapshots previous state as<name>.previous, leaves SDK-local skills untouched_extract_bundle()— ownsTemporaryDirectorylifecycle, shared between schema and skills extraction; cleans up correctly on extraction failurereplace_cache_from_bundle(bundle_root)— refactored to accept pre-extracted bundle root (wastgz_bytes + version)--no-skillsflag — skip skills sync; wired tocheck-schema-driftMakefile target so schema-only drift checks don't write toskills/skills/*.previousadded to.gitignoreschemas/exclusion, snapshot create/replace, local-only untouched, path traversal (../evilandgood/../evil), non-string name, missing bundle dir (with pre-existing dst preserved), multiple skillsWhat was tested
pytest tests/test_sync_schemas.py— 15 passedpytest tests/ --ignore=tests/integration --ignore=tests/conformance/signing/test_ip_pinned_transport.py) — 2173 passed, 0 failures from this change (one pre-existing network test fails onexample.comreturning 403 in the sandbox)ruff check scripts/sync_schemas.py— clean (scripts/ excluded per pyproject.toml)Pre-PR review
good/../evil, encoding onread_text); one nit surfaced in PR body (_extract_bundlecould be a@contextmanager— follow-up)Download latest schemasis stale — cannot fix,.github/**is off-limits per repo policy)Nits surfaced (not fixed — follow-up candidates)
_extract_bundlereturns an unmanagedTemporaryDirectory; a@contextmanagerwould be harder to misuseDownload latest schemasis now mildly stale (also syncs skills) — blocked by.github/**edit restrictionSession: https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}
Generated by Claude Code