Repository navigation
Skills trial: 26 pinned skills, portable installer, status check and invoke-rate monitoring (all hosts) - #301
Merged
seathatflowsinourveins merged 15 commits intoSep 26, 2026
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
seathatflowsinourveins
enabled auto-merge (squash)
September 25, 2026 23:19
…th audits, gaps and listing states Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
tools/adoption/install_skills.py installs adoption/skills/manifest.json's pinned skills through the upstream `skills` CLI: idempotent skip when a skill's canonical SKILL.md hash and lock skillFolderHash already match the manifest, refusal of an unlocked local modification unless --force, and a verify-after-add with automatic rollback (skills remove) on a hash/tree mismatch. Lock-path resolution honors $XDG_STATE_HOME exactly like the upstream CLI. Adds --dry-run, --only, --print-codex-config and --json. tests/test_install_skills.py exercises it against a fake 'skills' executable (emulating add/remove/--version from a fixture map) for dry-run, idempotent skip, fresh install+verify, tree/sha mismatch rollback, local-modified refusal (with and without --force), the XDG_STATE_HOME lock path, version refusal, --only and --print-codex-config. tests/test_skills_manifest.py checks the manifest's schema/shape, per-skill hash/URL formatting, budget arithmetic, and cross-file consistency against adoption/templates/claude.settings.template.json's new skillOverrides block and the three shared kept-skill pins in catalogs/landscape/native-practice.json. adoption/templates/claude.settings.template.json gets a top-level skillOverrides object mapping every manifest skill to its claude_listing (including "on" explicitly, so a later demotion is a state change, not an add). Confirmed apply_claude_settings.py deep-merges an unrecognized nested object key wholesale and render_config.py still renders/round-trips the template; the existing render/apply/install-profile and test_adoption* suites (383 tests total) pass unchanged against it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…no SKILL.md classify_skill() used skill_md.is_file() as the only proxy for "does the unlocked folder already exist". When no lock entry exists and the canonical folder exists but has no SKILL.md (e.g. a partial install or an unrelated file dropped there by hand), that check was False and execution fell through to the unconditional 'install' return -- the same status as a genuinely empty/nonexistent folder. process_skill() then ran `skills add` unmodified, which recreates the target directory from scratch, silently discarding whatever local content was actually there without ever reaching the 'local-modified' refusal and without --force being required. Reproduced directly: an unlocked canonical folder containing an unrelated file and no SKILL.md was wiped by a plain run (no --force), reported "installed and verified", exit 0. Fix: check the canonical folder itself (skill_dir.exists()) instead of skill_md.is_file() for the unlocked-folder branch, so any existing unlocked folder without a manifest-matching SKILL.md -- including one with no SKILL.md at all -- is refused as local-modified (overridable with --force) rather than passed to `skills add`. Adds a LocalModifiedTests case that pre-creates an unlocked canonical folder with an unrelated file and no SKILL.md, and asserts the run refuses with 'local-modified' and leaves that file in place. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Nonmutating, value-free status check for adoption/skills/manifest.json, separate from scripts/adoption_status.py per task scope. For each pinned skill it checks: the canonical ~/.agents/skills/<name>/SKILL.md exists with a matching sha256; the global skills-CLI lock (XDG_STATE_HOME-aware, version 3) has a matching skillFolderHash; ~/.claude/skills/<name> exists and resolves to the canonical folder (reporting relative/absolute/copy); ~/.claude/settings.json skillOverrides[name] matches claude_listing (a missing key defaults to "on" per Claude's own docs); and ~/.codex/config.toml has, or lacks, a [[skills.config]] disabling table matching codex_enabled. Also reports (informational only): extra skills present on disk or in the lock but absent from the manifest, canonical folders with no lock entry, and the Claude/Codex description-character budget vs a fresh computed sum. Optional --skills-bin compares "<bin> --version" against the pinned CLI version. Exit 0 only when every required check passes, 1 otherwise, 2 on an unreadable/invalid manifest. 37 unittest cases cover all-pass, sha/lock drift, a missing link, an absolute link (reported but passing), a plain-directory "copy" link (failing), a wrong override and a missing one (default-to-"on"), a missing and a present-but-empty Codex disable table, the codex_enabled=true rejection direction, the XDG_STATE_HOME lock path, extra/unlocked-folder warnings, --json shape, exit-2 manifest paths, --skills-bin match/mismatch/ not-found, budget drift/over-cap reporting, and that no fixture sentinel or raw file content ever reaches stdout/stderr in any mode. adoption/skills/manifest.json (read-only input, unchanged) will need a manifests/evidence.json entry for these two new files; left for the coordinator's registration commit per task scope. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…symlink-loop RuntimeError
Repair round on independent-review findings against the skills_status checker:
- check_claude_listing returned overrides.get(name) verbatim as report[...]["actual"]
with no check that it was one of the four documented skillOverrides strings, so a
non-enum value (secret-shaped or otherwise) reached both text and --json output,
contradicting the module's own value-free contract. It now clamps to one of the
four known states or a fixed "invalid_value" placeholder, guarded with an
isinstance(str) check so an unhashable value (a list/dict) can't raise either.
- check_claude_link's except clause only caught (OSError, RecursionError) around
Path.resolve(); a symlink cycle instead raises RuntimeError("Symlink loop from
...") on CPython 3.11 and 3.12 (verified empirically on both interpreters here;
3.13+ tolerates the cycle silently in non-strict mode instead), crashing the
whole status check for every skill instead of reporting the one looping skill
as cannot_resolve. Two of this repo's own CI workflows pin python-version 3.12.
Added four regression tests: a secret-shaped and a non-string (unhashable) override
value, both asserted to never reach stdout/stderr in either output mode and not to
crash; and a symlink-loop test that forces the RuntimeError via mock so it holds
under whichever interpreter runs the suite, since a real filesystem loop no longer
reproduces the exception on 3.13+. All 40 tests in tests/test_skills_status.py pass
(uv run --no-project --with jsonschema --with pyyaml python -m unittest, Python 3.14.7).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Joins adoption/skills/manifest.json with real usage on each client that lists a skill: Claude via native /skill-doctor (--claude-skill-doctor FILE parses a captured stream-json array or plain-text table; --run-skill-doctor runs it directly and refuses the result unless total_cost_usd == 0 and num_turns == 0, its documented zero-cost shape), and Codex via --codex-root (repeatable, no default search) scanning rollout-*.jsonl for a persisted tool/function call reading a path ending in "/<name>/SKILL.md" and a user message containing "$<name>". The rollout JSONL shape (RolloutLine/RolloutItem/ResponseItem) is grounded in the openai/codex source at tag rust-v0.155.1 (codex-rs/history/src/rollout_payload.rs, codex-rs/protocol/src/models.rs). Applies the manifest's own trial.prune_rule: a skill idle on every measured, enabled/listed client for at least trial.window_days is a prune candidate if its status is "trial", or flagged "verdict re-record required" (never demoted) if "kept". A window never actually scanned reads as unmeasured (null), not a fabricated zero, mirroring tools/token-report's evidence rules. Names and counts only: no paths, transcript text, or session/thread ids. Nothing is written unless --out is given, and --out inside the checkout is refused. Rollout fixtures are committed as *.jsonl.fixture (the repository's .gitignore has a blanket *.jsonl rule) and materialized under their real name by the test suite; no change to the shared .gitignore. Tests: 63 passed (uv run --no-project --with jsonschema --with pyyaml python -m unittest tests.test_skill_usage -v). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…patterns
An independent reviewer found two supported defects in the per-host skill
invoke-rate report committed at 1a453203:
1. tools/skill-usage/skill_usage.py: when a manifest skill is Claude-listed
(claude_listing != "off") but has no row in the parsed /skill-doctor
table, build_report() set claude.uses to a fabricated 0 instead of the
unmeasured None it already uses for a Codex window that was never
scanned. Reproduced against the real 26-skill manifest joined with the
real /skill-doctor capture: 21/26 skills came back with a confirmed-zero
that was never actually observed. Fixed by always reporting uses=None
for a row absent from the capture, matching the "never scanned reads as
unmeasured, never a fabricated zero" rule already documented for Codex.
Added a regression test and a matching README bullet extending that rule
to the Claude side explicitly.
2. tests/fixtures/skill_usage/**/*.jsonl.fixture and tests/test_skill_usage.py
used "/home/tester/" and "/home/x" personal-home-path placeholders, and
two fixtures' session_meta "id" (and matching filenames) used UUID-shaped
placeholders. scripts/validate.py's PRIVATE_CONTENT scan (personal home
path / local session identifier patterns) flags both, breaking the
validate.py gate .github/workflows/validate.yml runs on every PR. Fixed
by substituting the neutral "/home/example/" path validate.py already
exempts and replacing the UUID-shaped ids and matching filename segments
with plain "session-fixture-{a,b,c}" labels; skill_usage.py never reads
an "id"/"call_id" field or inspects the username segment of a path (only
the "/<skill>/SKILL.md" suffix), so this is content-neutral and does not
change any test's scored counts. `python3 scripts/validate.py` now exits
0; it exited 1 with 6 findings before this commit.
Verified both independently against source before fixing; re-ran the full
suite after (uv run --no-project --with jsonschema --with pyyaml python -m
unittest tests.test_skill_usage -v -> Ran 64 tests, OK: the 63 pre-existing
tests plus the one new regression test).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…l-scope supersession
- docs/decisions/2026-09-25-skills-trial-and-usage.md (new): the decision record that
adoption/skills/manifest.json already names as its decision_record. Writes up the
2026-09-25 native /skill-doctor baseline on host nativestack-5975wx-20260925 (5 kept
skills at 0 uses/never; 8 synced and 15 plugin skills never invoked; a read-only scout
count over 1,189 Claude transcripts and 76 Codex rollouts corroborates it as a scout
measurement, not a receipt), the manifest's selection rule/listing policy/prune rule
verbatim, a table of all 26 pinned skills and the 12 excluded groups, the rejected
monitoring alternatives (OTel, a custom hook, skills-lock.json + experimental_install,
launchd/systemd timers), an evidence-class table, and overturn conditions. Explicitly
supersedes, for the two named trial skills only, the 2026-09-24 dispositions for
anthropics/skills ("Not installed or vendored...") and obra/superpowers ("Keep the
catalog's `alternative` disposition: no plugin, SessionStart bootstrap or skill
copy."), quoting both exactly; every other skill in each source repository stays
excluded under its original reason. Does not edit catalogs/landscape/foundation.json;
the instructions-skills row is unchanged per the manifest's own verdict boundary.
- blueprints/native-skill-practice/README.md: "Install only the selected skills" now
points at adoption/skills/manifest.json plus tools/adoption/install_skills.py
(--check/--write) and scripts/skills_status.py, with the mechanics (pinned CLI in an
isolated npm prefix, DISABLE_TELEMETRY=1, lock entry, relative Claude symlink). The
old three-skill npx commands are kept as clearly labeled superseded history.
- recipes/claude-native-profile.md: the ECC skills-install section (search-first,
iterative-retrieval) now describes the same pinned CLI route at the same reviewed
ref (2b6e839...); the old Codex-only installer script plus manual Claude symlink
step is kept as superseded history.
- adoption/update.md: one added step, "Apply the skills manifest" (dry run, write,
status, /skill-doctor baseline), marked added after the current pinned release
`v2026.09.25.2`.
tools/adoption/install_skills.py and scripts/skills_status.py are referenced per the
coordinator's task but do not exist in this worktree (scripts/ and tools/ are outside
this branch's owned paths); tests/test_adoption_docs_consistency.py's
ScriptPathTests.test_every_documented_script_exists_at_head fails here as a result and
is expected to pass once the coordinator merges the sibling branch that adds them.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
An independent review found that the Mechanics section (source lines ~52-57) stated, as verified source review, that Claude Code's skillOverrides settings key is "propagated to a host by adoption/templates/claude.settings.template.json's skillOverrides block through tools/adoption/apply_claude_settings.py's deep merge." Re-reading both named files in full (the template, 239 lines; the script, 263 lines) confirms the finding: neither file mentions skillOverrides anywhere. The claim just restated the manifest's own equally-unverified settings_propagation field rather than the actual source. Fix, within my owned path (docs/decisions/2026-09-25-skills-trial-and-usage.md only): keep the verified upstream client fact -- Claude Code 2.1.282's skillOverrides key and its four listing values -- and replace the false in-repo-infrastructure claim with today's actual state: the template has no such key, the apply script never references it, and no host's live ~/.claude/settings.json is set from a manifest claude_listing value yet. Installing or removing a skill still goes through the skills CLI alone (the canonical copy and the claude-code symlink). Kept, reframed as a conditional design note rather than a fact already in production: apply_claude_settings.py's existing deep_merge_dict is already a generic recursive merge that never deletes a base key the incoming template omits, so a future demotion would still need to write an explicit state rather than delete a key -- true today, useful for whoever builds the block, no longer presented as already working. Verified: full read of adoption/templates/claude.settings.template.json (239 lines, no skillOverrides key) and tools/adoption/apply_claude_settings.py (263 lines, no skillOverrides reference); a repo-wide grep for skillOverrides across every *.json/*.py/*.md file matches only three files -- the manifest's own settings_propagation field, docs/decisions/2026-09-24-community-sweep.md's M10/Anthropic-sources rows (the abstract client feature, not this repo's template), and this record -- matching the reviewer's finding exactly. Tests: uv run --no-project --with jsonschema --with pyyaml python -m unittest discover -s tests -p "test_adoption_docs*.py" -v -> Ran 33 tests: 32 passed, 1 skipped (ProfileTableTests: "no profile's coverage differs between the pinned release and HEAD", unrelated), 1 failed (ScriptPathTests.test_every_documented_script_exists_at_head: adoption/update.md references scripts/skills_status.py and tools/adoption/install_skills.py, owned by a sibling worktree and absent from this checkout). That failure is pre-existing from the prior commit on this branch (2d8f0f2e already documented it as expected pending the coordinator's merge) and is confirmed unchanged by this edit: stashing this commit's diff and re-running the same test reproduces byte-identical failure output. test_ecosystem_manifest.py (also grep-matches my owned paths) -> Ran 48 tests: all passed. python3 scripts/validate.py: same three pre-existing hash mismatches as before this edit (adoption/update.md, blueprints/native-skill-practice/README.md, recipes/claude-native-profile.md); output confirmed byte-identical with and without this change via git stash, so this repair neither fixes nor worsens registration status. Still pending coordinator registration, as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s wired up" This reverts commit 84f8f38c49f63882585e2aeadee60d74a7d86a91.
…ad-back evidence Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ath looks like a personal home Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…er verifies (not installs) the CLI, record provenance Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… clear-text-storage false positive on a variable named secret) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
seathatflowsinourveins
force-pushed
the
claude/skills-trial-usage-20260925
branch
from
September 26, 2026 00:00
35693a8 to
6fa25db
Compare
seathatflowsinourveins
deleted the
claude/skills-trial-usage-20260925
branch
September 26, 2026 00:14
This was referenced Sep 26, 2026
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.
Scope and base
lane:foundation. This change serves token efficiency and the agent tooling behind every north-star action.origin/mainat the time of the hot-file commit.What changes
adoption/skills/manifest.json— the single registry for the trial. It holds 26 skills: 5 kept verdict winners at their recorded pins, and 21 trial skills. For each skill it records:SKILL.mdsha256;It also carries 13 excluded groups, each with a reason and an overturn condition. The pinned CLI is
skills@1.7.0; its tarball sha512 matches the npm integrity, and it runs withDISABLE_TELEMETRY=1.tools/adoption/install_skills.py— installs through the upstream CLI at tree URLs pinned to exact commits. It then checks two things: the lock'sskillFolderHashequalstree_sha, and theSKILL.mdsha256 matches.skills remove.--dry-runis opt-in.--print-codex-configprints the Codex[[skills.config]]lines.adoption/templates/claude.settings.template.json— askillOverridesblock with an explicit state for every skill, applied byapply_claude_settings.py.scripts/skills_status.py— a standalone, nonmutating, value-free check. It compares the manifest against the installed folders, links, lock (XDG-aware), CLI version,skillOverridesand the Codex config, and reports the budget.tools/skill-usage/— the invoke-rate report./skill-doctor, which costs 0 API tokens headless.Docs:
docs/decisions/2026-09-25-skills-trial-and-usage.md— covers the selection rule, listing policy, measurement, the rejected alternatives (OTel with tool details, a custom hook, projectskills-lock.json, timers), and the verdict boundary. For the trial only, it supersedes the 09-24 anthropics/skills and superpowers dispositions.adoption/update.md.Listing policy (token practice)
name-only: typesafe-ai, iterative-retrieval, search-first, agent-browser.user-invocable-only: find-skills, grill-me, improve-codebase-architecture./skill-doctorread about 600 → about 2,460 tokens per turn. It is not mixed into token-efficiency receipts.SOTA sources
src/update.ts(check == update;-g),src/add.ts(tree URL at a ref;skillFolderHash= git tree SHA),src/remove.ts,src/skill-lock.ts(XDG lock path) andsrc/telemetry.ts(DISABLE_TELEMETRY). npmskills@1.7.0; tarball sha512 = registrydist.integrity.skillOverrides, "Find unused skills"/skill-doctor, listing budget)UserPromptExpansion)claude_code.skill_activated,custom_skillredaction)codex-rs/config/src/skills_config.rsat tagrust-v0.155.1([[skills.config]]nameselector); https://developers.openai.com/codex/skillsadoption/skills/manifest.json. Each entry records commit, tree SHA andSKILL.mdsha256:Evidence-class table
SKILL.mdsha256) and licensesauditsskills update -g: no-op, lock unchangedsteps.skills_updatesteps.installer_applyskillOverridesapplied (only that key changed; backup written)steps.claude_settings< 20, user-only-)/skill-doctorread-back, 0 cost, 0 turnsskill_doctor_aftersteps.codex_configCommands run (this host, nativestack-5975wx-20260925)
Review
Writer units: six, each in its own worktree, each followed by an adversarial verifier and one repair round. All high and medium findings were confirmed and fixed. One docs "repair" was reverted after integration, because the template now carries
skillOverrides.Independent review: a separate headless
claude -psession (Sonnet; the Opus weekly limit runs until 2026-09-30). Result: needs_changes → 4 findings, all fixed in the repair commit:--check/--writeflags; the README said the installer installs the CLI.The reviewer independently verified the rest as clean: 6 pins re-derived (tree SHA +
SKILL.mdsha256), both budgets recomputed, 144 tests OK,skills_statusok on this host,validate.pypassed, no personal paths, and the Codex read-back gap honestly disclosed.Codex cross-family review: skipped. Codex is at its usage limit until 2026-09-30.
Remaining
host_receipts.py recordreceipts go in a follow-up after merge, so thatcatalog_revisionis published.🤖 Generated with Claude Code