diff --git a/docs/specs/native-overlap-integration-design.md b/docs/specs/native-overlap-integration-design.md new file mode 100644 index 0000000000..aa6e956681 --- /dev/null +++ b/docs/specs/native-overlap-integration-design.md @@ -0,0 +1,63 @@ +--- +outcome: early-exit +tier: B +reason: one new store field with a closed enum, one new convention grammar, and a per-lane integrity shape inside existing scripts; every contract decision was locked in the Brief, so a full design pass would re-derive settled threads +--- + +# Design resolution, audit-native-overlap + +## Type sketch + +### Store row (`docs/native-surfaces/records.json`, schema 1, additive) + +```text +row.integration : "route" | "wrap" | "suggest" required on every row + invariants (enforced by overlap.py validate_row): + native.class == "builtin-command" -> integration in {route, suggest} + native.class == "bundled-skill" -> integration in {route, wrap} + native.class == "bundled-skill" and "model-invocation-disabled" in native.markers + -> integration == "suggest" (the model never lists it, so a route phrase is dead text) + native.class == "plugin-backed-builtin"-> integration in {route, wrap} + native.class == "marketplace-plugin" -> integration in {route, wrap} (wrap grammar owned by seam-phrasing) + native.class == "session-skill" -> integration == "route" + verdict == "defer" -> integration == "route" (nothing is baked from a defer row) + integration in {wrap, suggest} -> an evidence line names the observed invocation mode + (Skill-tool reach is per surface: /init and /security-review are + reachable built-ins; /export and /skill-doctor are non-prompt + command types; the class rules are a floor, not the determinant) + marker source: the registration the row's evidence names, never the bare name + (on 2.1.263 "design" is two bundled registrations: the canvas, model-invocable; the hub, model-disabled) +row.baked : {description_phrase, boundary_section, native_step, suggest_sentence} two flags added + native_step: true only when integration == "wrap" and the body carries "## Native step: ()" + suggest_sentence: true only when integration == "suggest" and the body carries the suggest token +``` + +### Inventory integrity (`inventory.py`, schema 1, additive) + +```text +integrity.status : "ok" | "degraded" | "broken" unchanged, now the worst lane +integrity.lanes : { builtin_commands: LaneStatus, bundled_skills: LaneStatus, plugin_backed: LaneStatus } +LaneStatus : { status: "ok" | "degraded" | "broken", problems: [str], advisories: [str] } +``` + +`overlap.py detect` reads `integrity.lanes` when present and falls back to the top-level status when absent, so an older inventory file still parses. + +### Convention grammars (`docs/conventions/native-references`) + +```text +route : description phrase carrying the gate token "resolves in your session" (existing) +wrap : body section "## Native step: ()" carrying the gate token, the identity + check by class, the mutation clause, the skip-and-report contract for five states + (does not resolve, invocation refused with the reason named, identity mismatch, + mutation detected after a scoped invocation, resolved but degraded with the surface's + own disclosure relayed), and the enable path (new) +suggest : body sentence "If / is available in your session (), run it for ." + carrying the token "available in your session", the basis pointing at a same-file + four-part verification record (new) +``` + +Class table: bundled-skill, plugin-backed-builtin, and marketplace-plugin may take route or wrap (the marketplace-plugin wrap grammar is seam-phrasing's, per the playgrounds precedent); a bundled-skill row marked model-invocation-disabled takes suggest; builtin-command may take route or suggest; session-skill may take route only. Amended 2026-09-11 after the round-4 validation pass. + +## Threads resolved in the Brief + +Wrap semantics, degradation contract, identity verification, suggestion placement, per-row field placement, description trimming, and filing shape are all locked in `../PLAN.md` `## Brief`. No thread is open. diff --git a/docs/specs/native-overlap-route-to-compose.md b/docs/specs/native-overlap-route-to-compose.md new file mode 100644 index 0000000000..1804a0e8d7 --- /dev/null +++ b/docs/specs/native-overlap-route-to-compose.md @@ -0,0 +1,433 @@ +# Native overlap: fix the tooling, then route to compose + +## Status (graduated 2026-09-11) + +This is the approved plan for the native-overlap effort tracked in +melodic-software/claude-code-plugins#4047, graduated from the topic slice +`docs/topics/audit-native-overlap/` when unit 1 shipped, so the remaining units have a durable +contract to resume from. The design resolution it references is +[`native-overlap-integration-design.md`](native-overlap-integration-design.md). The interview +ledger and plan checklist lived in the gitignored memory tier and are not graduated; every decision +they held is in `## Brief`, `## Open questions`, and Phase 4 below. + +- Unit 1 (#4048) shipped in #4092: Phases 1 to 3 are done, with each phase's deviations recorded + under its heading. The audit-native-overlap description was trimmed under the 1,024-character + Skills API cap in the same PR, so Phase 5 no longer owns that finding. +- Main moved while unit 1 was in flight. The native-references convention reached 1.1.0 (#4058): + every non-`defer` extraction row lands with a Boundary section naming its surface as a code span, + the overlap self-check fails (exit 1) on a row without one, Boundary-only baking may batch across + plugins, and those sections were baked across code-tidying, review, testing, visualization, + prototype, and claude-ops. Three `claude-api` rows were added (`claude-config:audit-instructions`, + `evals:methodology`, `playbooks:fable-5`), so the store holds 19 rows. +- Consequences for the remaining units, to apply when unit 2 starts: Phase 4's major bump builds + on 1.1.0, not 1.0.1, and keeps its Boundary-section rule; the Phase 4 row count is 21 (19 plus + the two `export` rows), and the three `claude-api` rows take an `integration` value at the Phase + 4 gate (`route` on today's evidence: each component already owns its job and carries its + Boundary section); the sweep units' Boundary work is already done, so each unit is its + description phrase, Native step, or suggest sentence only, and Phase 5 through Phase 11 shrink + accordingly. +- Units 2 to 9 are deferred as filed (#4049 to #4056) with the notes above recorded on each issue. + Unit 2 is C4 (`work-class: structural`, `needs-human`); a person reviews and merges it. + +## Brief + +**Spec container:** melodic-software/claude-code-plugins#4047 + +### TLDR + +- Repair the native-overlap tooling so it reports honestly on Claude Code 2.1.263: the inventory extractor's bundled-skill lane, per-lane integrity, the reverse-parity blind spot, and the seeded-pairs drift. +- Move the marketplace posture from route-only to compose-where-sensible: a skill runs the native surface as a named step when it resolves and layers its own value around it. +- Add a per-row `integration` field (`route`, `wrap`, `suggest`) to the native-surfaces store, separate from `verdict`, and a `suggest` grammar to the native-references convention. +- Degrade gracefully: when a native surface does not resolve, skip the step, report it with the gating axes, offer the enable path, never re-implement the native job. +- Deliver as one parent issue with per-unit sub-issues: the tooling fix first, then claude-ops, then every other plugin holding a store row, one unit in flight at a time. + +### Goal + +A run of `/claude-ops:audit-native-overlap` on the current Claude Code build tells the truth lane by lane instead of collapsing to `broken`, and the skills this marketplace ships stop competing with the native surfaces they overlap: where a bundled skill does a strict subset of a skill's job, the skill composes it as a step and adds its own value (deeper inventory, reconciliation, its own agents); where a built-in command does related work a skill cannot invoke, the skill tells the user it exists at the moment that matters; and in every session where the native surface is gated, disabled, or absent, the skill still completes its own part and says what it skipped and why. + +### Constraints + +- A skill never asserts that a native surface is present, absent, enabled, or unavailable. The native-references convention's presence-gate rule holds for every new phrase and body section. +- A skill never invokes a target it cannot identify. Bundled skills: name in the listing plus an advisory description check, skip with a warning on mismatch. Plugin skills (any marketplace, including melodic-software): the namespaced `plugin:skill` form, plus marketplace-qualified provenance from `claude plugin list` when the CLI resolves. Built-in commands: never invoked. +- Whether a session is unattended is declared by the caller, never sniffed. An unattended run records a built-in suggestion in its output instead of asking. +- Verdicts stay human-gated. No component is edited before its store row carries a verdict and an `integration` value. +- The sweep contract stands: one plugin is one unit, closed only when its PR merges green, never two units in flight. +- Every touched description must fit the 1,536-character per-entry cap after baking; trimming existing text to make room is allowed and gated by `/skill-quality:check`. +- Baked text is self-contained: no phrase or Boundary section cites a file outside its own plugin. +- Plain-bullet acceptance criteria (`acceptance_criteria_format` resolved to `free-text` from the default; the repo declares no convention home). + +### Acceptance criteria + +- `inventory.py --binary-only` on Claude Code 2.1.263 reports integrity `ok` or `degraded` with a non-empty bundled-skill lane, its eval suite passes, and `validated_against` moves to 2.1.263. +- `overlap.py detect` on an inventory with exactly one broken lane exits 3, writes the candidates file, states per-lane floors, and marks only that lane's rows as not re-derivable. +- `overlap.py self-check` flags a frontmatter description that names a bundled surface behind a presence condition without a gate token. +- Every store row carries `integration` with one of `route`, `wrap`, `suggest`; the self-check rejects any other value, rejects `wrap` on a built-in command row, and rejects `wrap` or `suggest` on a session-provided row. +- The generated view renders the `integration` column and `--check` stays in sync. +- The two human-added `audit-skill-visibility` rows appear in `reference/canonical-pairs.json`, and a consumer-repo `detect` proposes them. +- The native-references convention documents three grammars (`route` phrase, `wrap` Boundary section, `suggest` body sentence) with the class table stating which grammar each provenance class may use. +- A wrapped skill, run where the native surface resolves, invokes it as a named step and reports the native result alongside its own. +- A wrapped skill, run where the native surface does not resolve, completes its own part and reports the skipped step naming the four gating axes and the enable path. +- While a bundled skill is present under `skillOverrides: name-only`, the wrapping skill treats it as not resolving. +- A skill with a `suggest` row surfaces the suggestion at the start of the run when the built-in command covers everything the skill does, and at the end when coverage is partial, phrased conditionally. +- Every touched skill passes `/skill-quality:check`, and each sweep unit's plugin takes its version bump and CHANGELOG entry. +- The parent issue quotes the affected store rows, and each sub-issue names its unit and its predecessor. + +### Captured assumptions + +- The two coverage cases above (native absent, `name-only` override) are the author's proposal; the user approved the Brief as a whole without ruling on them separately. Revisit if a sweep unit finds a third trigger or state the wrapped skill must handle. +- Wrap candidates on the current store are the bundled-skill rows: `doctor` (three rows), `simplify` (two), `run`, `design` (two), and the plugin-backed `security-review`. `code-review` against the CI lane is expected to stay `route` because a CI lane cannot type into a session. Each row's final `integration` value is still a human verdict recorded in the store. Revisit if a row's verdict moves to `superseded`. +- Built-in command rows (`export`, `skill-doctor`) and the `design` access command take `suggest`. Revisit if Claude Code exposes a built-in command through the Skill tool. +- The `skill-doctor` row keeps its `gated` marker on the docs basis (v2.1.252+, absent when feature-flag fetching is skipped); the extractor's `gated: false` reading is a heuristic blind to feature-flag gates. Revisit when the extractor learns to read that gate. +- `design` is two surfaces sharing one name: the canvas bundled skill (the store row) and a gated built-in access command. The row's evidence must name both. Revisit if either is renamed. +- Replace (our skill retired in favor of the native one) is expressed by the existing `superseded` verdict, so `integration` needs no `replace` value. Revisit if a row reaches `superseded` and still needs a runtime relationship. +- The extractor fix targets the 2.1.263 bundle layout, where the readable export map no longer carries `registerBundledSkill:()=>…`; the new registration shape is an implementation finding for the plan. Revisit on the next CLI release that moves `validated_against` again. +- Session-provided rows (`morning`) stay `defer` with `integration: route` withheld until an in-session capture protocol exists. + +Scope note, 2026-09-10, added at planning after the fresh-context review: three statements about `defer` rows disagreed (the assumption above says `route` is withheld for `morning`, the acceptance criterion says every row carries `integration`, and the plan's verdict pass proposes `route`). The acceptance criterion is the stronger, approved statement, so every row carries `integration`, `morning` included, with `route` as the only value a `defer` row may take. The assumption's "withheld" is superseded by this note. + +Scope note, 2026-09-10, added at planning after the devil's-advocate pass: the wrap-candidate assumption above rests on model-invocability that the 2.1.263 binary contradicts. Each bundled-skill registration carries an invocation-control field, and `doctor`, `design`, `design-sync`, `batch`, `debug`, and `run-skill-generator` are registered with model invocation disabled, which the official skills reference defines as "Claude cannot invoke" and this repository's invocation-mode convention restates as "cannot be invoked by any other skill". `simplify` and `run` are registered model-invocable. So the three `doctor` rows and the two `design` rows cannot take `wrap` on any host; their available integrations are `route` and `suggest`. This changes which rows the sweep can compose and is the user's decision, recorded under `## Open questions` as USER-RESERVED, not resolved here. + +Scope note, 2026-09-11, added after the round-4 re-interview and its two-validator research pass (independent fresh-context Fable 5.1 and Opus agents against the official skills, commands, settings-reference, sub-agents, env-vars, and errors references, the 2.1.263 binary bytes, and this repository's conventions): the previous scope note over-reads `design`. On 2.1.263 the name `design` carries two bundled-skill registrations: the design canvas skill (registered by `registerDesignCanvasSkill`, `userInvocable`, no invocation-control field, so model-invocable, gated on the artifact capability) and the claude.ai/design hub (registered model-disabled, gated on a design-sync entitlement), plus a `local` access command. Both `design` store rows describe the canvas, which is model-invocable and present in this session's own roster, so those rows do not carry `model-invocation-disabled` and `wrap` is admissible for them; the user's accepted value is `route` on job merits (both components already own their artifact-producing job, so there is no sub-step to wrap), revisitable in Phases 9 and 10. The `doctor` rows stand as model-disabled. The marker must therefore be keyed per registration, not per name, which Phase 1 now carries. The assumption above that built-in commands are never exposed through the Skill tool has also fired on its premise: the official skills reference exposes `/init` and `/security-review` through the Skill tool, so reach is a per-surface property the store records as evidence, and the class rules are a floor, not the determinant. The `export` and `skill-doctor` rows still take `suggest` because those two commands are non-prompt command types the Skill tool never lists. + +### Out-of-scope + +- Re-implementing any native job inside a marketplace skill as a fallback. +- Retiring any skill under `superseded` in this effort. +- Wrapping or suggesting session-provided skills. +- The improvement items beyond the four broken ones: a `recheck` subcommand for trigger evaluation, a marker-basis field, name-collision keying in `detect` (the bundled-vs-bundled case moved into Phase 1 on 2026-09-11; the seeded-vs-observed case stays here), an apply-time per-entry cap precheck, durable report output from a bare run, and a `reverify` helper. These are filed as follow-up scope in the parent issue, not built here. +- Two follow-ups surfaced by the 2026-09-11 validation, filed on the parent issue: `/verify` has no store row although it is the closest bundled analogue to `testing:run-e2e` and `verification:confirm` (candidate pairs `verify` x `testing:run-e2e` and `verify` x `verification:confirm`; it is model-disabled by a flag-driven field, so `suggest` at most); and a Native step identity check cannot see behaviour drift under a correct identity (the `simplify` two-prompt split), so a post-step assertion on what the surface reported doing is a candidate grammar addition. + +### Deferred questions + +- none; every registered question closed answered. + +## Plan + +### Goal + +**What:** repair the native-overlap tooling for Claude Code 2.1.263, add the `integration` axis to the store and the `wrap` and `suggest` grammars to the native-references convention, then sweep every plugin holding a store row, one closed unit at a time. +**Why:** the 2026-09-08 run reported `broken` for one stale lookup pattern and could re-derive nothing, and the fleet's skills still compete with the bundled surfaces they overlap instead of composing them. + +### Standards grounding + +No standards index resolves (`.claude/standards.yaml` absent, `docs/standards/` absent). Inferred from repository context, rung 4 of the ladder: this repository's standards live under `docs/conventions/`. Loaded for the surfaces this plan touches: `native-references` (phrase grammar, Boundary section, self-containment, one owning description per plugin, enforceability tiers, versioning rule), `seam-phrasing` (gate plus fallback plus ownership framing, the install-recipe carve-out, and the owner of any wrap of a marketplace plugin), `upstream-drift` (four-part verification records for every upstream fact a body restates), `rendered-views` (markdown is the record; the generated view stays markdown), `invocation-mode` (every touched skill keeps its explicit `disable-model-invocation` key; a model-disabled skill cannot be invoked by another skill), `topic-docs` (contract slice pruned before merge), the path rule `.claude/rules/skill-bodies-state-current-rules.md` (bodies state the current rule, never the incident; `## Next` placement), and `.claude/rules/vendor-docs-are-not-style.md` (no em dashes in instruction surfaces). The `standards` convention itself and the two personal layers contributed nothing. Persisting an index at `docs/standards/README.md` is an offer for the user, not a write this plan makes. + +### Approach + +Nine closed units, executed strictly in sequence per the sweep contract (one plugin is one unit, closed only when its PR merges green, never two in flight). Phases 1 to 3 form the tooling-fix unit. Phase 4 is the policy unit and settles every verdict, with its pre-flights, before any sweep unit starts. Phases 5 to 11 are the sweep, one plugin each. Phase 0 files the tracker container that carries the rest. + +Build technique: the one viability unknown (recovering bundled-skill registrations from the 2.1.263 layout) was resolved by two throwaway probes during planning, the second of which corrected the first. The kept slice is Phase 1: a walking skeleton whose sanity check is the real binary reporting the named surfaces the store depends on. + +Probe findings (2026-09-10, `node_modules/.bin/claude` 2.1.263, Linux ELF): the bundle is a `// @bun @bytecode` layout fragmented into thousands of printable runs; the CJS getter shape the extractor keys on (`registerBundledSkill:()=>xu`) is gone, replaced by an ESM export list (`eo as registerBundledSkill`) that names the registrar directly; bundled-skill registrations are calls to that minified function (`eo({name:"doctor",aliases:["checkup"],…})`) scattered across runs as small as 1.3 KB, so a size floor of 64 KiB recovers 13 names while a 256-byte floor recovers 33; a global constant map over the joined source produced one phantom name (`ehrpd`, a telecom string bound to a same-named identifier in an unrelated chunk), so constants resolve within their own chunk only; the built-in command table sits in the largest run, which is why the builtin lane extracted. Every registration carries `userInvocable` and `disableModelInvocation` fields; `doctor`, `design`, `design-sync`, `batch`, `debug`, and `run-skill-generator` are model-disabled, `simplify` and `run` are model-invocable, and `doctor` also carries `terminalOriented`, the documented-by-code reason it is absent on web and cloud hosts. A 256-byte regex floor runs in about 3 seconds on the 215 MB file; a 64 KiB floor takes minutes because the regex backtracks on every shorter run. + +Literals every unit's sanity check greps for, fixed here so the grammar and the checks agree: + +- Wrap heading: `## Native step: ()`, for example `## Native step: simplify (bundled skill)`. +- Suggest sentence shape: `If / is available in your session (), run it for .` The reverse-parity scan keys on the shape `If /` … `is available in your session (`, never on the bare phrase, because that phrase already occurs in unrelated prose in the fleet. +- Axis line, verbatim in every skip report: `settings or environment, plan, platform or provider, host surface`. +- Skip report opener: `did not resolve in this session`. +- Unattended declaration: the bare argument `unattended`, following the `overengineering:audit` precedent. +- Version bumps: "next minor at PR time" for every touched plugin; no literal numbers, because claude-ops receives unrelated PRs weekly. + +### Phase 0: File the parent issue and its sub-issues [DONE] + +Executed by `/work-items:decompose` after this plan is approved; the plan fixes the shape so decompose does not re-derive it. + +Done 2026-09-10: container #4047; sub-issues #4048 (unit 1, tooling fix), #4049 (unit 2, policy), #4050 (claude-ops), #4051 (code-tidying), #4052 (testing), #4053 (review), #4054 (visualization), #4055 (prototype), #4056 (session-flow), each attached as a native sub-issue of #4047 and each naming its predecessor in its `## Blocked by` section. Deviation: the native blocked-by dependency edges were not written, because the session's GitHub token could not drive the tracker seam and the GitHub MCP surface exposes no dependency verb; the edges are body text only until a session with seam access runs `link-blocks` for the eight predecessor pairs. + +1. **Search before create.** Query open issues for `native-overlap`, `audit-native-overlap`, `native-surfaces`, and `inventory.py 2.1.263`. A match with the same scope is the pivot path: attach this Brief to it as a comment and use it as the parent instead of creating one. Record the search outcome in the sanity check. +2. Create the parent issue: body is the Brief verbatim, followed by a `## Affected store rows` section quoting every row of `docs/native-surfaces/records.json` by native name, component, verdict, and current `integration` (or `pending` before Phase 4), and a `## Follow-up scope` section listing the improvement items from `### Out-of-scope` plus the two tooling findings the devil's-advocate pass surfaced that the tooling unit does not absorb (the registrar-export advisory's blindness to the ESM form, and phantom constant names). Sub-issue PRs cite their sub-issue with the closing keyword the PR-body contract expects; the issue body itself carries no closing keyword. +3. Publish all nine sub-issues at once with native dependency edges, blockers-first, each blocked on its predecessor, so the sweep contract's one-unit-in-flight rule is carried by the edges rather than by withholding issues. Sub-issues 3 to 9 name their unit as "integration values written in Phase 4 decide the unit's content". + +**Sanity Check:** + +- The search ran and its result (no match, or the matched issue number) is written into the parent issue body. +- `gh issue view --json body -q .body | grep -c "## Affected store rows"` prints 1 and `… | grep -c '`doctor`'` prints at least 3; nine linked sub-issues exist and eight of them are blocked. + +### Phase 1: Extractor bundled-skill lane on 2.1.263 [DONE] + +Done 2026-09-11. Deviation from step 3 as planned: bundle markers are not module boundaries in the bytecode layout (the hoisted name constants sit about 8.7 MB and 173 marker occurrences ahead of their registrations, measured on 2.1.263), so a marker-scoped constant map resolves nothing. The rule built instead is locality: a computed name resolves to its nearest preceding `ident="kebab"` binding, a farther binding never wins over a nearer one (the `ehrpd` phantom is the negative fixture), a single-character identifier is trusted only within a 64 KiB window (which recovers the design canvas `var r="design"` and refuses a loop variable bound megabytes away), and a loop or template-literal registration is a `dynamic_roster` note. A call to the registrar identifier whose object carries no `name:` is another module's function and is counted apart. Measured wall clock on this container: about 14 seconds (region pass 4 s, brace map 5 s, name resolution 2 s), so the sanity bound below is 30 seconds, not 15. + +Files: `plugins/claude-ops/skills/inventory/scripts/inventory.py` (MODIFY), `plugins/claude-ops/skills/inventory/scripts/test_inventory.py` (MODIFY). + +1. **Red, region selection.** `read_bundle` gets its first unit tests, driven with bytes fixtures: (a) a marker followed by a run above 1 MB carrying only command registrations, then non-printable bytes, then a 2 KB run carrying `eo({name:"doctor",aliases:["checkup"]…})`, then a third run carrying the hoisted constant for a second registration; assert the returned source contains all three. (b) The same layout wrapped in a PE-shaped container (an `MZ` header). (c) The legacy single-run layout still returns the same source as today. (d) A registration in a run smaller than the floor is either recovered or counted in `sources.binary.runs_below_floor`, never silently lost. +2. **Red, registrar discovery.** Three fixtures, one per route: the CJS getter (`registerBundledSkill:()=>xu`), the ESM export list (`eo as registerBundledSkill`), and neither of those but the canary present. `bundled_skill_notes.registrar_route` reads `export-map`, `esm-export`, or `canary`; with none, the lane is broken with the existing error text. +3. **Red, constant resolution.** A constant bound in one chunk and reused as a registration name in another chunk resolves only when the binding sits in the same chunk (between consecutive bundle markers); the `ehrpd` shape (same identifier bound to a different string in an unrelated chunk) is a negative fixture; identifiers of length 1 are always unresolved; a loop registration (`for(… of …)eo({name:e,…})`) is recorded as a `dynamic_roster` note, not one unresolved identifier. +4. **Red, invocation fields.** Each registration records `user_invocable`, `disable_model_invocation`, `terminal_oriented`, and `survives_kill_switch` when present. A function-valued `disable_model_invocation` (the `verify` registration's shape) is recorded as `true` with a `flag_driven: true` note, matching the serializer's own reading. +4b. **Red, same-name bundled collision.** Two bundled registrations sharing one name (on 2.1.263, `design`: the canvas skill from `registerDesignCanvasSkill`, model-invocable, and the claude.ai/design hub, model-disabled) are both kept, keyed by registrar, under `bundled_skills.` as a list of registrations with a `collision: true` note, never merged and never last-writer-wins; the fixture is the two `design` objects. `detect` and the Phase 4 marker read pick the registration whose description matches the row's evidence and record which. +5. **Green.** Region rule, stated exactly: from the first bundle marker to end of file, every printable run of at least 256 bytes, found with one `re.finditer(rb"[\t\n\r\x20-\x7e]{256,}")` pass, joined with newlines; `sources.binary` records `runs`, `joined_bytes`, `region_rule`, `runs_below_floor`, and `elapsed_seconds`. The longest-run path stays as the fallback when no marker is found. Registrar discovery order: CJS getter, ESM export list, canary. The registrar-shaped-export advisory scan gains the ESM form (`\w+ as (register[A-Za-z]*(?:Skill|Command|Agent))`) and `KNOWN_REGISTRAR_EXPORTS` gains `registerDesignCanvasSkill` and `registerWorkflowAuthoringSkill`. +6. `VALIDATED_AGAINST` moves to `2.1.263` in Phase 3, not here. + +**Sanity Check:** + +- `python3 plugins/claude-ops/skills/inventory/scripts/test_inventory.py` exits 0 and `grep -c "def test_read_bundle" plugins/claude-ops/skills/inventory/scripts/test_inventory.py` prints at least 4. +- `time python3 plugins/claude-ops/skills/inventory/scripts/inventory.py --binary-only --out /tmp/inv.json` exits 0 in under 30 seconds of wall clock (measured about 14 seconds), and `jq -e '[.bundled_skills.doctor, .bundled_skills.simplify, .bundled_skills.run, .bundled_skills.design, .bundled_skills["code-review"]] | all(. != null)' /tmp/inv.json` prints `true`, and `jq -e '.plugin_backed["security-review"] != null' /tmp/inv.json` prints `true`. +- `jq -e '.bundled_skills.doctor.disable_model_invocation == true and .bundled_skills.simplify.disable_model_invocation == false and .bundled_skills.doctor.terminal_oriented == true' /tmp/inv.json` prints `true`; `jq -r .bundled_skill_notes.registrar_route /tmp/inv.json` prints `esm-export`; `jq -r .integrity.status /tmp/inv.json` prints `ok` or `degraded`, never `broken`; `jq '.bundled_skills | keys' /tmp/inv.json` does not contain `ehrpd`. +- `jq -e '.bundled_skills.design | (type == "array" and length == 2) and (map(.disable_model_invocation) | sort == [false, true])' /tmp/inv.json` prints `true` (the canvas and hub registrations both kept). + +### Phase 2: Per-lane integrity and honest exits in both tools [DONE] + +Done 2026-09-11 as planned, plus a `plugin_backed` canary (`security-review`) so that lane has a break condition of its own, and `re_derivable: null` for session-provided and marketplace candidates, which have no extraction lane. A name collision in the extraction is listed in the candidate's evidence per registration with its invocation mode; the pairs file carries no description to match against, so `detect` records every registration and the Phase 4 marker read picks the one the row's evidence names. + +Files: `inventory.py` (MODIFY), `test_inventory.py` (MODIFY), `plugins/claude-ops/skills/inventory/reference/extraction.md` (MODIFY), `plugins/claude-ops/skills/audit-native-overlap/scripts/overlap.py` (MODIFY), `plugins/claude-ops/skills/audit-native-overlap/scripts/test_overlap.py` (MODIFY), `plugins/claude-ops/skills/inventory/SKILL.md` (MODIFY, the integrity paragraph), `plugins/claude-ops/skills/audit-native-overlap/SKILL.md` (MODIFY, "The two substrates" and "Detection posture"). + +Pre-flight consumer check, first work item: `Grep` for readers of the integrity block. Known today: `overlap.py cmd_detect` (reads `integrity.status`, `cli_version`, `validated_against`), `inventory.py --self-check` (maps status to exit through `{"ok": 0, "broken": 1, "degraded": 3}`), `inventory/reference/extraction.md` (documents the block), `inventory/evals/evals.json` (expectations name the status), and `scripts/validate-plugins.sh` (reads overlap's exit code only). Record any further reader the grep finds before editing. + +One rule for one state, stated exactly: top-level `integrity.status` becomes the worst lane; `broken` at the top level means every lane is broken or the binary is unreadable; a run with at least one healthy lane is at most `degraded`. The existing exit mappings in both tools already send `degraded` to 3 and `broken` to 1, so the exit code contract does not change; what changes is which state a single broken lane produces. + +1. **Red.** `check_integrity` returns `lanes` with `{status, problems, advisories}` for `builtin_commands`, `bundled_skills`, and `plugin_backed`. Tests: a broken bundled lane with a healthy builtin lane yields lane statuses `ok`/`broken`/`ok` and top-level `degraded` carrying the bundled lane's problem as an advisory prefixed with the lane name; all lanes broken yields top-level `broken`; an unreadable binary yields `broken`. +2. **Red.** `cmd_detect` on an inventory with one broken lane states per-lane floors in `integrity` and marks every candidate whose lane is broken with `re_derivable: false`. The lane is decided by the seeded class when the native name is absent from the extraction, and by the observed class when it is present; a candidate is marked in both directions when either class maps to the broken lane. A fixture with a synthetic collision (one name seeded bundled, observed builtin) asserts the both-directions rule; the plan makes no claim that the fixed extractor reproduces the 2026-09-08 collision, because `build_report` drops a builtin whose name a bundled skill also carries. An inventory without `lanes` keeps today's behaviour. +3. **Green** both; update `extraction.md` and the evals expectations to the per-lane wording. +4. Re-word the audit-native-overlap SKILL.md rule from "if it reports broken, the report carries no native-side counts at all" to per-lane: counts are omitted only for a broken lane; the report names the lane and its cause. + +**Sanity Check:** + +- `python3 plugins/claude-ops/skills/audit-native-overlap/scripts/test_overlap.py` and `python3 plugins/claude-ops/skills/inventory/scripts/test_inventory.py` exit 0. +- A fixture inventory with `lanes.bundled_skills.status == "broken"` and healthy other lanes makes `overlap.py detect … --out /tmp/c.json` exit 3 and `jq '[.candidates[] | select(.re_derivable == false)] | length' /tmp/c.json` prints a number greater than 0. +- `grep -c "no native-side counts at all" plugins/claude-ops/skills/audit-native-overlap/SKILL.md` prints 0. + +### Phase 3: Reverse-parity blind spot, seeded pairs, evals, claude-ops release [DONE] + +Done 2026-09-11. Two additions: the presence-mention advisory requires an availability word in the clause (`available`, `installed`, `enabled`, `present`, `resolves`, `exists`, `ships`), so "use when the user asks about a built-in command" is not a presence condition, and it judges the gate token per clause, since `prototype:explore-directions` carries the marketplace token in one clause and the ungated design mention in another. `--upstream-sha` now repeats, one value per upstream repository the store cites: the store records commits in two repositories (`anthropics/claude-code` for the `skill-doctor` row, `anthropics/claude-plugins-official` for the playground rows), so one value always drifted and the sanity check below could never hold as written. The Windows run was not reachable from this container; the CHANGELOG states the Linux-only basis. + +Files: `overlap.py` (MODIFY), `test_overlap.py` (MODIFY), `plugins/claude-ops/skills/audit-native-overlap/reference/canonical-pairs.json` (MODIFY), `plugins/claude-ops/skills/inventory/evals/evals.json` (MODIFY only where an expectation names the old build or the old integrity wording), `plugins/claude-ops/.claude-plugin/plugin.json` (MODIFY), `plugins/claude-ops/CHANGELOG.md` (MODIFY). The native-references Enforceability row ("candidate check named, not built" becomes built) is edited in Phase 4's convention bump, not here, so the convention takes one versioned change. + +1. **Red.** Self-check flags a frontmatter description that names a native surface by class and name behind a presence condition (`bundled|built-in|plugin-backed built-in|session-provided` followed within a few words by `skill|command`, inside a clause starting `when|where|if`) without the gate token. Fixture: the `visualize` wording "where the bundled design skill is available". Negative fixtures: a description carrying the token; a seam-phrasing "if that plugin is installed" clause; a Not-for clause that names a surface with no presence condition. +2. **Green.** The flag is an advisory (exit 3), not a break, because the two live cases are legitimate pending rows; the message names the row to add or the token to use. +3. Seed the two `audit-skill-visibility` pairs (`doctor` bundled-skill, `skill-doctor` builtin-command) into `canonical-pairs.json` with a `why`; `test_shipped_canonical_pairs_file_validates` covers the shape. +4. Eval re-validation, stated exactly: the repo's evals are `evals.json` in the skill-quality format, and `claude plugin eval` consumes `case.yaml` or `prompt.md` plus graders, so the CLI runner does not apply. "Evals pass" means `bash plugins/skill-quality/scripts/check-evals-quality.sh plugins/claude-ops/skills/inventory/evals/evals.json` exits 0 and `check-jsonschema` validates the file, after any expectation that names 2.1.228 or the old integrity wording is updated. Then set `VALIDATED_AGAINST = "2.1.263"`. +5. Windows basis: where a Windows host with the 2.1.263 `claude.exe` is reachable (the fleet's desktop over `/fleet:reach`), run `inventory.py --binary-only --self-check` there and record the result; where it is not, the CHANGELOG entry states that 2.1.263 was validated on the Linux ELF container only and the PE path is covered by the Phase 1 fixture. +6. Bump `claude-ops` to the next minor at PR time (additive `integrity.lanes`, the two new registrar routes, new advisories, new registration fields), write the CHANGELOG entry stating the rules, not the incident. +7. Open the unit's PR as a draft, body per the PR-body contract, quoting the two seeded pairs; flip to ready when green. + +**Sanity Check:** + +- `python3 overlap.py self-check --upstream-sha ed404106fcd80ba98ecb7c851e531dcb626d13b7 --upstream-sha d7dbd9a09f59775726ed14bbea8fc9dfdff62f7b` exits 3 with no problems, and its advisories are exactly the version-drift advisory (until Phase 4 refreshes observations) plus two reverse-parity advisories naming `visualization:visualize` and `prototype:explore-directions` (`… 2>&1 | grep -c "presence condition without a gate token"` prints 2). +- `jq '.pairs | length' canonical-pairs.json` prints 16 and `jq '.pairs[] | select(.native.name=="skill-doctor")' canonical-pairs.json` is non-empty. +- `grep -n 'VALIDATED_AGAINST = "2.1.263"' inventory.py` matches; `jq -r .version plugins/claude-ops/.claude-plugin/plugin.json` is greater than `0.45.2`; `bash plugins/skill-quality/scripts/check-evals-quality.sh plugins/claude-ops/skills/inventory/evals/evals.json` exits 0; `scripts/affected-tests.sh --run` passes. + +### Phase 4: Policy unit, the `integration` axis and the two new grammars [TODO] + +Files: `docs/native-surfaces/records.json` (MODIFY, every row, plus two new rows), `docs/NATIVE-SURFACES.md` (regenerated), `overlap.py` (MODIFY), `test_overlap.py` (MODIFY), `docs/conventions/native-references/README.md` (MODIFY), `docs/conventions/native-references/CHANGELOG.md` (MODIFY), `plugins/claude-ops/skills/audit-native-overlap/SKILL.md` (MODIFY, "Verdicts and the human gate", "The apply step"), `plugins/claude-ops/.claude-plugin/plugin.json` and `CHANGELOG.md` (MODIFY). + +Pre-flight consumer check, first work item: `Grep` for readers of `records.json` and of the `baked` object; today they are `overlap.py` and the generated view only. The change is additive. + +0. **Pre-flights, answered 2026-09-11 by the two-validator research pass and quoted in the PR body before any verdict is written.** (a) Model-invocability per row, keyed per registration from the Phase 1 extraction: `doctor` and `design-sync` disabled; `simplify`, `run`, `code-review`, and the design canvas skill enabled (the model-disabled `design` registration is the claude.ai/design hub, which no row describes). (b) `simplify` takes a `[]` argument and its prompt prepends the review target, so `batch-simplify` scopes it per file set. (c) CI lane: `allowed-tools` grants and never restricts, so the `security-review` skill's frontmatter proves nothing about reach; the row takes `route` because the lane runs a pinned reusable workflow in `melodic-software/ci-workflows` whose tool allowlist and plugin roster this repository does not control, and because the native command is unusable there (it diffs against `origin/HEAD`, which the Actions checkout does not provide; the skill body already records this), even though the official skills reference lists `/security-review` among the built-ins the Skill tool can invoke. +1. **Red, schema.** `validate_row` requires `integration` in `route|wrap|suggest` with the class rules from `design/design-resolution.md`: `builtin-command` takes `route` or `suggest`; `bundled-skill`, `plugin-backed-builtin`, and `marketplace-plugin` take `route` or `wrap`; a `bundled-skill` row carrying the new marker `model-invocation-disabled` takes `suggest` only (a route phrase on a surface the model never lists is dead text); `session-skill` takes `route`; a `defer` verdict takes `route`. `NATIVE_MARKERS` gains `model-invocation-disabled`, set from the extraction registration the row's evidence names, never from the bare name. Every `wrap` or `suggest` row carries an evidence line naming the observed invocation mode (the class rules are a floor; Skill-tool reach is per surface). `baked` gains booleans `native_step` (true only with `wrap`) and `suggest_sentence` (true only with `suggest`). One test per rule. +2. **Red, parity.** Forward parity for `native_step` looks for the literal `## Native step: ()` heading in the component body; forward parity for `suggest_sentence` looks for the suggest sentence shape in the body. Reverse parity gains a body scan for the suggest sentence shape only (`If /` … `is available in your session (`), with the `claude-ops:changelog` prose line as a negative fixture, so a body carrying the shape with no store row is an orphan. +3. **Green.** Render the view with an `Integration` column in the summary table and an `Integration:` line per row. +4. **Convention, one major bump.** Add the `wrap` grammar: the body section `## Native step: ()` carrying, in order, the gate token; the identity check by class (bundled: name in the listing, invoke by alias where the Skill tool resolves one, advisory description check; a description that reads as a different surface is a likely user or project shadow, so skip with a warning; a name with no description, which `name-only` and budget overflow both produce, is invoked with a stated "identity confirmed by name alone" warning, matching the playgrounds precedent; plugin: namespaced form plus marketplace provenance when the CLI resolves); the mutation clause (a mutating surface may be wrapped only where the wrapping skill's own contract mutates the same thing, the invocation passes an explicit scope or report-only argument, the skill fingerprints what the surface may write before the step and diffs after, and an unexpected diff is reported as a fourth state, "mutation detected after a scoped invocation", with the run exiting degraded); the invocation form; what our part adds before or after; the skip-and-report contract for the states `did not resolve in this session`, invocation refused by the tool or permissions (the report names the refusal reason and never retries: not in the session's skills allowlist, disabled for model invocation by `disableBundledSkills` or `skillOverrides`, or a permission deny, which is alias-aware, so `Skill(review)` blocks the bundled `code-review`), identity mismatch, and a fifth state, resolved but degraded (the native surface ran a weaker procedure and said so, as `simplify` does when the Agent tool is absent and it falls back to a single pass; the wrapper relays the surface's own disclosure into its report and never restates the step as the documented full procedure), each naming the axis line and the enable path; a note that the wrapped body enters context once and stays there, so a Native step is priced like any other body load; and the `unattended` argument, declared by the caller, under which the skill records instead of asks and never invokes a mutating surface. Add the `suggest` grammar: the sentence shape above, addressed to the person, where `` is a same-file four-part verification record per upstream-drift that names the surface's own gate (for `/doctor`, `DISABLE_DOCTOR_COMMAND` or a `skillOverrides` entry, since it is the one registration that survives `disableBundledSkills`; for every other bundled skill, `disableBundledSkills` as well), placed at the start of the run when coverage is total and at the end when partial, and where a model-disabled bundled skill is suggested as `/` typed by the person exactly as a built-in command is; the wording is "reserved for the person to run", never "cannot be invoked" as an absolute; unattended runs record it in output. State explicitly that the suggest token `is available in your session (` differs from the route token by design (the route token is a condition the model observes in its listing, the suggest token one the person checks), that it is not the rejected assertion phrasing `always available`, and that parity keys on the sentence shape. Add the class table with the `marketplace-plugin` row naming seam-phrasing as the owner of that class's wrap grammar and the model-disabled bundled-skill row (`suggest` only). Amend the one-owner rule: one owning description phrase per plugin per surface stays; a second skill in the same plugin carries a Native step or suggest section with a same-plugin pointer to the owner's Boundary, never a second phrase; the rule governs description phrases only, so sibling skills may each carry a suggest sentence for the same surface. Flip the Enforceability row for the gate-token check to built (Phase 3 built it). This is a major version of the convention on the convention's own trigger list: an enforceability verdict changes and the canonical-token clause gains a second token; the one-owner amendment in the Boundary section contributes. The CHANGELOG entry cites those triggers, not the Boundary section alone. +5. **Re-derive observations.** Run `detect` on the Phase 1 extraction and refresh `observation.detail` and `recheck.verified` to 2.1.263 for every extraction-class row whose native surface the extraction resolves and whose evidence still holds, so the version advisory clears; rows whose evidence moved keep their old record and gain an evidence line saying what moved; every bundled-skill row gains or drops the `model-invocation-disabled` marker from the registration its evidence names. Reason-text corrections in the same pass: the `export` row's "built-ins are user-invoked only" becomes the per-surface reason (`/export` and `/skill-doctor` are non-prompt command types the Skill tool never lists; `/init` and `/security-review` are prompt-typed and reachable); the `security-review` row's reason drops any `allowed-tools` basis for the CI-harness and `origin/HEAD` basis; the `code-review` row's reason rests on cost and shape (the native surface fans out its own agents; the CI lane cannot be typed into a session), not reach; the two `design` rows name the canvas registration, the hub registration, and the `local` access command, with the recheck trigger widened to "which registrar wins `isEnabled`"; the `morning` row gains this session's roster as a second live observation; the `skill-doctor` row notes the native report excludes bundled skills. This step runs before step 6, since the `design` conclusions change under a 2.1.263 read. +6. **Verdict pass, values confirmed by the user on 2026-09-11 and validated by the research pass; the PR body quotes this table and the user re-confirms it there.** `wrap` for `run` (run-e2e) and `simplify` (batch-simplify); `suggest` for all three `doctor` rows (audit-install-state and audit-skill-visibility own a sentence each; audit-performance carries a pointer-carrying sentence, since a `route` row cannot carry the sentence Phase 5 writes it and a route phrase on a model-disabled surface is dead text), for `skill-doctor`, and for the three `export` rows; `route` for `code-review`, `security-review`, `morning`, the two `playground` rows, the two `design` rows (on job merits, not reach; `wrap` is admissible and stays a Phase 9 and 10 revisit), `design-sync`, and `tidy` (`simplify` works the current diff, tidy works a rotated lane of resting code with no diff, and `simplify` applies fixes where tidy is structure-only). Two new rows: `export` against `session-flow:handoff` and `export` against `session-flow:retro`, both `suggest`, because each body carries a suggest sentence and every baked line must trace to a row. Each row's evidence gains the substrate observations from the 2026-09-08 report where they apply (`skill-doctor` gate basis; `design` name collision) and the `budget_caveat` note that `audit-performance` and `audit-skill-visibility` have under 60 characters of description headroom. +7. **Apply step.** Update the skill body: preconditions add "the row's `integration` is not `route` when a Native step or suggest sentence is to be written" and "the row does not carry `model-invocation-disabled` when a Native step is to be written"; the emitted artefacts add the Native step section and the suggest sentence; the per-entry cap precheck is a hard precondition (`description` plus `when_to_use` after baking at most 1,536 characters, measured by `bash plugins/skill-quality/scripts/check-skill.sh `); every wrapped or suggesting skill declares the `unattended` argument in its `argument-hint`. +8. Bump `claude-ops` to the next minor at PR time; draft PR; ready when green. + +**Sanity Check:** + +- `python3 overlap.py self-check --upstream-sha ed404106fcd80ba98ecb7c851e531dcb626d13b7 --upstream-sha d7dbd9a09f59775726ed14bbea8fc9dfdff62f7b` exits 0 with no problems and no advisories; `jq '[.rows[] | select(.integration == null)] | length' docs/native-surfaces/records.json` prints 0; `jq '.rows | length' docs/native-surfaces/records.json` prints 18; `python3 overlap.py generate --check` exits 0. +- `grep -c "## Native step: ()" docs/conventions/native-references/README.md` prints at least 1; `grep -c "is available in your session (" docs/conventions/native-references/README.md` prints at least 1; `grep -c "marketplace-plugin" docs/conventions/native-references/README.md` prints at least 1; `grep -c "model-invocation-disabled" docs/conventions/native-references/README.md` prints at least 1. +- `jq '[.rows[] | select(.native.class=="builtin-command" and .integration=="wrap")] | length' records.json` prints 0; `jq '[.rows[] | select((.native.markers | index("model-invocation-disabled")) and .integration!="suggest")] | length' records.json` prints 0; `jq '[.rows[] | select(.native.name=="design" and (.native.markers | index("model-invocation-disabled")))] | length' records.json` prints 0; `jq '[.rows[] | select(.verdict=="defer" and .integration!="route")] | length' records.json` prints 0; `jq '[.rows[] | select(.integration=="suggest")] | length' records.json` prints 7 and `jq '[.rows[] | select(.integration=="wrap")] | length' records.json` prints 2. + +### Phase 5: Sweep unit, claude-ops [TODO] + +Files: `plugins/claude-ops/skills/audit-install-state/SKILL.md`, `audit-skill-visibility/SKILL.md`, `audit-performance/SKILL.md` (MODIFY), `records.json` and the view (baked flags), `plugins/claude-ops/.claude-plugin/plugin.json` and `CHANGELOG.md` (MODIFY). `morning-brief` is KEEP (defer row). + +A suggest unit, confirmed 2026-09-11: `doctor` is model-disabled, so no skill in this plugin can compose it, and all three `doctor` rows take `suggest`. The suggest bases name `DISABLE_DOCTOR_COMMAND` and the `skillOverrides` entry as the gate, not `disableBundledSkills`, which `/doctor` alone survives. + +1. `audit-install-state`: keep the description phrase; replace the Boundary section's routing paragraph with the suggest sentence for `/doctor` at the end of the run (coverage is partial: doctor fixes, this skill inventories), its basis pointing at the section's existing verification record; declare `unattended` and record the suggestion under it. +2. `audit-skill-visibility`: a suggest sentence for `/skill-doctor` at the start of the run (its coverage of "unused versus cost" is the whole ask when the user wants only that list) and one for `/doctor` at the end, both with bases pointing at the skill's existing verification record; declare `unattended`. +3. `audit-performance`: no description change. A suggest sentence for `/doctor` at the end of the run that opens with a same-plugin pointer to audit-install-state's section for the shared surface facts; declare `unattended`. +4. Set `baked.suggest_sentence` on the four rows; regenerate the view; run the skill-quality check per touched skill; bump to the next minor; draft PR quoting the four store rows; ready when green. + +**Sanity Check:** + +- `bash plugins/skill-quality/scripts/check-skill.sh ` passes for each of the three skills (its per-entry cap check covers the description length). +- `python3 overlap.py self-check --upstream-sha ed404106fcd80ba98ecb7c851e531dcb626d13b7 --upstream-sha d7dbd9a09f59775726ed14bbea8fc9dfdff62f7b` exits 0; `grep -c "If /doctor is available in your session (" plugins/claude-ops/skills/audit-install-state/SKILL.md` prints 1; `grep -c "unattended" plugins/claude-ops/skills/audit-install-state/SKILL.md` prints at least 2 (argument-hint and body). +- In this cloud session, invoking `/claude-ops:audit-install-state unattended` yields a report whose final section records the `/doctor` suggestion without asking; the transcript excerpt is quoted in the PR body. + +### Phase 6: Sweep unit, code-tidying [TODO] + +Files: `plugins/code-tidying/skills/tidy/SKILL.md`, `batch-simplify/SKILL.md` (MODIFY), `records.json` and the view, `plugins/code-tidying/.claude-plugin/plugin.json` and `CHANGELOG.md`. + +The first wrap unit. `simplify` is model-invocable and mutates the working tree, which is batch-simplify's own contract, so the mutation clause is satisfied by scope: the invocation names the file set, the pre-step fingerprint is the set of tracked files outside that scope, and any change outside it is the fourth state. + +1. `batch-simplify`: `## Native step: simplify (bundled skill)` invoking `simplify` over the scope the Phase 4 pre-flight established when it resolves, our batching and ordering around it; the three-state skip report; `unattended` declared, under which the step still runs because the skill's own contract already edits files unattended. +2. `tidy`: phrase only unless the human gate chose `wrap`; if `wrap`, the Native step runs `simplify` on the lane's changed files after the structural tidyings and states that the resulting PR is no longer structure-only. +3. Baked flags, view, skill-quality check, next minor, draft PR, ready when green. + +**Sanity Check:** + +- skill-quality check passes for both skills; `python3 overlap.py self-check --upstream-sha ed404106fcd80ba98ecb7c851e531dcb626d13b7 --upstream-sha d7dbd9a09f59775726ed14bbea8fc9dfdff62f7b` exits 0; `grep -c "## Native step: simplify (bundled skill)" plugins/code-tidying/skills/batch-simplify/SKILL.md` prints 1. +- Positive path in this cloud session, where `simplify` resolves: invoking `/code-tidying:batch-simplify` over a small changed set yields a report with a `Native step` result block and no change outside the named scope; negative path on a host with `disableBundledSkills` set: the report carries `did not resolve in this session` and the axis line; both transcript excerpts are quoted in the PR body. + +### Phase 7: Sweep unit, testing [TODO] + +Files: `plugins/testing/skills/run-e2e/SKILL.md` (MODIFY), `records.json` and the view, `plugins/testing/.claude-plugin/plugin.json` and `CHANGELOG.md`. + +1. `run-e2e`: phrase plus `## Native step: run (bundled skill)` that launches the app through `run` when it resolves and layers evidence capture (screenshots, responses, logs) on top; the three-state skip report falling back to the skill's own launch playbook; `unattended` declared. `run` starts processes rather than editing files, so the mutation clause's fingerprint is the tracked tree, which must be unchanged after the step. Identity check: the official skill precedence lets a project skill named `run` shadow the bundled one, and the bundled `run` itself defers to a project skill that covers launching the app, so a listed `run` whose description reads as a project launcher is a legitimate target, not a mismatch; the step records which it invoked. +2. Baked flags, view, skill-quality check, next minor, draft PR, ready when green. + +**Sanity Check:** + +- skill-quality check passes; self-check with the pinned SHA exits 0; `grep -c "settings or environment, plan, platform or provider, host surface" plugins/testing/skills/run-e2e/SKILL.md` prints at least 1; positive and negative transcript excerpts quoted in the PR body. + +### Phase 8: Sweep unit, review [TODO] + +Files: `plugins/review/skills/code-review/SKILL.md`, `plugins/review/skills/security-review/SKILL.md` (MODIFY, phrase only), `records.json` and the view, `plugins/review/.claude-plugin/plugin.json` and `CHANGELOG.md`. + +1. Both skills: the route phrase only, per the Phase 4 pre-flight (c). No `allowed-tools` change. +2. Baked flags, view, skill-quality check, next minor, draft PR, ready when green. + +**Sanity Check:** + +- skill-quality check passes for both; self-check with the pinned SHA exits 0; `jq -r '.rows[] | select(.component.plugin=="review") | .integration' records.json` prints `route` twice. + +### Phase 9: Sweep unit, visualization [TODO] + +Files: `plugins/visualization/skills/visualize/SKILL.md` (MODIFY), `records.json` and the view, `plugins/visualization/.claude-plugin/plugin.json` and `CHANGELOG.md`. + +1. Bring the description's "where the bundled design skill is available" under the gate token so the reverse-parity advisory from Phase 3 closes; set `baked.description_phrase`. +2. The row is `route` (confirmed 2026-09-11): the design canvas skill is model-invocable, so the gate-token phrase is live, and the existing Boundary is kept with its evidence naming the canvas registration, the model-disabled hub registration, and the `local` access command. `wrap` is admissible here and is the one revisit this phase may raise before it starts: a Native step would only make sense if visualize gained a sub-step the canvas skill performs, which today it does not. The `design-sync` row stays `defer`. +3. Baked flags, view, skill-quality check, next minor, draft PR, ready when green. + +**Sanity Check:** + +- `python3 overlap.py self-check --upstream-sha ed404106fcd80ba98ecb7c851e531dcb626d13b7 2>&1 | grep -c "visualization:visualize"` prints 0; skill-quality check passes. + +### Phase 10: Sweep unit, prototype [TODO] + +Files: `plugins/prototype/skills/explore-directions/SKILL.md` (MODIFY), `records.json` and the view, `plugins/prototype/.claude-plugin/plugin.json` and `CHANGELOG.md`. + +1. Same treatment as Phase 9 for the `design` row; the existing `playground` phrase (`installed from its marketplace`) is untouched. +2. Baked flags, view, skill-quality check, next minor, draft PR, ready when green. + +**Sanity Check:** + +- `python3 overlap.py self-check --upstream-sha ed404106fcd80ba98ecb7c851e531dcb626d13b7 2>&1 | grep -c "prototype:explore-directions"` prints 0; skill-quality check passes. + +### Phase 11: Sweep unit, session-flow [TODO] + +Files: `plugins/session-flow/skills/clean-stop/SKILL.md`, `handoff/SKILL.md`, `retro/SKILL.md` (MODIFY), `records.json` and the view, `plugins/session-flow/.claude-plugin/plugin.json` and `CHANGELOG.md`. + +1. Re-phrase the three existing `/export` suggestion sites to the `suggest` sentence shape (the basis pointing at a same-file verification record, end-of-run placement, which is where they already sit); each of the three skills declares `unattended` and records the suggestion in output under it. +2. Set `baked.suggest_sentence` on the three `export` rows (clean-stop, handoff, retro); `integration: suggest` was written in Phase 4. +3. View, skill-quality check, next minor, draft PR, ready when green. + +**Sanity Check:** + +- `grep -c "If /export is available in your session (" plugins/session-flow/skills/clean-stop/SKILL.md plugins/session-flow/skills/handoff/SKILL.md plugins/session-flow/skills/retro/SKILL.md` prints 1 or more for each file; skill-quality check passes for the three skills; self-check with the pinned SHA exits 0 with no orphan advisory. + +### Files affected (whole plan) + +| File | Action | Phase | +|---|---|---| +| `plugins/claude-ops/skills/inventory/scripts/inventory.py` | MODIFY | 1, 2, 3 | +| `plugins/claude-ops/skills/inventory/scripts/test_inventory.py` | MODIFY | 1, 2 | +| `plugins/claude-ops/skills/inventory/reference/extraction.md` | MODIFY | 2 | +| `plugins/claude-ops/skills/inventory/evals/evals.json` | MODIFY if an expectation names the old build or wording | 2, 3 | +| `plugins/claude-ops/skills/inventory/SKILL.md` | MODIFY | 2 | +| `plugins/claude-ops/skills/audit-native-overlap/scripts/overlap.py` | MODIFY | 2, 3, 4 | +| `plugins/claude-ops/skills/audit-native-overlap/scripts/test_overlap.py` | MODIFY | 2, 3, 4 | +| `plugins/claude-ops/skills/audit-native-overlap/reference/canonical-pairs.json` | MODIFY | 3 | +| `plugins/claude-ops/skills/audit-native-overlap/SKILL.md` | MODIFY | 2, 4 | +| `docs/native-surfaces/records.json` | MODIFY | 4, 5 to 11 | +| `docs/NATIVE-SURFACES.md` | REGENERATE | 4, 5 to 11 | +| `docs/conventions/native-references/README.md`, `CHANGELOG.md` | MODIFY (one major bump) | 4 | +| `plugins/claude-ops/.claude-plugin/plugin.json`, `CHANGELOG.md` | MODIFY | 3, 4, 5 | +| `plugins/claude-ops/skills/{audit-install-state,audit-skill-visibility,audit-performance}/SKILL.md` | MODIFY | 5 | +| `plugins/claude-ops/skills/morning-brief/SKILL.md` | KEEP | 5 | +| `plugins/code-tidying/skills/{tidy,batch-simplify}/SKILL.md` plus manifest and CHANGELOG | MODIFY | 6 | +| `plugins/testing/skills/run-e2e/SKILL.md` plus manifest and CHANGELOG | MODIFY | 7 | +| `plugins/review/skills/{code-review,security-review}/SKILL.md` plus manifest and CHANGELOG | MODIFY (phrase only) | 8 | +| `plugins/visualization/skills/visualize/SKILL.md` plus manifest and CHANGELOG | MODIFY | 9 | +| `plugins/prototype/skills/explore-directions/SKILL.md` plus manifest and CHANGELOG | MODIFY | 10 | +| `plugins/session-flow/skills/{clean-stop,handoff,retro}/SKILL.md` plus manifest and CHANGELOG | MODIFY | 11 | +| `scripts/validate-plugins.sh` | KEEP (exit-code contract unchanged) | 2 | + +### Test strategy + +TDD throughout, Red-Green-Refactor per work item, with Red items restricted to behaviour that does not exist yet (the Phase 2 exit mappings already exist and are not claimed as Red). Test boundaries, all existing public interfaces: + +- `inventory.py` functions `read_bundle` (first tests, the regression boundary for the actual bug), `extract_bundled_skills`, `discover_registrar`, `build_const_map`, and `check_integrity`, driven by `test_inventory.py` with synthetic bundles (existing pattern) plus the fragmented-run, PE-container, legacy, below-floor, three-registrar-route, cross-chunk-constant, and phantom-name fixtures; the real binary is the Phase 1 sanity check, not a unit test. +- `overlap.py` functions `validate_row`, `check_baked_parity`, `cmd_detect`, `cmd_self_check`, `render_view`, driven by `test_overlap.py`'s `TempRepo` fixture (existing pattern), with the synthetic collision, marker, and suggest-shape fixtures added. +- Skill bodies are verified by `plugins/skill-quality/scripts/check-skill.sh` per touched skill and by `overlap.py self-check` parity, both deterministic. +- Behavioural criteria are verified twice per wrap unit: the positive path (surface resolves, native result reported, no change outside scope) in this cloud session for `simplify` and `run`, which resolve here, and the negative path on a host with `disableBundledSkills` set, with both transcript excerpts quoted in the PR body. Suggest units verify the unattended recording path in this session. +- `scripts/affected-tests.sh --run` closes every unit before its PR. + +Test type per change: unit for the Python engines, contract for the store and view, static for skill bodies, one manual runtime probe per path per unit for the composition and degradation behaviour. No test is skipped, disabled, or quarantined to reach green. + +### Alternatives considered + +- **Canary-derived registrar as the primary route.** Rejected after the second probe: the ESM export list names the registrar directly and survives a reordering of the `doctor` object; the canary stays as the third route. Switch condition: a build with neither the CJS getter nor an ESM export list. +- **A 64 KiB run floor.** Rejected: measured on the real binary it recovers 13 of 33 names and the regex takes minutes. Switch condition: a build that stops fragmenting the bundle. +- **Fold the policy unit into the tooling-fix PR.** Rejected: mixes a repair with a routing-policy change and hides the human verdict pass inside a bug fix. Switch condition: the user prefers one review over two. +- **Reuse the `resolves in your session` token for `suggest`.** Rejected: a built-in command or a model-disabled skill never appears in the model's listing, so the token's read-time meaning does not hold; the suggest sentence is addressed to the person who can check. Switch condition: Claude Code starts listing those surfaces to the model. +- **Treat a name-only listing as "not resolving".** Rejected: the model cannot tell `skillOverrides: name-only` from a budget-dropped description, and the playgrounds precedent already rules that a present name is not evidence of absence. Switch condition: the listing starts marking overridden entries. +- **Wrap `doctor` anyway through a report-only argument.** Rejected: the registration disables model invocation, so no skill can reach it regardless of arguments. Switch condition: a release flips the flag, which the Phase 1 field and the row's recheck trigger will show. The binary carries an undocumented exception (a skill the person typed by name in the same turn may be invoked from the main session), which the plan does not design on. +- **Wrap the design canvas on visualize and explore-directions.** Admissible after the 2026-09-11 finding that the canvas registration is model-invocable; rejected on job merits, because both components already own the artifact-producing job and have no sub-step the canvas skill performs. Switch condition: the user chooses `wrap` at Phase 9 or 10, or either component gains such a sub-step. +- **One gate token for both grammars.** Rejected: the route token names a condition the model observes in its listing and the suggest token one the person checks; parity keys on the sentence shape either way. The near-collision with the rejected phrasing `always available` is an accepted authoring hazard the convention names. Switch condition: the shape-keyed parity proves insufficient in practice. +- **Move #4049 to `work-class: scoped` and `agent-ready` once the verdicts are recorded.** Rejected by both validators on the repository's own definitions: the unit adds a required field to the store and takes a major version of a convention that calls itself a contract, which is C4 by `work-classes.md`, and a C4 item never carries `agent-ready`. A carve-out of the data work would be feature-shaped C3 and still human-gated at the work-loop admission gate. Switch condition: the admission gate admits feature-shaped C3, or the convention bump leaves the unit. +- **Wrap every bundled row uniformly, no human pass.** Rejected by the Brief. +- **Run the sweep as parallel per-plugin agents.** Rejected: the sweep contract forbids two units in flight because description edits are routing-affecting. Switch condition: the contract is amended. + +### Risks and mitigations + +- **A later CLI release changes the registration shape again.** Three discovery routes plus per-lane integrity make the failure a named broken lane, not a silent short list; the registrar-export advisory now sees the ESM form. Mitigation is the existing eval re-validation trigger. +- **A wrapped mutating surface writes outside its scope.** The mutation clause's fingerprint-and-diff makes that a reported fourth state; `unattended` never invokes a mutating surface. `[FALLBACK, confirm or override]`: on a detected out-of-scope change the run exits degraded and the unit's PR body records it. +- **The Skill tool refuses the invocation headless or in a subagent.** The three-state skip report makes a refusal a reported state, not a silent one; `AskUserQuestion` is absent in subagents and under `--permission-prompts none`, so a wrapped skill never depends on the native surface asking a question. +- **The reverse-parity heuristic produces false positives on prose.** It is an advisory, keyed on a class word plus `skill|command` inside a presence clause, with negative fixtures for seam-phrasing and Not-for clauses; the suggest scan keys on the full sentence shape. +- **Nine sequential PRs take weeks.** Accepted by the Brief; the parent issue tracks progress and each unit is independently valuable. + +## Blast radius + +MEDIUM. Files: over 30 across eight plugins and one convention at a major bump. Other sessions: the store, the generated view, `inventory.py --self-check`, and `validate-plugins.sh` are shared CI surfaces; every change is additive and the exit-code contracts are unchanged. Reversible by git revert per unit. Two stress-test triggers matched: a new convention grammar (constrains future skill authoring) and a multi-step implementation touching undocumented binary layout. + +## Stress-test summary + +Fresh-context plan review (Step 3): 1 critical, 14 important, 8 suggestions; every finding verified against the code or docs and folded in. Devil's-advocate pass (Step 4): 2 critical, 6 high, 5 medium, 4 low; every finding re-verified on the binary and the repository before it was applied. The two critical findings changed the plan's substance: the 64 KiB region rule was replaced by a 256-byte rule with chunk-scoped constant resolution and the ESM export list as the primary registrar route, and the discovery that `doctor` and `design` are registered model-disabled moved their rows from `wrap` to `suggest` or `route`, which is now a USER-RESERVED question because it changes the Brief's wrap-candidate assumption. One research-iterate loop ran (a second binary probe); no third was needed. Round-4 validation (2026-09-11): two independent fresh-context validators (Fable 5.1, Opus) checked the seven policy decisions against the official references, the binary, and the conventions; both confirmed every integration value, both rejected the `agent-ready` flip for #4049, and both found that `design` is two bundled registrations (the canvas is model-invocable) and that `allowed-tools` never restricts reach; those corrections are folded into Phases 1, 4, 5, 7, 9 and 10 and the Brief's third scope note. Reports: scratchpad `validation-fable.md` and `validation-opus.md`, quoted in the #4049 body. + +## Execution shape + +Fully sequential. Phase 0 gates everything; Phases 1 to 3 share `inventory.py` and `overlap.py`; Phase 4 gates every sweep unit because it writes the `integration` values and runs every pre-flight; Phases 5 to 11 are serialized by the sweep contract. No parallel wave is recommended. `[EXEC-SHAPE]` + +| Phase | Surface | Basis | +|---|---|---| +| 0 | main session via `/work-items:decompose` | tracker write with a search-before-create pivot | +| 1 to 3 | main session | judgment-heavy: binary layout, integrity semantics, evals | +| 4 | main session | pre-flights and the human verdict pass row by row | +| 5 to 11 | main session, or one sub-agent worker per unit with the unit's SKILL.md files as its ALLOWED list and `records.json`, `PLAN.md`, other plugins FORBIDDEN | mechanical per unit once the grammar exists; still one unit at a time; the runtime probes stay main-session because subagents lack the question tool | + +Sequential fallback: not applicable, the shape is already sequential. + +## Open questions + +- Resolved 2026-09-11 (round-4 re-interview, user accepted all recommendations conditioned on a two-validator research pass, which confirmed every value and amended four bases; ledger `.work/audit-native-overlap/interview-checklist.md`): the three `doctor` rows take `suggest`; the two `design` rows and `design-sync` take `route`, the `design` rows on job merits because the canvas registration is model-invocable; `tidy` takes `route`; `simplify` on batch-simplify and `run` on run-e2e take `wrap`; the remaining rows take the bundle values in Phase 4 step 6; the convention change is a major bump on the enforceability and token triggers; #4049 stays `work-class: structural` and `needs-human` with the verdict table recorded in its body as the C4 plan approval. The `name-only` coverage case stays as downgraded (name present, description absent, invoke with a warning). **arbiter: USER, validated** +- None open. + +## Handoff to implementation + +### User-approval gates + +- Every `integration` value written in Phase 4 (confirmed by the user on 2026-09-11 and validated; the PR body quotes the table and the user re-confirms it there as the C4 plan approval). #4049 stays human-gated: `work-class: structural`, `needs-human`, human review and human merge. +- The mutation-clause fourth state when it fires in a wrap unit. +- Creating each sweep sub-issue's PR only after its predecessor unit closes. + +### Execution shape ([EXEC-SHAPE] tagged) + +- Policy schema as its own unit (Phase 4) rather than inside the tooling fix. +- The suggest sentence shape and its parity keyed on the shape, not the bare phrase. +- `baked.native_step` and `baked.suggest_sentence` as the third and fourth baked flags; `model-invocation-disabled` as a third native marker set from the extraction. +- Two new store rows for the `export` suggestion sites in `handoff` and `retro`, so parity is enforceable. +- The amended one-owner rule: pointer-carrying Native step or suggest sections in sibling skills, never a second phrase. +- One major bump of the native-references convention in Phase 4, carrying the Phase 3 enforceability flip. +- Unit order after claude-ops: code-tidying, testing, review, visualization, prototype, session-flow. +- "Next minor at PR time" for every touched plugin, matching claude-ops's one-version-per-PR CHANGELOG practice. +- The Phase 1 region rule (first marker to end of file, runs of at least 256 bytes, one regex pass), registrar route order (CJS getter, ESM export, canary), and chunk-scoped constant resolution. +- Sub-issues published all at once with dependency edges, blocked, rather than created one at a time. + +### Mechanical work + +- One draft PR per unit, body per the PR-body contract, closing keyword pointing at the unit's sub-issue, affected store rows quoted. +- Before each push: `scripts/affected-tests.sh --run`, `python3 overlap.py self-check --upstream-sha `, `python3 overlap.py generate --check`, `check-skill.sh` per touched skill, markdownlint on touched markdown. +- The contract slice `docs/topics/audit-native-overlap/` is pruned in the final commit of the last unit's PR per the topic-docs convention. +- Phase tags advance `[TODO]` to `[DOING]` to `[DONE]` in this file, edited only from the main session. diff --git a/plugins/claude-ops/.claude-plugin/plugin.json b/plugins/claude-ops/.claude-plugin/plugin.json index 8e134d5d28..149a268e5e 100644 --- a/plugins/claude-ops/.claude-plugin/plugin.json +++ b/plugins/claude-ops/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "claude-ops", - "version": "0.53.0", + "version": "0.54.0", "description": "Claude Code operations toolkit. Twelve skills: audit-skill-visibility (audit whether each installed skill is actually VISIBLE to the model, and diagnose why most of a fleet never gets used: a skill is invisible when its description is dropped by Claude Code's skill-listing context budget, which sheds descriptions lowest-score-first so an unused skill loses the keywords that would let it be matched, from skills genuinely not wanted, from skills the run cannot observe at all; computes whether the listing overflows from documented settings, and withholds every cold verdict the data cannot support rather than reporting absence of data as absence of use), inventory (read-only enumeration of the complete invocable surface: every built-in CLI command with aliases and hidden/gated status, every bundled skill, and every component of every installed plugin across all marketplaces; reads the shipped binary because upstream publishes no built-in command list, and carries an integrity verdict so a drifted build reports counts as floors rather than silently short totals), audit-install-state (read-only audit of the machine-scope ~/.claude installation directory and ~/.claude.json: full inventory split into an authored surface and rolled-up bulk trees, product-managed retention vs genuinely unmanaged state, filename-scheme resolution before any process-liveness check, and deliberate/mid-experiment detection; reports, never deletes), audit-performance (read-only slowness-diagnostic capture run at the moment the machine or a session feels slow: CLI version, retention-sweep health including the silent unparsable-settings pause, a timed census walk of the install tree as a sweep-cost proxy, active-session and plugin-fleet counts, a process census, and the fan-out layer, which covers a load-labelled no-op spawn baseline, every hook that will fire bucketed per-tool-call versus per-turn with its invocation shape, the configured statusline, subagent concurrency and spawn-depth ceilings against documented defaults, whether running sessions predate the settings file they are judged by, and orphan attribution by parent liveness rather than age, plus on Windows a kernel-object census (Token objects against uptime, paged pool) that names a host-level leak beneath all four suspects; read against a bundled known-performance-issues reference that also records the causes tested and cleared; separates the four documented suspects of accumulated state, version regression, component bloat, and per-spawn fan-out cost, and routes remediation out; reports, never mutates, and never executes a discovered hook or statusline command), audit-native-overlap (map native Claude Code surfaces, namely built-in CLI commands, bundled skills, plugin-backed built-ins, and session-provided skills, against the current repo's plugin skills and agents, so a custom component never silently duplicates what Claude Code itself ships; bare invocation is a read-only overlap report carrying the extraction's integrity floors and a shared-listing-budget exposure section, verdicts are human-gated in a committed store rendered into a generated registry whose every row carries an observable recheck trigger, and only an explicit apply step bakes presence-gated native references into descriptions and Boundary sections), observability (read locally captured telemetry from the OTEL store, the collector, the per-session hook event log and hook-event JSONL, and ccusage, with trend reports, a per-session report of what fired, what was blocked and the event timeline, and store pruning), known-issues (search known Claude product GitHub bugs, check service health, maintain a persistent tracked-issue registry), changelog (ingest Claude Code changelog entries and integrate them into the current repo), plugins (bring a machine's plugin fleet current on demand: marketplace refresh, effective-scope updates including in-repo project/local installs, new-plugin install per policy, scope-divergence detection and explicit convergence), morning-brief (read-only gh-based operator morning view: queue-label counts, merge-ready PRs, parked decisions with their RECOMMENDED lines, and loop-lane telemetry freshness), lanes (start/restart/stop/status loop lanes as named background Claude Code sessions seeded from canonical prompt files, with per-lane model/effort, a repo-pull + marketplace-refresh launch step, and a consume-restarts action, an OS-schedulable reader that relaunches stopped lanes whose telemetry carries a restart_request), and a re-runnable setup action that settles where the known-issues registry, the skill-usage log and the hook log root live, places the root's self-ignoring guard, and detects retired conventions. Plus an opt-in, default-off per-session hook event log (one JSON line per hook event on every event the generated registry marks observable, written to /sessions/.jsonl, with SessionEnd retention by session count or age and an optional detached pre-prune command), a family of eight advisory *-audit hooks (API errors, config changes, instruction loads, permission denials, pre-compaction, skill usage, tool failures, and unsurfaced hook failures. The last also warns the user via systemMessage, since a hook that fails to launch enforces nothing and Claude Code surfaces the failure to nobody) that emit the shared hook-telemetry envelope, and a reference sink that routes envelopes under the same root: per session when the envelope carries a session id, else into the shared hook-events.jsonl the observability skill reads.", "author": { "name": "Melodic Software", diff --git a/plugins/claude-ops/CHANGELOG.md b/plugins/claude-ops/CHANGELOG.md index f88c456ecf..514784c703 100644 --- a/plugins/claude-ops/CHANGELOG.md +++ b/plugins/claude-ops/CHANGELOG.md @@ -3,6 +3,63 @@ All notable changes to the `claude-ops` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.54.0] + +### Added + +- **`inventory.py` reads a bytecode-fragmented bundle.** Region rule: from the first bundle marker + to end of file, every printable run of at least 256 bytes, joined with newlines, in one regex + pass; `sources.binary` records `runs`, `joined_bytes`, `region_rule`, `runs_below_floor` + (registration tokens sitting under the floor, counted rather than lost, and degrading the + bundled-skill lane when positive), and `elapsed_seconds`. A build with no marker keeps the + largest-run fallback. +- **Three registrar discovery routes.** The CJS getter, then the ESM export list + (` as registerBundledSkill`), then the canary registration; `bundled_skill_notes` records + `registrar_route`. The registrar-shaped-export advisory sees both export shapes, and the known + set gains `registerDesignCanvasSkill` and `registerWorkflowAuthoringSkill`. +- **Computed names resolve by locality.** A hoisted constant resolves to its nearest preceding + binding, never a farther one, so an unrelated module's binding of the same identifier cannot + shadow the real one; a single-character identifier is trusted only within a locality window; a + loop or template-literal registration is a `dynamic_roster` note, not an unresolved name; a call + whose object carries no `name:` is another module's function and is counted apart. +- **Per-registration invocation fields.** `user_invocable`, `disable_model_invocation`, + `terminal_oriented`, and `survives_kill_switch` when present; a function-valued field reads as + true with the key listed under `flag_driven`. +- **Same-name registrations are both kept.** Two distinct bundled registrations sharing a name are + a list under that name with `collision: true` and are named in `bundled_skill_notes.collisions`; + `registrations_of(entry)` reads either shape. +- **Integrity per lane.** `integrity.lanes` carries `builtin_commands`, `bundled_skills`, and + `plugin_backed`, each with its own status, problems, and advisories; the top-level status is the + worst lane, and top-level `broken` means every lane is broken or the binary is unreadable, so one + broken lane is a named `degraded` rather than a run with no counts. `plugin_backed` gains a canary + (`security-review`). Exit mappings are unchanged in both `inventory.py --self-check` (which now + prints each lane) and `overlap.py detect`. +- **`overlap.py detect` reads the lanes.** The candidate report carries per-lane floors (a lane's + counts are totals only when the lane is ok and no run-wide advisory such as an unvalidated CLI + version stands; a lane-attributed advisory degrades only its own lane), and every + candidate carries `re_derivable`: false when the lane its seeded or observed class maps to is + broken (both directions on a class collision), null for session-provided and marketplace classes, + which have no lane. A name collision lists every registration with its invocation mode. An + inventory without `lanes` keeps the previous reading. +- **`overlap.py self-check` flags a presence-gated native mention without the gate token.** A + description that names a native surface by class and kind inside a `when`, `where`, or `if` + clause with an availability word, and carries no gate token in that clause, is an advisory (exit + 3) naming the row to add or the token to use. Judged per clause, so a gated marketplace clause + never excuses an ungated native clause beside it. +- **`--upstream-sha` repeats.** One value per upstream repository the store cites; a recorded + commit matches when any provided value matches it. +- **Seeded pairs** gain `doctor` and `skill-doctor` against `claude-ops:audit-skill-visibility`. + +### Changed + +- `VALIDATED_AGAINST` is `2.1.263`, validated on the Linux ELF build in a container; the PE + container path is covered by a byte-layout fixture, not a Windows run. +- The audit-native-overlap description is under the 1,024-character Skills API cap: the same + claims and every trigger phrase, in fewer words. +- The inventory skill's integrity table, the audit-native-overlap detection posture, and + `reference/extraction.md` describe the lanes, the region rule, the locality rule, and the + collision list; the inventory eval for degraded counts names dynamic rosters and lane statuses. + ## [0.53.0] ### Added diff --git a/plugins/claude-ops/skills/audit-native-overlap/SKILL.md b/plugins/claude-ops/skills/audit-native-overlap/SKILL.md index 7c768407ed..ada2844129 100644 --- a/plugins/claude-ops/skills/audit-native-overlap/SKILL.md +++ b/plugins/claude-ops/skills/audit-native-overlap/SKILL.md @@ -1,5 +1,5 @@ --- -description: "Map native Claude Code surfaces (built-in CLI commands, bundled skills, plugin-backed built-ins, session-provided skills) against the current repo's plugin skills and agents, so a custom component never silently duplicates what Claude Code itself now ships. Bare invocation is a READ-ONLY report: overlap candidates with evidence, detection integrity floors, and a listing-budget exposure section. Verdicts are human-gated and recorded in a committed store rendered into a generated registry; only an explicit `apply` argument edits a component, baking presence-gated native references into descriptions and Boundary sections. Use when: 'does this skill duplicate a built-in', 'what does Claude Code already ship for this', 'audit native overlap', 'is our install-state audit the same as /doctor', 'refresh the native-surfaces registry', 'bake the native reference into this skill', 'which of our skills overlap bundled skills'. Not for: enumerating what this machine can invoke (use /claude-ops:inventory), MCP tool overlap (use /mcp-tools:audit), plugin fleet currency (use /claude-ops:plugins), or ingesting a CLI release (use /claude-ops:changelog)." +description: "Map native Claude Code surfaces (built-in commands, bundled skills, plugin-backed built-ins, session skills) against this repo's skills and agents, so no component silently duplicates what Claude Code ships. Bare invocation is READ-ONLY: overlap candidates with evidence, per-lane integrity floors, and listing-budget exposure. Verdicts are human-gated in a committed store rendered to a generated registry; only an explicit `apply` argument edits a component, baking presence-gated native references into it. Use when: 'does this skill duplicate a built-in', 'what does Claude Code already ship for this', 'audit native overlap', 'is our install-state audit the same as /doctor', 'refresh the native-surfaces registry', 'bake the native reference into this skill', 'which of our skills overlap bundled skills'. Not for: enumerating what this machine can invoke (/claude-ops:inventory), MCP tool overlap (/mcp-tools:audit), plugin fleet currency (/claude-ops:plugins), or ingesting a CLI release (/claude-ops:changelog)." argument-hint: "[report|apply ] [--store ] [--inventory ]. Bare runs the read-only report" user-invocable: true disable-model-invocation: false @@ -96,9 +96,13 @@ for the repo's test discovery. Under-recall stated honestly beats confident completeness. Three rules: -- **Carry the integrity floor through.** If the inventory reports `degraded`, every native-side - count in the report is a floor and the report says so in the same sentence as the number. If it - reports `broken`, the report carries no native-side counts at all. +- **Carry the integrity floor through, per lane.** The inventory reports integrity per lane + (`builtin_commands`, `bundled_skills`, `plugin_backed`). A `degraded` lane makes every count from + that lane a floor, and the report says so in the same sentence as the number. A `broken` lane's + counts are omitted, the report names the lane and its cause, and every candidate whose lane is + broken is marked `re_derivable: false` (its presence or absence in that lane proves nothing + this run); the other lanes' counts stand. Only when every lane is broken does the report omit + every native-side count. - **Never auto-verdict.** Detection emits candidates with evidence. The verdict column is empty until a human fills it. - **Accept human-added candidates.** A pair nobody's heuristic found is a first-class row; add it @@ -110,8 +114,8 @@ Under-recall stated honestly beats confident completeness. Three rules: # Native overlap — , ## Detection integrity -Inventory status (ok | degraded | broken), cli_version vs validated_against, and what that -means for every count below. +Inventory status per lane (ok | degraded | broken), cli_version vs validated_against, and what +that means for every count below; a broken lane is named with its cause. ## Overlap candidates One row per (native surface, our component): native name + provenance class + hidden/gated diff --git a/plugins/claude-ops/skills/audit-native-overlap/reference/canonical-pairs.json b/plugins/claude-ops/skills/audit-native-overlap/reference/canonical-pairs.json index fdd17ab8d7..82e2b6c358 100644 --- a/plugins/claude-ops/skills/audit-native-overlap/reference/canonical-pairs.json +++ b/plugins/claude-ops/skills/audit-native-overlap/reference/canonical-pairs.json @@ -37,6 +37,16 @@ "component": { "plugin": "claude-ops", "skill": "audit-performance", "kind": "skill" }, "why": "Both diagnose a slow or unhealthy install; doctor also offers to fix." }, + { + "native": { "name": "doctor", "class": "bundled-skill" }, + "component": { "plugin": "claude-ops", "skill": "audit-skill-visibility", "kind": "skill" }, + "why": "doctor's checkup covers unused extensions and skill visibility as one pass of a broader health check; the audit asks the whole-fleet visibility question." + }, + { + "native": { "name": "skill-doctor", "class": "builtin-command" }, + "component": { "plugin": "claude-ops", "skill": "audit-skill-visibility", "kind": "skill" }, + "why": "Both ask which loaded skills go unused and what each costs; the built-in is a one-shot report that excludes bundled skills, the audit is the fleet-wide visibility question." + }, { "native": { "name": "morning", "class": "session-skill" }, "component": { "plugin": "claude-ops", "skill": "morning-brief", "kind": "skill" }, diff --git a/plugins/claude-ops/skills/audit-native-overlap/scripts/overlap.py b/plugins/claude-ops/skills/audit-native-overlap/scripts/overlap.py index f745c8ccda..4d74013c4c 100755 --- a/plugins/claude-ops/skills/audit-native-overlap/scripts/overlap.py +++ b/plugins/claude-ops/skills/audit-native-overlap/scripts/overlap.py @@ -71,6 +71,26 @@ COMPONENT_KINDS = ("skill", "agent") NATIVE_MARKERS = ("hidden", "gated") +# Extraction lanes the sibling extractor reports integrity for, and the +# native class each one carries. Session-provided and marketplace classes have +# no lane: nothing about them is derivable from the binary. +LANE_ORDER = ("builtin_commands", "bundled_skills", "plugin_backed") +LANE_OF_CLASS = { + "builtin-command": "builtin_commands", + "bundled-skill": "bundled_skills", + "plugin-backed-builtin": "plugin_backed", +} + + +def _registrations(entry: Any) -> list[dict[str, Any]]: + """Every registration behind one native name (a list on a name collision).""" + if isinstance(entry, list): + return [item for item in entry if isinstance(item, dict)] + if isinstance(entry, dict): + return [entry] + return [] + + # Lane order in the generated view: (class, section heading, singular noun used # in a row's own prose). Provenance classes are never merged into one list - # they carry different disable switches and different rosters per host. @@ -100,6 +120,20 @@ # token instead; the reverse-parity scan keys on both, per class. MARKETPLACE_GATE_TOKEN = "installed from its marketplace" +# A description that names a native surface by class and kind inside a +# presence clause (`when|where|if ... the bundled design skill is available`) +# without the gate token is routing on a presence condition the registry +# cannot see: no store row records it and no parity check protects it. The +# availability word is what separates a presence condition from a plain +# mention ("Not for: the bundled doctor skill's fix pass"). +PRESENCE_MENTION_RE = re.compile( + r"\b(?:when|where|if)\b[^.;()]{0,80}?" + r"\b(?:bundled|built-in|plugin-backed built-in|session-provided)\s+" + r"(?:[\w/-]+\s+){0,3}?(?:skill|command)s?\b" + r"[^.;()]{0,40}?\b(?:available|present|installed|enabled|resolves?|exists?|ships)\b", + re.IGNORECASE, +) + START_MARKER = "" END_MARKER = "" @@ -771,6 +805,48 @@ def check_baked_parity(repo: Path, rows: list[dict[str, Any]]) -> list[str]: return problems +def check_presence_mentions(repo: Path) -> list[str]: + """Descriptions routing on a native surface's presence without the gate token. + + Advisory, never a break: the two shapes it catches on a real tree are + legitimate pending rows, and the fix is a store row plus a baked phrase + (or dropping the condition), not a red gate. A description carrying a + gate token anywhere is left to the parity checks, which own it. + """ + advisories: list[str] = [] + plugins_dir = repo / "plugins" + if not plugins_dir.is_dir(): + return advisories + for skill_md in sorted(plugins_dir.glob("*/skills/*/SKILL.md")): + try: + frontmatter, _ = split_frontmatter(skill_md.read_text(encoding="utf-8")) + except OSError: + continue + description = frontmatter_description(frontmatter) + if not description: + continue + plugin = skill_md.parents[2].name + skill = skill_md.parent.name + for match in PRESENCE_MENTION_RE.finditer(description): + # Token presence is judged per clause, not per description: one + # skill may carry a gated marketplace clause and an ungated native + # clause, and the second is the one this check exists to find. + clause_end = len(description) + for stop in ".;)": + at = description.find(stop, match.start()) + if at != -1: + clause_end = min(clause_end, at) + clause = description[match.start() : clause_end] + if GATE_TOKEN in clause or MARKETPLACE_GATE_TOKEN in clause: + continue + advisories.append( + f"{plugin}:{skill} names a native surface behind a presence condition " + f'without a gate token ("{match.group(0).strip()}"); add a store row ' + f'and bake the phrase with "{GATE_TOKEN}", or drop the condition' + ) + return advisories + + # --------------------------------------------------------------------------- # Subcommands # --------------------------------------------------------------------------- @@ -816,6 +892,19 @@ def cmd_detect(args: argparse.Namespace) -> int: status = ( integrity.get("status", "unknown") if isinstance(integrity, dict) else "unknown" ) + # Per-lane floors, when the extractor reports them. An older inventory + # without `lanes` is read through the top-level status alone, so a + # consumer on a newer plugin against an older extraction still parses. + lanes = integrity.get("lanes") if isinstance(integrity, dict) else None + if not isinstance(lanes, dict): + lanes = None + + def lane_state(lane: str | None) -> dict[str, Any] | None: + if lane is None or lanes is None: + return None + entry = lanes.get(lane) + return entry if isinstance(entry, dict) else None + components = scan_components(repo) known_skills = set(components["skills"]) known_agents = set(components["agents"]) @@ -847,15 +936,59 @@ def cmd_detect(args: argparse.Namespace) -> int: evidence.append( f"`{native.get('name')}` present in the extraction as {seen['class']}" ) - entry = seen["entry"] - if isinstance(entry, dict): + registrations = _registrations(seen["entry"]) + if len(registrations) > 1: + evidence.append( + f"name collision: {len(registrations)} distinct registrations share " + "this name in the extraction; a per-registration property (model " + "invocability, gating) is read from the registration the row's " + "evidence names, never from the bare name" + ) + for position, entry in enumerate(registrations, start=1): + tag = f"[{position}] " if len(registrations) > 1 else "" markers = [m for m in NATIVE_MARKERS if entry.get(m)] if markers: - evidence.append(f"markers: {', '.join(markers)}") + evidence.append(f"{tag}markers: {', '.join(markers)}") if entry.get("aliases"): - evidence.append(f"aliases: {', '.join(entry['aliases'])}") + evidence.append(f"{tag}aliases: {', '.join(entry['aliases'])}") if entry.get("description"): - evidence.append(f"native description: {entry['description']}") + evidence.append(f"{tag}native description: {entry['description']}") + if "disable_model_invocation" in entry: + mode = ( + "disabled" if entry["disable_model_invocation"] else "enabled" + ) + flagged = "disable_model_invocation" in ( + entry.get("flag_driven") or [] + ) + evidence.append( + f"{tag}model invocation: {mode}" + + (" (flag-driven at runtime)" if flagged else "") + ) + # The lane a candidate's re-derivability depends on: the seeded class + # when the name is absent from the extraction, the observed class when + # present, and both when the two disagree (a class collision), so a + # broken lane on either side marks the candidate. + seeded_lane = LANE_OF_CLASS.get(native.get("class")) + observed_lane = LANE_OF_CLASS.get(seen["class"]) if seen else None + relevant_lanes = {lane for lane in (seeded_lane, observed_lane) if lane} + re_derivable: bool | None + if not relevant_lanes: + re_derivable = None # session-provided and marketplace rows have no lane + else: + broken = [ + lane + for lane in sorted(relevant_lanes) + if (lane_state(lane) or {}).get("status") == "broken" + ] + re_derivable = not broken + for lane in broken: + problems = (lane_state(lane) or {}).get("problems") or [] + evidence.append( + f"the `{lane}` lane of this extraction is broken" + + (f" ({'; '.join(problems)})" if problems else "") + + " - presence or absence in that lane is not re-derivable from " + "this run" + ) kind = component.get("kind", "skill") pool = known_agents if kind == "agent" else known_skills target_present = target in pool @@ -873,26 +1006,55 @@ def cmd_detect(args: argparse.Namespace) -> int: }, "component": component, "component_present": target_present, + "re_derivable": re_derivable, "verdict": None, "evidence": evidence, } ) + report_integrity: dict[str, Any] = { + "status": status, + "cli_version": integrity.get("cli_version") + if isinstance(integrity, dict) + else None, + "validated_against": ( + integrity.get("validated_against") if isinstance(integrity, dict) else None + ), + "counts_are": "floors" if status != "ok" else "totals", + } + if lanes is not None: + # A run-wide advisory (an unvalidated CLI version) applies to every + # lane's numbers, so an ok lane under it still reports floors. A lane + # -attributed advisory (prefixed with the lane name by the extractor) + # degrades only its own lane: a healthy lane beside a broken one keeps + # its totals, which is the point of reporting per lane. + lane_prefixes = tuple(f"{lane}:" for lane in LANE_ORDER) + tuple( + f"{lane} lane broken:" for lane in LANE_ORDER + ) + run_wide = [ + advisory + for advisory in (integrity.get("advisories") or []) + if isinstance(advisory, str) and not advisory.startswith(lane_prefixes) + ] + report_integrity["lanes"] = { + lane: { + "status": (lane_state(lane) or {}).get("status", "unknown"), + "counts_are": ( + "not reportable" + if (lane_state(lane) or {}).get("status") == "broken" + else "floors" + if (lane_state(lane) or {}).get("status") != "ok" or run_wide + else "totals" + ), + } + for lane in LANE_ORDER + if lane_state(lane) is not None + } + report = { "schema": 1, "repo": str(repo), - "integrity": { - "status": status, - "cli_version": integrity.get("cli_version") - if isinstance(integrity, dict) - else None, - "validated_against": ( - integrity.get("validated_against") - if isinstance(integrity, dict) - else None - ), - "counts_are": "floors" if status != "ok" else "totals", - }, + "integrity": report_integrity, "target_scan": { "skills": len(components["skills"]), "agents": len(components["agents"]), @@ -913,10 +1075,18 @@ def cmd_detect(args: argparse.Namespace) -> int: _fail("inventory integrity is broken - no native-side counts are reportable") return 1 if status != "ok": - print( - f"degraded: inventory integrity is {status}; every native-side count is a floor", - file=sys.stderr, + broken_lanes = [ + lane + for lane in LANE_ORDER + if (lane_state(lane) or {}).get("status") == "broken" + ] + detail = ( + f"; lane(s) {', '.join(broken_lanes)} broken, their counts are not " + "reportable and their candidates are marked re_derivable: false" + if broken_lanes + else "; every native-side count is a floor" ) + print(f"degraded: inventory integrity is {status}{detail}", file=sys.stderr) return 3 return 0 @@ -1001,6 +1171,7 @@ def cmd_self_check(args: argparse.Namespace) -> int: ) problems.extend(check_baked_parity(repo, rows)) + advisories.extend(check_presence_mentions(repo)) # A verdict lives for the model only once the component's body carries its # Boundary section; the store alone ships to nobody. Broken, not degraded: @@ -1070,30 +1241,36 @@ def cmd_self_check(args: argparse.Namespace) -> int: if recorded_shas: # Mirrors the --cli-version seam: the registry never fetches upstream # itself, so without the flag the comparison is honestly undecidable. - if args.upstream_sha: - current_sha = args.upstream_sha.strip().lower() - if not re.fullmatch(r"[0-9a-f]{8,40}", current_sha): + # The flag repeats, one value per upstream repository the store cites + # (a changelog commit in one repository, a plugin commit in another), + # and a recorded commit matches when any provided value matches it. + current_shas: list[str] = [] + for raw in args.upstream_sha or []: + candidate = raw.strip().lower() + if not re.fullmatch(r"[0-9a-f]{8,40}", candidate): # A malformed value must not silently pass: an empty or short # prefix would match every recorded SHA via startswith. advisories.append( - f"--upstream-sha {args.upstream_sha!r} is not an 8-40 character " + f"--upstream-sha {raw!r} is not an 8-40 character " "hex commit prefix; the recorded upstream commit(s) " - f"{', '.join(recorded_shas)} were not checked" + f"{', '.join(recorded_shas)} were not checked against it" ) - current_sha = None - else: - current_sha = None - if current_sha: + continue + current_shas.append(candidate) + if current_shas: drifted = [ sha for sha in recorded_shas - if not (sha.startswith(current_sha) or current_sha.startswith(sha)) + if not any( + sha.startswith(current) or current.startswith(sha) + for current in current_shas + ) ] if drifted: advisories.append( f"recorded upstream commit(s) {', '.join(drifted)} differ from " - f"--upstream-sha {current_sha}; rows sourced from them are " - "stale-but-honest until re-derived" + f"--upstream-sha {', '.join(current_shas)}; rows sourced from them " + "are stale-but-honest until re-derived" ) elif not args.upstream_sha: advisories.append( @@ -1186,11 +1363,13 @@ def add_paths(sub: argparse.ArgumentParser) -> None: ) self_check.add_argument( "--upstream-sha", + action="append", default=None, help=( - "compare upstream-source rows' recorded commits against this SHA (offline " - "seam; the registry never fetches upstream itself - without the flag the " - "comparison is reported as not locally decidable)" + "compare upstream-source rows' recorded commits against this SHA; repeat " + "the flag once per upstream repository the store cites (offline seam; the " + "registry never fetches upstream itself - without the flag the comparison " + "is reported as not locally decidable)" ), ) add_paths(self_check) diff --git a/plugins/claude-ops/skills/audit-native-overlap/scripts/test_overlap.py b/plugins/claude-ops/skills/audit-native-overlap/scripts/test_overlap.py index db38dd363c..fbf8f66393 100755 --- a/plugins/claude-ops/skills/audit-native-overlap/scripts/test_overlap.py +++ b/plugins/claude-ops/skills/audit-native-overlap/scripts/test_overlap.py @@ -158,7 +158,11 @@ def self_check(self, cli_version=FIXTURE_CLI_VERSION, upstream_sha=None): if cli_version is not None: args += ["--cli-version", cli_version] if upstream_sha is not None: - args += ["--upstream-sha", upstream_sha] + shas = ( + [upstream_sha] if isinstance(upstream_sha, str) else list(upstream_sha) + ) + for sha in shas: + args += ["--upstream-sha", sha] return overlap.main(args) def cleanup(self): @@ -711,6 +715,28 @@ def test_self_check_with_matching_upstream_sha_is_ok(self): repo.generate() self.assertEqual(repo.self_check(upstream_sha=FIXTURE_UPSTREAM_SHA), 0) + def test_self_check_accepts_one_sha_per_upstream_repository(self): + # Two upstream-source rows citing two repositories: a recorded commit + # matches when ANY provided value matches it, so neither drifts. + other = deep_copy(MARKETPLACE_ROW) + other["component"]["skill"] = "other-skill" + other["observation"]["detail"] = ( + "anthropics/claude-code at commit d7dbd9a09f59775726ed14bbea8fc9dfdff62f7b" + ) + repo = TempRepo([BASE_ROW, MARKETPLACE_ROW, other]) + self.addCleanup(repo.cleanup) + repo.generate() + self.assertEqual( + repo.self_check( + upstream_sha=[ + FIXTURE_UPSTREAM_SHA, + "d7dbd9a09f59775726ed14bbea8fc9dfdff62f7b", + ] + ), + 0, + ) + self.assertEqual(repo.self_check(upstream_sha=[FIXTURE_UPSTREAM_SHA]), 3) + def test_self_check_with_short_prefix_sha_still_matches(self): repo = TempRepo(rows=[BASE_ROW, MARKETPLACE_ROW]) self.addCleanup(repo.cleanup) @@ -1026,6 +1052,196 @@ def test_shipped_canonical_pairs_file_validates(self): payload = json.loads(shipped.read_text(encoding="utf-8")) self.assertEqual(overlap.validate_pairs(payload), []) + def lanes(self, **statuses): + lanes = {} + for lane in overlap.LANE_ORDER: + status = statuses.get(lane, "ok") + lanes[lane] = { + "status": status, + "problems": [f"{lane} failed"] if status == "broken" else [], + "advisories": [], + } + return lanes + + def test_a_broken_lane_marks_its_candidates_not_re_derivable(self): + # Seeded bundled-skill `doctor`, absent from a broken bundled lane: + # the seeded class decides the lane, and its absence proves nothing. + self.write_inventory( + bundled_skills={}, + integrity={ + "status": "degraded", + "cli_version": FIXTURE_CLI_VERSION, + "validated_against": FIXTURE_CLI_VERSION, + "lanes": self.lanes(bundled_skills="broken"), + }, + ) + out = self.repo.root / "candidates.json" + self.assertEqual(self.detect(out), 3) + report = json.loads(out.read_text(encoding="utf-8")) + self.assertEqual( + report["integrity"]["lanes"]["bundled_skills"]["counts_are"], + "not reportable", + ) + self.assertEqual( + report["integrity"]["lanes"]["builtin_commands"]["counts_are"], "totals" + ) + candidate = report["candidates"][0] + self.assertFalse(candidate["re_derivable"]) + self.assertTrue( + any( + "lane of this extraction is broken" in item + for item in candidate["evidence"] + ) + ) + + def test_a_healthy_lane_keeps_its_candidates_re_derivable(self): + self.write_inventory( + integrity={ + "status": "degraded", + "cli_version": FIXTURE_CLI_VERSION, + "validated_against": FIXTURE_CLI_VERSION, + "lanes": self.lanes(builtin_commands="broken"), + } + ) + out = self.repo.root / "candidates.json" + self.assertEqual(self.detect(out), 3) + candidate = json.loads(out.read_text(encoding="utf-8"))["candidates"][0] + self.assertTrue(candidate["re_derivable"]) + + def test_a_class_collision_is_marked_in_both_directions(self): + # Seeded as a bundled skill, observed as a built-in command: a broken + # lane on EITHER side marks the candidate. + collided = { + "builtin_commands": { + "help": {"name": "help"}, + "doctor": {"name": "doctor"}, + }, + "bundled_skills": {}, + } + for broken in ("builtin_commands", "bundled_skills"): + self.write_inventory( + **collided, + integrity={ + "status": "degraded", + "cli_version": FIXTURE_CLI_VERSION, + "validated_against": FIXTURE_CLI_VERSION, + "lanes": self.lanes(**{broken: "broken"}), + }, + ) + out = self.repo.root / "candidates.json" + self.assertEqual(self.detect(out), 3) + candidate = json.loads(out.read_text(encoding="utf-8"))["candidates"][0] + self.assertEqual(candidate["native"]["class"], "builtin-command") + self.assertEqual(candidate["native"]["seeded_class"], "bundled-skill") + self.assertFalse(candidate["re_derivable"], broken) + + def test_top_level_drift_makes_every_ok_lane_report_floors(self): + # An unvalidated CLI version degrades the run, not a lane; the lane + # labels must agree with the top-level `counts_are`. + self.write_inventory( + integrity={ + "status": "degraded", + "cli_version": FIXTURE_CLI_VERSION, + "validated_against": "2.1.228", + "advisories": [ + f"cli {FIXTURE_CLI_VERSION} differs from the last validated build " + "2.1.228; counts are believed, not verified" + ], + "lanes": self.lanes(), + } + ) + out = self.repo.root / "candidates.json" + self.assertEqual(self.detect(out), 3) + report = json.loads(out.read_text(encoding="utf-8")) + self.assertEqual(report["integrity"]["counts_are"], "floors") + for lane in overlap.LANE_ORDER: + self.assertEqual(report["integrity"]["lanes"][lane]["status"], "ok") + self.assertEqual(report["integrity"]["lanes"][lane]["counts_are"], "floors") + + def test_a_lane_attributed_advisory_degrades_only_its_lane(self): + lanes = self.lanes() + lanes["bundled_skills"]["status"] = "degraded" + lanes["bundled_skills"]["advisories"] = [ + "1 registration(s) register a dynamic roster" + ] + self.write_inventory( + integrity={ + "status": "degraded", + "cli_version": FIXTURE_CLI_VERSION, + "validated_against": FIXTURE_CLI_VERSION, + "advisories": [ + "bundled_skills: 1 registration(s) register a dynamic roster" + ], + "lanes": lanes, + } + ) + out = self.repo.root / "candidates.json" + self.assertEqual(self.detect(out), 3) + report = json.loads(out.read_text(encoding="utf-8")) + self.assertEqual( + report["integrity"]["lanes"]["bundled_skills"]["counts_are"], "floors" + ) + self.assertEqual( + report["integrity"]["lanes"]["builtin_commands"]["counts_are"], "totals" + ) + + def test_an_inventory_without_lanes_keeps_the_old_behaviour(self): + self.write_inventory() + out = self.repo.root / "candidates.json" + self.assertEqual(self.detect(out), 0) + report = json.loads(out.read_text(encoding="utf-8")) + self.assertNotIn("lanes", report["integrity"]) + self.assertTrue(report["candidates"][0]["re_derivable"]) + + def test_a_session_skill_candidate_has_no_lane(self): + self.write_inventory() + self.write_pairs( + { + "schema": 1, + "pairs": [ + { + "native": {"name": "morning", "class": "session-skill"}, + "component": {"plugin": "demo", "skill": "demo-audit"}, + } + ], + } + ) + out = self.repo.root / "candidates.json" + self.assertEqual(self.detect(out), 0) + self.assertIsNone( + json.loads(out.read_text(encoding="utf-8"))["candidates"][0]["re_derivable"] + ) + + def test_a_name_collision_lists_every_registration(self): + self.write_inventory( + bundled_skills={ + "doctor": [ + { + "name": "doctor", + "description": "Hub", + "disable_model_invocation": True, + "collision": True, + }, + { + "name": "doctor", + "description": "Canvas", + "gated": True, + "user_invocable": True, + "collision": True, + }, + ] + } + ) + out = self.repo.root / "candidates.json" + self.assertEqual(self.detect(out), 0) + evidence = json.loads(out.read_text(encoding="utf-8"))["candidates"][0][ + "evidence" + ] + self.assertTrue(any(item.startswith("name collision: 2") for item in evidence)) + self.assertIn("[1] model invocation: disabled", evidence) + self.assertIn("[2] markers: gated", evidence) + self.assertIn("[2] native description: Canvas", evidence) + def test_absent_native_is_reported_as_an_extraction_statement(self): self.write_inventory(bundled_skills={}) out = self.repo.root / "candidates.json" @@ -1040,6 +1256,107 @@ def test_absent_native_is_reported_as_an_extraction_statement(self): ) +class PresenceMentionTests(unittest.TestCase): + """A presence-gated native mention without the gate token is an advisory.""" + + def setUp(self): + self.repo = TempRepo() + self.addCleanup(self.repo.cleanup) + self.repo.generate() + + def check(self): + buffer = io.StringIO() + with contextlib.redirect_stdout(buffer): + code = self.repo.self_check() + return code, buffer.getvalue() + + def test_presence_clause_without_the_token_is_an_advisory(self): + self.repo.write_skill( + "viz", + "visualize", + description=( + "Picks a form (a mermaid diagram, or, where the bundled design skill is " + "available, a hand-editable design canvas)." + ), + ) + code, out = self.check() + self.assertEqual(code, 3) + self.assertIn( + "viz:visualize names a native surface behind a presence condition", out + ) + + def test_a_description_carrying_the_token_is_left_to_parity(self): + row = deep_copy(BASE_ROW) + row["baked"]["description_phrase"] = True + self.repo.write_store(make_store([row])) + self.repo.generate() + self.repo.write_skill( + "demo", + "demo-audit", + description=( + "When the bundled doctor skill resolves in your session, prefer it for the " + "quick pass." + ), + extra=BOUNDARY_EXTRA, + ) + code, out = self.check() + self.assertEqual(code, 0) + self.assertNotIn("presence condition", out) + + def test_the_token_is_judged_per_clause_not_per_description(self): + # A gated marketplace clause does not excuse an ungated native clause + # elsewhere in the same description. + claimed = deep_copy(MARKETPLACE_ROW) + claimed["component"]["skill"] = "mixed" + claimed["baked"]["description_phrase"] = True + self.repo.write_store(make_store([BASE_ROW, claimed])) + self.repo.generate() + self.repo.write_skill( + "demo", + "mixed", + description=( + "Builds mockups (or, where the bundled design skill is available, a canvas). " + "Not for an explorer: that is the playground skill, routed via /playgrounds:use " + "where the upstream playground plugin is installed from its marketplace." + ), + ) + code, out = self.check() + self.assertEqual(code, 3) + self.assertIn("demo:mixed names a native surface", out) + + def test_seam_phrasing_plugin_clause_is_not_flagged(self): + self.repo.write_skill( + "demo", + "seam", + description="Routes chart craft to the dataviz plugin if that plugin is installed.", + ) + code, out = self.check() + self.assertEqual(code, 0) + self.assertNotIn("presence condition", out) + + def test_not_for_clause_without_a_presence_condition_is_not_flagged(self): + self.repo.write_skill( + "demo", + "notfor", + description="Audits an install directory. Not for: the bundled doctor skill's fix pass.", + ) + code, out = self.check() + self.assertEqual(code, 0) + self.assertNotIn("presence condition", out) + + def test_a_plain_mention_in_a_when_clause_is_not_flagged(self): + # "when the user asks about a built-in command" states no presence + # condition; only an availability word makes one. + self.repo.write_skill( + "demo", + "plain", + description="Use when the user asks whether a built-in command is a real command.", + ) + code, out = self.check() + self.assertEqual(code, 0) + self.assertNotIn("presence condition", out) + + class ScanTests(unittest.TestCase): def test_repo_tree_scan_finds_skills_and_agents(self): repo = TempRepo() diff --git a/plugins/claude-ops/skills/inventory/SKILL.md b/plugins/claude-ops/skills/inventory/SKILL.md index b62dd12314..80d12ee16a 100644 --- a/plugins/claude-ops/skills/inventory/SKILL.md +++ b/plugins/claude-ops/skills/inventory/SKILL.md @@ -43,7 +43,8 @@ python3 "${CLAUDE_PLUGIN_ROOT}/skills/inventory/scripts/inventory.py" --out ./cl ``` Python 3.11+ is the only requirement. No `strings`, no `jq`, no PowerShell, no third-party -packages. The run takes a few seconds, dominated by reading the executable once. +packages. The run takes about fifteen seconds, dominated by reading and tokenizing the executable +once. Useful flags: `--binary ` (else auto-detected) · `--config-dir ` (else `$CLAUDE_CONFIG_DIR`, else `~/.claude`) · `--binary-only` / `--disk-only` to skip a source. @@ -119,8 +120,14 @@ hooks live in its own manifest. Report the map as read and tell the user to run `/claude-ops:plugins audit` for the verdict. **Counts that disagree are data, not noise.** `bundled_skill_notes` carries `registrations_seen` -alongside `resolved`. When they differ, some registration used a dynamically computed name; say so -rather than reporting the smaller number as complete. +alongside `resolved`, and `dynamic_roster` when a registration builds its names at runtime (a loop +over a table, or a template literal). When seen exceeds resolved, say so rather than reporting the +smaller number as complete. + +**One name can be two registrations.** When two distinct bundled registrations share a name, the +extraction keeps both as a list under that name with `collision: true`, and +`bundled_skill_notes.collisions` names them. Report each registration on its own line with its +description and invocation fields; never pick one and present it as the name's single meaning. **A marketplace checkout is not an installation, and neither is enablement.** Three different sets: a cached marketplace is a catalog of what is *available*, `disk.installed_plugins` is what is @@ -165,14 +172,19 @@ Cite the version that introduced or removed a command rather than asserting it c Claude Code updates constantly, and this skill reads its internals. The design assumption is not that the build holds still. It is that **drift must never be silent**. -**Read the integrity block before quoting any number.** Every run carries one, and it states whether -the counts are verified or merely believed: +**Read the integrity block before quoting any number, lane by lane.** Every run carries one, and it +states per lane (`builtin_commands`, `bundled_skills`, `plugin_backed`) whether that lane's counts +are verified or merely believed; the top-level status is the worst lane: | Status | Means | Do | |---|---|---| -| `ok` | Build matches the last validated release, every check passed | Report counts as totals | -| `degraded` | Extraction worked, but something is unaccounted for | Report counts as **floors**, and say what is unaccounted for | -| `broken` | A canary command is missing or nothing resolved | Do not report counts at all; say the extractor needs updating | +| `ok` | Build matches the last validated release, every check passed | Report that lane's counts as totals | +| `degraded` | Extraction worked, but something is unaccounted for | Report that lane's counts as **floors**, and say what is unaccounted for | +| `broken` | A canary is missing or nothing resolved in that lane | Do not report that lane's counts at all; name the lane and its cause; the other lanes' counts stand | + +A single broken lane makes the top level `degraded`, not `broken`, so a healthy command list is +never withheld because the bundled-skill lane failed. Top-level `broken` means every lane is broken +or the binary could not be read. The distinction earns its keep because the dangerous failure is not a crash. A renamed export throws and is obvious. A *new registration path* returns a clean, confident, short list, so the checks are diff --git a/plugins/claude-ops/skills/inventory/evals/evals.json b/plugins/claude-ops/skills/inventory/evals/evals.json index 4e45be4f1a..9ba56a7b55 100644 --- a/plugins/claude-ops/skills/inventory/evals/evals.json +++ b/plugins/claude-ops/skills/inventory/evals/evals.json @@ -18,12 +18,13 @@ "id": 2, "name": "degraded-counts-are-floors", "prompt": "How many bundled skills does Claude Code ship? Just give me the number.", - "expected_output": "Reads the integrity block first. Because the current build reports degraded — some bundled-skill registrations use computed names — the count is reported as a floor with the shortfall named, not as a verified total. Does not round the unresolved registrations away or present the resolved number as complete.", + "expected_output": "Reads the integrity block first, lane by lane. Because the bundled_skills lane reports degraded — some registrations build their names at runtime (a dynamic roster) — the count is reported as a floor with the shortfall named, not as a verified total, while the healthy lanes' counts stand as totals. Does not round the dynamic registrations away or present the resolved number as complete.", "files": [], "expectations": [ "Output reports the bundled-skill count as a floor or minimum rather than asserting it as an exact total", - "Output names the reason the count is not exact, referring to registrations whose names could not be resolved", - "Output does not silently drop the unresolved registrations from the explanation to present a cleaner number" + "Output names the reason the count is not exact, referring to registrations whose names are built at runtime or could not be resolved", + "Output does not silently drop the unresolved or dynamic registrations from the explanation to present a cleaner number", + "Output reads the lane statuses and does not downgrade a healthy lane's count because another lane is degraded" ] }, { diff --git a/plugins/claude-ops/skills/inventory/reference/extraction.md b/plugins/claude-ops/skills/inventory/reference/extraction.md index a87ae86d72..4a19317e21 100644 --- a/plugins/claude-ops/skills/inventory/reference/extraction.md +++ b/plugins/claude-ops/skills/inventory/reference/extraction.md @@ -7,7 +7,7 @@ editor needs. ## Why the binary is read at all Claude Code's documentation does not publish its built-in slash commands. `docs/en/slash-commands` -now serves the skills page. The two URLs return byte-identical markdown, because commands were +now serves the skills page: the two URLs return byte-identical markdown, because commands were merged into skills, so no upstream page enumerates `/clear`, `/rewind`, `/artifacts`, or the rest. Plugin components are on disk and need no such measure; the binary read exists only for the built-in and bundled surfaces, which have no other complete source. @@ -26,48 +26,76 @@ documentation and reduce the binary read to a cross-check. ## Shape of the artifact The shipped executable is a Bun standalone build: a native container with the JavaScript bundle -appended. On the build observed while writing this (2.1.228, Windows, PE32+), the layout was: +appended. Two layouts have been observed: -| Property | Value | -|---|---| -| Container | PE32+ x86-64, 12 sections | -| Payload section | `.bun`, ~212 MB, ~71% of the file | -| CLI bundle | ~25 MB of minified JS, header `// @bun @bytecode @bun-cjs` | -| Trailer | `\n---- Bun! ----\n` at EOF | +| Layout | Observed on | Readable source | +|---|---|---| +| Single run | 2.1.228 (PE32+, Windows) | one ~25 MB printable run under a `// @bun @bytecode @bun-cjs` header | +| Fragmented bytecode | 2.1.263 (ELF, Linux) | ~3,200 printable runs from 256 bytes to several MB, scattered through ~214 MB after the first `// @bun` marker | -None of those specifics matter to the script, and that is deliberate. Parsing the PE -section table would work on Windows and then need a Mach-O load-command reader for macOS and an ELF -section reader for Linux, three parsers to maintain against a packer that may rename its section -anyway. The script instead treats the file as bytes and finds the bundle by content. +In the fragmented layout the registrations sit in small runs (the `doctor` registration in about +1.3 KB), the hoisted name constants sit megabytes ahead of the calls that use them, and the export +map is an ESM export list rather than a CJS getter. None of the container specifics are +load-bearing in the script, and that is deliberate: parsing the PE section table would work on +Windows and then need a Mach-O load-command reader for macOS and an ELF section reader for Linux. +The script treats the file as bytes and finds the source by content. -## The three extraction decisions +## The extraction decisions -### 1. Anchor on an export name, not the chunk header +### 1. Region rule: every printable run above a floor, from the first marker to end of file -The obvious anchor is the `// @bun` header. It fails: the header appears in several small helper -chunks, and the *first* occurrence is a few hundred bytes of the wrong one, which yields a tiny -"bundle" and zero commands. +The previous rule took the single largest printable run around an anchor. On a fragmented build +that run holds commands and nothing else, so the bundled-skill lane returned empty while the +command lane looked healthy. -The script anchors on `registerBundledSkill`, a string that occurs only in the CLI bundle, expands -to the surrounding printable run, and takes the largest candidate, rejecting anything under 1 MB. -Chunk headers remain as fallbacks in `BUNDLE_MARKERS` for a build that renames the export. +The rule now, stated exactly: from the first `BUNDLE_MARKERS` occurrence to end of file, every +printable run of at least `MIN_RUN_BYTES` (256) bytes, found with one `RUN_RE` pass, joined with +newlines. `sources.binary` records `runs`, `joined_bytes`, `region_rule`, `runs_below_floor`, and +`elapsed_seconds`. `runs_below_floor` counts registration tokens (`({name:`) that sit in runs +shorter than the floor: a registration below the floor is counted, never silently lost. The floor +is what keeps the pass cheap; measured on 2.1.263, 256 bytes recovers every named surface in about +4 seconds, while a 64 KiB floor recovers 13 of 33 names and a 1-byte floor takes a minute. -### 2. Discover registrar names, never hardcode them +A build with no marker at all falls back to the largest run around an anchor, which is what the +single-run layout needed. -Minified identifiers are regenerated on every build. The bundled-skill registrar is `xu` in 2.1.228 -and will be something else next release, so `re.findall(r'xu\(\{', src)` dates the script to a -single version. +### 2. Discover the registrar by three routes, never hardcode it -The bundle's module export maps keep the original names: +Minified identifiers are regenerated on every build (`xu` in 2.1.228, `eo` in 2.1.263). The +readable half of an export is what upstream maintains, so `discover_registrar_route` tries, in +order: -```js -pt(QCd,{ ... registerBundledSkill:()=>xu, ... }) -``` +| Route | Shape | Recorded as | +|---|---|---| +| CJS getter | `registerBundledSkill:()=>xu` | `export-map` | +| ESM export list | `eo as registerBundledSkill` | `esm-export` | +| Canary | the callee immediately before `({name:"doctor",aliases:["checkup"]` | `canary` | + +`bundled_skill_notes.registrar_route` says which one resolved. With none, the bundled lane is broken +with the existing error text. + +### 3. Resolve computed names by locality, never by a single global value + +Names arrive three ways. Literals (`name:"doctor"`); hoisted constants (`name:kYe` where +`kYe="simplify"` is bound elsewhere); and runtime names (a template literal such as +`` name:`artifact-${e}` ``, or a loop `for(let{kind:e,...}of ls)eo({name:e,...})`), which are one +call registering a family and are recorded as a `dynamic_roster` note, not as one unresolved +identifier. -`discover_registrar(src, "registerBundledSkill")` reads the minified name out of that map at -runtime. The readable half is what upstream maintains; the minified half is what changes. +For a hoisted constant, the locality rule is: the binding is the nearest `ident="kebab-case"` +before the registration, and a farther binding never wins over a nearer one. In 2.1.263 the same +identifier `oO` is bound to `"ehrpd"` in an unrelated module and to `"artifact-design"` closer in; +nearest-preceding picks the real one. A single-character identifier is a function-local name reused +everywhere, so it is trusted only when that nearest binding lies within +`SHORT_IDENT_LOCALITY_BYTES`; the design canvas registration (`var r="design"` a few KB ahead of +`eo({name:r,...})`) resolves, while a loop variable whose only binding is megabytes away does not. +No preceding binding is unresolved, never guessed. -### 3. Resolve enclosing objects by brace depth, not by a text window +Bundle markers are not module boundaries in the fragmented layout: the real bindings sit about 170 +marker occurrences ahead of their registrations, so a marker-scoped rule finds nothing. Locality is +measured in bytes. + +### 4. Resolve enclosing objects by brace depth, not by a text window Minified object literals sit flush against one another: @@ -75,27 +103,35 @@ Minified object literals sit flush against one another: ...,IWp=N7b});var $7b,DWp;var MWp=E(()=>{QH();$7b={type:"local-jsx",name:"artifacts",... ``` -A fixed ±N-character window around `type:"local-jsx"` spans the neighbouring command and mixes its -`description` in. `build_brace_map` tokenizes the whole bundle once, tracking string, template, -regex, and comment states so a `{` inside a string is not counted, and records every matched pair. -Each command's fields are then read from its own literal. +A fixed window around `type:"local-jsx"` spans the neighbouring command and mixes its `description` +in. `build_brace_map` tokenizes the whole joined source once, tracking string, template, regex, and +comment states so a `{` inside a string is not counted, and records every matched pair. Each +command's and each registration's fields are then read from its own literal. A regex-only pass over +this bundle goes wrong in one of two ways: it misses `/artifacts` entirely, or it invents `/alias` +and `/todos` as commands. -This is the single most important correctness property in the script. A regex-only pass over this -bundle goes wrong in one of two ways: it misses `/artifacts` entirely, or it invents `/alias` and -`/todos` as commands. +A call to the registrar identifier whose object carries no `name:` is another module's function +sharing the minified name, not a registration; it is counted in `same_identifier_calls_skipped` and +never inflates the resolved-versus-seen gap. -## The three registration paths +### 5. Keep both registrations when two share a name -| Path | Shape | Yields | -|---|---|---| -| Command objects | `{type:"local"\|"local-jsx"\|"prompt", name, description, aliases, isEnabled, isHidden}` | Built-in CLI commands | -| `registerBundledSkill` | `xu({name, aliases, menuDescription, isEnabled, requires})` | Bundled skills | -| Cloud registrar | a thin wrapper registering remote-backed commands | `ultraplan`, `ultrareview`, `teleport`, `remote-control`, `schedule`, `autofix-pr` | +2.1.263 registers `design` twice: the canvas skill (from `registerDesignCanvasSkill`, model-invocable, +gated on the artifact capability) and the claude.ai/design hub (model-disabled). A name-keyed map +would keep whichever the bundle placed last, and a consumer deciding model invocability would read +the wrong one. Two distinct registrations sharing a name are both kept as a list under +`bundled_skills.`, each carrying `collision: true`, and `bundled_skill_notes.collisions` names +them. The same registration seen twice is not a collision. Read through `registrations_of(entry)` +so neither shape is a special case. -Names arrive two ways in the second path. Some are literals; others are hoisted constants -(`xu({name:gme,...})` where `gme="code-review"`), which is why `build_const_map` exists: a -literal-only scan silently drops roughly a third of the bundled skills, including `code-review`, -`simplify`, and the artifact family. +### 6. Read the invocation-control fields + +Each registration records `user_invocable`, `disable_model_invocation`, `terminal_oriented`, and +`survives_kill_switch` when the object carries them (`!0` true, `!1` false). A function-valued +field, the `verify` registration's `disableModelInvocation:()=>...`, reads as true with the key +listed under `flag_driven`, matching the binary's own serializer. On 2.1.263 `doctor` reads +model-disabled and terminal-oriented and is the one registration that survives the bundled kill +switch; `simplify` and `run` carry no invocation-control field and are model-invocable. ## Known non-commands @@ -113,23 +149,45 @@ reading its surrounding code: The `type:` requirement plus brace-depth resolution excludes all of these. `INTERNAL_NAMES` marks the remainder that are real registrations but never user-typed. +## Integrity, per lane + +`check_integrity` returns `lanes` (`builtin_commands`, `bundled_skills`, `plugin_backed`), each with +its own `status`, `problems`, and `advisories`. One rule for one state: the top-level `status` is +the worst lane. `broken` at the top level means every lane is broken or the binary is unreadable; a +run with at least one healthy lane is at most `degraded`, with each broken lane's problems restated +as top-level advisories prefixed by the lane name. The exit mapping is unchanged (`ok` 0, `broken` +1, `degraded` 3); what changed is that a single broken lane no longer voids the healthy lanes' +counts. + +| Lane | Breaks on | Degrades on | +|---|---|---| +| `builtin_commands` | a canary command absent; command yield under `MIN_COMMAND_YIELD` | nothing lane-specific | +| `bundled_skills` | no bundled skill resolved | an unknown registrar-shaped export (either export shape); computed names unresolved; a dynamic roster; registration literals in runs below the floor | +| `plugin_backed` | a `PLUGIN_BACKED_CANARY` name absent | nothing lane-specific | + +The CLI-version advisory is top-level, not a lane's. + ## When a build changes -Work the integrity block, not the symptom. `--self-check` names which check failed, and each maps to -one edit: +Work the integrity block, not the symptom. `--self-check` prints each lane's status and names which +check failed, and each maps to one edit: | Verdict | Cause | Fix | |---|---|---| -| `broken`: canary commands absent | Bundle found but parsing yields little | Confirm the bundle size looks right; if so the object shape changed, so re-derive from a known command | -| `broken`: registrar lookup failed | Export renamed upstream | Update the name passed to `discover_registrar` | -| `broken`: no bundle found | Packer layout changed | Add the new anchor to `BUNDLE_MARKERS` | -| `degraded`: unrecognised registrar export | A new registration path may exist | Inspect it; add to `KNOWN_REGISTRAR_EXPORTS` if it funnels into the known registrar, otherwise extract it | -| `degraded`: computed names unresolved | Registration built its name dynamically | Usually acceptable, so report as a floor. Extend `build_const_map` only if the count grows | +| `builtin_commands` broken: canary commands absent | Source found but parsing yields little | Confirm `joined_bytes` looks right; if so the object shape changed, so re-derive from a known command | +| `bundled_skills` broken: registrar lookup failed | All three routes missed | Read the export shape around `registerBundledSkill` and add a fourth route | +| `plugin_backed` broken: canary absent | The `pluginName` field moved | Re-derive `extract_plugin_backed` from `security-review` | +| broken: joined region under 1 MB | Packer layout changed | Add the new marker to `BUNDLE_MARKERS`, or lower the floor after measuring it | +| `bundled_skills` degraded: unrecognised registrar export | A new registration path may exist | Inspect it; add to `KNOWN_REGISTRAR_EXPORTS` if it funnels into the known registrar, otherwise extract it | +| `bundled_skills` degraded: computed names unresolved | A binding shape the locality rule does not see | Report as a floor; extend `_KEBAB_BINDING_RE` only if the count grows | +| `bundled_skills` degraded: dynamic roster | A family registered in a loop or template | Acceptable; the names are enumerable only by running the binary | After revalidating, bump `VALIDATED_AGAINST`. Leaving it stale is not a bug: every report then says its counts are believed rather than verified, which is the honest state until someone checks. ## Cost -Reading the executable dominates. On the observed build: ~2.7 s wall clock, ~300 MB read once, -~25 MB tokenized into ~190,000 brace pairs. The file is opened read-only and never executed. +Reading and scanning the executable dominates. On 2.1.263 in a Linux container: about 214 MB read +once, the region pass about 4 seconds, the brace map over the 37 MB joined source about 5 seconds, +name resolution about 2 seconds, roughly 14 seconds wall clock in all. The file is opened read-only +and never executed. diff --git a/plugins/claude-ops/skills/inventory/scripts/inventory.py b/plugins/claude-ops/skills/inventory/scripts/inventory.py index a0bf2b4d66..bc2e717cba 100755 --- a/plugins/claude-ops/skills/inventory/scripts/inventory.py +++ b/plugins/claude-ops/skills/inventory/scripts/inventory.py @@ -26,6 +26,7 @@ import re import shutil import sys +import time from dataclasses import dataclass, field from pathlib import Path from typing import Any @@ -36,7 +37,7 @@ # the skill's evals. Drift from it is not an error - the extraction is designed # to survive ordinary releases - but it downgrades every count from "verified" # to "believed", which the report has to say out loud. -VALIDATED_AGAINST = "2.1.228" +VALIDATED_AGAINST = "2.1.263" # Commands that have shipped in every build observed. Their absence means the # extraction broke, not that Anthropic deleted /help. This is the cheapest @@ -59,6 +60,8 @@ "registerRunSkillGeneratorSkill", "registerScheduleRemoteAgentsSkill", "registerAgentProxyEnvFn", + "registerDesignCanvasSkill", + "registerWorkflowAuthoringSkill", } ) @@ -66,11 +69,40 @@ # brace reader is failing to resolve enclosing objects and the list is partial. MIN_COMMAND_YIELD = 0.40 +# Plugin-backed built-ins that have shipped in every build observed. Absence +# means the `pluginName` scan broke, not that the product dropped the command. +PLUGIN_BACKED_CANARY = ("security-review",) + # The bundle is minified JS. These markers sit at the top of the embedded CLI # chunk and are stable across the releases observed so far; each is tried in # turn so one rename does not break discovery. BUNDLE_MARKERS = (b"// @bun @bytecode @bun-cjs", b"// @bun @bun-cjs", b"// @bun") +# Region rule. A bytecode-fragmented bundle scatters its readable JavaScript +# across thousands of printable runs, some only a few kilobytes, with +# registrations in the small ones. From the first bundle marker to end of file, +# every printable run of at least this many bytes is taken and the runs are +# joined with newlines. The floor is what keeps the single regex pass cheap; +# below it the regex costs minutes and above it registrations go missing. +MIN_RUN_BYTES = 256 +RUN_RE = re.compile(rb"[\t\n\r\x20-\x7e]{%d,}" % MIN_RUN_BYTES) +# Every registration literal, of any registrar, opens with this token. Counting +# it in the raw region against the joined source is how a registration that +# sits in a run below the floor is counted rather than silently lost. +REGISTRATION_TOKEN = b"({name:" + +# A single-character identifier is a function-local minifier name reused +# everywhere, so its nearest preceding string binding is only trusted when it +# lies within this many bytes of the registration; a longer identifier is a +# module-level constant and its nearest preceding binding is trusted at any +# distance (the constants are hoisted megabytes ahead of the registrations in +# the bytecode layout). +SHORT_IDENT_LOCALITY_BYTES = 65_536 + +# Extraction lanes, in the order the self-check prints them. Each lane carries +# its own status so one broken lane never silently voids the others' counts. +LANES = ("builtin_commands", "bundled_skills", "plugin_backed") + # Component types a plugin may ship, from the plugin manifest schema and the # standard plugin layout. Directory is the default location; the manifest may # redirect most of them, which is why the manifest is read before the tree. @@ -204,7 +236,59 @@ def read_bundle(binary: Path) -> tuple[str | None, dict[str, Any]]: return None, meta meta["container"] = detect_container(data) + return _select_region(data, meta) + + +def _select_region( + data: bytes, meta: dict[str, Any] +) -> tuple[str | None, dict[str, Any]]: + """Apply the region rule to raw bytes; the legacy longest-run path is the fallback. + + Region rule: from the first bundle marker to end of file, every printable + run of at least `MIN_RUN_BYTES`, joined with newlines. A build with no + marker at all falls back to the largest printable run around a known + anchor, which is what every build before the bytecode layout needed. + """ + started = time.perf_counter() + first = -1 + marker_used = "" + for marker in BUNDLE_MARKERS: + pos = data.find(marker) + if pos >= 0 and (first < 0 or pos < first): + first, marker_used = pos, marker.decode("ascii") + if first < 0: + return _select_longest_run(data, meta) + + runs = [m.group(0) for m in RUN_RE.finditer(data, first)] + joined = b"\n".join(runs) + meta["anchor"] = marker_used + meta["bundle_offset"] = first + meta["region_rule"] = ( + f"first bundle marker to end of file; printable runs of at least " + f"{MIN_RUN_BYTES} bytes joined with newlines" + ) + meta["runs"] = len(runs) + meta["joined_bytes"] = len(joined) + meta["runs_below_floor"] = max( + 0, data.count(REGISTRATION_TOKEN, first) - joined.count(REGISTRATION_TOKEN) + ) + meta["elapsed_seconds"] = round(time.perf_counter() - started, 3) + if len(joined) < 1_000_000: + meta["error"] = ( + f"joined printable region is only {len(joined)} bytes - " + "this build does not embed the CLI bundle where expected" + ) + return None, meta + return joined.decode("latin1"), meta + +def _select_longest_run( + data: bytes, meta: dict[str, Any] +) -> tuple[str | None, dict[str, Any]]: + """Legacy region selection: the largest printable run around a known anchor.""" + meta["region_rule"] = ( + "no bundle marker found; largest printable run around an anchor" + ) printable = bytearray(256) for c in b"\t\n\r": printable[c] = 1 @@ -519,56 +603,206 @@ def _read_aliases(body: str) -> list[str]: return re.findall(r'"([^"]+)"', m.group(1)) -def discover_registrar(src: str, export_name: str) -> str | None: - """Resolve a minified registrar function via its readable export name. +_IDENT = r"[A-Za-z_$][A-Za-z0-9_$]*" +# The canary registration: `doctor` has shipped in every build observed and its +# alias makes the literal unambiguous. The callee immediately before it is the +# registrar when no export map names one. +_CANARY_REGISTRATION = r'\(\{name:"doctor",aliases:\["checkup"\]' + + +def discover_registrar_route( + src: str, export_name: str +) -> tuple[str | None, str | None]: + """Resolve a minified registrar function, and say which route resolved it. Minified identifiers are regenerated every release, so hardcoding one dates - the script immediately. The bundle's export maps keep the original names - (`registerBundledSkill:()=>xu`), which makes them a stable way in. + the script immediately. Three routes, tried in order, each keyed on + something the bundle keeps readable: + + export-map the CJS getter `registerBundledSkill:()=>xu` + esm-export the ESM export list `xu as registerBundledSkill` + canary the callee immediately preceding the `doctor` registration + (only for the bundled-skill registrar) + + Returns (identifier, route), or (None, None) when no route resolves. """ - m = re.search(re.escape(export_name) + r":\(\)=>([A-Za-z_$][A-Za-z0-9_$]*)", src) - return m.group(1) if m else None + m = re.search(re.escape(export_name) + r":\(\)=>(" + _IDENT + ")", src) + if m: + return m.group(1), "export-map" + m = re.search(r"\b(" + _IDENT + r") as " + re.escape(export_name) + r"\b", src) + if m: + return m.group(1), "esm-export" + if export_name == "registerBundledSkill": + m = re.search(r"\b(" + _IDENT + r")" + _CANARY_REGISTRATION, src) + if m: + return m.group(1), "canary" + return None, None + + +def discover_registrar(src: str, export_name: str) -> str | None: + """The registrar identifier alone; see `discover_registrar_route`.""" + return discover_registrar_route(src, export_name)[0] + + +_KEBAB_BINDING_RE = re.compile( + r"\b(" + _IDENT + r')\s*=\s*"([a-z][a-z0-9]*(?:-[a-z0-9]+)*)"' +) -def build_const_map(src: str) -> dict[str, str]: - """Map single-valued identifiers to their kebab-case string literal. +def build_const_index( + src: str, idents: set[str] | None = None +) -> dict[str, list[tuple[int, str]]]: + """Every `ident="kebab-case"` binding, keyed by identifier, in source order. Several bundled skills are registered as `xu({name:gme,...})` where `gme` - is a hoisted constant, so a literal-only scan silently misses them. + is a hoisted constant, so a literal-only scan silently misses them. The + positions are kept because a name is resolved by locality, never by a + single global value: the same identifier is bound to other strings in + unrelated modules, and in the bytecode layout the real binding sits + megabytes ahead of the registration. Passing the identifiers that need + resolving narrows the scan to those names, which is a fraction of the cost + of indexing every binding in a 37 MB source. + """ + if idents is not None: + if not idents: + return {} + pattern = re.compile( + r"\b(" + + "|".join(re.escape(i) for i in sorted(idents, key=len, reverse=True)) + + r')\s*=\s*"([a-z][a-z0-9]*(?:-[a-z0-9]+)*)"' + ) + else: + pattern = _KEBAB_BINDING_RE + index: dict[str, list[tuple[int, str]]] = {} + for m in pattern.finditer(src): + index.setdefault(m.group(1), []).append((m.start(), m.group(2))) + return index + + +def resolve_name_ident( + ident: str, at: int, index: dict[str, list[tuple[int, str]]] +) -> str | None: + """Resolve a registration's name identifier by its nearest preceding binding. + + Locality rule, stated exactly: the binding is the nearest `ident="..."` + before the registration; a farther binding never wins over a nearer one, + which is what keeps an unrelated module's `oO="ehrpd"` from shadowing the + real `oO="artifact-design"` bound closer in. A single-character identifier + is trusted only when that nearest binding lies within + `SHORT_IDENT_LOCALITY_BYTES`, because such names are function-local and a + far binding belongs to some other function. No preceding binding at all is + unresolved, never guessed. + """ + bindings = index.get(ident) + if not bindings: + return None + k = bisect.bisect_left(bindings, (at, "")) - 1 + if k < 0: + return None + pos, value = bindings[k] + if len(ident) == 1 and at - pos > SHORT_IDENT_LOCALITY_BYTES: + return None + return value + + +_FOR_HEAD_RE = re.compile(r"for\((?P[^()]*)\)\s*\{?\s*$") +_INVOCATION_FIELDS: tuple[tuple[str, str], ...] = ( + ("user_invocable", "userInvocable"), + ("disable_model_invocation", "disableModelInvocation"), + ("terminal_oriented", "terminalOriented"), + ("survives_kill_switch", "survivesBundledKillSwitch"), +) + + +def _is_loop_registration(src: str, call_start: int, ident: str) -> bool: + """True when the registration sits inside a `for(... of ...)` whose head binds `ident`. + + Such a registration is a dynamic roster (one call registering many + skills from a table), not one skill with a computed name. """ - seen: dict[str, set[str]] = {} - for m in re.finditer( - r'\b([A-Za-z_$][A-Za-z0-9_$]{0,8})\s*=\s*"([a-z][a-z0-9]*(?:-[a-z0-9]+)*)"', src - ): - seen.setdefault(m.group(1), set()).add(m.group(2)) - return {k: next(iter(v)) for k, v in seen.items() if len(v) == 1} + pre = src[max(0, call_start - 300) : call_start] + m = _FOR_HEAD_RE.search(pre) + if not m: + return False + head = m.group("head") + return bool( + re.search(r"\bof\b", head) and re.search(r"\b" + re.escape(ident) + r"\b", head) + ) + + +def read_invocation_fields(body: str) -> dict[str, Any]: + """The invocation-control fields a registration carries, when present. + + `!0` is true and `!1` false in the minified source. A function-valued + field (`disableModelInvocation:()=>...`) is recorded as true with + `flag_driven`, matching the binary's own serializer, which treats any + function as disabled. + """ + out: dict[str, Any] = {} + for key, field_name in _INVOCATION_FIELDS: + m = re.search( + r"\b" + field_name + r":(!0|!1|\(\)=>|function\b|[A-Za-z_$])", body + ) + if not m: + continue + token = m.group(1) + if token == "!0": + out[key] = True + elif token == "!1": + out[key] = False + else: + out[key] = True + out.setdefault("flag_driven", []).append(key) + return out + + +def registrations_of(entry: Any) -> list[dict[str, Any]]: + """Every registration behind one bundled-skill name. + + A name maps to one registration object, or to a list when two distinct + registrations share the name (a collision). Consumers read through this + helper so neither shape is a special case. + """ + if isinstance(entry, list): + return [e for e in entry if isinstance(e, dict)] + if isinstance(entry, dict): + return [entry] + return [] def extract_bundled_skills( src: str, braces: BraceMap -) -> tuple[dict[str, dict[str, Any]], dict[str, Any]]: +) -> tuple[dict[str, Any], dict[str, Any]]: """Bundled skills, keyed by name, plus notes about resolution. Each registration's fields are bound to its own `{...}` via the brace map, for the same reason command extraction is: registrations sit flush against one another, so a fixed-width window around one silently adopts the next one's description or aliases whenever a field is absent. + + Two distinct registrations sharing one name are both kept, as a list under + that name with a `collision` note, never merged and never last-writer-wins: + a consumer deciding a per-registration property such as model + invocability would otherwise read whichever the bundle happened to place + last. """ notes: dict[str, Any] = {} - fn = discover_registrar(src, "registerBundledSkill") + fn, route = discover_registrar_route(src, "registerBundledSkill") notes["registrar"] = fn + notes["registrar_route"] = route if not fn: notes["error"] = "registerBundledSkill export not found - build layout changed" return {}, notes - consts = build_const_map(src) - out: dict[str, dict[str, Any]] = {} - unresolved: list[str] = [] + # First pass: bound every call and read its name expression. A call whose + # object carries no `name:` is another module's function that happens to + # share the minified identifier, not a registration, so it is counted + # apart and never inflates the resolved-versus-seen gap. + calls: list[tuple[int, str, re.Match[str] | None]] = [] unbounded = 0 - seen = 0 - + same_ident_calls = 0 + name_re = re.compile(r"\bname:(?:" + _STR + r"|(" + _IDENT + r")|(`[^`]*`))") for m in re.finditer(re.escape(fn) + r"\(\{", src): - seen += 1 open_i = m.end() - 1 # the '{' captured by the pattern close_i = braces.pairs.get(open_i) if close_i is None: @@ -577,19 +811,45 @@ def extract_bundled_skills( unbounded += 1 continue body = src[open_i : close_i + 1] - nm = re.search(r"name:(?:" + _STR + r"|([A-Za-z_$][A-Za-z0-9_$]{0,8}))", body) + nm = name_re.search(body) if not nm: + same_ident_calls += 1 continue + calls.append((m.start(), body, nm)) + + idents = {nm.group(2) for _, _, nm in calls if nm is not None and nm.group(2)} + index = build_const_index(src, idents) + out: dict[str, Any] = {} + unresolved: list[str] = [] + dynamic_rosters = 0 + dynamic_patterns: list[str] = [] + seen = 0 + collisions: list[str] = [] + + for call_start, body, nm in calls: + seen += 1 + assert nm is not None if nm.group(1) is not None: name = _unescape(nm.group(1)) + elif nm.group(3) is not None: + # A template literal builds the name at runtime: one call + # registering a family, enumerable only by running it. + dynamic_rosters += 1 + dynamic_patterns.append(nm.group(3)) + continue else: ident = nm.group(2) - if ident not in consts: + if _is_loop_registration(src, call_start, ident): + dynamic_rosters += 1 + dynamic_patterns.append(f"for(... of ...) over {ident}") + continue + resolved = resolve_name_ident(ident, call_start, index) + if resolved is None: unresolved.append(ident) continue - name = consts[ident] + name = resolved desc = _MENUDESC_RE.search(body) - out[name] = { + rec: dict[str, Any] = { "name": name, "source": "bundled-skill", "description": _unescape(desc.group(1)) if desc else "", @@ -597,16 +857,49 @@ def extract_bundled_skills( "gated": "isEnabled" in body, "hidden": "isHidden" in body, } + rec.update(read_invocation_fields(body)) + prev = out.get(name) + if prev is None: + out[name] = rec + continue + existing = registrations_of(prev) + if any(_same_registration(e, rec) for e in existing): + continue + # A genuine collision: keep every registration, keyed by name. + for e in existing: + e["collision"] = True + rec["collision"] = True + out[name] = [*existing, rec] + if name not in collisions: + collisions.append(name) notes["registrations_seen"] = seen - notes["resolved"] = len(out) + notes["resolved"] = sum(len(registrations_of(e)) for e in out.values()) if unresolved: notes["unresolved_dynamic_names"] = sorted(set(unresolved)) + if dynamic_rosters: + notes["dynamic_roster"] = dynamic_rosters + notes["dynamic_roster_patterns"] = dynamic_patterns if unbounded: notes["unbounded_registrations"] = unbounded + if same_ident_calls: + notes["same_identifier_calls_skipped"] = same_ident_calls + if collisions: + notes["collisions"] = sorted(collisions) return out, notes +def _same_registration(a: dict[str, Any], b: dict[str, Any]) -> bool: + # `flag_driven` is part of the identity: a constant-true invocation field + # and a function-valued one read as the same boolean, and the difference + # (decided at runtime versus fixed) is exactly the evidence a collision + # exists to preserve. + keys = ("description", "aliases", "gated", "hidden", "flag_driven") + tuple( + k for k, _ in _INVOCATION_FIELDS + ) + return all(a.get(k) == b.get(k) for k in keys) + + def extract_plugin_backed(src: str) -> dict[str, str]: """Commands the build registers as plugin-backed (`pluginName`).""" out: dict[str, str] = {} @@ -636,80 +929,155 @@ def detect_cli_version(src: str) -> str | None: return best[0] if best[1] >= 20 else None +def _lane_status(problems: list[str], advisories: list[str]) -> str: + return "broken" if problems else ("degraded" if advisories else "ok") + + def check_integrity( src: str, commands: dict[str, Any], skills: dict[str, Any], skill_notes: dict[str, Any], + plugin_backed: dict[str, str] | None = None, + runs_below_floor: int = 0, ) -> dict[str, Any]: - """Decide whether this extraction can be trusted, and say why. + """Decide whether this extraction can be trusted, per lane, and say why. A drifted build usually degrades quietly: the script still returns rows, just fewer than exist. Every check here exists to convert that quiet shortfall into a stated one, so a downstream reader is never handed a confident short list. + + One rule for one state: each lane (`builtin_commands`, `bundled_skills`, + `plugin_backed`) carries its own status, and the top-level status is the + worst lane. `broken` at the top level means every lane is broken or the + binary is unreadable; a run with at least one healthy lane is at most + `degraded`, with each broken lane's problems restated as top-level + advisories prefixed by the lane name, so the healthy lanes' counts stay + reportable and the broken lane is named rather than hidden. """ - problems: list[str] = [] - advisories: list[str] = [] + lanes: dict[str, dict[str, Any]] = { + lane: {"status": "ok", "problems": [], "advisories": []} for lane in LANES + } + top_advisories: list[str] = [] version = detect_cli_version(src) if version is None: - advisories.append( + top_advisories.append( "could not read a CLI version from the bundle; drift against the last " f"validated build {VALIDATED_AGAINST} cannot be checked" ) elif version != VALIDATED_AGAINST: - advisories.append( + top_advisories.append( f"cli {version} differs from the last validated build {VALIDATED_AGAINST}; " "counts are believed, not verified - re-run the skill's evals to revalidate" ) + builtin = lanes["builtin_commands"] missing = [c for c in CANARY_COMMANDS if c not in commands] if missing: - problems.append( + builtin["problems"].append( f"canary commands absent: {', '.join(missing)} - extraction is broken, " "not merely drifted" ) - type_tokens = len(_TYPE_RE.findall(src)) yield_ratio = (len(commands) / type_tokens) if type_tokens else 0.0 if type_tokens and yield_ratio < MIN_COMMAND_YIELD: - problems.append( + builtin["problems"].append( f"resolved {len(commands)} commands from {type_tokens} type tokens " f"({yield_ratio:.0%}); the brace reader is not resolving enclosing objects" ) + bundled = lanes["bundled_skills"] + # Both export shapes: the CJS getter and the ESM export list. A + # registrar-shaped name in either that this script does not know is the + # signal of a registration path it is not reading. found_registrars = set(re.findall(r"\b(register[A-Za-z]*)\s*:\(\)=>", src)) + found_registrars |= set( + re.findall( + r"\b" + _IDENT + r" as (register[A-Za-z]*(?:Skill|Command|Agent))\b", src + ) + ) registrar_like = { r for r in found_registrars if re.search(r"(Skill|Command|Agent)", r) } unknown = sorted(registrar_like - KNOWN_REGISTRAR_EXPORTS) if unknown: - advisories.append( + bundled["advisories"].append( "unrecognised registrar-shaped exports: " + ", ".join(unknown) + " - a new registration path may exist and this run may under-report" ) - seen = skill_notes.get("registrations_seen") resolved = skill_notes.get("resolved") - if isinstance(seen, int) and isinstance(resolved, int) and seen > resolved: - advisories.append( - f"{seen - resolved} bundled-skill registration(s) used a computed name and " - "were not resolved; the bundled-skill list is a floor, not a total" + dynamic = skill_notes.get("dynamic_roster", 0) + if isinstance(seen, int) and isinstance(resolved, int): + gap = seen - resolved - (dynamic if isinstance(dynamic, int) else 0) + if gap > 0: + bundled["advisories"].append( + f"{gap} bundled-skill registration(s) used a computed name and " + "were not resolved; the bundled-skill list is a floor, not a total" + ) + if isinstance(dynamic, int) and dynamic > 0: + bundled["advisories"].append( + f"{dynamic} registration(s) register a dynamic roster (a loop over a " + "table); those names are not enumerable statically and the " + "bundled-skill list is a floor, not a total" ) - if not skills: - problems.append("no bundled skills resolved - the registrar lookup failed") + bundled["problems"].append( + "no bundled skills resolved - the registrar lookup failed" + ) + + if runs_below_floor > 0: + # A registration literal in a run under the floor was never read by + # any lane. It is attributed to the bundled-skill lane, where the + # small runs sit, so the roster is labelled a floor rather than a + # total; the literal is generic, so the command lane may be short too. + bundled["advisories"].append( + f"{runs_below_floor} registration literal(s) sit in printable runs shorter " + f"than the {MIN_RUN_BYTES}-byte floor and were not read; the bundled-skill " + "list (and possibly the command list) is a floor, not a total" + ) + + backed = lanes["plugin_backed"] + if plugin_backed is not None: + missing_backed = [c for c in PLUGIN_BACKED_CANARY if c not in plugin_backed] + if missing_backed: + backed["problems"].append( + f"canary plugin-backed built-in(s) absent: {', '.join(missing_backed)} - " + "the pluginName scan resolved nothing it should have" + ) + + problems: list[str] = [] + advisories: list[str] = list(top_advisories) + for lane in LANES: + lanes[lane]["status"] = _lane_status( + lanes[lane]["problems"], lanes[lane]["advisories"] + ) + all_broken = all(lanes[lane]["status"] == "broken" for lane in LANES) + for lane in LANES: + entry = lanes[lane] + if entry["status"] == "broken" and all_broken: + problems.extend(f"{lane}: {p}" for p in entry["problems"]) + elif entry["status"] == "broken": + advisories.extend( + f"{lane} lane broken: {p} (its counts are not reportable; the other " + "lanes' counts stand)" + for p in entry["problems"] + ) + advisories.extend(f"{lane}: {a}" for a in entry["advisories"]) - status = "broken" if problems else ("degraded" if advisories else "ok") + status = "broken" if all_broken else ("degraded" if advisories else "ok") return { "status": status, + "lanes": lanes, "cli_version": version, "validated_against": VALIDATED_AGAINST, "command_yield": round(yield_ratio, 3), "type_tokens": type_tokens, "registrars_seen": sorted(registrar_like), + "registrar_route": skill_notes.get("registrar_route"), "problems": problems, "advisories": advisories, } @@ -944,8 +1312,9 @@ def build_report(args: argparse.Namespace) -> dict[str, Any]: commands[name]["plugin_name"] = plugin commands[name]["source"] = "plugin-backed-builtin" elif name in skills: - skills[name]["plugin_name"] = plugin - skills[name]["source"] = "plugin-backed-builtin" + for registration in registrations_of(skills[name]): + registration["plugin_name"] = plugin + registration["source"] = "plugin-backed-builtin" # A name registered as a bundled skill is a skill, not a command. for name in list(commands): @@ -958,7 +1327,12 @@ def build_report(args: argparse.Namespace) -> dict[str, Any]: report["bundled_skill_notes"] = skill_notes report["plugin_backed"] = plugin_backed report["integrity"] = check_integrity( - src, commands, skills, skill_notes + src, + commands, + skills, + skill_notes, + plugin_backed, + int(meta.get("runs_below_floor", 0) or 0), ) if not args.binary_only: @@ -1040,6 +1414,8 @@ def main(argv: list[str] | None = None) -> int: f"{integrity['status'].upper()}: cli {integrity['cli_version']}, " f"validated against {integrity['validated_against']}" ) + for lane, entry in (integrity.get("lanes") or {}).items(): + print(f" lane {lane}: {entry['status']}") for p in integrity["problems"]: print(f" problem: {p}") for a in integrity["advisories"]: diff --git a/plugins/claude-ops/skills/inventory/scripts/test_inventory.py b/plugins/claude-ops/skills/inventory/scripts/test_inventory.py index 4373f22c03..7ba97298b4 100755 --- a/plugins/claude-ops/skills/inventory/scripts/test_inventory.py +++ b/plugins/claude-ops/skills/inventory/scripts/test_inventory.py @@ -67,7 +67,8 @@ def _extract(self, src: str) -> dict: def test_adjacent_literals_do_not_bleed(self) -> None: got = self._extract(self.ADJACENT) self.assertEqual( - got["artifacts"]["description"], "Browse your published and shared artifacts" + got["artifacts"]["description"], + "Browse your published and shared artifacts", ) self.assertEqual(got["btw"]["description"], "Ask a quick side question") @@ -75,7 +76,9 @@ def test_gated_and_hidden_flags(self) -> None: got = self._extract(self.ADJACENT) self.assertTrue(got["artifacts"]["gated"]) self.assertFalse(got["artifacts"]["hidden"]) - src = 'x={type:"local",name:"heapdump",description:"d",get isHidden(){return!0}};' + src = ( + 'x={type:"local",name:"heapdump",description:"d",get isHidden(){return!0}};' + ) self.assertTrue(self._extract(src)["heapdump"]["hidden"]) def test_aliases_are_collected(self) -> None: @@ -95,7 +98,9 @@ def test_shell_builtin_is_not_a_command(self) -> None: self.assertEqual(self._extract(src), {}) def test_userfacingname_is_a_fallback(self) -> None: - src = 'x={type:"local-jsx",userFacingName(){return"autofix-pr"},description:"d"};' + src = ( + 'x={type:"local-jsx",userFacingName(){return"autofix-pr"},description:"d"};' + ) self.assertIn("autofix-pr", self._extract(src)) def test_internal_names_are_marked(self) -> None: @@ -105,7 +110,7 @@ def test_internal_names_are_marked(self) -> None: class TestBundledSkills(unittest.TestCase): BUNDLE = ( - 'pt(Q,{registerBundledSkill:()=>xu,getBundledSkills:()=>dF});' + "pt(Q,{registerBundledSkill:()=>xu,getBundledSkills:()=>dF});" 'var gme="code-review",dvz="dataviz";' 'xu({name:gme,aliases:["review"],menuDescription:"Review the current diff"});' 'xu({name:dvz,menuDescription:"Chart and dashboard design guidance"});' @@ -113,7 +118,9 @@ class TestBundledSkills(unittest.TestCase): ) def test_registrar_is_discovered_not_hardcoded(self) -> None: - self.assertEqual(inv.discover_registrar(self.BUNDLE, "registerBundledSkill"), "xu") + self.assertEqual( + inv.discover_registrar(self.BUNDLE, "registerBundledSkill"), "xu" + ) def test_missing_export_returns_none(self) -> None: self.assertIsNone(inv.discover_registrar("var x=1;", "registerBundledSkill")) @@ -121,19 +128,23 @@ def test_missing_export_returns_none(self) -> None: def test_constant_names_resolve(self) -> None: # The failure this guards: a literal-only scan silently drops roughly a # third of the bundled skills, including code-review and dataviz. - skills, notes = inv.extract_bundled_skills(self.BUNDLE, inv.build_brace_map(self.BUNDLE)) + skills, notes = inv.extract_bundled_skills( + self.BUNDLE, inv.build_brace_map(self.BUNDLE) + ) self.assertEqual(notes["registrar"], "xu") self.assertIn("code-review", skills) self.assertIn("dataviz", skills) self.assertIn("doctor", skills) self.assertEqual(skills["code-review"]["aliases"], ["review"]) - def test_ambiguous_constant_is_not_resolved(self) -> None: - # An identifier bound to two different strings cannot be resolved - # safely, so it must be reported rather than guessed. + def test_a_rebound_constant_resolves_to_its_nearest_binding(self) -> None: + # An identifier bound to two different strings resolves by locality: + # the binding nearest before the registration is the one in scope. src = self.BUNDLE + 'var zz="a-one";var zz="a-two";xu({name:zz});' - _, notes = inv.extract_bundled_skills(src, inv.build_brace_map(src)) - self.assertGreater(notes["registrations_seen"], notes["resolved"]) + skills, notes = inv.extract_bundled_skills(src, inv.build_brace_map(src)) + self.assertIn("a-two", skills) + self.assertNotIn("a-one", skills) + self.assertEqual(notes["registrations_seen"], notes["resolved"]) class TestIntegrity(unittest.TestCase): @@ -142,10 +153,12 @@ def _src(self, extra: str = "") -> str: # ignores a version mentioned only once, since that is a dependency's # version rather than the build's. return ( - 'pt(Q,{registerBundledSkill:()=>xu});' + "pt(Q,{registerBundledSkill:()=>xu});" + f'"{inv.VALIDATED_AGAINST}"' * 30 - + "".join(f'x{i}={{type:"local",name:"{n}",description:"d"}};' - for i, n in enumerate(inv.CANARY_COMMANDS)) + + "".join( + f'x{i}={{type:"local",name:"{n}",description:"d"}};' + for i, n in enumerate(inv.CANARY_COMMANDS) + ) + extra ) @@ -155,46 +168,454 @@ def _commands(self, src: str) -> dict: def test_ok_when_everything_resolves(self) -> None: src = self._src() got = inv.check_integrity( - src, self._commands(src), {"a": {}}, {"registrations_seen": 1, "resolved": 1} + src, + self._commands(src), + {"a": {}}, + {"registrations_seen": 1, "resolved": 1}, ) self.assertEqual(got["status"], "ok") - def test_broken_when_canary_missing(self) -> None: - src = 'x={type:"local",name:"help",description:"d"};' - got = inv.check_integrity(src, self._commands(src), {"a": {}}, {}) - self.assertEqual(got["status"], "broken") - self.assertTrue(any("canary" in p for p in got["problems"])) + def test_canary_missing_breaks_the_builtin_lane_only(self) -> None: + # One rule for one state: a broken lane beside a healthy one is a + # degraded run whose advisory names the lane, never a broken run that + # voids the healthy lane's counts. + src = ( + 'x={type:"local",name:"help",description:"d"};' + + f'"{inv.VALIDATED_AGAINST}"' * 30 + ) + got = inv.check_integrity( + src, + self._commands(src), + {"a": {}}, + {}, + {"security-review": "security-review"}, + ) + self.assertEqual(got["lanes"]["builtin_commands"]["status"], "broken") + self.assertEqual(got["lanes"]["bundled_skills"]["status"], "ok") + self.assertEqual(got["lanes"]["plugin_backed"]["status"], "ok") + self.assertEqual(got["status"], "degraded") + self.assertEqual(got["problems"], []) + self.assertTrue( + any( + a.startswith("builtin_commands lane broken: canary") + for a in got["advisories"] + ) + ) - def test_broken_when_no_skills_resolve(self) -> None: + def test_no_skills_breaks_the_bundled_lane_only(self) -> None: src = self._src() - got = inv.check_integrity(src, self._commands(src), {}, {}) + got = inv.check_integrity( + src, self._commands(src), {}, {}, {"security-review": "security-review"} + ) + self.assertEqual(got["lanes"]["bundled_skills"]["status"], "broken") + self.assertEqual(got["status"], "degraded") + self.assertTrue( + any(a.startswith("bundled_skills lane broken:") for a in got["advisories"]) + ) + + def test_every_lane_broken_is_broken(self) -> None: + src = ( + 'x={type:"local",name:"help",description:"d"};' + + f'"{inv.VALIDATED_AGAINST}"' * 30 + ) + got = inv.check_integrity(src, self._commands(src), {}, {}, {}) + self.assertEqual( + [got["lanes"][lane]["status"] for lane in inv.LANES], + ["broken", "broken", "broken"], + ) self.assertEqual(got["status"], "broken") + self.assertTrue(all(":" in p for p in got["problems"])) + + def test_missing_plugin_backed_canary_breaks_that_lane(self) -> None: + src = self._src() + got = inv.check_integrity( + src, + self._commands(src), + {"a": {}}, + {"registrations_seen": 1, "resolved": 1}, + {}, + ) + self.assertEqual(got["lanes"]["plugin_backed"]["status"], "broken") + self.assertEqual(got["status"], "degraded") + + def test_dynamic_roster_is_an_advisory_not_an_unresolved_name(self) -> None: + src = self._src() + got = inv.check_integrity( + src, + self._commands(src), + {"a": {}}, + {"registrations_seen": 2, "resolved": 1, "dynamic_roster": 1}, + {"security-review": "security-review"}, + ) + self.assertEqual(got["status"], "degraded") + self.assertTrue(any("dynamic roster" in a for a in got["advisories"])) + self.assertFalse(any("computed name" in a for a in got["advisories"])) + + def test_registrations_below_the_floor_degrade_the_bundled_lane(self) -> None: + # A literal in a run under the floor was never read, so the roster is + # a floor even when every read registration resolved. + src = self._src() + got = inv.check_integrity( + src, + self._commands(src), + {"a": {}}, + {"registrations_seen": 1, "resolved": 1}, + {"security-review": "security-review"}, + runs_below_floor=2, + ) + self.assertEqual(got["lanes"]["bundled_skills"]["status"], "degraded") + self.assertEqual(got["status"], "degraded") + self.assertTrue( + any("shorter than" in a and "floor" in a for a in got["advisories"]) + ) + + def test_esm_export_list_feeds_the_registrar_advisory(self) -> None: + src = self._src("export{zz as registerSomethingNewSkill};") + got = inv.check_integrity( + src, + self._commands(src), + {"a": {}}, + {"registrations_seen": 1, "resolved": 1}, + {"security-review": "security-review"}, + ) + self.assertTrue( + any("registerSomethingNewSkill" in a for a in got["advisories"]) + ) + self.assertEqual(got["lanes"]["bundled_skills"]["status"], "degraded") def test_degraded_on_unknown_registrar(self) -> None: # The silent-drift signal: a registration path the script does not know. src = self._src("pt(Q,{registerSomethingNewSkill:()=>zz});") got = inv.check_integrity( - src, self._commands(src), {"a": {}}, {"registrations_seen": 1, "resolved": 1} + src, + self._commands(src), + {"a": {}}, + {"registrations_seen": 1, "resolved": 1}, ) self.assertEqual(got["status"], "degraded") - self.assertTrue(any("registerSomethingNewSkill" in a for a in got["advisories"])) + self.assertTrue( + any("registerSomethingNewSkill" in a for a in got["advisories"]) + ) def test_degraded_when_registrations_exceed_resolved(self) -> None: src = self._src() got = inv.check_integrity( - src, self._commands(src), {"a": {}}, {"registrations_seen": 5, "resolved": 3} + src, + self._commands(src), + {"a": {}}, + {"registrations_seen": 5, "resolved": 3}, ) self.assertEqual(got["status"], "degraded") self.assertTrue(any("floor" in a for a in got["advisories"])) - def test_low_yield_is_broken(self) -> None: + def test_low_yield_breaks_the_builtin_lane(self) -> None: # Many registration tokens present but few commands resolved means the # brace reader stopped working - the exact shape of a quiet shortfall. src = self._src() + 'type:"local"' * 200 got = inv.check_integrity( - src, self._commands(src), {"a": {}}, {"registrations_seen": 1, "resolved": 1} + src, + self._commands(src), + {"a": {}}, + {"registrations_seen": 1, "resolved": 1}, + {"security-review": "security-review"}, ) - self.assertEqual(got["status"], "broken") + self.assertEqual(got["lanes"]["builtin_commands"]["status"], "broken") + self.assertEqual(got["status"], "degraded") + + +class TestReadBundleRegionRule(unittest.TestCase): + """Region selection against synthetic byte layouts. + + A bytecode-fragmented build scatters the readable source across thousands + of printable runs; the doctor registration sat in a 1.3 KB run on the + build that broke the previous single-longest-run rule. + """ + + MARKER = b"// @bun @bytecode @bun-cjs\n" + BIG = b'x={type:"local",name:"help",description:"d"};' * 30_000 # > 1 MB + GAP = bytes(range(0x80, 0x100)) * 8 # non-printable bytes + DOCTOR = b'eo({name:"doctor",aliases:["checkup"],menuDescription:"Health-check"});' + CONST = b'var kYe="simplify";' + b"a" * 300 + + def _write(self, layout: bytes) -> pathlib.Path: + import tempfile + + tmp = tempfile.NamedTemporaryFile(delete=False, suffix=".bin") + self.addCleanup(lambda: pathlib.Path(tmp.name).unlink(missing_ok=True)) + tmp.write(layout) + tmp.close() + return pathlib.Path(tmp.name) + + def _fragmented(self) -> bytes: + return ( + self.MARKER + + self.BIG + + self.GAP + + self.DOCTOR + + b"b" * 2000 + + self.GAP + + self.CONST + + b"c" * 300 + ) + + def test_read_bundle_joins_every_run_above_the_floor(self) -> None: + src, meta = inv.read_bundle(self._write(self._fragmented())) + self.assertIsNotNone(src) + assert src is not None + self.assertIn('name:"help"', src) + self.assertIn('name:"doctor"', src) + self.assertIn('kYe="simplify"', src) + self.assertEqual(meta["runs"], 3) + self.assertEqual(meta["runs_below_floor"], 0) + self.assertIn("256", meta["region_rule"]) + self.assertGreaterEqual(meta["elapsed_seconds"], 0) + + def test_read_bundle_pe_container_uses_the_same_rule(self) -> None: + src, meta = inv.read_bundle(self._write(b"MZ" + self.GAP + self._fragmented())) + self.assertEqual(meta["container"], "PE") + assert src is not None + self.assertIn('name:"doctor"', src) + self.assertEqual(meta["runs"], 3) + + def test_read_bundle_legacy_single_run_is_unchanged(self) -> None: + legacy = self.MARKER + b"registerBundledSkill:()=>xu;" + self.BIG + self.DOCTOR + src, meta = inv.read_bundle(self._write(legacy)) + assert src is not None + self.assertEqual(src.encode("latin1"), legacy) + self.assertEqual(meta["runs"], 1) + + def test_read_bundle_without_a_marker_falls_back_to_the_longest_run(self) -> None: + legacy = b"registerBundledSkill:()=>xu;" + self.BIG + self.DOCTOR + src, meta = inv.read_bundle( + self._write(self.GAP + legacy + self.GAP + b"tail" * 100) + ) + assert src is not None + self.assertEqual(src.encode("latin1"), legacy) + self.assertIn("no bundle marker", meta["region_rule"]) + + def test_read_bundle_counts_a_registration_below_the_floor(self) -> None: + # A run shorter than the floor carrying a registration is counted, + # never silently lost. + tiny = b'eo({name:"tiny"});' + layout = ( + self.MARKER + + self.BIG + + self.GAP + + tiny + + self.GAP + + self.DOCTOR + + b"b" * 300 + ) + src, meta = inv.read_bundle(self._write(layout)) + assert src is not None + self.assertNotIn('name:"tiny"', src) + self.assertEqual(meta["runs_below_floor"], 1) + + def test_read_bundle_rejects_a_region_under_one_megabyte(self) -> None: + src, meta = inv.read_bundle(self._write(self.MARKER + self.DOCTOR + b"b" * 300)) + self.assertIsNone(src) + self.assertIn("does not embed", meta["error"]) + + +class TestRegistrarRoutes(unittest.TestCase): + CANARY = 'eo({name:"doctor",aliases:["checkup"],menuDescription:"Health-check"});' + + def test_cjs_getter_route(self) -> None: + src = "pt(Q,{registerBundledSkill:()=>xu});" + self.CANARY + self.assertEqual( + inv.discover_registrar_route(src, "registerBundledSkill"), + ("xu", "export-map"), + ) + + def test_esm_export_list_route(self) -> None: + src = "export{eo as registerBundledSkill,dF as getBundledSkills};" + self.CANARY + self.assertEqual( + inv.discover_registrar_route(src, "registerBundledSkill"), + ("eo", "esm-export"), + ) + + def test_canary_route_when_no_export_names_the_registrar(self) -> None: + self.assertEqual( + inv.discover_registrar_route(self.CANARY, "registerBundledSkill"), + ("eo", "canary"), + ) + + def test_no_route_is_none_and_the_lane_is_broken(self) -> None: + self.assertEqual( + inv.discover_registrar_route("var x=1;", "registerBundledSkill"), + (None, None), + ) + skills, notes = inv.extract_bundled_skills( + "var x=1;", inv.build_brace_map("var x=1;") + ) + self.assertEqual(skills, {}) + self.assertIn("export not found", notes["error"]) + self.assertIsNone(notes["registrar_route"]) + + def test_route_is_recorded_in_the_notes(self) -> None: + src = "export{eo as registerBundledSkill};" + self.CANARY + _, notes = inv.extract_bundled_skills(src, inv.build_brace_map(src)) + self.assertEqual(notes["registrar_route"], "esm-export") + + +class TestNameLocality(unittest.TestCase): + """Computed names resolve by locality, never by a single global value.""" + + HEAD = "export{eo as registerBundledSkill};" + + def _skills(self, src: str) -> tuple[dict, dict]: + return inv.extract_bundled_skills(src, inv.build_brace_map(src)) + + def test_nearest_preceding_binding_wins_over_a_farther_one(self) -> None: + # The phantom this guards: `oO="ehrpd"` in an unrelated module far + # ahead must not shadow `oO="artifact-design"` bound closer in. + src = ( + self.HEAD + + 'var oO="ehrpd";' + + "z" * 500 + + 'var oO="artifact-design";' + + 'eo({name:oO,menuDescription:"Design"});' + ) + skills, notes = self._skills(src) + self.assertIn("artifact-design", skills) + self.assertNotIn("ehrpd", skills) + self.assertNotIn("unresolved_dynamic_names", notes) + + def test_a_lone_far_binding_of_a_short_identifier_is_not_resolved(self) -> None: + src = ( + self.HEAD + + 'var e="linux";' + + "z" * (inv.SHORT_IDENT_LOCALITY_BYTES + 10) + + 'eo({name:e,menuDescription:"D"});' + ) + skills, notes = self._skills(src) + self.assertEqual(skills, {}) + self.assertEqual(notes["unresolved_dynamic_names"], ["e"]) + + def test_a_short_identifier_bound_nearby_resolves(self) -> None: + # The design canvas registration: `var r="design"` a few KB ahead of + # `eo({name:r,...})` inside the same module. + src = ( + self.HEAD + + 'var r="design";' + + "z" * 4000 + + 'eo({name:r,menuDescription:"Draft"});' + ) + skills, _ = self._skills(src) + self.assertIn("design", skills) + + def test_a_long_identifier_bound_far_ahead_resolves(self) -> None: + src = ( + self.HEAD + + 'var kYe="simplify";' + + "z" * (inv.SHORT_IDENT_LOCALITY_BYTES * 2) + + 'eo({name:kYe,menuDescription:"Clean up"});' + ) + skills, _ = self._skills(src) + self.assertIn("simplify", skills) + + def test_a_binding_after_the_registration_does_not_resolve_it(self) -> None: + src = self.HEAD + 'eo({name:zz,menuDescription:"D"});var zz="later";' + skills, notes = self._skills(src) + self.assertEqual(skills, {}) + self.assertEqual(notes["unresolved_dynamic_names"], ["zz"]) + + def test_a_template_literal_name_is_a_dynamic_roster(self) -> None: + src = ( + self.HEAD + "eo({name:`artifact-${e}`,menuDescription:t,userInvocable:!0});" + ) + skills, notes = self._skills(src) + self.assertEqual(skills, {}) + self.assertEqual(notes["dynamic_roster"], 1) + self.assertEqual(notes["dynamic_roster_patterns"], ["`artifact-${e}`"]) + + def test_a_same_identifier_call_without_a_name_is_not_a_registration(self) -> None: + # Another module's `eo(...)` taking an options object: no `name:`, + # so it is neither a registration nor an unresolved one. + src = ( + self.HEAD + + 'eo({check:"overwrite",tx:e});eo({name:"run",menuDescription:"Launch"});' + ) + skills, notes = self._skills(src) + self.assertEqual(list(skills), ["run"]) + self.assertEqual(notes["registrations_seen"], 1) + self.assertEqual(notes["same_identifier_calls_skipped"], 1) + + def test_a_loop_registration_is_a_dynamic_roster(self) -> None: + src = ( + self.HEAD + + 'var e="linux";' + + "for(let{kind:e,menuDescription:o}of ls)eo({name:e,menuDescription:o});" + ) + skills, notes = self._skills(src) + self.assertEqual(skills, {}) + self.assertEqual(notes["dynamic_roster"], 1) + self.assertNotIn("unresolved_dynamic_names", notes) + self.assertEqual(notes["registrations_seen"], 1) + + +class TestInvocationFieldsAndCollisions(unittest.TestCase): + HEAD = "export{eo as registerBundledSkill};" + DOCTOR = ( + 'eo({name:"doctor",aliases:["checkup"],isEnabled:()=>!a.X,survivesBundledKillSwitch:!0,' + 'requires:{workspace:!0},terminalOriented:!0,menuDescription:"Health-check",' + "userInvocable:!0,disableModelInvocation:!0});" + ) + SIMPLIFY = 'eo({name:"simplify",menuDescription:"Clean up",userInvocable:!0});' + VERIFY = 'eo({name:"verify",description:As,userInvocable:!0,disableModelInvocation:()=>!P1e()});' + + def _skills(self, src: str) -> tuple[dict, dict]: + return inv.extract_bundled_skills(src, inv.build_brace_map(src)) + + def test_invocation_fields_are_read_when_present(self) -> None: + skills, _ = self._skills(self.HEAD + self.DOCTOR + self.SIMPLIFY) + doctor = skills["doctor"] + self.assertTrue(doctor["user_invocable"]) + self.assertTrue(doctor["disable_model_invocation"]) + self.assertTrue(doctor["terminal_oriented"]) + self.assertTrue(doctor["survives_kill_switch"]) + self.assertTrue(doctor["gated"]) + simplify = skills["simplify"] + self.assertTrue(simplify["user_invocable"]) + self.assertNotIn("disable_model_invocation", simplify) + self.assertNotIn("terminal_oriented", simplify) + + def test_a_function_valued_field_reads_as_true_and_flag_driven(self) -> None: + skills, _ = self._skills(self.HEAD + self.VERIFY) + self.assertTrue(skills["verify"]["disable_model_invocation"]) + self.assertEqual(skills["verify"]["flag_driven"], ["disable_model_invocation"]) + + def test_two_registrations_sharing_a_name_are_both_kept(self) -> None: + # 2.1.263 registers `design` twice: the canvas skill (model-invocable) + # and the claude.ai/design hub (model-disabled). Neither may win. + hub = 'eo({name:"design",menuDescription:"Work with Claude Design",disableModelInvocation:!0,userInvocable:!0});' + canvas = 'var r="design";eo({name:r,menuDescription:"Draft a design on a canvas",isEnabled:o,userInvocable:!0});' + skills, notes = self._skills(self.HEAD + hub + canvas) + design = skills["design"] + self.assertIsInstance(design, list) + self.assertEqual(len(design), 2) + self.assertEqual( + sorted(r.get("disable_model_invocation", False) for r in design), + [False, True], + ) + self.assertTrue(all(r["collision"] for r in design)) + self.assertEqual(notes["collisions"], ["design"]) + self.assertEqual(notes["resolved"], 2) + self.assertEqual(len(inv.registrations_of(design)), 2) + + def test_a_flag_driven_twin_of_a_constant_field_is_a_collision(self) -> None: + # Same boolean reading, different basis: one fixed, one decided at + # runtime. That difference is evidence, so both registrations stay. + fixed = 'eo({name:"verify",description:As,userInvocable:!0,disableModelInvocation:!0});' + skills, notes = self._skills(self.HEAD + fixed + self.VERIFY) + self.assertIsInstance(skills["verify"], list) + self.assertEqual(len(skills["verify"]), 2) + self.assertEqual(notes["collisions"], ["verify"]) + + def test_the_same_registration_seen_twice_is_not_a_collision(self) -> None: + skills, notes = self._skills(self.HEAD + self.SIMPLIFY + self.SIMPLIFY) + self.assertIsInstance(skills["simplify"], dict) + self.assertNotIn("collisions", notes) class TestContainerAndVersion(unittest.TestCase): @@ -216,20 +637,24 @@ def test_finds_plugin_name(self) -> None: 'x={name:"security-review",description:"Complete a security review",' 'pluginName:"security-review",pluginCommand:"security-review"};' ) - self.assertEqual(inv.extract_plugin_backed(src), {"security-review": "security-review"}) + self.assertEqual( + inv.extract_plugin_backed(src), {"security-review": "security-review"} + ) class TestBundledSkillFieldBinding(unittest.TestCase): """A registration missing a field must not adopt the next one's.""" BLEED = ( - 'pt(Q,{registerBundledSkill:()=>xu});' + "pt(Q,{registerBundledSkill:()=>xu});" 'xu({name:"first"});' 'xu({name:"second",aliases:["s"],menuDescription:"Second description"});' ) def test_missing_description_does_not_bleed_forward(self) -> None: - skills, _ = inv.extract_bundled_skills(self.BLEED, inv.build_brace_map(self.BLEED)) + skills, _ = inv.extract_bundled_skills( + self.BLEED, inv.build_brace_map(self.BLEED) + ) self.assertEqual(skills["first"]["description"], "") self.assertEqual(skills["first"]["aliases"], []) self.assertEqual(skills["second"]["description"], "Second description") @@ -240,15 +665,20 @@ class TestManifestComponentPaths(unittest.TestCase): def test_dotted_key_resolves(self) -> None: m = {"experimental": {"themes": "./custom-themes/"}} - self.assertEqual(inv._manifest_paths(m, "experimental.themes"), ["./custom-themes/"]) + self.assertEqual( + inv._manifest_paths(m, "experimental.themes"), ["./custom-themes/"] + ) def test_absent_key_is_none(self) -> None: self.assertIsNone(inv._manifest_paths({}, "agents")) - self.assertIsNone(inv._manifest_paths({"experimental": {}}, "experimental.themes")) + self.assertIsNone( + inv._manifest_paths({"experimental": {}}, "experimental.themes") + ) def test_array_form(self) -> None: self.assertEqual( - inv._manifest_paths({"commands": ["./a/", "./b/"]}, "commands"), ["./a/", "./b/"] + inv._manifest_paths({"commands": ["./a/", "./b/"]}, "commands"), + ["./a/", "./b/"], ) def test_declared_dir_replaces_default(self) -> None: @@ -262,17 +692,17 @@ def test_declared_dir_replaces_default(self) -> None: (root / "custom" / "declared.md").write_text("x", encoding="utf-8") spec = {"dir": "agents", "manifest": "agents", "kind": "dir-of-files"} self.assertEqual( - inv._scan_component(root, spec, {"agents": ["./custom/"]}), ["declared.md"] + inv._scan_component(root, spec, {"agents": ["./custom/"]}), + ["declared.md"], ) self.assertEqual(inv._scan_component(root, spec, {}), ["default.md"]) class TestSelfCheckDiagnostic(unittest.TestCase): def test_unknown_version_is_an_advisory(self) -> None: - src = ( - 'pt(Q,{registerBundledSkill:()=>xu});' - + "".join(f'x{i}={{type:"local",name:"{n}",description:"d"}};' - for i, n in enumerate(inv.CANARY_COMMANDS)) + src = "pt(Q,{registerBundledSkill:()=>xu});" + "".join( + f'x{i}={{type:"local",name:"{n}",description:"d"}};' + for i, n in enumerate(inv.CANARY_COMMANDS) ) got = inv.check_integrity( src, @@ -281,7 +711,9 @@ def test_unknown_version_is_an_advisory(self) -> None: {"registrations_seen": 1, "resolved": 1}, ) self.assertEqual(got["status"], "degraded") - self.assertTrue(any("could not read a CLI version" in a for a in got["advisories"])) + self.assertTrue( + any("could not read a CLI version" in a for a in got["advisories"]) + ) if __name__ == "__main__": diff --git a/scripts/skill-description-cap-baseline.txt b/scripts/skill-description-cap-baseline.txt index 51e52035a0..de8367d703 100644 --- a/scripts/skill-description-cap-baseline.txt +++ b/scripts/skill-description-cap-baseline.txt @@ -25,7 +25,6 @@ plugins/architecture/skills/improve # 1105 plugins/claude-config/skills/audit-prompting-postures # 1228 -plugins/claude-ops/skills/audit-native-overlap # 1153 plugins/claude-ops/skills/audit-performance # 1528 plugins/claude-ops/skills/audit-skill-visibility # 1467 plugins/code-tidying/skills/audit-dead-code # 1094