j4k-oss/agent-skills
Watch
1
0
Fork
You've already forked agent-skills
 0
Code Issues 6 Pull requests Releases Packages Activity Actions

fix(skills): route skill references by their own headings instead of copied indexes #99

Closed
jercik wants to merge 1 commit from fix/skill-indexes-point-to-sources into main
pull from: fix/skill-indexes-point-to-sources
merge into: j4k-oss:main
Conversation 3 Commits 1 Files changed 25 +40 -139
jercik commented 2026-10-02 08:29:07 +00:00
Owner
Copy link

verify-unixy-cli, verify-tests, and design each listed every file in their rules directory, so adding a rule needed a second edit and the copy could drift. They now have the agent read each file's opening lines instead: the rule heading in verify-unixy-cli, the Covers line in design. Routing details only the design index held moved into those Covers lines.

This applies the restated-sets rule from j4k/review#91, like #96 and #97. In five paired runs on real repos, agents using the old and revised skills loaded about the same files.

Follow-ups: verify-use-effect and node keep full indexes, and writing-for-agents/references/skill-packaging.md still tells authors to index every topic file.

`verify-unixy-cli`, `verify-tests`, and `design` each listed every file in their rules directory, so adding a rule needed a second edit and the copy could drift. They now have the agent read each file's opening lines instead: the rule heading in `verify-unixy-cli`, the `Covers` line in `design`. Routing details only the design index held moved into those `Covers` lines. This applies the restated-sets rule from [j4k/review#91](https://code.j4k.dev/j4k/review/pulls/91), like #96 and #97. In five paired runs on real repos, agents using the old and revised skills loaded about the same files. Follow-ups: `verify-use-effect` and `node` keep full indexes, and `writing-for-agents/references/skill-packaging.md` still tells authors to index every topic file.
fix(skills): route by each rule file's own heading instead of a copied index
All checks were successful
commit-msg / commitlint (pull_request) Successful in 25s
Details
Node tests / node:test (pull_request) Successful in 2m6s
Details
Review / Review (pull_request_target) Successful in 14m26s
Details
0ac1a658d4
 
verify-unixy-cli, verify-tests, and design each kept a table or list of every
file in their rules directory, so adding a rule needed a second edit and the
copy could drift. Each skill now points at the directory and tells the agent
which line of each file to scan. Design's guideline files already carried a
Covers line; it absorbs the routing detail only the index held.
forgejo-actions commented 2026-10-02 08:43:32 +00:00
Copy link

Review 01M3XVPJD4RMXD28QHEGQEEK9J — head 0ac1a658d4e0f4dace9286cd8bcc123f2eb24c55

Review — j4k-oss/agent-skills @ 734cafd76e

Scope: diff against base tree eba0a2665e6d
Status: dispatched — coverage complete (3/3 slots terminal)
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v3",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (3)

medium — Prompt summary omits the permitted INT3 confirmation path

  • claim: 01M3XW927H0KGN1FWTAVX197ME
  • anchor: skills/verify-unixy-cli/references/int1-non-interactive-by-default.md (snippet)
| Prompts    | User input requests        | `--interactive` flag (opt-in)       |
  • lens: writing-quality · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • disposition: none

I read the full INT1 rule, the INT3 TTY-Based Confirmations rule, and the CLI skill's rule-selection instructions. INT1's own note calls INT3 a compliant alternative for destructive actions, and INT3 allows a yes/no confirmation when stdin is a TTY if --yes, --no-interactive, and CI suppress it. The moved mental-model table instead states that prompts require --interactive, and its closing sentence says flags express intent. An auditor using this summary can wrongly reject an INT3-compliant delete command or remove its allowed confirmation path. Qualify the Prompts row and closing principle: ordinary input prompts require --interactive; destructive confirmations may use INT3's TTY and escape-hatch contract. This keeps the useful distinction between display capability and general prompt intent. This is a static reading of the two rule files; an explicit INT3 exception in this summary would resolve the conflict.

medium — The valid TSV example omits its trailing empty field

  • claim: 01M3XW9VHSPNQ5MDA9840PQV6A
  • anchor: skills/verify-unixy-cli/references/io7-tabular-output-format.md (snippet)
user1	admin	admin@example.com
user2	member
user3	-	guest@example.com
  • lens: writing-quality · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • disposition: none

I read the complete IO7 rule and checked the displayed 'Good: empty field preserves column alignment' sample with awk using a tab field separator. The header and surrounding rows have three fields, but user2<TAB>member has only two: the empty EMAIL field needs a trailing tab, or a placeholder in the third field. The text below the sample requires equal column counts, so an agent copying or using this example as an audit standard can emit malformed three-column TSV. Change the row to user2<TAB>member<TAB> (show the final tab visibly in the example or annotate it), or give EMAIL a placeholder. This preserves the intended lesson about missing values and makes the sample satisfy it. The field count is observed from the file; behavior in any particular CLI was not tested.

low — Reference module headings do not match the declared Covers line format

  • claim: 01M3XVWMEJEF8A3ZZX8YK4J8TD
  • anchor: skills/design/design-guidelines.md (snippet)
- Each file in this skill's `guidelines/` directory covers one topic, and its third line, `Covers: …`, lists what it applies to. Before writing UI code, read the first three lines of each file there and load every rule file that could apply.
  • lens: general-bug · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
  • duplicates: 01M3XWC1TSTAT4EHPTQ72CNSGC (writing-quality)
  • disposition: none

I read the design skill's Load Contract and the first three lines of every file in its guidelines directory. The contract says each third line is Covers: …, but assets-api.md and font-recommendations.md start their third lines with Covers (reference module; …): instead. Those are the two files that the next Load Contract bullet expects to identify as reference modules. A consumer selecting applicable files by the documented Covers: prefix will omit both and can miss asset URL parameters or font sourcing guidance. Align those headings with Covers: while keeping the reference-module marker, or describe the alternate syntax in the contract. I did not find a parser in the subject tree, so the effect on free-form human reading is unverified.

Other claims

  • grounding-pending (0)
  • ungrounded (0)
  • rejected (0)
  • duplicate-of (1)
    • 01M3XWC1TSTAT4EHPTQ72CNSGC low — The load contract names a Covers field the reference modules do not use → 01M3XVWMEJEF8A3ZZX8YK4J8TD
  • unadjudicated (0)

Coverage

Coverage pass: 01M3XVPJG71B3WX42Z2820E7AS
Accounting: complete
Slot health: healthy

lens part arm unit status runs loss
general-bug whole default claims-emitted 1 no
writing-quality whole default claims-emitted 1 no
test-trimming whole default no-claims 1 no
<!-- review:summary --> **Review** `01M3XVPJD4RMXD28QHEGQEEK9J` — head `0ac1a658d4e0f4dace9286cd8bcc123f2eb24c55` # Review — j4k-oss/agent-skills @ 734cafd76e66 Scope: diff against base tree `eba0a2665e6d` Status: dispatched — coverage complete (3/3 slots terminal) Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v3", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (3) ### medium — Prompt summary omits the permitted INT3 confirmation path - claim: `01M3XW927H0KGN1FWTAVX197ME` - anchor: `skills/verify-unixy-cli/references/int1-non-interactive-by-default.md` (snippet) ``` | Prompts | User input requests | `--interactive` flag (opt-in) | ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - disposition: none > I read the full INT1 rule, the INT3 TTY-Based Confirmations rule, and the CLI skill's rule-selection instructions. INT1's own note calls INT3 a compliant alternative for destructive actions, and INT3 allows a yes/no confirmation when stdin is a TTY if --yes, --no-interactive, and CI suppress it. The moved mental-model table instead states that prompts require --interactive, and its closing sentence says flags express intent. An auditor using this summary can wrongly reject an INT3-compliant delete command or remove its allowed confirmation path. Qualify the Prompts row and closing principle: ordinary input prompts require --interactive; destructive confirmations may use INT3's TTY and escape-hatch contract. This keeps the useful distinction between display capability and general prompt intent. This is a static reading of the two rule files; an explicit INT3 exception in this summary would resolve the conflict. ### medium — The valid TSV example omits its trailing empty field - claim: `01M3XW9VHSPNQ5MDA9840PQV6A` - anchor: `skills/verify-unixy-cli/references/io7-tabular-output-format.md` (snippet) ``` user1 admin admin@example.com user2 member user3 - guest@example.com ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - disposition: none > I read the complete IO7 rule and checked the displayed 'Good: empty field preserves column alignment' sample with awk using a tab field separator. The header and surrounding rows have three fields, but `user2<TAB>member` has only two: the empty EMAIL field needs a trailing tab, or a placeholder in the third field. The text below the sample requires equal column counts, so an agent copying or using this example as an audit standard can emit malformed three-column TSV. Change the row to `user2<TAB>member<TAB>` (show the final tab visibly in the example or annotate it), or give EMAIL a placeholder. This preserves the intended lesson about missing values and makes the sample satisfy it. The field count is observed from the file; behavior in any particular CLI was not tested. ### low — Reference module headings do not match the declared Covers line format - claim: `01M3XVWMEJEF8A3ZZX8YK4J8TD` - anchor: `skills/design/design-guidelines.md` (snippet) ``` - Each file in this skill's `guidelines/` directory covers one topic, and its third line, `Covers: …`, lists what it applies to. Before writing UI code, read the first three lines of each file there and load every rule file that could apply. ``` - lens: general-bug · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - duplicates: `01M3XWC1TSTAT4EHPTQ72CNSGC` (writing-quality) - disposition: none > I read the design skill's Load Contract and the first three lines of every file in its guidelines directory. The contract says each third line is `Covers: …`, but `assets-api.md` and `font-recommendations.md` start their third lines with `Covers (reference module; …):` instead. Those are the two files that the next Load Contract bullet expects to identify as reference modules. A consumer selecting applicable files by the documented `Covers:` prefix will omit both and can miss asset URL parameters or font sourcing guidance. Align those headings with `Covers:` while keeping the reference-module marker, or describe the alternate syntax in the contract. I did not find a parser in the subject tree, so the effect on free-form human reading is unverified. ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (1) - `01M3XWC1TSTAT4EHPTQ72CNSGC` low — The load contract names a Covers field the reference modules do not use → `01M3XVWMEJEF8A3ZZX8YK4J8TD` - unadjudicated (0) ## Coverage Coverage pass: 01M3XVPJG71B3WX42Z2820E7AS Accounting: complete Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | claims-emitted | 1 | no | | writing-quality | whole | default | claims-emitted | 1 | no | | test-trimming | whole | default | no-claims | 1 | no |
forgejo-actions reviewed 2026-10-02 08:43:33 +00:00
skills/design/design-guidelines.md
@ -5,3 +5,3 @@
## Load Contract
- Before writing UI code, scan the rule-file index below and load every rule file that could apply.
- Each file in this skill's `guidelines/` directory covers one topic, and its third line, `Covers: …`, lists what it applies to. Before writing UI code, read the first three lines of each file there and load every rule file that could apply.
forgejo-actions commented 2026-10-02 08:43:33 +00:00
Copy link

low — Reference module headings do not match the declared Covers line format
lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the design skill's Load Contract and the first three lines of every file in its guidelines directory. The contract says each third line is Covers: …, but assets-api.md and font-recommendations.md start their third lines with Covers (reference module; …): instead. Those are the two files that the next Load Contract bullet expects to identify as reference modules. A consumer selecting applicable files by the documented Covers: prefix will omit both and can miss asset URL parameters or font sourcing guidance. Align those headings with Covers: while keeping the reference-module marker, or describe the alternate syntax in the contract. I did not find a parser in the subject tree, so the effect on free-form human reading is unverified.

claim 01M3XVWMEJEF8A3ZZX8YK4J8TD of review 01M3XVPJD4RMXD28QHEGQEEK9J

<!-- review:claim:01M3XVWMEJEF8A3ZZX8YK4J8TD --> **low** — Reference module headings do not match the declared Covers line format lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the design skill's Load Contract and the first three lines of every file in its guidelines directory. The contract says each third line is `Covers: …`, but `assets-api.md` and `font-recommendations.md` start their third lines with `Covers (reference module; …):` instead. Those are the two files that the next Load Contract bullet expects to identify as reference modules. A consumer selecting applicable files by the documented `Covers:` prefix will omit both and can miss asset URL parameters or font sourcing guidance. Align those headings with `Covers:` while keeping the reference-module marker, or describe the alternate syntax in the contract. I did not find a parser in the subject tree, so the effect on free-form human reading is unverified. claim `01M3XVWMEJEF8A3ZZX8YK4J8TD` of review `01M3XVPJD4RMXD28QHEGQEEK9J`
skills/verify-unixy-cli/references/int1-non-interactive-by-default.md
@ -48,0 +41,4 @@
| Layer | What it controls | How to detect/configure |
| ---------- | -------------------------- | ----------------------------------- |
| Formatting | Colors, spinners, progress | `stdout.isTTY`, `NO_COLOR` |
| Prompts | User input requests | `--interactive` flag (opt-in) |
forgejo-actions commented 2026-10-02 08:43:33 +00:00
Copy link

medium — Prompt summary omits the permitted INT3 confirmation path
lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain

I read the full INT1 rule, the INT3 TTY-Based Confirmations rule, and the CLI skill's rule-selection instructions. INT1's own note calls INT3 a compliant alternative for destructive actions, and INT3 allows a yes/no confirmation when stdin is a TTY if --yes, --no-interactive, and CI suppress it. The moved mental-model table instead states that prompts require --interactive, and its closing sentence says flags express intent. An auditor using this summary can wrongly reject an INT3-compliant delete command or remove its allowed confirmation path. Qualify the Prompts row and closing principle: ordinary input prompts require --interactive; destructive confirmations may use INT3's TTY and escape-hatch contract. This keeps the useful distinction between display capability and general prompt intent. This is a static reading of the two rule files; an explicit INT3 exception in this summary would resolve the conflict.

claim 01M3XW927H0KGN1FWTAVX197ME of review 01M3XVPJD4RMXD28QHEGQEEK9J

<!-- review:claim:01M3XW927H0KGN1FWTAVX197ME --> **medium** — Prompt summary omits the permitted INT3 confirmation path lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I read the full INT1 rule, the INT3 TTY-Based Confirmations rule, and the CLI skill's rule-selection instructions. INT1's own note calls INT3 a compliant alternative for destructive actions, and INT3 allows a yes/no confirmation when stdin is a TTY if --yes, --no-interactive, and CI suppress it. The moved mental-model table instead states that prompts require --interactive, and its closing sentence says flags express intent. An auditor using this summary can wrongly reject an INT3-compliant delete command or remove its allowed confirmation path. Qualify the Prompts row and closing principle: ordinary input prompts require --interactive; destructive confirmations may use INT3's TTY and escape-hatch contract. This keeps the useful distinction between display capability and general prompt intent. This is a static reading of the two rule files; an explicit INT3 exception in this summary would resolve the conflict. claim `01M3XW927H0KGN1FWTAVX197ME` of review `01M3XVPJD4RMXD28QHEGQEEK9J`
jercik commented 2026-10-02 08:43:46 +00:00
Author
Owner
Copy link

Closing: these indexes let an agent pick which rule file to read before reading it, so they stay; the restated-sets rule now exempts tables of contents.

Closing: these indexes let an agent pick which rule file to read before reading it, so they stay; the restated-sets rule now exempts tables of contents.
jercik closed this pull request 2026-10-02 08:43:47 +00:00
All checks were successful
commit-msg / commitlint (pull_request) Successful in 25s
Details
Node tests / node:test (pull_request) Successful in 2m6s
Details
Review / Review (pull_request_target) Successful in 14m26s
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
j4k-oss/agent-skills!99
No description provided.