Phase D: skill manifest dependencies + consistency test + host-conditional install gate#156
Merged
Conversation
…ST-001) Define the draft-2020-12 sub-block schema validating a skill's optional requires-env/requires-pip/requires-cmd/requires-skills declarations, with additionalProperties:false to catch typos. Expose SKILL_REQUIREMENTS_KEYS as the single derivable authority so the upcoming consistency test and pre-install check derive the field list rather than hardcoding it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-002) Add the requires-cmd declaration to the single-source skill-rules.json.jinja for map-state (its scripts invoke git); render propagates to both generated trees. The other 13 skills ship no scripts and declare no requirements. make check-render green; tests/test_skills.py (223) pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New tests/test_skills_consistency.py scans each shipped skill's scripts (*.sh via a conservative POSIX-builtin-aware scanner, *.py via AST) and asserts the declared requires-* (skill-rules.json) is a superset of the scanner-detected deps, validates each requires-* sub-block against SKILL_REQUIREMENTS_SCHEMA, and rejects dangling requires-skills targets. Green on all 14 skills (map-state: git detected + declared). Includes a teeth self-test proving under-declaration fails, plus scanner-edge unit tests (getenv-default, comments, builtins, awk programs, heredoc bodies, apostrophes). SCANNABLE_KEYS derives from SKILL_REQUIREMENTS_KEYS (INV-2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…004) Gate each shipped skill on its declared requires-* before install: skip (and print [skipped: <skill>: missing <kind> <name>]) when a required cmd (shutil.which), pip module (find_spec), or env var NAME (os.environ) is absent on the host. requires-skills is warning-only, never a skip. The gate lives inside create_skill_files so both `mapify init` (providers.py) and `upgrade` (__init__.py) are guarded by one site; happy-path install is unchanged. A module-level assertion enforces _REQUIRES_CHECKER covers every blocking key derived from SKILL_REQUIREMENTS_KEYS, so a new schema key fails loudly at import rather than going silently unchecked. Security: env check reads variable NAMES only — never values, never .env files. Also fixes a latent mypy error on SKILL_REQUIREMENTS_KEYS (tuple of a dict subscript) surfaced by `make check`. Adds tests/test_file_copier.py covering missing-dep skip, deps-present identity, upgrade-path guard, and the name-only env security boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Correctness/integrity: - Scanner (test_skills_consistency): detect here-doc openers AFTER comment + quote stripping with a quote-parity guard, so a `<<WORD` token in a comment or string no longer falsely opens a here-doc and swallows the rest of the file — which would have made the consistency check vacuous (real undeclared commands after such a line going undetected). Quoted/indented delimiters (<<'EOF', <<"EOF", <<-EOF) still register. Adds regression tests for both directions. - create_skill_files: prune skipped skills from the INSTALLED skill-rules.json so the catalog never advertises an absent skill (no dangling trigger entry on a host missing a declared dependency). On a fully-satisfied host nothing is pruned, so the catalog stays byte-identical. - codex_copier: apply the same requires-* gate as the Claude provider via the shared helpers, so the contract is enforced identically across providers (no-op today — the Codex tree ships no skill-rules.json — but no future divergence). Robustness/cleanup: - Replace the import-time `assert` enforcing checker/schema parity with an explicit `raise RuntimeError`, so the invariant is not stripped under `python -O`. - _extract_requires_block: validate each requires-* sub-block against SKILL_REQUIREMENTS_SCHEMA and surface malformed entries as a warning instead of silently disabling the gate; treat a non-dict entry as no-requirements (no crash). - _check_requires_cmd: mirror check_tool()'s Claude-CLI special case (~/.claude/local/claude) so requires-cmd:["claude"] is not wrongly skipped. - Source stdlib module names from sys.stdlib_module_names instead of a hand- maintained 168-entry frozenset (no drift on Python upgrades). - test_vc2 happy-path: force all checkers present so the identity test is host-independent (no longer fails on a CI image without git). make check green (1893 passed); verified in-process: git-less host installs 13 skills with catalog==dirs, Codex unaffected, module imports under -O. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…el path) templates_src/**/*.jinja already lands in the wheel under mapify_cli/templates_src via packages=["src/mapify_cli"] plus the global [tool.hatch.build] include/artifacts patterns. The force-include added the same files a second time at the identical archive path; newer hatchling rejects this with "A second file is being added to the wheel archive at the same path: mapify_cli/templates_src/CLAUDE.md.jinja" (it previously deduped silently, which is why main was green). Removing the force-include fixes the build; verified the wheel still contains all 95 templates_src .jinja files and the generated templates/ tree, with no duplicates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What & why
Phase D of the roadmap: give every shipped MAP skill a machine-checked declaration of its runtime dependencies (
requires-env/requires-pip/requires-cmd/requires-skills) inskill-rules.json, add a consistency test that fails on any undeclared script dependency, and makemapify init/upgradeskip a skill (with a visible[skipped: …]message) when a declared dependency is genuinely absent on the host. Builds on Phase C (#155, single-source Jinja render).Changes
SKILL_REQUIREMENTS_SCHEMA+ derivedSKILL_REQUIREMENTS_KEYSinschemas.py(draft 2020-12,additionalProperties:false) — single authority for therequires-*field listmap-state→requires-cmd:["git"]in the single-sourceskill-rules.json.jinja, rendered to all generated treestests/test_skills_consistency.py— scans skill scripts (*.shregex,*.pyAST), asserts declaredrequires-*⊇ scanner-detected, validates each sub-block vs the schema, rejects danglingrequires-skills; includes a teeth self-testcreate_skill_files(one site guards both init and upgrade); env check reads variable names only (never values, never.env);requires-skillsis warning-onlyCode-review fixes (final commit)
A high-effort review pass surfaced 10 findings, all addressed:
<<WORDin a comment/string can't swallow lines and hide an undeclared command.skill-rules.jsonso the catalog never advertises an absent skill.-Osafety: the import-time invariant uses an explicitraise, notassert._check_requires_cmdmirrors the Claude-CLI special case; stdlib names come fromsys.stdlib_module_names(no drift); the happy-path identity test is now host-independent (won't fail on a git-less CI image).Verification
make checkgreen: 1893 passed, ruff + mypy + pyright clean (0/0/0),check-renderbyte-identical.python -O..jinjasource hand-edited; generated trees produced bymake render-templates.🤖 Generated with Claude Code