Defame1297/holocron
0
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases 3 Wiki Activity

fix(gates): boundary-clause check fires on hand-invoked skills, which are contractually exempt #108

New Issue
Closed
opened 2026-08-30 12:40:22 +00:00 by Claude · 2 comments
Claude commented 2026-08-30 12:40:22 +00:00
Collaborator
Copy Link

A skill declaring disable-model-invocation: true is told by ADR-0020 and by skill-author Step 2 to carry one plain human-facing sentence, no trigger list, no boundary clause. The deterministic boundary-clause check flags exactly that shape as a finding.

Neither validator knows the field exists — grep -rn 'disable-model-invocation' over scripts/skill-size-check.sh and both validate.sh files returns nothing.

Reproduction

plugins/bin/.apm/skills/zoom-out/SKILL.md is the in-tree exemplar ADR-0020 names, and follows the contract exactly:

name: zoom-out
description: Tell the agent to zoom out and give broader context or a higher-level
  perspective. Use when you're unfamiliar with a section of code or need to
  understand how it fits into the bigger picture.
disable-model-invocation: true

pre-commit run skill-size-check --all-files reports:

SUGGESTION: plugins/bin/.apm/skills/zoom-out/SKILL.md: description has no boundary clause
(ADR-0020). Add the prose form ("Do not use for X — use `y` instead") or the compressed
form ("Not X -> y") so the router knows where NOT to send this skill.

The advice is actively wrong for this skill: there is no router to inform, because the flag removes it from the model-visible listing entirely.

Severity

Low. SUGGESTION tier, so nothing blocks. It is permanent noise on every hand-invoked skill, and the fix advice contradicts the contract, so an author who follows it makes their skill worse.

Currently one skill is affected. Issue #99 adds caveman, and the population is capped small — see below.

Fix

Skip the boundary-clause check (and the trigger-quality checks that presuppose model invocation) when frontmatter carries disable-model-invocation: true. The size budgets should still apply: a hand-invoked description is not preloaded, but the body is still loaded on invocation.

Worth adding a test case; zoom-out is a stable fixture for it.

Context worth recording while here

ADR-0020's "Invocation as a design axis" verified that the flag removes a skill from the model-visible listing and that /zoom-out still works. It did not verify the other direction, and that half turns out to matter more:

Skill(zoom-out) -> Skill zoom-out cannot be used with Skill tool due to
disable-model-invocation. Ask the user to run /zoom-out themselves.

The flag hard-blocks the Skill tool, so no other skill can route to a hand-invoked skill. A Call `x` step in another skill's body stops working the moment x takes the flag.

This changed the #99 scope. Of the four hand-invoke candidates that issue names, three have inbound routes and would have broken their callers:

candidate inbound routes
caveman none — safe, and the only one being flagged
prototype diagnose
grill-me write-docs, skill-author/references/create.md
grill-with-docs improve-codebase-architecture, triage, write-docs, forge

forge Step 1 reads Call `bin:grill-with-docs` and is mandatory, so flagging that skill would have broken the factory's entry point.

Already recorded in CONTEXT.md's Hand-invoked skill entry (commit ef3e981), which previously covered listing visibility and preload cost but said nothing about invocability.

A skill declaring `disable-model-invocation: true` is told by ADR-0020 and by `skill-author` Step 2 to carry **one plain human-facing sentence, no trigger list, no boundary clause**. The deterministic boundary-clause check flags exactly that shape as a finding. Neither validator knows the field exists — `grep -rn 'disable-model-invocation'` over `scripts/skill-size-check.sh` and both `validate.sh` files returns nothing. ## Reproduction `plugins/bin/.apm/skills/zoom-out/SKILL.md` is the in-tree exemplar ADR-0020 names, and follows the contract exactly: ```yaml name: zoom-out description: Tell the agent to zoom out and give broader context or a higher-level perspective. Use when you're unfamiliar with a section of code or need to understand how it fits into the bigger picture. disable-model-invocation: true ``` `pre-commit run skill-size-check --all-files` reports: ``` SUGGESTION: plugins/bin/.apm/skills/zoom-out/SKILL.md: description has no boundary clause (ADR-0020). Add the prose form ("Do not use for X — use `y` instead") or the compressed form ("Not X -> y") so the router knows where NOT to send this skill. ``` The advice is actively wrong for this skill: there is no router to inform, because the flag removes it from the model-visible listing entirely. ## Severity Low. SUGGESTION tier, so nothing blocks. It is permanent noise on every hand-invoked skill, and the fix advice contradicts the contract, so an author who follows it makes their skill worse. Currently one skill is affected. Issue #99 adds `caveman`, and the population is capped small — see below. ## Fix Skip the boundary-clause check (and the trigger-quality checks that presuppose model invocation) when frontmatter carries `disable-model-invocation: true`. The size budgets should still apply: a hand-invoked description is not preloaded, but the body is still loaded on invocation. Worth adding a test case; `zoom-out` is a stable fixture for it. ## Context worth recording while here ADR-0020's "Invocation as a design axis" verified that the flag removes a skill from the model-visible listing and that `/zoom-out` still works. It did **not** verify the other direction, and that half turns out to matter more: ``` Skill(zoom-out) -> Skill zoom-out cannot be used with Skill tool due to disable-model-invocation. Ask the user to run /zoom-out themselves. ``` **The flag hard-blocks the Skill tool, so no other skill can route to a hand-invoked skill.** A ``Call `x` `` step in another skill's body stops working the moment `x` takes the flag. This changed the #99 scope. Of the four hand-invoke candidates that issue names, three have inbound routes and would have broken their callers: | candidate | inbound routes | |---|---| | `caveman` | none — safe, and the only one being flagged | | `prototype` | `diagnose` | | `grill-me` | `write-docs`, `skill-author/references/create.md` | | `grill-with-docs` | `improve-codebase-architecture`, `triage`, `write-docs`, **`forge`** | `forge` Step 1 reads ``Call `bin:grill-with-docs` `` and is mandatory, so flagging that skill would have broken the factory's entry point. Already recorded in `CONTEXT.md`'s **Hand-invoked skill** entry (commit `ef3e981`), which previously covered listing visibility and preload cost but said nothing about invocability.
Claude added this to the Skills & Agents milestone 2026-08-30 12:40:22 +00:00
Claude added the Kind/Bug
Reviewed
Confirmed
1
Priority
Low
4
 labels 2026-08-30 12:40:22 +00:00
Claude commented 2026-08-31 08:09:23 +00:00
Author
Collaborator
Copy Link

Fixed on refactor/adr0020-skill-retrofit (not yet pushed — the PR body will carry the close)

A shared hand_invoked(fm_text) now reads disable-model-invocation as a boolean, not as key presence. PyYAML already resolves the unquoted YAML 1.1 booleans, so the extra branch catches only a quoted "true" (which a host reads as truthy); false is not the carve-out. A frontmatter parse failure returns False rather than raising — the field's validity is already decided and reported by description_value() on the same text, and raising twice would diagnose one broken file two different ways.

What it lifts — the routing rules only:

  • the boundary-clause SUGGESTION
  • the multi-target arrow SUGGESTION (#107)
  • the 250-character description target

What it keeps — for the reason this issue gives:

  • the 400-character description FAIL (not preloaded, but still the line the user reads in the / menu; the ceiling is an outlier stop, not a routing-quality target)
  • both body-word tiers, 600 SUGGESTION / 900 FAIL — the body is loaded on invocation like any other and competes with the caller's live conversation the same way

Verified on the two live cases. caveman and zoom-out both carry disable-model-invocation: true and both exit 0 clean under scripts/skill-size-check.sh.

Tests (tests/test-skill-size-check.sh) pin both halves, and each exemption case is paired with a control that removes only the flag:

  • a 300-char description with no boundary clause is silent when hand-invoked; the identical fixture without the flag produces both findings
  • a hand-invoked description over 400 chars still FAILs
  • a hand-invoked body over 900 words still FAILs; over 600 still suggests
  • disable-model-invocation: false is not the carve-out (still reports "has no boundary clause")

Implemented in all three hosts: scripts/skill-size-check.sh, skill-audit/scripts/validate.sh, agent-audit/scripts/validate.sh. The contract side is stated in skill-author/SKILL.md and skill-audit/SKILL.md.

## Fixed on `refactor/adr0020-skill-retrofit` (not yet pushed — the PR body will carry the close) A shared `hand_invoked(fm_text)` now reads `disable-model-invocation` **as a boolean**, not as key presence. PyYAML already resolves the unquoted YAML 1.1 booleans, so the extra branch catches only a quoted `"true"` (which a host reads as truthy); `false` is not the carve-out. A frontmatter parse failure returns `False` rather than raising — the field's validity is already decided and reported by `description_value()` on the same text, and raising twice would diagnose one broken file two different ways. **What it lifts** — the routing rules only: - the boundary-clause SUGGESTION - the multi-target arrow SUGGESTION (#107) - the 250-character description target **What it keeps** — for the reason this issue gives: - the 400-character description FAIL (not preloaded, but still the line the user reads in the `/` menu; the ceiling is an outlier stop, not a routing-quality target) - both body-word tiers, 600 SUGGESTION / 900 FAIL — the body is loaded on invocation like any other and competes with the caller's live conversation the same way **Verified on the two live cases.** `caveman` and `zoom-out` both carry `disable-model-invocation: true` and both exit 0 clean under `scripts/skill-size-check.sh`. **Tests** (`tests/test-skill-size-check.sh`) pin both halves, and each exemption case is paired with a control that removes only the flag: - a 300-char description with no boundary clause is silent when hand-invoked; the identical fixture without the flag produces both findings - a hand-invoked description over 400 chars still FAILs - a hand-invoked body over 900 words still FAILs; over 600 still suggests - `disable-model-invocation: false` is **not** the carve-out (still reports "has no boundary clause") Implemented in all three hosts: `scripts/skill-size-check.sh`, `skill-audit/scripts/validate.sh`, `agent-audit/scripts/validate.sh`. The contract side is stated in `skill-author/SKILL.md` and `skill-audit/SKILL.md`.
Claude commented 2026-09-01 13:59:33 +00:00
Author
Collaborator
Copy Link

Fixed by #129, squash-merged to main as 598a7c3.

The boundary-clause check (and the trigger-quality checks that presuppose model invocation) now skip when frontmatter carries disable-model-invocation: true. Size budgets still apply, per the fix note in this issue.

Verified against the merged tree on a fixture with the flag set and no boundary clause at all: zero output, rc 0. The permanent SUGGESTION noise on hand-invoked skills is gone, and with it the advice that contradicted the contract.

caveman took the flag in this PR, so the affected population is now zoom-out + caveman — both clean.

Stayed open after the merge because Gitea's closing-keyword parser consumed only the first reference in Closes #99, #107, …. Closing manually.

Fixed by #129, squash-merged to `main` as `598a7c3`. The boundary-clause check (and the trigger-quality checks that presuppose model invocation) now skip when frontmatter carries `disable-model-invocation: true`. Size budgets still apply, per the fix note in this issue. Verified against the merged tree on a fixture with the flag set and no boundary clause at all: **zero output, rc 0**. The permanent SUGGESTION noise on hand-invoked skills is gone, and with it the advice that contradicted the contract. `caveman` took the flag in this PR, so the affected population is now `zoom-out` + `caveman` — both clean. Stayed open after the merge because Gitea's closing-keyword parser consumed only the first reference in `Closes #99, #107, …`. Closing manually.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label Kind/Bug
Priority
Low
4
Reviewed
Confirmed
1
Milestone
No items
No Milestone Skills & Agents
Projects
Clear projects
No project
Assignees
Clear assignees
Claude renovate-bot Defame1297
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Defame1297/holocron#108
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?