feat(claude): wire the kit's Claude-side mechanisms (frontmatter, hooks, memory budget)#13
Conversation
Every .claude/commands/*.md lacked YAML frontmatter, so Claude Code surfaced each command's raw first line as its description (a workflow-doc pointer, or a v0-status blockquote for the two engine-blocked skills) instead of a real one. Add `description:` (reusing the wording already written in the matching .agents/skills/*/SKILL.md where one exists) and `argument-hint:` where a command takes a structured argument. No command body text changed.
triage-friction-log.md and post-merge-systemize.md both opened with a "v0 kit status — engine not yet vendored" banner, which reads as unwritten work. It isn't: working deterministic engines for both already exist in the kit maintainer's own toolkit repo, blocked on a tracker-adapter abstraction so they can target any configured tracker.backend instead of one hardcoded client. Rewrite both banners to say that plainly and link the tracking issues (agentic-dev-kit#6 for triage, #7 for systemize). Also adds frontmatter to both (folded into this commit since it touches the same banner block).
Port the PR follow-through nag from a reference implementation (cs-toolkit's scripts/hooks/pr_followup_hook.py) so the kit's Principle #5 policy ("watch the PR through to green") is a PostToolUse hook instead of prose asking the agent to run /pr-watch unasked (Principle #8: "a rule that lives only in a doc is a wish"). Generalized for the kit: reads review.bots, review.fallback_commands.claude, and paths.engines from config/dev-model.yaml via scripts/lib/kitconfig.py instead of naming a review bot as a literal. Falls back to generic wording if config can't be read, rather than skipping the reminder. No-ops when JOB_NAME is set (cron/CI) and always exits 0. Tests cover: triggers on `gh pr create` / `gh pr ready`, does not trigger on `gh pr view` or unrelated commands, no-ops under JOB_NAME, exits 0 on malformed/unexpected stdin shapes, and that the reminder text is genuinely config-sourced rather than hardcoded.
Port the doc-budget tripwire applied to Claude Code's own per-project auto-memory index (MEMORY.md) from a reference implementation (cs-toolkit's scripts/check_memory_budget.py). Mirrors check_doc_budget.py's CLI exactly (--quiet/--strict/--json, same exit-code contract) and reuses scripts/lib/kitconfig.py for portable repo-root discovery (walking up for .git, so the derived repo-slug stays correct when the kit is vendored under a nested paths.engines dir) instead of the reference's fixed-depth `parent.parent`. Deliberately NOT config/dev-model.yaml `doc_budgets`-driven: that list is iterated unconditionally by check_doc_budget.py, which requires every entry's path to resolve to an existing repo-relative file with archive/remedy fields. The memory index lives outside the repo at a machine- and repo-slug-specific path (not portable to commit, not swept to an in-repo archive), so adding it there would make every check_doc_budget.py run — including the SessionStart hook it now shares — fail loudly wherever no memory has been saved yet. The byte/line-length budgets stay documented platform constants (overridable via --max-bytes / --max-line-chars), same as check_doc_budget.py keeps its own tuning out of adopter config. config/dev-model.yaml is untouched in this change.
The doc-budget tripwire (check_doc_budget.py) already advertised itself as "safe to run from a SessionStart hook" but nothing wired it, so it only fired if someone remembered to run it during /wrap-up — memory, not mechanism (Principle #8). Wire it, plus the new check_memory_budget.py, into a SessionStart hook (matcher "startup"); wire the new pr_followup_hook.py into a PostToolUse hook (matcher "Bash", pre-filtered with `if: "Bash(gh pr *)"`). Both SessionStart hooks are guarded `[ -z "$JOB_NAME" ] && … || true` so a cron/CI session never runs them and the hook always exits 0. Also ships a `permissions.allow` list for the kit's own read-only forge calls (gh pr view/list/checks, gh run view, uv run scripts/pr_watch.py) so a lane doesn't stall on a permission prompt for calls the kit itself makes routinely. Hardcoded, documented compromise: the hook commands reference scripts/check_doc_budget.py, scripts/check_memory_budget.py, and scripts/hooks/pr_followup_hook.py directly rather than resolving paths.engines dynamically. A settings.json hook command is a static string; resolving paths.engines first would require invoking an interpreter to read config before knowing which interpreter/path to invoke, which is circular for a bare shell command. An adopter who vendors the kit's engines under a different paths.engines value must edit these three command strings by hand (one line each) after adopting. permissions.allow's `uv run scripts/pr_watch.py:*` entry carries the same caveat. Verified against the official Claude Code docs (code.claude.com/docs/en/ hooks.md and hooks-guide.md), not assumed from the reference example alone: SessionStart matcher values (startup/resume/clear/compact/fork), PostToolUse matcher "Bash", the {type, command, timeout-in-seconds} hook-entry shape, the `if` field's permission-rule syntax and that it applies to PostToolUse (not just PreToolUse), $CLAUDE_PROJECT_DIR as a real hook env var, the `hookSpecificOutput.additionalContext` shape for PostToolUse specifically (confirmed via a direct fetch of hooks.md after an initial secondary source claimed, incorrectly, that additionalContext was UserPromptSubmit-only), and the permissions.allow `Bash(cmd:*)` colon-suffix form as equivalent to a trailing `Bash(cmd *)`.
`install-hooks` does not reimplement the pre-push hook shim installer: init.sh's own install_hooks() (git-worktree-aware, honors core.hooksPath, regenerates a shim rather than copying the hook body or symlinking it) stays the single source of truth. Running the whole of `./init.sh` here would also re-trigger its interactive project/tracker prompts and its docs/ narrative-template seeding step — confirmed by hand: on a repo whose narrative docs still carry the `devkit-template: unrendered` marker (this kit's own repo, before an adopter has claimed them), a bare `./init.sh` run rewrites docs/handoff.md and friends as a side effect, which is out of bounds for a target named "install-hooks" and, in this repo specifically, out of this lane's own touch scope. So the target extracts and runs just the install_hooks() function body straight out of init.sh via `sed` + `eval` — it always reflects init.sh's current logic, never a stale/diverged copy — after resolving paths.engines the same way init.sh does, via scripts/lib/kitconfig.py. `test` runs the lane contract's own local gate: the state_paths + scripts pytest suites.
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
* feat: kit_doctor + /upgrade — make an already-adopted install diagnosable Phase 2. Re-running ./init.sh (landed in #8) already handles the config half of an upgrade. This is the engine half, which was the part nothing could see. The problem, measured rather than assumed. Surveying the real installs turned up four distinct shapes, none of them versioned at the engine level: · an ancestor with no config surface at all · a v1-schema repo with 2 of 6 engines and a paths.engines value pointing at a directory containing no engine — so every workflow's <engine-dir>/… reference resolved to nothing, silently, because nothing validates that value · a correctly namespaced v2 repo whose pr_watch.py was 3 features behind · the template itself Nothing could tell an older engine from a hand-patched one, which is what makes "just copy the new files in" unsafe. scripts/kit_doctor.py — read-only drift report against a released kit-manifest.json (path -> sha256). Per kit-owned file: unchanged / differs / missing / unknown-version. Plus the four installation-level checks nothing else performs: schema generation, whether paths.engines actually holds engines, whether the pre-push hook is installed rather than merely shipped, and whether the narrative docs were rendered. Two deliberate design calls: · `differs` does NOT claim a cause. A hash mismatch cannot distinguish "older version" from "hand-edited", and labelling an old file "locally modified" sends someone hunting for edits they never made — this was caught by running the tool against a real adopter, which reported 4 files as locally-modified when they were almost certainly just old. It now narrows by schema version and leaves the call to the operator. · paths.engines indirection is handled by remapping the manifest's kit-layout prefix, so a repo with engines under scripts/devkit/ compares correctly against an unmodified manifest. Adopter-owned paths are never compared — they are supposed to differ, and reporting them would bury the signal. That list is explicit (not inferred) so an installer can consume the same boundary. /upgrade (.claude/commands/upgrade.md) — drives the sequence: detect which of the four shapes this repo is, migrate config via init.sh, then refresh per file keyed on kit_doctor's states. Routes a config-less repo to /adopt instead of guessing a config into existence. Names the case worth routing UPSTREAM: a local edit that is genuinely ahead of the kit should become a kit PR, not be overwritten. Also closes #5: check_doc_budget.py and archive_plan_sessions.py now read config through the stdlib kitconfig instead of PyYAML, so every shipped engine is dependency-free — including the SessionStart hook path added in #13. This is the same invariant kit_doctor asserts: an engine an adopter must edit (or install a dependency for) can never be safely replaced. CI gates the manifest against the tree, since a stale manifest silently degrades every adopter's report to unknown-version. 184 tests pass. Verified against both real adopter repos read-only: brain's missing-engine breakage and OpenKitchen's 4 drifted files are both correctly diagnosed, with the engines-dir failure called out explicitly. Closes #5. * fix(kit_doctor): honor core.hooksPath when detecting the installed hook Found by the fallback review pass on this PR. `.git/hooks` is not the only place git reads hooks from — core.hooksPath overrides it, and pre-commit plus several monorepo layouts set it. Checking only `.git/hooks` reports a correctly-installed hook as missing, so the report would tell an adopter to re-run an install that had already worked. This is the same bug class fixed in init.sh for the WRITE side earlier in this stack, reintroduced on the READ side. Read via a plain .git/config scan rather than shelling out to git, keeping kit_doctor subprocess-free. Also strip a trailing slash from paths.engines before remapping. --------- Co-authored-by: Topi Jarvinen <topi.jarvinen@gmail..com>
Summary
The Claude-side wiring of agentic-dev-kit was weaker than its Codex-side wiring. This closes that gap:
.claude/commands/*.mdlacked YAML frontmatter, so Claude Code surfaced each command's raw first line as its description — a workflow-doc pointer, or a> **v0 kit status...**blockquote fragment for the two engine-blocked skills. Addeddescription:(reusing the wording already in the matching.agents/skills/*/SKILL.mdwhere one exists; freshly written foradopt,triage-friction-log,post-merge-systemize) andargument-hint:where a command takes a structured argument (pr-watch,parallel,triage-friction-log,post-merge-systemize). No command body text changed..claude/settings.jsonships (Task 2).check_doc_budget.py's own docstring advertised "safe to run from a SessionStart hook" but nothing wired it — memory, not mechanism. Wires it (plus the newcheck_memory_budget.py) into aSessionStarthook, and the newpr_followup_hook.pyinto aPostToolUsehook. Also ships apermissions.allowlist for the kit's own read-only forge calls.pr_followup_hook.pyported (Task 3). APostToolUsehook that fires ongh pr create/gh pr readyand injectsadditionalContextmandating the watch-and-fix loop — a real mechanism for Principle check_doc_budget.py requires PyYAML; make the config reader stdlib-only #5/Fix the adoption path and make engines upgradable (kit schema v2) #8 instead of prose asking the agent to run/pr-watchunasked. Generalized from a reference implementation to readreview.bots,review.fallback_commands.claude, andpaths.enginesfromconfig/dev-model.yamlviascripts/lib/kitconfig.py— never a hardcoded bot name.check_memory_budget.pyported (Task 4). The doc-budget tripwire applied to Claude Code's own per-project auto-memory index (MEMORY.md). Mirrorscheck_doc_budget.py's CLI exactly; reuseskitconfig.pyfor portable repo-root discovery.#14banners corrected (Task 6).triage-friction-log.mdandpost-merge-systemize.mdboth said their engine was "not yet vendored in kit v0," which reads as unwritten. It isn't — working engines exist in the kit maintainer's own toolkit repo, blocked on a tracker-adapter abstraction. Rewrote both banners and linked Extract the friction-triage engine from cs-toolkit behind a tracker adapter #6 and Extract the post-merge-systemize engine (fetch + digest + heartbeat) from cs-toolkit #7.Makefile(Task 5). Thin:install-hooksandtest.What I hardcoded, and why
.claude/settings.json(scripts/check_doc_budget.py,scripts/check_memory_budget.py,scripts/hooks/pr_followup_hook.py) are literal strings, not resolved frompaths.engines. A settings.json hook command is a static string; genuinely resolvingpaths.enginesfirst would require invoking an interpreter to read config before knowing which interpreter/path to invoke — circular for a bare shell command. This is the documented compromise the lane brief explicitly allows. An adopter who vendors the kit's engines under a differentpaths.enginesvalue must edit these three command strings by hand (one line each).permissions.allow'sBash(uv run scripts/pr_watch.py:*)entry carries the same caveat.check_memory_budget.pyis deliberately NOTdoc_budgets-driven, despite the lane brief's "generalize it to be driven by the existingdoc_budgetsconfig list." I traced through why and concluded it's structurally unsafe:check_doc_budget.py(which I'm not allowed to touch) iteratesdoc_budgetsunconditionally, requiring every entry'spathto resolve to an existing, repo-relative file witharchive/remedyfields. The memory index lives outside the repo at a path keyed by this machine's Claude Code config dir and this repo's absolute path — not portable to commit, and not swept to an in-repo archive. Adding it as adoc_budgetsentry would make everycheck_doc_budget.pyrun (including the SessionStart hook it now shares) printerror: configured doc not found: …on any machine/session with no saved memories yet. I verified this by hand (see below) before deciding not to do it.config/dev-model.yamlis untouched in this PR — I did not need the one conditionally-alloweddoc_budgetsentry.MAX_BYTES/MAX_LINE_CHARSincheck_memory_budget.pystay hardcoded platform constants (overridable via--max-bytes/--max-line-chars), matching howcheck_doc_budget.pyitself keeps its own tuning out of adopter config.Settings.json schema verification
I did not ship the hook syntax on faith from the reference example. I verified each piece against the official docs (
code.claude.com/docs/en/hooks.md,hooks-guide.md,permissions.md) via direct fetch:SessionStartmatcher values (startup/resume/clear/compact/fork) — confirmed.PostToolUsematcher"Bash"— confirmed.{type: "command", command, timeout}, timeout in seconds — confirmed.iffield's permission-rule syntax ("Bash(gh pr *)") and that it applies toPreToolUse,PostToolUse,PostToolUseFailure,PermissionRequest,PermissionDenied— confirmed (this one is easy to get wrong; a first pass from a sub-agent using indirect knowledge incorrectly reportedadditionalContextasUserPromptSubmit-only. I re-verified directly againsthooks.mdand confirmedPostToolUsedoes supporthookSpecificOutput.additionalContext, appearing "next to the tool result" — the sub-agent's report was wrong).$CLAUDE_PROJECT_DIRas a real hook environment variable — confirmed.permissions.allowBash(cmd:*)colon-suffix as equivalent to a trailingBash(cmd *)— confirmed.I manually exercised every hook command from a shell (cron-guard on/off, JSON validity, the PostToolUse trigger regex,
make install-hooksin isolation) rather than relying on Claude Code itself to fire them during this session — see commit messages for exact commands run.Left undone / open questions
/Users/topi/Coding/dev-model-sessions/wiring/wt) failstest_state_root_defaults_to_repo_root_stateandtest_resolve_read_path_single_source_when_no_sandboxinscripts/lib/state_paths/tests/test_state_paths.py— both untouched by this PR (scripts/lib/state_paths/**is out of my scope). I traced this to the worktree's own.devkit_state_rootmarker file overriding the tests' monkeypatched env vars during repo-root discovery — an artifact of running the suite from inside an active lane worktree, not a regression: the same suite is 53/53 green when run from a plain checkout with no marker file, and CI checks out fresh viaactions/checkout@v4so it won't see this. Full run from this worktree: 129 passed, 2 failed (106 original − 2 environmental + 25 new tests added here). Flagging per the lane contract rather than silently working around it, since I can't touch the files involved.post-merge-systemize.md/triage-friction-log.mdbanners now link Extract the friction-triage engine from cs-toolkit behind a tracker adapter #6/Extract the post-merge-systemize engine (fetch + digest + heartbeat) from cs-toolkit #7 but their doctrine bodies are otherwise untouched (out of scope — Task 6 was banner-only).Test plan
uv run --with pytest --with pyyaml python -m pytest scripts/lib/state_paths/tests scripts/tests -q— 129 passed, 2 failed (pre-existing/environmental, see above; 25 of the 129 are new in this PR)python3 -c "import json; json.load(open('.claude/settings.json'))"— validpr_followup_hook.pyvia stdin forgh pr create,gh pr ready,gh pr view,JOB_NAMEset, and 7 malformed-stdin shapesSessionStarthook commands and thePostToolUsehook command end-to-end with$CLAUDE_PROJECT_DIRset, confirming exit 0 in every casemake install-hooksandmake testrun cleanly with no unintendedgit statuschanges (verifieddocs/**andconfig/dev-model.yamlstay untouched)