Skip to content

refactor(skill): a small core with a dispatch table, flows in reference files, hooks for the hard rules - #41

Merged
pasrom merged 7 commits into
mainfrom
refactor/skill-core-and-references
Nov 7, 2026
Merged

pasrom merged 7 commits into
mainfrom
refactor/skill-core-and-references

Conversation

@pasrom

@pasrom pasrom commented Nov 7, 2026 •

Copy link
Copy Markdown
Owner

Problem

skills/km/SKILL.md had grown to 774 lines (57 KB, 21.5k tokens as the API counts them). Three things followed from that:

  • After a compaction Claude Code re-attaches only the first 5,000 tokens of an invoked skill, so most flows were gone exactly when a long session needed them.
  • The hard rules sat at the end of the file (## Rules at line 769, the hard-rule blocks of brain fix and contribute in the middle), the last place a truncated skill keeps.
  • The description was at 1016 of 1024 characters and fired on bare words such as "review" and "PR".

Design

  • Core + reference files. SKILL.md keeps what every flow shares (rules, routing, citation, consuming vs. authoring, search, writing content, the sync rule, GitHub access) and a dispatch table that maps every /km verb and request shape to the files its flow needs, all of them (brain fix -> brains.md + review-and-pr.md + validate-and-gate.md; queue -> queue.md + github-reads.md + review-and-pr.md). The flows live in skills/km/reference/*.md, one level deep, each with a contents list. help is defined in the table. The core tells Claude that after a compaction only SKILL.md is re-attached, so a flow's files are read again.
  • Rules first. The old ## Rules list moved to the top and gained the hard rules that were buried in the flows (no push to a brain's default branch, no force push without a lease, no edit of a mounted brain, km approve as its own commit, a doc with contribution: is edited on its PR head, no parent-repo private content in a brain PR).
  • Description shortened to one trigger per activity, qualified where a bare word would fire on unrelated work ("review a brain doc or brain PR"), no when_to_use.
  • Hooks for two hard rules, so they hold after a compaction and for a Claude that never read the flow:
    • hooks/guard-brain-edit.sh, a PreToolUse hook on Edit, Write, MultiEdit and NotebookEdit: refuses (exit 2, reason on stderr) an edit of a file whose directory lies in a git submodule when both the submodule and its superproject are brains (CONVENTIONS.md plus a schema file). Detection through git rev-parse --show-superproject-working-tree, not a path pattern; a mount that is not checked out yet (an empty gitlink directory, where a file would block the checkout) is refused too. Bash builtins plus one git call, no jq: the path comes out of the JSON with a bash regex; a Windows path arrives as C:\\Users\\... and is read as C:/Users/..., so it runs in Git Bash (the Windows CI job runs that case). Everything else exits 0 at once (no path in the input, no git, a plain repository, a shared resource, a brain mounted in a repository that is no brain, a standalone clone, the brain's own brains/ folder).
    • a pre-push hook in a team brain's pre-commit config (templates/team/no-push-hook.yaml, a repo: local hook on PRE_COMMIT_REMOTE_BRANCH) that refuses a push to refs/heads/main, next to the existing no-commit-to-branch; the config now sets default_install_hook_types: [pre-commit, pre-push] so pre-commit install writes both git hooks, and default_stages: [pre-commit] so the hooks without stages: (no-commit-to-branch, km-validate) keep running at commit only. km init --team writes it; km upgrade adds whichever of the two hooks and the install types a team brain lacks and says to run pre-commit install again; km doctor fails when the config names a pre-push hook git does not have. A personal brain gets none of it.
    • What the hooks do not do: a throwaway clone has no hooks, so the prose rule stays; --no-verify still pushes (a guard against mistakes, not a boundary, like no-commit-to-branch); force pushes without a lease stay a prose rule; a brain mounted in a non-brain repository is not guarded.
  • Version bump to 1.9.0 (plugin.json, __init__.py, README together), since without it no brain or plugin install would see the change.

Evidence

  • Nothing lost (commit A). tests/skill_inventory.py runs in CI (tests/skill_smoke.sh) against the frozen pre-split SKILL.md (tests/fixtures/skill-baseline.md): every baseline paragraph (blank-line separated, a code fence as one) must be a substring of exactly one new file after the 20 enumerated pointer rewrites in REWRITES; every heading, inline code span and sentence with a hard-rule word must appear in the union. 116 paragraphs, 753 atoms, 0 problems. Mutation-checked: a dropped paragraph, a duplicated one, a stale rewrite, a lost code span and a changed word each fail it. git diff --color-moved=zebra: 1032 moved lines, 121 plain, all of them pointer rewrites, contents lists and the reference list.
  • Core size. Measured with a one-turn haiku call (input tokens with the file appended to the system prompt minus without): old SKILL.md 21,502 tokens; new core 4,593 (151 lines, 11,524 bytes; it was 4,194 before the two review passes added rows and a clause, see open questions); reference files 1,273 to 5,457 each. CI guards the core with a byte limit of 12,000 in tests/skill_inventory.py (CORE_BYTES), a proxy for the 5,000-token compaction budget calibrated from that measurement (2.5 bytes per token, so about 4,780 tokens at the limit; padding the core past it fails the test, checked).
  • Dispatch completeness. The inventory test also asserts that every /km <verb> in the README and the baseline has a row in the table and that help is defined.
  • Prose tightening (commit D) is deliberately small: two wall-of-text sections split into paragraphs with the wording untouched, a heading added, one duplicated rule bullet dropped from the core. The checker lists the two paragraphs in REWRAPPED and requires them once with whitespace normalised; a changed word still fails (checked). Every other paragraph stays under the strict check, so the relaxation is exactly two entries the reviewer can read.
  • Test suite as CI runs it: all green (gate 116, index 30, pins 37, cites 87, doctor 50, sync 63, list 40, skill 1, hook 23, team 123, encoding 4), plus the wheel install step. tests/hook_smoke.sh runs on the Windows job too (CI installs pre-commit for the push cases).
  • Hook mutation checks: without the superproject-is-a-brain check the "brain mounted in a non-brain" case fails; exit 0 instead of exit 2 fails five cases; refs/heads/master instead of main in the template fails the push test; dropping default_stages makes a hook without stages: run at push (a test case asserts exactly that). The hook costs 19 ms per call on the common path (2 execs: cat and git).
  • Evals (claude plugin eval, Claude Code 2.1.319, fixture brain via km init in the scaffold; the first four cases with read-only tools, the nine dispatch cases with --allow-tools Bash Write Edit in the sandbox). Every row of the dispatch table has a case; each dispatch case has a reads-reference grader (the flow's reference file was read) and a behaviour grader on the flow's key step (the km command run, the file written, the line answered) that the old plugin can pass too. Scores are the mean over runs; n/a marks a grader the old plugin cannot pass because it has no reference files.
Case (dispatch row) New plugin, 3 runs Old plugin (origin/main) Behaviour grader
lookup question 1.00 (W/OUT 1.00, Δ 0) 1.00 (3 runs) answer carries the fact and cites the doc; skill fired 3/3 in both
"fix this in @peer" 1.00 (W/OUT 0.00, Δ +1) 0.67 (3 runs; reads-reference n/a) PR route in a throwaway clone (llm), never an edit of the mount
/km queue 1.00 (W/OUT 0.00, Δ +1) 0.00 (3 runs; reads-reference n/a, the only grader) reads queue.md
"review this C function" (negative) 1.00 (W/OUT 1.00) 1.00 (3 runs) the skill never fires, the bug is found
save content 1.00 1.00 (1 run) Write of inbox/<slug>.md with status: draft and type: decision
help 1.00 1.00 (1 run) the answer lists queue and brain fix
doctor ("prüfe meine Einrichtung") 1.00 0.50 (1 run; behaviour pass, reads-reference n/a) runs km doctor
sync 1.00 0.50 (1 run; behaviour pass, reads-reference n/a) runs km sync
lint 1.00 0.50 (1 run; behaviour pass, reads-reference n/a) runs km validate
review <doc> 1.00 0.50 (1 run; behaviour pass, reads-reference n/a) runs km validate on the doc (step 1)
pr 1.00 0.50 (1 run; behaviour pass, reads-reference n/a) finds the branch's docs (git log/diff/merge-base) or checks gh auth status
brain list + brain remove 1.00 0.67 (1 run; both behaviour graders pass, reads-reference n/a) lists peer, then git submodule deinit/git rm
repin 1.00 after one skill fix (first batch 0.83: one run read the reference, then stopped for a confirmation before km repin. Cause: the core's confirm bullet added in the review pass, which the pre-split skill never had at that level; it is dropped again, the baseline's "Always confirm before writing" stays under Writing content and the flows keep their own OK steps. Re-run 3/3 with the bullet scoped to writes the user did not ask for; the drop is the simpler form of the same fix and was not re-run for budget) 0.50 (1 run; behaviour pass, reads-reference n/a) runs km repin

Cost: first four cases $3.22 new (24 runs, both arms) + $2.66 old (12 runs) + $2.04 final re-run; the nine dispatch cases $7.90 new (27 runs) + $3.52 old (9 runs) + $0.85 for the repin re-run (3 runs), plus $0.82 lost to an attempt that the account's usage limit killed before any run was graded: $13.09 for this follow-up against a $12 cap (the overrun is the lost attempt plus the re-run). The old-plugin runs of the nine dispatch cases were cut to one run per case for that budget; three would have cost about $7.90 more. The suite is in evals/ (see evals/README.md); evals/results/ is ignored.

Review passes

  • /simplify (four angles). Applied: the eval fixtures source tests/lib.sh instead of re-implementing km and the fake km remote; one git-options array in tests/lib.sh; the edit guard uses bash builtins and one git call (26 ms to 19 ms per call on the common path, 6 execs to 2) and lost its test seam; init.py keeps one table of the team hooks, parses the config once and tests the install types by meaning; upgrade derives its messages from that table; doctor shares one "installed" test and checks one git hook per type in default_install_hook_types; the inventory checker dropped three empty exception tables; the dispatch table uses full paths throughout and merged rows that read the same files. Left out: retiring the containment test after merge (the brief asked for it to stay; see open questions); a shared hooks/lib.sh for the brain predicate (two standalone scripts); a filesystem pre-check before calling git in the guard (changes the contract from "ask git"); per-step file loading for the three-file rows (the plan wants every file a flow needs named in its row).
  • /code-review (medium), eight findings, all applied with a test each: pre-commit inspects the first ref of a push only and skips a push whose commits the remote already has, so the pre-push guard's claim is scoped to git push origin main in the template and README; default_stages: [pre-commit] is written with the install types so hooks without stages: (no-commit-to-branch, km-validate) do not run at push; a doctor regression that took another tool's executable hook for a non-executable one of pre-commit's; default_install_hook_types parsed in block and quoted form; the edit guard refuses a Write into a mount that is not checked out yet (an empty gitlink directory, where the file would block the checkout); with_team_hooks appends pre-push to a one-line install types list instead of giving up; the core says the hook covers the file-edit tools (a shell edit is not caught) and points at the heading that exists in setup.md; the eval fixture's fake km remote lives under mktemp, not next to the workspace.
  • /simplify and /code-review low on the eval additions (second round): seven identical fixtures became two helpers in evals/lib/brain.sh; the save-content grader became two plain ones; the README shrank; the core's confirm bullet went (see the repin row). Left out, since each would need paid re-runs: lower max_turns, empty brains for the cases that need no doc, narrower graders for pr (git log/diff only) and brains-list-remove (the list grader's peer also appears in the prompt), prompt wording that tells doctor/sync/pr to run the command once and stop.
  • /code-review low (second round) flagged the dropped confirm bullet as removing a guard; checked: the baseline never had it, and its rules all stand ("Always confirm before writing" under Writing content, "wait for explicit OK" in brain fix and contribute, "on the user's OK" for approve, the PR push and the rebase), so the skill is back to the state that is merged today. If you would rather keep a core bullet, the gated form ("unless the user asked for exactly that command") passed repin 3/3. The README now says which rows have a reads-reference grader (help and save content name only the core).
  • Fresh independent review (diff, plan and eval results only): one blocking finding and seven to fix, all applied with tests: git ls-files --stage -- <dir> is a prefix pathspec, so the empty-mount check refused a write into the brain's own brains/ folder (now the entry's path must equal the folder; four cases); default_stages is now written whenever a hook is added and the config sets none, not only with the install types (a config that already installs pre-push would otherwise run no-commit-to-branch at push); the question row names brains.md for @<name>/@all (shared resources, @all sync live there) and the review row pins-and-cites.md; the sync caveat ("never a reset, a force or a moved mount") is back in the core; the "confirm before every write" bullet, which contradicted the ordered km sync and later made an explicit /km repin stop for an OK, is dropped again (the baseline rule stays under Writing content); the README carries both pre-push gaps and the pre-commit version it needs; the /km queue case drops its unpassable skill-fired grader; setup.md names ${CLAUDE_SKILL_DIR}/bin/km; the description keeps Meeting-Notiz, ADR and freigeben; github-reads.md and pins-and-cites.md get the headings they lacked; team init says the first push of a fresh brain takes --no-verify once. Checked and found right by that reviewer: every baseline rule survives (hard rules of brain fix and contribute included), every cross-file pointer names an existing heading, every verb has a row, the hook uses only builtins, git, sed and cut and exits 0 on every input outside a mount, no non-public name anywhere in the diff.

Housekeeping for the maintainer

During the code-review pass, a reviewer agent's experiment ran in the wrong directory and pushed two stray refs to this repository: refs/heads/feature and refs/tags/v1, both at an interim commit of this branch (ba9f75e). No workflow ran on them (CI triggers on main and pull requests only). Deleting refs on the remote is outside what the agent may do, so please run git push origin --delete feature v1 (and git tag -d v1 in the local worktree).

Open questions

  • The core measures 4,593 tokens, above the ~4,000 the plan aimed at (4,194 before the reviews asked for two more rows, a merged clause and the full ${CLAUDE_SKILL_DIR} paths in every row) and under the 5,000 a compaction keeps. If you would rather have the margin, the rows for @<name>/@all and review could name their second file in the flow's own text instead.
  • The edit guard requires the superproject to be a brain as well (the same test as hooks/session-start.sh). A brain mounted in a project that is no brain is therefore not guarded; the alternative (guard every mounted brain) would also block an unrelated repository that happens to be a submodule.
  • The pre-push guard is a repo: local bash hook rather than a hook id in km's own .pre-commit-hooks.yaml, so a brain pinned to an older km keeps a valid config and km init --pin <older> still works; the cost is that it needs bash on the PATH where git runs hooks (Git for Windows provides it).
  • tests/fixtures/skill-baseline.md freezes the pre-split text (57 KB) so the inventory test runs on a shallow CI checkout. Two reviewers asked whether the paragraph check should stay a permanent gate: every later edit of the reference prose needs a REWRITES or REWRAPPED entry. It stays because the brief asked for it; it can be retired after merge, keeping only the structural checks (limits, pointers, dispatch rows), once the frozen copy has done its job.

@pasrom
pasrom force-pushed the refactor/skill-core-and-references branch from c950f45 to 8de2632 Compare November 7, 2026 02:38
…iles

SKILL.md keeps the core (running km, sync, GitHub access, routing, citation,
consuming vs. authoring, search, writing content, rules) and names the seven
reference files under skills/km/reference/ that now hold the flows: setup,
brains, github-reads, validate-and-gate, review-and-pr, queue, pins-and-cites.
The text moved verbatim; only cross-file pointers changed, each listed in
tests/skill_inventory.py, which checks against the frozen pre-split SKILL.md
(tests/fixtures/skill-baseline.md) that every paragraph lands in exactly one
file and that every heading, code span and rule sentence survives.
tests/skill_smoke.sh runs it in CI.
The core now opens with the rules, then a table that maps every /km verb
and request shape (help included) to the reference files its flow needs,
with the note that after a compaction only SKILL.md is re-attached, so a
flow's files are read again before it goes on. The long 'Running km' and
'Keeping the copy current' paragraphs moved verbatim to setup.md; the core
keeps the sync rule in four lines. The description is shorter, one trigger
per activity, qualified where a bare word would fire on unrelated work
(a brain doc or brain PR), and the rules moved to the top of the file.
Measured with a one-turn haiku call: the core is 4.6k tokens (the whole
old SKILL.md was 21.5k, of which a compaction keeps 5k); the inventory
test gains a check that every /km verb of the README and the baseline has
a row in the table.
…rain's main

Two of the skill's hard rules become hooks, so they hold after a compaction
and for a Claude that never read the flow:

- hooks/guard-brain-edit.sh, a PreToolUse hook on Edit, Write, MultiEdit
  and NotebookEdit: the file's directory lies in a git submodule whose
  checkout and superproject are both brains (CONVENTIONS.md plus a schema
  file), found through git rev-parse --show-superproject-working-tree, not
  a path pattern; a mount that is not checked out yet (an empty gitlink
  directory, where a file would block the checkout) is refused too. It
  exits 2 with the fix flows named; everything else exits 0 at once. Bash
  builtins plus one git call (a second one next to an empty mount), no jq:
  the path comes out of the JSON with a bash regex, and a Windows path
  arrives as C:\\... and is read as C:/..., so it runs in Git Bash too.
- a pre-push hook in a team brain's pre-commit config (templates/team/
  no-push-hook.yaml, a local hook on PRE_COMMIT_REMOTE_BRANCH) that refuses
  a push to refs/heads/main, next to the no-commit-to-branch hook; the
  config now sets default_install_hook_types so pre-commit install writes
  both git hooks, and default_stages so the hooks without stages keep
  running at commit only. pre-commit inspects the first ref of a push only
  and skips a push whose commits the remote already has, so git push
  origin main is refused while a push of several refs is not; the template
  and the README say so. km init --team writes it, km upgrade adds
  whichever of the hooks, the install types (appended to a one-line list)
  and default_stages a team brain lacks and says to run pre-commit install
  again, km doctor checks one git hook per installed type. A personal
  brain gets none of it; a throwaway clone has no hooks, there the prose
  rule holds by itself. Force pushes without a lease stay a prose rule.

tests/hook_smoke.sh pipes the hook JSON of each tool in and out of a real
mount, an empty mount, a shared resource, a standalone clone and a plain
repository, and pushes through pre-commit where it is installed (CI
installs it). It runs on the Windows job too, where the C:\\... case runs.
@pasrom
pasrom force-pushed the refactor/skill-core-and-references branch from 8de2632 to 601b38e Compare November 7, 2026 05:29
The prose part is deliberately small, since the point of this change is
that nothing gets lost: the two wall-of-text sections (cited files, index
completeness) are split into paragraphs at their sentence boundaries with
the wording untouched, brains.md, github-reads.md and pins-and-cites.md
get the top-level headings they lacked, and the core drops a rule bullet
that repeated the last sentence of 'GitHub access'. The inventory check
stays strict for every other paragraph; the two re-wrapped ones are
listed in REWRAPPED and must still appear once with whitespace
normalised, so a changed word fails the test (checked).

This commit also carries what the three review passes changed in the
core and the pointers, folded here rather than into the reorder commit:
full ${CLAUDE_SKILL_DIR}/reference/ paths in every dispatch row and
merged rows that read the same files; a row for @<name>/@ALL naming
brains.md and pins-and-cites.md on the review row; the sync caveat
(never a reset, a force or a moved mount) back in the core; the confirm
bullet of the reorder commit dropped again, the baseline rule stays under Writing content; the hook rule worded as covering
the file-edit tools; the description keeps Meeting-Notiz, ADR and
freigeben; setup.md names ${CLAUDE_SKILL_DIR}/bin/km and the team init
says the first push of a fresh brain takes --no-verify once; two
reference pointers name their file. Each pointer change is an entry of
REWRITES. Core: 4.6k tokens.
The skill's core and reference files, the plugin's edit guard and the
team brain's pre-push hook ship with this version; without the bump no
brain or plugin install would see them.
Thirteen cases for claude plugin eval, each scaffolding a brain with km init
from this checkout (evals/lib/brain.sh reuses tests/lib.sh and its fake km
remote, since km init pins a release by its commit): a lookup question, "fix
this in @peer" on a mounted brain, /km queue, a C review that must not fire
the skill, and one case per remaining row of the dispatch table (save
content, help, doctor, sync, lint, review a doc, pr, brain list + remove,
repin). A case whose row names a reference file has a reads-reference
grader; every case has a grader on the flow's key step (the km command it
runs, the file it writes, the line it answers), which the pre-split skill
passes too. Graders are tool_used and regex, plus one llm grader. Run from
the plugin root with --scaffold --no-publish --ablation none --allow-tools
Bash Write Edit (evals/README.md); evals/results/ is ignored.
@pasrom
pasrom force-pushed the refactor/skill-core-and-references branch from 601b38e to 99e67d5 Compare November 7, 2026 06:08
@pasrom
pasrom merged commit 9ce28e0 into main Nov 7, 2026
4 checks passed
@pasrom
pasrom deleted the refactor/skill-core-and-references branch November 7, 2026 07:13
pasrom added a commit that referenced this pull request Nov 7, 2026
The check that every paragraph of the frozen pre-split SKILL.md still
appears in the split skill guarded one refactor (#41). Since then it
makes every ordinary edit of the skill register a rewrite entry. It goes
with its fixture; the structural checks stay: 500 lines, CORE_BYTES, the
description length, reference paths that resolve, contents lists, and a
dispatch row for every /km verb.

The verb check now reads the skill's own files instead of the baseline,
so approve, doctor, lint, pr and review, which the README does not name,
stay covered. A row counts only where the verb opens a code span or a
quoted phrase in the table, not where it is a word in another row's
text, and skill_smoke shows the check fails without the review/approve
row.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant