4a5c3c0cff1d3ce779d1a5a8191c448f92937a34
80 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| 4a5c3c0cff |
feat(kyberforge): enforce the ADR-0020 context contract for skills and agents
Skill name+description pairs are preloaded into every session, costing ~6,200 tokens across 39 skills before any skill is invoked. The authoring rules mandated that growth: skill-author:104 and description-quality.md:21 both required padding, while skill-author:102 (the deflating rule) had no FAIL condition behind it. Gates (blocking, no baseline file): - description 250 chars SUGGESTION / 400 FAIL, measured on the folded YAML value - body-only 600 words SUGGESTION / 900 FAIL, independent of the unchanged whole-file 2770-word / 500-line spec backstop - every boundary-clause routing target must resolve to a real skill or agent; catches skill-improve, neuledge-context and gitea-labels - agents take the description gates but deliberately no body gate; a test pins that absence Vale: DescriptionOpener widened to ^This\b, new CompositionNote rule banning architecture notes from descriptions. 10 hits, 0 false positives. Kyberforge's own four skills retrofitted: descriptions 3,364 -> 938 chars (-72%), bodies 8,306 -> 2,487 words (-70%), all via the apm-workflow dispatch pattern. Fixes the skill-improve dangling route and the agent-author misroute to manual review. Also fixes a pre-existing false positive where any line-initial 'read ' was flagged as interactive input, which had already caused two scripts to be rewritten around it. Refs: ADR-0020 |
|||
| ae178a95a2 |
fix(kyberforge): detect a single stale package at SessionStart
apm prints "1 outdated dependency found" in the singular when exactly one package is behind (apm_cli/commands/outdated.py). check-apm-current.sh matched only "outdated dependencies found", so one stale package was invisible: the hook exited 0 silently and no refresh ran. With six packages merging independently, one-behind is the ordinary case, so the freshness mechanism failed most often in the situation it exists for. Three further defects in the same hook: - The host timeout was below the script's own budget. hooks.json declared 320s while the script allows `timeout 60` plus `timeout 300` = 360s, so a slow remote let the host kill the hook mid-update and leave .claude/skills/ half-deployed with nothing emitted. Now 380. A test asserts the invariant rather than the literal: it sums every `timeout N` parsed out of the script and requires hooks.json to exceed it, so changing either side alone fails. - The lockfile guard was cwd-relative, so a session opened in a subdirectory no-opped silently and ran both apm calls against the wrong directory. Now anchored on CLAUDE_PROJECT_DIR, falling back to the cwd so the hook stays inert under a host that does not set it. - Every assertion mocked apm, so the suite was green over code that could not detect its own most common trigger. That blind spot is what hid the singular/plural bug, and it is the same shape as the deleted post-push tests. The suite now stages a genuinely outdated dependency against a local git remote — offline, via url.<path>.insteadOf, so the pass-under-unshare property survives — runs the real `apm outdated`, and replays its output through the real hook. Reverting the grep to plural-only fails it. 23 -> 35 assertions. Each fix mutation-tested individually. kyberforge stays at 1.5.0: it is untagged, so this changes what 1.5.0 ships rather than superseding it, and executables.allow needs no edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 099cf5846c |
ci: gate the executables.allow key against kyberforge's version
apm approves a package's hooks and bin by an exact dictionary lookup on a composed `name#version` key (apm_cli/security/executables.py, is_package_approved). There is no wildcard and no version-less form, so bumping plugins/kyberforge/apm.yml without editing root apm.yml's `kyberforge#<version>` key errors nowhere: the entry stops matching, the SessionStart hook stops deploying, and the install goes quietly stale. ADR-0019 already named that as a live failure mode, mitigated only by a comment in the executables block. This repo gates generated-content drift, marketplace mirror drift and vale style drift deterministically, and a silent-staleness failure is worse than any of them — a comment does not survive the release that breaks it. check-executables-allow-sync.sh parses the version out of the plugin manifest and asserts the matching key exists in the root manifest. It uses PyYAML where importable and falls back to a two-shape scan otherwise, so a missing pip package cannot become the thing that blocks every push; the test asserts both readers agree. 23 assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| dee56c506a |
feat(kyberforge): refresh the apm install at SessionStart, not at push
Why --- ADR-0018 left deployed skills tracking the remote default branch with nothing watching for drift. The mechanism that was supposed to cover this, scripts/git-hooks/post-push, could never have worked: git has no client-side post-push hook. install.sh copied it into .git/hooks/ so it looked installed, and it had never once fired. Issue #78 reported it as skipping the gitea plugin; it was skipping everything. Refreshing on push was also the wrong shape. Your install goes stale when someone else merges, so a push of your own is neither necessary nor sufficient for staleness to have occurred. Implementation notes -------------------- kyberforge ships a SessionStart hook (startup matcher only) that runs `apm outdated`, and when anything is behind runs `apm update --yes` and returns reloadSkills:true so the running session picks up redeployed content. It exits silently with no apm.lock.yaml present, which keeps it inert for hosts that installed this plugin natively rather than through apm. Two findings drove the wiring, both verified rather than assumed: - apm resolves ${CLAUDE_PLUGIN_ROOT} against the installed package root, and `apm pack` keeps only *.json from .apm/hooks/. A .../hooks/<script> reference therefore points into the generated mirror where the script does not exist — apm reports "Hook script not found" and deploys a hook aimed at nothing. The reference must be .apm/-relative, and a test pins it. - apm's executable-trust gate is OFF unless apm.yml carries an `executables:` block; until now every hook, bin and MCP primitive a dependency shipped would have deployed unprompted. Root apm.yml now enables it. The allow key is version-pinned by apm's design, so a kyberforge version bump silently blocks the hook until the key is bumped too — called out in the block and the ADR. Also corrects ADR-0018 and AGENTS.md, which named `apm install` as the refresh command. It is not: `apm install` deploys from apm.lock.yaml's pinned commit and does not re-resolve refs. `apm update` does. Impact ------ Session startup costs ~0.7s when current and ~10.4s when six packages are behind. Auto-refresh rewrites apm.lock.yaml, so an unexplained modification to it after opening a session is expected; the emitted notice says so. .claude/settings.json stops being exactly {"hooks": {}} once the hook lands there — the merged entry is apm's own output, and the rule that nothing repo-authored goes in that file is unchanged. .claude/hooks/ and the .claude/apm-hooks.json sidecar are gitignored install output. The hook cannot install itself: dependencies resolve from the remote, so it takes effect only after this merges and `apm update` runs once against the new default branch. scripts/git-hooks/ is now empty. install.sh's copy block is kept and test-git-hooks-install.sh synthesizes its own fixture, so the mechanism stays tested without requiring a dead hook to exist. ADR: 0019 Refs: #78 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 2e8732a8e5 |
build(apm): consume holocron plugins through apm instead of plugin install
Why:
The repo published apm packages but consumed them the old way — `claude plugin install
<name>@holocron`, six plugins enabled per project. Dogfooding stopped one layer short of the
install tooling kyberforge itself ships.
Implementation notes:
- Root apm.yml declares the six packages as dependencies.apm git+path objects against the
holocron remote. Object form over `<name>@holocron` aliases on purpose: an alias first needs
`apm marketplace add`, which writes to ~/.apm/marketplaces.json — user scope, absent on a fresh
clone. Unpinned against the default branch, matching the autoUpdate the native install had.
- apm.lock.yaml is committed; .claude/skills/, .claude/agents/ and apm_modules/ are gitignored
regenerable install output. Committing the deployed skills would add a third mirror of content
ADR-0017 already governs two copies of.
- .mcp.json is generated by apm from plugins/bin/.mcp.json, so the obsidian MCP server survives
the switch.
- .claude/settings.json is reduced to {"hooks": {}}. apm replays the install into a scratch tree
and diffs, so any repo-owned key there is permanent drift that fails apm-audit-ci. Nothing was
lost: enabledPlugins was empty after the uninstall and the only hooks entry was PreToolUse: [].
- tests/run-bats.sh and tests/run-tests.sh exclude apm_modules/. It holds a full copy of every
plugin, and a copied .bats file resolves its helpers against the dependency root rather than
this repo — 334 tests, 167 failures before the exclusion.
Impact:
Skills are now unnamespaced — `git-commits`, not `git:git-commits` — because apm deploys plain
project skills with no plugin to prefix. AGENTS.md, CONTEXT.md and docs/spec/architecture.md are
updated accordingly. Root apm.yml now declares dependencies, which arms apm-audit-ci's
lockfile-exists check for the root manifest. External consumers are unaffected: the marketplace
manifests are untouched and `apm pack --check-clean` stays clean. Project scope only.
ADR: 0018
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
|
|||
| cf625229f7 |
fix(scripts): detect the .apm/ content the mirror loses silently
apm's bundle exporter drops symlinks entirely, so a symlink under .apm/ never
reaches the mirror -- and no gate could see it, because every existing check
diffs the live mirror against a bundle-derived copy and both sides lack the
file. It is an absence with nothing left to mismatch against, the only class of
.apm/ content that vanishes without a trace. check_apm_symlinks reads the .apm/
source tree, where the loss is visible, and fails both modes. Reported rather
than resolved: dereferencing would make a real sync emit content the bundle does
not contain, which is the reimplementation ADR-0017 rejects.
--check --all could also pass having verified fewer plugins than the marketplace
lists: a plugin whose .apm/ had gone was SKIPped rather than counted, and the
earlier floor only caught zero. The count is now checked against the marketplace's
own local-package list. There is no exempt state -- ADR-0015 makes .apm/ the sole
authoring source for every local plugin, so a listed plugin without one is drift.
On the Copilot hooks gap, the decision is to document, not implement. Copilot
declares no hooks path and apm emits none, which looks like the mcpServers case
-- but that exception holds because .mcp.json is one host-agnostic format both
ecosystems read, so a pointer to it is true whatever it contains. Hooks have no
shared format: Claude expects nested matcher groups under PascalCase events,
Copilot requires version: 1, camelCase, and a bash/powershell split. apm merges
.apm/hooks/*.json into exactly one file, at Claude's convention path. A pointer
would assert a Claude-shaped file is Copilot-shaped -- an incomplete manifest
traded for a wrong one -- and it is not inert today either, since {"hooks": {}}
lacks Copilot's mandatory version key. A test pins the decision, so restoring the
pointer fails until someone confronts the schema mismatch.
Tests: 77 -> 92 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
|
|||
| a155af6827 |
ci: audit every apm package, and validate the agents the validator governs
apm-audit-ci ran against root apm.yml alone, so it audited none of the six plugin packages, and its description claimed a lockfile/policy/hidden-content gate while delivering one vacuous check. It now loops all seven manifests, and the description says only what runs. Proven load-bearing: a malformed dependency in plugins/lint/apm.yml passed the old root-only entry at exit 0 and passed apm pack --check-clean too, because that gate never parses plugin dependencies; the loop catches it and names the file. policy.fetch_failure_default: block was considered and rejected. apm's org-policy discovery understands github.com and Azure DevOps; this repo's remote is self-hosted Gitea, so no policy source is discoverable and the setting makes the hook exit 1 on every push forever. Fail-closed is right when there is a control to fail closed on -- a permanently red gate is one people learn to SKIP=, which is worse than an accurate description. agent-audit's validate.sh had never run against the four real .apm/agents files it governs, only against synthetic fixtures. That is why an amended ADR-0016 and a validator that still rejected the field it blessed could disagree unnoticed until someone ran it by hand. check-apm-agents-valid.sh closes it, deriving the expected set from git ls-files rather than a count, failing on zero discovered files, and replaying validate.sh's own reason under each failing filename. Also makes the pretty-format-json exclude consistently root-anchored: it mixed (^|/) for five paths with ^ for one, so a nested fixture at .../.claude-plugin/plugin.json was silently exempt from formatting. Pre-push goes 12 repo-defined to 13, 14 total to 15; AGENTS.md's counts, hook list and offline-skip note are updated to match. The new hook needs no network. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| a700b3771c |
fix(scripts): make the mirror's mode check umask-independent
The previous round widened path_manifest from the exec bit to full permission bits, and that made check-plugin-content-sync fail at pre-push on a pristine tree. hooks/hooks.json is not copied from the bundle -- sync_hooks_json writes it with printf, i.e. at the runtime umask -- while the real side comes from the checkout. On a umask-002 clone the two disagree, 664 vs 644, and no commit can reconcile them because git tracks no non-exec mode. The rule adopted: record a mode for a path this pipeline copies, never for one it writes. A copied path's mode traces to the same checkout on both sides, so comparing it means something; a written path's mode is the writer's umask on one side and the checkout's on the other, which are independent. That is the same rationale the directory exclusion already carried -- what broke was the premise that files are immune. Normalising instead was rejected: pinning the generated side cannot fix a checked-out side that is already 664. The unconditional chmod 644 in reinject_mcp_servers goes for the same reason; writing through the destination inode already closed the original 0600 bug. The mode coverage added for the two plugin.json manifests is removed rather than documented, because it measured nothing on any axis. In check mode the expected side is a cp -a of the real plugin root, so apm rewrites an existing inode and inherits its mode; and a symlinked manifest is copied as a symlink and written straight through, so both sides agreed no matter what. That symlink case is a real hazard -- the re-injection corrupts the link's target -- so it is now asserted directly instead. Also: an unparseable or non-object per-plugin plugin.json killed the manifest walk mid-loop; the source-less-entry guard closed only source: null and let every other malformed value through; the select it backstops was extracted so a test can exercise it independently, which nothing could before; and two more `|| pwd` fallbacks now hard-error -- with a decoy marketplace.json in $PWD, --all derived its plugin list from it. Tests: 63 -> 77 and 23 -> 31 assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| aa15fc850c |
test: fail the gate when a suite is skipped or never reports
Three gaps left by the previous round, all the same shape: a gate reporting success having verified less than it appears to. run_bats() hard-failed on a missing or non-executable runner but never checked that the runner produced anything. An empty, executable run-bats.sh exits 0, and the dispatcher printed a green summary with 166 bats tests silently absent. It now requires an "N tests, M failures" line with a non-zero count. run-tests.sh's skip listing is swallowed by pre-commit on a pass, so on a machine without vale three suites exited 77 and the pre-push gate went green having run 14 of 17. The hook now invokes it as --strict, where a skip fails and the error names each suite and the reason it skipped. An ad-hoc local run still skips gracefully -- at pre-push a skip means a documented dependency is missing, which is a setup error, not a legitimate state. Deliberately not wired to the vale downgrade's env var: one flag must not disarm two gates. BATS_FILE_FLOOR is replaced by an expectation derived from git ls-files. A floor of 8 against a real count of 10 let two files and eleven tests disappear green, and the number needed an edit whenever a plugin was added. The derived set needs no number, and catches an addition as well as a removal -- a .bats file staged into the index and deleted from disk is now demanded back. The vale opt-out announced its downgrade to nobody: pre-commit prints nothing for a passing hook, so the summary line AGENTS.md tells the reader to check was unreachable in exactly the situation it exists for. The hook is now verbose. Also corrects the PROBES_CHECKED guard, whose commit message described a state that cannot occur -- the .vale.ini loop errs first. Its two reachable triggers, a gutted probe heredoc and a probe row naming a missing directory, had no test; they do now, each asserting the guard is the sole cause. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 52bbd62286 |
test: stop the runners reporting green on suites that never ran
run_bats() was an if with no else, so a missing or non-executable run-bats.sh made all 166 bats tests disappear with no diagnostic while the run printed a green summary and exited 0. --bats-only became a total no-op. This is the same defect the PR fixed one level down, left open in the dispatcher that pre-push actually invokes -- and nothing tested run-tests.sh at all. run-bats.sh's aggregation was asserted by nothing. Three separate mutations to its failure-detection line all survived the existing suite, because real bats emits both a nonzero exit and "not ok" lines, so each signal masked the other. The new cases produce each signal without the other; all three mutants now die. Also in this pass: - zero discovered .bats files exited 0, so a widened path exclusion retired the suite silently. Replaced with a file-count floor: a collapse to one or two files is the same failure as a collapse to zero - an existing-but-empty status file counted as a pass, because [[ "" -eq 0 ]] is arithmetic-true and the || echo 1 fallback only covered a missing file. The repro is deterministic: the stub truncates its own status file, then kill -9s its parent so the real exit-code write never happens Adds tests/test-run-tests.sh, 9 cases pinning the exit 0 / 1 / 77 three-way split against a single summary line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| af085ed057 |
fix(scripts): fail the vale and scope gates when they cannot verify
check-vale-style-sync.sh's glob-coverage probe silently self-disabled when vale was absent from PATH, exiting 0 on the one-character glob typo it exists to catch. pre-commit swallows a passing hook's output, so the pre-push hook reported Passed. The script already hard-fails on a bad REPO_ROOT for exactly this reason -- "a clean exit 0 here would read as 'checked, in sync' when nothing ran at all" -- and six of its assertions are vale invocations. Absence now fails; the opt-out is an env var that must be set deliberately, and it downgrades the run to text-level assertions while saying so in the summary. Neither script had a floor on its rewritten .apm/ paths, so relocating .apm/ made both exit 0 -- and this PR's whole change to them was a path rewrite, the exact edit that failure mode survives. A third gap the directory check could not see: relocating only assets/vale/ left both audit skill directories in place while every probe continued past its missing .vale.ini, skipping the whole table with FAIL=0. A zero-probe run is now an error. Both test suites encoded the vacuous pass as a passing case. Those cases are now scoped to "no plugins/kyberforge at all" and assert the fixture really lacks it, with new counterparts covering the drift shape and new positive cases requiring each script to report a non-zero inspected-target count. Also removes the HOOK_REGEX_CACHE memoization: every call site was a command substitution, so the writes happened in a subshell and the lookup always missed. Measured at 14ms of an ~870ms run, all of which is the six vale invocations. Deleted rather than repaired -- 35 lines claiming a benefit they never delivered is worse than no cache -- with a comment recording why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 3f1ee47f1e |
fix(scripts): stop check-manifests passing on entries it cannot parse
A marketplace entry missing its source key disabled both directions of the check at once. The helper required source to be a string, so a source-less entry was skipped and its plugin.json existence check never ran; the name axis selected on (.source | type) != "string", and null != "string" is true, so the same entry also marked its on-disk directory as listed. Delete source from an entry and delete its plugin.json and the script exited 0. Because sync-plugin-content.sh --all derives its work list from the same helper, that plugin silently dropped out of the content-mirror gate too. Also in this pass: - a wrongly typed skills value crashed the script mid-loop with a raw jq error and no "Manifest check failed:" line, leaving every later plugin unchecked. Note skills is legally string|string[] per both host schemas, so a string now resolves as a single path rather than erroring - array- and object-valued pointer fields were reported missing even when they resolved, because the whole JSON value was pretty-printed into a path test - an unparseable marketplace.json died inside a process substitution, so the run reported six "no entry in marketplace.json" errors that sent the reader to edit apm.yml when the real fault was a corrupt manifest - a missing marketplace.json exited 0 even with plugin directories present Tests: 14 -> 23 assertions. Every failure case asserts on message text, not exit code alone, since exit 1 here is reachable by several causes that call for opposite fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 9e612fd183 |
fix(scripts): re-inject .mcp.json as a pointer, not resolved content
reinject_mcp_servers copied .mcp.json's mcpServers into the compiled Copilot
manifest verbatim via jq. apm's own path does not: collect_mcp_servers runs
_sanitize_mcp_servers(), which drops env/headers/authorization and redacts
secret-shaped keys, because copying them into a committed manifest exfiltrates
them into the distributed artefact. The re-injection was the only route around
that sanitizer, and it wrote to a tracked, marketplace-distributed file.
Both host schemas document mcpServers as "string or object -- config path or
inline definitions", so the pointer form is valid and carries no resolved
content. It also preserves the ${VAR} indirection the sanitizer strips.
Also in this pass:
- mktemp+mv left the manifest at 0600 while --check compared content only, so
a real sync silently demoted a mode the gate could not see
- --check --all exited 0 when the marketplace yielded zero plugins, including
on unparseable JSON: the one gate whose work list comes from a generated file
could be silenced by regenerating its own input
- sync_dir took an unguarded $target_dir despite a comment claiming otherwise
- basename '.'/'..' escaped $SCRATCH_ROOT and made bundle selection arbitrary
- path_manifest compared only the exec bit, so check and sync disagreed
- sync-marketplace-mirror.sh fell back to pwd outside a worktree and reported
no drift on a tree it never identified
Mode comparison is deliberately files-only: directory modes come from umask on
one side and checkout on the other and git tracks neither, so comparing them
reports the runner's umask rather than a property of the mirror.
Tests: 44 -> 67 and 15 -> 19 assertions, each verified to fail under the
mutation it exists to catch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
|
|||
| c442f7eb85 |
fix(scripts): decide .vale.ini readability by reading it, not by access(2)
Issue #97 item 1 reports the unreadable-.vale.ini guard as untested. It was worse: it was dead. `[[ -r ]]` is access(2), which asks whether the permission bits would allow a read -- and for uid 0 that is yes even on a mode-000 file. This hook runs at pre-push and the dev environment is root, so the guard could never fire where it exists to fire. That is why no uid-independent test for it existed; there was nothing to test. Readability is now decided by actually reading (`cat`), which is uid-independent and strictly stronger, catching EISDIR and EIO that access(2) reports on neither. `cat`, not a `< "$ini"` redirect: opening a directory for reading succeeds, only the read fails. The missing branch moves to `-e`, so a directory sitting where the file belongs is reported as unreadable rather than sending the reader hunting for a deleted file. The new case asserts the MESSAGE, not the exit code. With the guard removed the script still exits 1 -- the greps hit the unreadable path and blame a missing StylesPath on a file that has one. An exit-code-only test would have been green with the guard deleted. Also stops paying for vale in cases that only assert .vale.ini text: 21 of 28 script runs now mask it via the PATH_NO_VALE mechanism case 12 already builds, cutting the suite's bottleneck ~3.5x (issue #97 item 5). The helper falls back to an unmasked run rather than skipping, so a machine where masking is unavailable loses speed, never coverage. That masking is a coverage gain, not only a speedup. With vale on PATH, cases 8 and 9 could not detect deletion of the assertions they were written to catch: a dropped StylesPath also breaks the glob probe, so the script exited 1 for the wrong reason and both cases went green. Verified against the pre-change files -- the same mutation was caught by one incidental assertion before, and by three after. Refs #97 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 5a61b417c9 |
fix(scripts): bring the generated hooks/ directory under the mirror's ownership
`checked_paths` covered hooks/hooks.json but not the hooks/ directory holding it, so a stray file dropped inside, or an empty hooks/ left behind once .apm/hooks/ stopped producing anything, was invisible to --check. Check and sync agreed in both cases, so the invariant held -- but a stray in a directory the mirror owns should be drift, exactly as it is inside skills/ or agents/. A stray at the PLUGIN root stays out of scope by design: README.md, docs/, bin/, .mcp.json are hand-authored. hooks/ is now wiped and rebuilt like every MIRROR_DIRS destination, and the directory is listed in checked_paths so the recursive manifest sees one-sided entries. Issue #97 item 4 reports `prompts` as documented-but-unmirrored. That is refuted: MIRROR_DIRS lists DESTINATION directories, and apm folds .apm/prompts/ into commands/ (renaming *.prompt.md to *.md), verified empirically. A plugin adding .apm/prompts/ is mirrored today; adding a `prompts` entry would name an output directory apm never emits. Pinned with a characterization test that fires if that mapping ever changes, plus a comment so it is not refiled. Guards the new wipe with ${target_dir:?}: `set -u` aborts on an unset variable but not an empty one, which would make it `rm -rf /hooks`. Refs #97 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 73393b9d01 |
fix(scripts): correct shellcheck source directives that resolved to nothing
`tests/run-tests.sh` declared `source=lib/batch-run.sh`, which resolves to neither the repo root nor the script's own directory. A directive that does not resolve is silent: it blinds test-vale-wrap.sh's `sourced_files()` seeding exemption, and shellcheck's own SC1091 is `info` while .pre-commit-config.yaml pins `--severity=warning`. Issue #97 names run-bats.sh's `../scripts/lib/batch-run.sh` as the correct spelling. It is not. Directives resolve against the source-path, which under pre-commit is the repo root, so `../scripts/...` escapes the repo and trips SC1091 exactly as `lib/...` does -- verified directly. The spelling satisfying both shellcheck and `sourced_files()`'s two-candidate rule is repo-root-relative, matching scripts/install.sh. Fixes all three: run-tests.sh, run-bats.sh, and check-manifests.sh, the last unmentioned by the issue. Every directive in the repo now resolves, which the previous commit's case 27 asserts. Refs #97 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 49d21bcb4d |
fix(providers): guard the statusline's unguarded array expansion
`parts` is seeded empty and all seven appends are conditional, so
"${parts[@]}" at the join loop can expand an empty array. install.sh
deploys this file to every user machine.
Two things had to both hold for the bare form to be safe: this file
enabling no `set -u`, and the shell being bash 4.4+, which stopped
treating an empty-array expansion as unbound. On bash 3.2 -- macOS's
system bash, an explicit repo target -- adding `set -u` aborts here.
That is also why the hazard is unreproducible on a modern dev box and
why the enforcement is a static scan rather than a runtime test.
Adds the `providers` glob to test-vale-wrap.sh's bash-3.2 scan, which
excluded it precisely because of this defect. Floor is 1 rather than
"count minus slack": the glob holds one file, so any slack at all
means a floor of 0, which passes vacuously on a renamed directory.
Also adds case 27, the regression test for the stale `shellcheck
source=` directives fixed in the next commit (#97 item 2). It lives in
this file because that is where the exemption it guards lives.
Closes #96
Refs #97
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
|
|||
| 413a750819 |
fix(scripts): close gates that passed while the thing they guard was disabled
Four repo gates reported success in states they exist to reject. `check-vale-style-sync.sh` passed while a Kyberforge lint rule was silenced. The check matched a blocklist of severity values, but Vale's semantic is an allowlist: anything that is not exactly YES/error/warning/suggestion disables the rule. So `= false`, `= 0`, `= garbage`, an empty value and — worst — a lowercase `= yes` all killed enforcement while reading as "enabled" to a human. Inverted to an allowlist. Two sibling holes: dropping `KyberforgeCopilot` from `BasedOnStyles` unloaded the Copilot-only check silently, and narrowing a section glob to a location made Vale lint zero files, which is the "0 files, hook Passed" failure the script's own comment says it exists to catch. `sync-marketplace-mirror.sh --check` failed open when its source was missing, while its sibling correctly errored in the same state. `check-scope-walkup-sync.sh` wrote to hardcoded `/tmp/fN.out` paths and read one back, making it non-reentrant — a concurrent instance can flip a verdict, and this branch made the test runner concurrent. Now per-run `mktemp -d`. `check-manifests.sh` had no disk-to-marketplace pass, so a plugin directory absent from `marketplace.json` passed every gate while the `validate-plugins` hook globbed it. The "listed" match is restricted to remote-source entry names; matching any entry name let a genuine orphan through on a name coincidence. `run-bats.sh` reported an empty TAP stream as `0 tests, 0 failures`, exit 0 — a total harness failure reading as a pass. The test-side changes are the larger half, because the guards were the real problem. `test-sync-marketplace-mirror.sh` could overwrite the live tracked mirror under an inherited GIT_DIR, which is precisely the git-hook context it runs in. The bash-3.2 scan hand-maintained its file list, omitting the new shared runner, and had no rule for `wait -n` or `nproc` — the two hazards the previous review round found live. It now derives 43 files across three globs with per-glob floors. Several assertions were decoration: the concurrency checks caught the reentrancy defect 0 times in 10, the leak fix was green either way, and two manifest fixtures passed with the code they claimed to cover deleted. Every assertion now has a revert it provably fails against. Refs: #90 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| d4fa4b7153 |
fix(kyberforge): stop the content mirror amputating a shipped template asset
The mirror's `tests/` exclusion was depth-agnostic, so it deleted `skill-author/assets/templates/tests/` — a template the skill scaffolds FROM — alongside the depth-2 dev fixtures it was meant to drop. Since ADR-0017 makes the mirror the installed content, the shipped scaffolder was broken: the mirror copy of `new-skill.sh` exited 2 on `sed: can't read .../tests/README.md`, leaving a half-written skill, while the byte-identical `.apm/` copy exited 0. `--check` was green about it. Check mode was restructured rather than patched because `diff -x` matches a basename at any depth and cannot express the depth-2 scoping the fix needs — the two modes could not be made to agree by construction. Check mode now runs the real `sync_dir` into a throwaway root and diffs with no exclusions, leaving the exclusion rule and the hooks destination each in exactly one place. Also fixed here, all previously invisible to `--check`: - Merged hooks were written to `<plugin>/hooks.json`, which Claude Code does not convention-scan, while ADR-0017 itself quoted `hooks/hooks.json` as the contract. Moved, with the legacy path cleaned up as stale. No `hooks` pointer is added to `plugin.json`, so this does not reopen the option ADR-0017 rejected. - Only the first drift per plugin was reported: `diff | sed` returns 1 under `pipefail`, and `set -e` killed the subshell before the remaining checks and before `FAIL=1`. - File-mode and symlink drift were invisible, so `--check` and a real sync disagreed; a find-based type/mode manifest now covers both. The tests pinned almost none of this — the stale-skill wipe, the check-mode stale branch, three `MIRROR_DIRS` entries and the hooks newline normalization could each be deleted with the suite still green. All are now mutation-tested. Refs: #90 ADR: 0017 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| e79497b3cf |
fix(tests): guard remaining bash 3.2 hazards from PR #95 review
Review findings #5 and #7 on PR #95 flagged two bash-3.2-incompatible patterns despite the surrounding scripts claiming 3.2 safety: - tests/run-bats.sh used `mapfile` (bash 4.0+), which fails immediately under macOS's stock bash 3.2 before any batching logic runs. Replaced with the `while read` loop already established in tests/run-tests.sh, and guarded the two downstream `${TEST_FILES[@]}` expansions with `${arr[@]+"${arr[@]}"}` to match that file's convention. - `trap 'rm -rf "${CLEANUP_DIRS[@]}"' EXIT` was unguarded in tests/test-sync-marketplace-mirror.sh and tests/test-sync-plugin-content.sh: under `set -u`, if `mktemp -d` fails before the array is populated, the trap itself throws an unbound-variable error that masks the real test failure. A repo-wide grep for the same pattern turned up a third, unreviewed instance in tests/test-check-release-needed.sh. Fixed all three with the guarded idiom already used elsewhere in the repo. Extended the existing bash-3.2-hazard static check (test 16 in tests/test-vale-wrap.sh) to scan all four fixed files going forward, so a regression of either pattern fails the suite instead of only surfacing on a real bash 3.2 host. Refs: PR #95 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 23cef3627a |
fix(kyberforge): restore pointer-field validation for non-apm plugins
Skills/hooks/mcpServers/agents pointer-field validation in plugin.json was fully delegated to sync-plugin-content.sh --check, but that script explicitly skips any plugin directory lacking .apm/ (it has nothing to compile there). A plugin with no .apm/ and a hand-authored plugin.json whose pointer field points at a missing path was therefore left uncovered by either check -- currently latent since every plugin in this repo has .apm/, but a real gap for the first non-apm plugin added. Restores a fallback validation path here for exactly that case (no .apm/ directory), reusing the pre-delegation logic this script used to run unconditionally. apm-native plugins keep relying on the delegated check so the two never duplicate (or disagree) on the same manifest. Also switches the marketplace.json walk to the shared scripts/lib/marketplace-plugins.sh helper introduced alongside sync-plugin-content.sh's matching --all branch, replacing the near-identical hand-duplicated loop this script's own header comment already flagged as a duplication risk. Adds fixtures: a non-apm plugin with a broken skills pointer (caught), a non-apm plugin with a valid pointer (no false positive), and an apm-native plugin with a broken pointer (left to the delegated check, not double-validated here). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| fd70c8d65e |
fix(kyberforge): catch plugin.json and hooks.json drift in sync-plugin-content.sh --check
--check's throwaway pack copy seeded .claude-plugin/plugin.json and
.github/plugin/plugin.json from the real plugin dir, then packed without
--force -- apm pack silently skips regenerating a plugin.json that already
exists, so the diff always compared the copy against itself and never caught
drift in the compiled name/version/description/mcpServers. --force is now
always passed; in check mode it forces regeneration inside the throwaway copy
only, which sync_plugin_manifest() then diffs against the real committed
manifest.
sync_hooks_json() returned early whenever .apm/hooks/ was missing, without
checking whether a stale hooks.json was still sitting at the plugin root from
a prior sync -- unlike sync_dir(), which already detects that kind of orphaned
mirrored output. It now mirrors sync_dir()'s shape: flagged as drift in
--check, removed on a real sync.
Running the corrected --check --all against this repo's own plugins surfaced
3 real orphans: plugins/{git,gitea,core}/hooks.json, empty stubs added in
|
|||
| 07ea0aeb17 |
fix(kyberforge): stop union-masking drift between vale-audit-prefilter manifests
hook_file_regexes() unioned the `files:` regex from .pre-commit-hooks.yaml and .pre-commit-config.yaml before checking whether a probe path is in scope of a kyberforge vale-audit-prefilter hook. That union let a probe matching only the old, looser .pre-commit-hooks.yaml pattern pass even after .pre-commit-config.yaml's copy of the same hook had been narrowed (e.g. to require a `.agent.md` suffix) -- silently masking exactly the kind of hook-rescoping drift this check exists to catch. Per ADR-0014 the two manifests are meant to exercise the same resolution path an external consumer's hook would, so this divergence is real drift, not noise. hook_file_regexes() now takes the manifest path explicitly and caches per (skill, manifest) pair instead of per skill, so each manifest's regex set can be inspected on its own. The probe-validation loop computes in_hooks/in_config independently via a new matches_any_regex() helper. Probes carry a new third heredoc field, `shared` or `hooks-only`: `shared` probes (a file shape genuinely covered by both manifests, e.g. plugins/demo/.apm/agents/demo.agent.md) must agree between the two or the check now fails with a drift error; `hooks-only` probes (a Copilot .agent.md living outside this repo's own plugins/.apm/ layout, and the legacy bare-`.md`-under-agents/ shape kept only to exercise a distinct .vale.ini glob section in isolation) are exempt, since .pre-commit-hooks.yaml is deliberately broader there by design. The original "matches no regex in either manifest" staleness check is unchanged. Added case 11b to tests/test-check-vale-style-sync.sh: narrows a fixture's local config regex further while leaving .pre-commit-hooks.yaml untouched, and asserts the check now flags it. Confirmed red against the pre-fix script before applying the fix. Refs: #95 |
|||
| 2c731eb476 |
test(kyberforge): cover sync-marketplace-mirror.sh drift-guard
Its sibling sync-plugin-content.sh has thorough coverage (tests/test-sync-plugin-content.sh) but this drift guard -- wired into pre-commit as check-marketplace-mirror-sync, keeping .claude-plugin/marketplace.json and .github/plugin/marketplace.json in sync -- had none. A silent regression here would let the two marketplace manifests drift without any test catching it. Covers: missing-source no-op in both real-sync and --check modes, drift detection when the mirror is missing or stale, real sync producing a byte-identical mirror, --check clean immediately after sync, drift from an edited source cleared by re-sync, an already-in-sync mirror reporting no drift, usage-error rejection of unrecognized/extra arguments, and idempotency of repeated syncs. |
|||
| 4003c6a273 |
fix(kyberforge): make bats dispatcher bash-3.2 safe
run-bats.sh's new bounded parallel dispatcher used nproc/wait -n, which are bash 4.3+/GNU-only and silently drop the concurrency cap on macOS's stock bash 3.2 (the wait -n error is swallowed by `|| true`). Its sibling tests/run-tests.sh, changed in the same PR and explicitly bash-3.2-safe, already solves this with getconf + a batched wait. Ported that same pattern here for consistency and to actually meet the compatibility goal. Refs: #95 |
|||
| 9c140efa2e |
fix(kyberforge): harden plugin-content sync, reinject Copilot mcpServers
PR #95's review of the issue #90 apm-conversion work found several defects in scripts/sync-plugin-content.sh and the gate wired to it: - --check claimed never to mutate the plugin root, but apm pack still wrote .claude-plugin/plugin.json and .github/plugin/plugin.json into the real plugin_dir on first-time creation. --check now packs a throwaway copy instead. - check-plugin-content-sync hardcoded the six plugin directories instead of deriving them the way check-manifests.sh already does. Added an --all flag that parses .claude-plugin/marketplace.json, and simplified the pre-commit hook to use it. - A missing plugin_dir and one that legitimately has no .apm/ yet both reported SKIP/success; a missing directory now FAILs. - The dispatch loop backgrounded every plugin with no concurrency cap, unlike the JOBS-bounded pattern this same PR added to tests/run-bats.sh and tests/run-tests.sh. Added the same bash-3.2-safe getconf + batched-wait cap here for consistency. - Per-plugin scratch/log/status files were keyed only by basename, with no collision guard across arguments; added a fail-fast check. - sync_hooks_json()'s trailing-newline normalization was duplicated between its --check and write branches; factored into one helper. - tests/test-sync-plugin-content.sh set two competing `trap ... EXIT` statements, so the first (cleaning up $FIXTURE) was silently replaced by the second and its tmp dir leaked every run. Adopted the track()/CLEANUP_DIRS pattern already used in tests/test-check-release-needed.sh. Separately: apm's Copilot-ecosystem plugin.json builder unconditionally strips mcpServers, citing (in its own docstring) that the field is out of schema for Copilot -- a claim this repo's own researched Copilot plugin schema docs contradict. reinject_mcp_servers() narrowly restores it from the plugin's .mcp.json on real syncs only, regenerating plugins/bin/.github/plugin/plugin.json (the only plugin that currently declares any MCP servers). Documented as an amendment to ADR-0017, since it's a deliberate, narrow exception to that ADR's rejection of patching apm's compiled output -- apm's premise for stripping skills/agents/commands/hooks pointers is still accurate; its premise for stripping mcpServers is not. All 12 assertions in tests/test-sync-plugin-content.sh pass individually, plus 5 new regression tests added for this round; the full bats and shell-script suites are green; shellcheck is clean. Refs: #95 ADR: 0017 |
|||
| a8beff7d2c |
perf(kyberforge): parallelize the bats and shell test runners
Both runners spawned one process at a time and let it finish before starting the next, so wall time scaled with file count even though each file's own work (bats: mostly repeated apm/git subprocess startup; the vale-heavy shell scripts: repeated vale binary startup) rarely used a full core. run-bats.sh now backgrounds one `bats` invocation per file, bounded by core count, buffering each file's TAP output so concurrent streams can't interleave, then flushes in stable sorted order once every job finishes. run-tests.sh does the same for test-*.sh in fixed-size batches (plain `wait`, not `wait -n`, to stay on the bash-3.2-safe path test-vale-wrap.sh already enforces for this file) -- verified beforehand that every test-*.sh keeps its fixtures under its own mktemp dir rather than mutating the live repo tree, so running them concurrently is safe. Confirmed correct on both the pass and fail paths (a deliberately failing bats test and a scratch TEST_DIR with pass/fail/skip-77 scripts) before measuring: full bats suite 26-30s -> 7.5s, tests/run-tests.sh 68s -> ~45-50s, full pre-push gate 2m40s -> 1m12s. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT |
|||
| 38f1ba4e03 |
fix(kyberforge): bridge apm content to Claude Code's flat plugin discovery
Claude Code's (and Copilot's) native plugin installer has zero awareness of .apm/ nesting -- it convention-scans only flat skills/, agents/, commands/, hooks.json at each plugin's root. Confirmed via strings on the installed claude binary and live installs of git@holocron/gitea@holocron/kyberforge@ holocron, all reporting Skills(0) Agents(0) Hooks(0) post ADR-0015's apm conversion. Root cause (apm_cli/core/plugin_manifest.py): apm's plugin.json compiler deliberately strips skills/agents/commands keys, assuming the host already auto-discovers those convention directories -- it has no model of .apm/ being host-visible at all. Separately, apm's own bundle exporter (apm_cli/bundle/plugin_exporter.py, behind `apm pack --format plugin`) implements the correct .apm/ -> flat mapping, but only ever targeted build/<name>-<version>/, a path nothing in marketplace.json's source: points at. scripts/sync-plugin-content.sh wraps that bundle exporter and copies its agents/, skills/, commands/, instructions/, extensions/, and merged hooks.json back into each plugin's own root as a second tracked compiled-output category -- same governance status as .claude-plugin/plugin.json: generated from .apm/, never hand-edited. tests/ subdirectories are excluded from the mirror (dev fixtures, not host-visible runtime content; several hardcode a relative repo-root walk-up sized for the .apm/-nested depth, which breaks when duplicated one level shallower). Applied for real across all 6 plugins and verified two ways: `claude plugin validate --strict` passes on every real plugin directory, and a live `claude --plugin-dir <path> -p "list skills/agents"` behavioral test confirms content is now actually discovered. Also, from the same issue #90 review round: - scripts/check-manifests.sh pointed at each plugin's root-level plugin.json (checking skills/hooks/mcpServers/agents pointer fields) -- that file was a stale near-duplicate of .claude-plugin/plugin.json nothing else read or wrote, now deleted across all 6 plugins. check-manifests.sh is rewritten to validate .claude-plugin/plugin.json instead, and drops the pointer-field checks entirely (nothing to check -- those fields are correctly absent by design). Content-presence drift is now check-plugin-content-sync's job, a new pre-push hook wired in .pre-commit-config.yaml. docs/adr/0017 records the root cause and decision in full, including two rejected alternatives (patching plugin.json's path fields directly -- apm's compiler strips them on every run; pointing marketplace.json at apm pack's build/ output -- a version-suffixed non-source directory nothing can install from without an extra build step). ADR-0015 and CONTEXT.md are updated to point at it. Refs: #90 |
|||
| 5e232503c4 |
feat(kyberforge): execute plugin-to-apm marketplace conversion
Why: ADR-0015 established that Microsoft APM (apm.yml + .apm/) should replace this repo's hand-authored plugin.json/marketplace.json model, with those files becoming compiled output of `apm pack` instead of files edited by hand via the (now-retired) plugin-author/marketplace-author skills. Issue #90 was the deferred execution of that decision, gated on #88 (apm tooling) and #89 (apm-native agent-author/skill-author routing). Implementation notes: - All six plugins (bin, core, git, gitea, kyberforge, lint) now carry apm.yml + .apm/{skills,agents,hooks} as their authoring source. Skills moved with a plain git mv (content-identical across targets). Agents were re-authored, not moved: per ADR-0016, .apm/agents/*.agent.md compiles verbatim to both Claude and Copilot, so plugin-scope agents now carry only name/description/model/source_keys -- no tools: field, no Claude-only knobs (isolation, maxTurns, effort, memory, permissionMode). - Root apm.yml registers all 7 marketplace packages (6 local plus mattpocock-skills as a remote entry) under versioning: per_package, matching this repo's existing independent-plugin-versioning practice. - .claude-plugin/marketplace.json and every plugin's plugin.json are now apm-pack-compiled output, verified against the prior hand-maintained content: same names/descriptions/versions/licenses/authors, only cosmetic serialization differences (JSON key order, owner email vs. url, Unicode escaping). - plugin-author and marketplace-author are retired now that apm-based authoring fully replaces their job; kyberforge bumped 1.3.1 -> 1.4.0 for that removal, and the root marketplace catalog bumped 0.3.1 -> 0.3.2 to match, per the version-bump convention now documented in apm-workflow's reference docs instead of a dedicated script (apm has no native version-bump automation). - Fixed hardcoded pre-.apm/ path assumptions across .pre-commit-config.yaml, .pre-commit-hooks.yaml, scripts/check-scope-walkup-sync.sh, scripts/sync-vale-styles.sh, scripts/check-vale-style-sync.sh, six plugins' root plugin.json (stale skills/hooks/agents pointer fields that check-manifests.sh validates), and several tests/*.bats and tests/*.sh fixtures -- including a bats REPO_ROOT relative-path depth bug (10 files, one extra .apm/ directory level to walk up) and a vale probe-path isolation regression introduced mid-fix. - Corrected empirically-wrong assumptions surfaced this session in apm-workflow/apm-install's own reference docs: `apm marketplace package add` does not accept local paths (only owner/repo remote shorthand -- local packages are registered by editing apm.yml's marketplace.packages[] directly); `apm compile` is a consumer-side AGENTS.md/CLAUDE.md generator, not the plugin.json producer, and hard-fails on skill/agent-only packages without --clean; `apm plugin init <name>` nests a stray subdirectory when run with a positional name arg from inside a same-named directory; no native Copilot marketplace output profile exists; .mcp.json is merged into the compiled plugin.json content-aware and target-scoped, with no dependencies.mcp entry needed for simple passthrough; pipx is the correct pip fallback on externally-managed Python environments. - Renamed agent-author's copilot.agent.md template asset to copilot.agent.md.template so apm compile's recursive *.agent.md glob stops misparsing the placeholder template as a real agent primitive. Impact: plugin.json and marketplace.json are compiled artifacts from here on -- editing them by hand is no longer the workflow; edit apm.yml/.apm/ and run apm pack. CONTEXT.md's Plugin/Plugin marketplace glossary entries reflect this. ADR-0001 is marked superseded, ADR-0006 moot, and ADR-0010 updated for the new .apm/agents/ path (project/user scope unaffected, per ADR-0016). Full local verification: claude plugin validate --strict on all 6 plugins, apm audit --ci, apm marketplace check, check-manifests.sh, and the full test suite (165/165 bats, 13/13 shell scripts) all pass clean. Fixes: #90 Refs: #88, #89 ADR: 0015 ADR: 0016 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ub96PyaSRD9BHPktotj1pC |
|||
| 044b2d3f08 |
fix(kyberforge): fix HOME/git scope-walkup false-FAILs in agent-audit
validate.sh's detect_scope() and validate-provenance.sh's find_plugin_root() disagreed with new-agent.sh's already-correct, documented walk-up semantics on three points, each causing validate.sh to false-FAIL a legitimately-scaffolded project-scope agent pair: - a marker-less directory walked up into $HOME (no .git/apm.yml of its own) was classified as user scope instead of project scope - the .git-boundary branch returned the walked-to .git location instead of the conventional scope root, breaking any <root> that is a subdirectory of a larger git-tracked tree (monorepo package dirs) - the new conventional-root arithmetic introduced to fix the above two cases had no guard against non-conventional/hand-placed file paths, which could point it at the wrong ancestor Also adds scripts/check-scope-walkup-sync.sh, a behavioral drift-guard (per ADR-0014's no-cross-skill-path precedent) that cross-checks the four independently hand-ported walk-up implementations (validate.sh, validate-provenance.sh, new-agent.sh, new-skill.sh) against real fixture scaffolds, wired into .pre-commit-config.yaml at pre-push so future drift between the ports is caught automatically. Verified via bash tests/run-tests.sh (13/13) and targeted before/after reproduction of each bug this closes. |
|||
| cf5de2bd87 |
fix(lint): fail loudly on a nonexistent REPO_ROOT in check-vale-style-sync.sh
A bad or stale REPO_ROOT argument fell through to the "neither copy present" no-op guard and exited 0 — the exact "clean result can mean nothing was checked" anti-pattern this PR spent multiple review rounds eliminating elsewhere. That guard exists for a repo that legitimately has no kyberforge plugin installed, not for a typo'd path. Only the documented manual-invocation mode was affected: the shipped pre-push hook always calls this script with zero args, which resolves via `git rev-parse --show-toplevel` and is always valid inside a repo. Added a regression test asserting a nonexistent REPO_ROOT exits non-zero. Refs: #85 |
|||
| 389a4f0f7a |
fix(lint): avoid bash 4 associative arrays in check-vale-style-sync.sh
check-vale-style-sync.sh used `declare -A` for a per-skill regex cache.
Associative arrays are bash 4.0+; this script runs as an always-run
pre-push hook with `language: system`, so it inherits whatever bash is
first on the invoking user's PATH. On macOS's stock bash 3.2, `declare -A`
at top level aborts immediately under `set -euo pipefail` — every push
would hard-fail before the sync check ran anything.
Replaced with two parallel indexed arrays (HOOK_REGEX_CACHE_KEYS/_VALS),
linear-scanned by index — same caching behavior (avoids re-parsing both
pre-commit manifests when agent-audit is probed twice), but only ever
uses ${#arr[@]} and index access, never a bare ${arr[@]} expansion.
Extended test-vale-wrap.sh's existing bash-3.2 hazard sweep to scan this
file too, and added a check for `declare -A` itself — it previously only
caught unguarded ${arr[@]} expansions and mapfile/readarray, so this
exact regression had no test that would have caught it.
Refs: #85
|
|||
| 6910f1b5a5 |
fix(lint): reject checkpoint-suffixed tags as the release-gate baseline
git describe --match is a shell glob, not a regex: the trailing `*`s in 'v[0-9]*.[0-9]*.[0-9]*' match any suffix, so a tag like v1.2.3-checkpoint or v1.2.3-rc1 satisfied the pattern and could be picked as LAST_TAG instead of the true last release. That silently shifts the diff baseline and can let a push skip a required release. --exclude '*-*' rules out any tag carrying a hyphenated suffix. Added a regression test that tags a release-relevant change with a v1.0.1-checkpoint tag right after v1.0.0 and asserts the gate still fires — confirmed it fails against the pre-fix script and passes against the fix. |
|||
| 9a3f72b696 |
test(lint): stop the release-gate suite inheriting the caller's PRE_COMMIT refs
run_check set PRE_COMMIT_REMOTE_BRANCH and, when asked, PRE_COMMIT_TO_REF, but never cleared what was already in the environment. Standalone that is invisible — nothing sets those vars. Under the pre-push hook this suite exists to guard, pre-commit exports PRE_COMMIT_TO_REF and PRE_COMMIT_FROM_REF as shas of the real repo; the fixtures inherited them, the script resolved a rev that does not exist in the fixture, and 13 of 20 cases failed. The suite passed in every context except the only one that matters. The variables are now cleared in both branches, so a standalone run and a pre-push run are the same test. Verified 20/20 with the vars unset and with them set to real shas of this repo. Found by the pre-push hook rejecting the push, not by any test — the same shape as the --config regression: the local invocation exercised a different thing than the shipped one, and the two were indistinguishable by reading the file. Refs: #85 |
|||
| 302f6d0c19 |
fix(lint): tighten the SKILL.md word ceiling to 2770
MAX_WORDS=2900 was calibrated to the corpus median density and carried no
margin: at the densest observed 7.22 chars/word (~1.81 tokens/word) it permits
~5,240 tokens against the 5,000 it proxies for. 2770 holds the worst observed
density under the ceiling. The largest SKILL.md is 2,489 words, so the change
costs nothing today — 281 words of margin — and the header comment now argues
the new calibration rather than swapping the digits.
Both enforcement points move together, and a new test asserts they agree, since
a SKILL.md passing its own audit while the commit hook blocks it is the
disagreement this pair exists to prevent.
CONTEXT.md is deliberately left ungated: it is 2,816 words, and gating it would
block the build. Recorded here so the omission reads as a decision rather than
an oversight.
skill-audit's manual-fallback path listed only the line ceiling, so an agent
taking that path passed an oversized SKILL.md the hook then rejected. The word
ceiling is now named alongside it. agent-audit is deliberately unchanged: the
size hook scopes to SKILL.md only and agent-audit's validate.sh has no word
gate, so claiming it there would be false.
The Vale research doc still showed the MDX {/* vale off */} form under a
Markdown heading, contradicting CONTEXT.md and vale-run's troubleshooting
reference — that form suppresses nothing in plain .md. Fixed in both places it
appeared.
tests/run-tests.sh used mapfile (bash 4.0+) with unguarded array expansion,
though AGENTS.md tells contributors to run it and macOS ships bash 3.2. It now
collects via a while-read loop over process substitution and guards every
expansion. The newline-delimited find|sort pipeline is kept rather than -print0
with sort -z, whose BSD portability is the weaker link, and which matches
mapfile -t's previous behaviour exactly.
Refs: #85
ADR: 0013
|
|||
| f6eb0d295e |
fix(lint): derive the release gate from the pushed ref, reject multi-token entries
The gate hardcoded HEAD as its diff tip, but pre-commit exports PRE_COMMIT_TO_REF for exactly this. Pushing "somebranch:main" from another checkout diffed the wrong tip — a false negative when HEAD is older, a false positive when newer. Fixing only the diff tip leaves a second bug: git describe took the tag baseline from HEAD too, so a tag reachable only from HEAD becomes a baseline the pushed ref never saw. Both now resolve from the pushed ref, and an all-zeros ref (branch deletion) short-circuits before any rev resolution rather than surfacing as "could not diff". PRE_COMMIT_FROM_REF is deliberately not used: it is the remote's current tip, so diffing from it would let an untagged release-relevant commit already on main excuse the next push from cutting a tag — the drift this gate exists to catch. The baseline must stay the last release tag. collect_release_paths took tokens[0] as a path unconditionally. ADR-0014 makes bare single-path entries a binding constraint, but nothing enforced it, and the sibling .pre-commit-config.yaml already ships "entry: bash <script>". Under that shape add_release_path takes "bash", git diff accepts the non-matching pathspec silently, bundle_root becomes "." and is skipped — the hook's whole surface leaves the gate with no error, the same shape as the --config regression in LESSONS.md. Multi-token entries now fail loudly naming the hook and the ADR, and tokens[0] must resolve at HEAD or at the tag (the union is load-bearing: a per-scope check would reject the deletion cases). Six mutations verified, each restored. One correction worth recording: the first multi-token test passed with its guard removed, because the existence guard caught "bash" and printed a similar message. It now requires the verbatim entry text that only the multi-token diagnostic emits. Refs: #85 ADR: 0014 |
|||
| ad1e5aaa9b |
fix(kyberforge): carry apostrophes verbatim through a |- literal block
The flattener's last-resort branch rewrote ASCII ' to U+2019, justified as the one combination no YAML scalar can carry verbatim. That claim was false: a |- literal block with a single indented content line carries ', ", \ and ": " verbatim and keeps text.frontmatter.description matching — as the wrapper's own docstring already said of literal blocks. The rewrite fired on 12 of 54 in-scope files, silently disabling every rule whose token contains an apostrophe. Case 20 pinned only that the scope stayed alive, so it passed either way. The emission site now splits the emitted scalar on its first newline so a carried-over trailing comment stays on the "description: |-" header line rather than becoming part of the value, and pads by span_lines - 1 - newlines. The pad stays non-negative because the branch is only reachable when the original span is at least two lines. Verified across all 73 in-scope files: no line-count changes, and exactly the 12 expected files take the new branch. One reported position moves: an alert on a description that is itself flagged shifts from the key line to the block's content line, both inside the original span. YAML cannot put a literal block's content on the key's own line, so this is unavoidable; no line at or after the end of any description span moves. Also: --output no longer absolutises the built-in style names line, JSON and CLI, which a same-named file or directory in cwd turned into a template path (exit 2, E100 Runtime error). And case 19's empty-baseline guard no longer lets five dependent comparisons print vacuous passes — while fixing it the guard turned out to be unreachable, since under pipefail an alert-free report aborted the script at the assignment. Refs: #85 ADR: 0014 |
|||
| d25355077f |
fix(lint): attribute Vale alerts per hook and cover .vale.ini in the sync check
The external-consumer test asserted a combined alert count (>=2) across both shipped Vale hooks, but the SKILL.md fixture alone raises two alerts — so one working hook satisfied the threshold. Retargeting agent-audit's glob to match nothing left the suite reporting "3 passed" under the message "both hooks flatten and flag". The Skipped guard does not catch this: the hook still matches the file, Vale lints nothing, reports 0 errors in 1 file and exits 0, which pre-commit renders as Passed. An assertion aggregating over N subjects proves nothing about any individual subject. Each hook now runs individually and its alerts are attributed to the nearest preceding path header, so an alert is checked by path rather than by presence in the combined blob. The two fixtures carry distinct VagueWording tokens, so one hook's alert cannot be credited to another. Nothing in the repo read either .vale.ini — the sync check diffed only vale-wrap.sh and styles/Kyberforge, so a one-line glob typo silently disabled the prefilter for a whole file type. That was the enabling half of the same defect. The check now asserts the shared lines both copies must carry (StylesPath, a section naming Kyberforge as a whole word) without flagging their intentional divergence, and probes each glob section by asking Vale itself to lint a representative path. Regex-to-glob comparison was rejected as it means reimplementing doublestar semantics in bash; a file-count dry-run was rejected because a section whose glob matches but whose BasedOnStyles lost Kyberforge reports "1 file" with no alerts and would pass it. Every new assertion is bound to a failing case in both directions: breaking the artifact fails the suite, and neutering the assertion fails exactly one case. That reverse sweep exposed two assertions bound to no failing case at all, one masked by a stronger check running first. Refs: #85 |
|||
| 4ae2429840 |
fix(kyberforge): align the audit word ceiling and drop the fragile --config
Three divergences between what the audit skills claim and what the hooks enforce, each of which fails silently rather than loudly: - `skill-size-check.sh` blocked at 2900 words while `validate.sh` checked only the 500-line ceiling, so `/skill-audit` could report a skill ready to ship that the commit hook then rejected. `validate.sh` now checks the same pair on the same inclusive terms; the constants are duplicated with a comment naming the other file, because a plugin skill's scripts cannot read outside the plugin directory once installed to the cache. - Both audit skills' Step 1 passed `--config assets/vale/.vale.ini`, which is redundant (the wrapper self-locates its sibling config) and fragile: an agent that resolves the script path against the skill directory but not the config path gets E100, exit 2, which the surrounding fallback clause misreads as "vale unavailable" and downgrades to full LLM judgment with no signal. - The external-consumer test registered only the two Vale hooks, never the third shipped hook, so a lost executable bit would have broken every consumer while the local suite stayed green. Verified by mutation: `chmod 644` on the copied script now turns three passes into two failures. Also corrects the size hook's calibration comment, which claimed ~5.7-6.5 characters per word against a corpus whose measured median is 6.79 — the stated upper bound sat below the median, so the "calibrated with margin" claim was inverted for prose-dense files. MAX_WORDS is unchanged pending a decision; the comment is now explicit that the gate holds under 5,000 tokens for typical prose density, not for any file. Refs: #85 |
|||
| aa8cc22695 |
fix(lint): flatten every multi-line description form in vale-wrap.sh
Vale locates a frontmatter description by matching the parsed YAML value back against the source text, so any scalar whose value is not spelled out verbatim loses the `text.frontmatter.description` scope entirely. The wrapper only flattened `>` folded scalars, so plain, double-quoted and single-quoted multi-line descriptions silently reported zero alerts and exit 0 — a clean pass indistinguishable from a real one, in a prefilter whose callers are instructed not to re-derive its verdict by judgment. Implementation notes: - Classify the scalar kind after `^description:[ \t]*` and reuse one shared continuation-line generator for every form; `|` literal blocks keep their line breaks, stay verbatim-matchable, and are still left untouched. - Emit the flattened value in whichever scalar form needs no escape at all (plain, then single-quoted, then double-quoted), because any escape breaks the verbatim match. The old blanket `'` -> U+2019 substitution silently made apostrophe-bearing rule tokens unmatchable across 63% of the corpus; it now survives only for the one combination no YAML scalar can carry verbatim. - Terminate continuations at a line flush with the key, not only on a shallower indent — a `description:` followed by a flush-left line previously swallowed the rest of the frontmatter. - Route vale's value-taking flags explicitly instead of inferring targets by file existence, and absolutize relative `--output`/`--path` values the way `--config` already was, since the run `cd`s into the scratch mirror. - Fail loudly on a nonexistent path instead of inheriting bare vale's fallback to stdin, which rendered a typo'd path as `0 errors ... in stdin`, exit 0 — a form the callers' `0 files` NOT-RUN guard cannot match. - Follow symlinks when walking a directory argument, matching bare vale. Refs: #85 |
|||
| 16c038b178 |
fix(lint): guard empty array expansions in vale-wrap.sh for bash 3.2
Under set -u, "${arr[@]}" on an empty array aborts on bash before 4.4,
which is what macOS ships as /bin/bash. Three expansion sites now use
${arr[@]+"${arr[@]}"} consistently.
The hazard is not currently reachable: verified on a bash 3.2.57 built
from source that all seven invocation shapes succeed against the
previous code, including zero args, flags-only and an empty directory.
vale_args is provably non-empty at every site because the default
--config branch always appends first. The guard is kept because that
invariant is non-local and untested, so an edit to the default-config
branch would reintroduce a macOS-only crash silently.
Test fidelity is deliberately mixed. Case 16 is static and is the only
one that fails against the previous code, since no bash 5 host can
reproduce the abort at runtime. Case 17 runs the emptiest invocations
under the oldest bash it can find and names that shell in its output
so it cannot overclaim. Case 18 guards against the tempting wrong fix
of dropping the quotes, which also silences the abort but word-splits
a path containing a space.
No other bash 4.x construct is present; swept for mapfile, declare -A,
case modification, negative indices, globstar, wait -n and namerefs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58
|
|||
| 14c2c91521 |
fix(lint): flag release-relevant paths retired since the last tag
Coverage was derived from the worktree alone, so the -d guard on a hook's bundled assets/ tree meant deleting the whole tree removed it from the pathspec instead of flagging it — the gate stayed silent about a change that breaks every consumer at the next rev:. The path set is now derived twice, from the worktree manifest and from the manifest at $LAST_TAG, then unioned. A path the tag exposed but HEAD no longer does is a removal pinned consumers must be told about; a path only HEAD exposes is new contract surface. Both need flagging. Fails closed on an unreadable tagged tree (shallow clone), and treats a readable root tree with no manifest as "added since the tag". tokens[0] needed no exit-code fix — it carries no existence guard, so both deletion cases already exited non-zero. What was wrong was the reporting: a fully retired hook could no longer be named in the failure message. The tagged manifest fixes that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58 |
|||
| 348dd9f665 |
fix(lint): restore release-gate coverage of bundled Vale assets
The gate derived release-relevant paths from the dirname of each entry's --config target. Dropping --config from .pre-commit-hooks.yaml left that loop dead, silently removing both assets/vale/ trees from coverage — a Vale rule change could land on main without demanding a release tag, leaving consumers pinned to an old rev: with stale rules. Coverage now derives from tokens[0] instead: double-dirname for the .. normalization, guarded on the tree existing and on the bundle root not resolving to "." so skill-size-check.sh cannot invent a bogus path. The --config branch is removed rather than kept as dead code. Since pre-commit rewrites only entry[0], no argument in any entry can ever name a file this repo ships, so that shape is broken by design. Known gap: deleting a hook's entire assets/ tree is not flagged, as the candidate path stops existing. Deletions within a surviving tree are. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58 |
|||
| 714e8a0c78 |
fix(lint): fail the style-sync check when one copy is missing
The guard used `||`, so exactly one of the two audit skill directories missing also exited 0, where the intended silent no-op is both absent. A renamed skill-audit reported green instead of flagging that a canonical style copy had lost its counterpart. One-present now exits 1 naming the missing side and the remedy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58 |
|||
| 8c570e9659 |
fix(lint): make the Vale prefilter work for external consumers
pre-commit prefixes only entry[0] with the hook-repo clone path (cmd = (prefix.path(cmd[0]), *cmd[1:])), so the --config argument in .pre-commit-hooks.yaml resolved against the *consuming* repo's root and hard-failed every external run with E100. Two of the three hooks ADR-0014 promises were unusable. vale-wrap.sh now self-locates its config from BASH_SOURCE when no --config is supplied; an explicit --config still wins in all three argv forms and stays cwd-relative. Both manifests drop the argument and are kept byte-identical: the local repo: local config resolved --config correctly only because the consuming repo *was* this repo, and that divergence is why three review rounds missed the defect. Also in the wrapper: - replace GNU-only `realpath -m` with a portable abspath helper; -m is load-bearing (dest does not exist yet), so BSD realpath aborted the script under set -e on macOS - walk directory arguments instead of passing them through unflattened, which reported a clean 0-error run for files that fail when named explicitly - read/write with errors='surrogateescape' so one non-UTF-8 .md under a directory argument cannot abort the hook New test-vale-hooks-consumer.sh builds the hook repo from the working tree and points a file:// consumer at it, covering the manifest as a hook repo for the first time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58 |
|||
| acd2f1d422 |
fix(lint): harden check-release-needed.sh, script the vale-style sync
A review of PR #85's last two commits ( |
|||
| 4d018af03c |
fix(lint): hard-fail on main when a release tag is needed
.pre-commit-hooks.yaml now exposes hooks to external consumers pinning rev: <tag>, but nothing enforced that a tag actually gets cut when the files it references change — relying on memory is exactly what this repo's governance rules say to avoid for a repeatable, deterministic check. scripts/check-release-needed.sh hard-fails at pre-push, but only when PRE_COMMIT_REMOTE_BRANCH (set by pre-commit's hook-impl) is refs/heads/main: it diffs .pre-commit-hooks.yaml's referenced paths against the last tag reachable from HEAD, and fails if either no tag exists yet or something changed since. It's a silent no-op on every other branch — hard-failing on feature-branch pushes mid-review would force a premature tag on a commit that might not survive a squash-merge, the exact risk the repo: local (vs. pinned self- reference) decision in ADR-0014 already avoids for this repo's own dev-time gate. Verified against the real git pre-push hook path (not just the script in isolation): simulated stdin matching git's pre-push protocol through .git/hooks/pre-push, confirmed it correctly fires and fails when targeting main with no tag, and is silent otherwise. ADR: 0014 Refs: #87 |
|||
| 1164f3abad |
fix(lint): make Vale prefilter portable via the plugin
skill-audit/agent-audit's Step 1 resolved vale-wrap.sh/.vale.ini via `git rev-parse --show-toplevel`, which returns whichever repo the skill happens to run in. Inside ai-development that works; in any external repo that installs kyberforge@holocron as a plugin, it resolves to that repo's own root, which has no .vale.ini — the prefilter silently fell back to full LLM judgment. ADR-0013 named this as a deliberately deferred gap. Vale's config/styles/wrapper now ship inside the plugin itself: a canonical copy in agent-audit/assets/vale/ (Kyberforge + KyberforgeCopilot, the superset agent-audit needs) and a smaller duplicate in skill-audit/assets/vale/ (Kyberforge only) — per the no-cross-skill-path rule already established for plugin cache-installs. Both skills resolve these relative to their own directory, same as scripts/validate.sh already does. A new root .pre-commit-hooks.yaml exposes both copies plus skill-size-check so any external repo can enforce the same rules via `repo: <this-repo-url>, rev: <tag>` in its own pre-commit config, independent of Claude Code entirely — the same mechanism covers CI. This repo's own pre-commit hook now consumes the identical plugin-bundled copies via repo: local (not a third root copy, and not a pinned self-reference, which would lint working-tree edits against the last tagged release instead of the change being made). Split into vale-audit-prefilter-skill/-agent hooks after confirming, by diffing the full corpus against both old and new config before deleting the old files, that one combined hook pointed at only one copy silently 0-file- skips the other file type. scripts/check-vale-style-sync.sh guards the two copies against drift, wired at pre-push alongside check-manifests. ADR: 0014 |
|||
| 149d564f6a |
fix(lint): make the Vale gate actually gate, drop VagueQualifier
Round-3 review of PR #85 found the "enforcing" pre-commit hook enforced nothing. Vale's exit code keys on error-level alerts alone: five of the six rules were level: warning, so they exited 0, and pre-commit hides output from a passing hook — the alerts were invisible and blocked nothing. ADR-0013 rejected a report-only trial tier and then shipped one by accident. Flatten every rule to level: error. Vale's own exit code is then correct, so the hook entry drops to a bare vale-wrap.sh call and the graded error->FAIL / warning->SUGGESTION mapping disappears from both audit skills: every alert is a FAIL, in the gate and the audit alike. No ignorable tier, matching shellcheck, the test suite and conventional-pre-commit. Delete Kyberforge.VagueQualifier. Measured against the 41 skill/agent files as they stood before the rule ever ran: 2 hits. One marginal ("very different" -> "fundamentally different"), one an unfixable false positive — caveman/SKILL.md quotes "of course" as an example of filler, a mention not a use — which forced the only Vale suppression comments in the repo. Those four lines go with it; two of them were dead anyway, suppressing a frontmatter-scoped rule on a body line. Held-out prose (273 files) fired 15 times, 9 inside out-of-scope research examples and the rest one word in two idioms in a single doc. SentenceOpenerThereIs survives: 22 held-out hits, both in-corpus hits clean rewrites, zero suppressions. Widen .vale.ini's globs to [**/SKILL.md], [**/agents/*.md] and [**/*.agent.md]. The plugins/*/-prefixed globs scoped nothing — Vale's * crosses /, so they already matched docs/research/examples/**/agents/*.md and assets/templates/SKILL.md, the two paths CONTEXT.md claimed they excluded. Scoping is and was the hook's files: regex. The old globs also hid a silent false negative: a skill outside plugins/ matched no section, so Vale reported 0 files and exited 0, which both audits read as clean. They now treat a 0-file run as NOT RUN and fall back to full judgment. Also: - vale-wrap.sh resolves relative --config values and file arguments against the caller's cwd, as vale does, instead of the repo root, which hard-errored from a subdirectory and silently skipped flattening for file args that did not resolve from the root. Absolute paths inside the cwd are relativized so reports cite resolvable paths, not scratch ones. - vale-run's exit-code model was documented backwards ("exits non-zero whenever it finds an alert at or above MinAlertLevel") and would have led anyone following it to build a gate that passes everything. Its Markdown suppression syntax was MDX-only and does not suppress in .md; corrected in the skill and its troubleshooting reference, with backtick/fence exemption documented as the first resort. - skill-size-check.sh fails only above 500 lines, agreeing with skill-audit's validate.sh <= 500 pass. - ADR-0013 and CONTEXT.md amended to match, recording why graded severities cannot gate. Verified: 9 test scripts / 15 vale-wrap cases pass; vale-audit-prefilter, skill-size-check and shellcheck pass --all-files; check-manifests and claude plugin validate --strict clean. New tests fail against the old script (3 of them) and pass against the new one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58 |
|||
| 792d3e1852 |
fix(lint): resolve round-1 and round-2 review findings on the Vale prefilter
Addresses PR #85's outstanding review items after grilling the open
questions against ADR-0013/CONTEXT.md/ADR-0010:
Blocking fixes:
- vale-wrap.sh: replace json.dumps() escaping (which silently defeated
Vale's frontmatter scope on any description containing a quote,
backslash, or non-ASCII char — ~58% of the corpus) with a single-quoted
YAML scalar, substituting a Unicode right single quote for embedded
apostrophes rather than '' doubling (Vale's frontmatter scanner isn't a
full YAML parser and silently truncates on '' too).
- vale-wrap.sh: fix a blank-line-inside-a-folded-description truncation
bug via indentation-based, blank-line-tolerant body capture; narrow
flattening to `>`-style scalars only (`|` already works unflattened).
- skill-audit/agent-audit Step 1: make the vale-wrap.sh invocation
cwd-independent via git rev-parse --show-toplevel, fixing a bug where
no single cwd satisfied all three Step 1 commands.
- styles/Kyberforge/VagueQualifier.yml: prune 17 tokens verified
false-positive-dominated on this repo's own voice via a real corpus
sweep (obvious, clearly, usually, several, simple, easy, completely,
simply, tiny, etc.), keep 13 with real or unattested noise. Revert the
28 prose "fixes" those tokens drove across 14 skill files back to their
original, correct wording, including a functional regression to
caveman/SKILL.md's own filler-word list (a mention, not a use) — now
guarded with vale-off comments against recurrence.
Gaps:
- --minAlertLevel=warning on the pre-commit hook and Step 1 invocation
so warning-level rules actually surface, without collapsing the
FAIL/SUGGESTION severity mapping skill-audit/agent-audit rely on.
- vale-wrap.sh: fix --config=<path> equals-form, absolute-path silent
no-op, and a zero-file-argument stdin hang.
- Route vale-run and lint-runner through a documented wrapper script
when a target repo has one, instead of unconditionally recommending
bare `vale`.
- Wire Kyberforge.VagueQualifier/SentenceOpenerThereIs into skill-audit/
agent-audit's dimension-mapping prose (Body discipline).
- Add plugins/lint/sources.md provenance for lint-runner (ADR-0010).
- Sync both marketplace.json lint-entry descriptions with plugin.json.
- Retune skill-size-check.sh's MAX_WORDS 5000->2900 (measured ~1.6-1.7
tokens/word on this repo's corpus, the old value gated at ~8,500
tokens against a stated 5,000 ceiling); fix the >/>= line-count
boundary and wc -l undercount on files with no trailing newline.
- Document the vale binary as a Setup prerequisite in AGENTS.md.
- Fix SentenceOpenerThereIs's dead regex alternative and add a real
sentence-start anchor/scope.
- Fix a stale docs/research/docs/vale/ index pointer in kyberforge's
docs README (moved to plugins/lint/ in
|
