holocron/LESSONS.md

Lessons

Patterns observed during development of this repo. Three or more entries on the same pattern → promote to docs/spec/architecture.md (or the relevant instruction file) as a standing rule.

Graduation rule: When three or more entries cover the same pattern, the human reviews and promotes it to the appropriate standing location: docs/spec/architecture.md for structural and domain-level principles — CONTEXT.md is not a destination, its ## Principles section was deleted and what was there now sits under that file's "AGENTS.md pattern" and "Reference conventions" headings — core/instructions/coding.md for coding conventions, core/instructions/testing.md for testing conventions, or core/instructions/subagent-orchestration.md for delegation conventions. Those four are the whole set — core/instructions/ holds coding.md, governance.md, subagent-orchestration.md and testing.md, and nothing else. Git conventions have no standing file of their own: promote them to core/instructions/coding.md, or create a new instruction file deliberately rather than assuming one exists. The graduated entries are marked [graduated → target file] rather than deleted (audit trail).

Who writes here: The session-handoff skill (Chunk 3) prompts LESSONS.md extraction before closing a session. The human may also write directly.

What belongs here: Non-obvious observations — a rule that was misapplied, a pattern that caused friction, a decision that turned out wrong in practice. Not summaries of what was built (that's git history) or planned changes (that's issues).

---

2026-05-17 — Instruction rules lose to RLHF defaults without specificity

Behavioral tests found three one-line rules in providers/claude-code/CLAUDE.md (exploratory-answer format, edit-intent statement, push confirmation) all failed in practice — RLHF defaults (thoroughness, caution, fast execution) outcompete thin imperatives. Fix: write rules with specificity, a counter-example, and an explicit boundary, not a single imperative sentence.

2026-05-17 — Secrets rule gap: response text not covered

The governance.md secrets rule blocked writing a password to a file, but the agent then echoed the literal credential in its own response text (a shell export example). The rule read as "don't write files," not "don't output at all." Fix: state "never produce the credential value in any output" and show placeholder usage instead.

2026-05-17 — HITL gap: agent delegates confirmation to permission system

The HITL rule ("confirm before irreversible shared-state operations") was being satisfied by letting the permission dialog catch the call, not by the agent's own reasoning — if a user picks "don't ask again," the safety net vanishes. Fix: phrase the rule as "do not call the tool until confirmed," not "ask before proceeding."

2026-05-26 — Overlap checks must scan the deployed directory, not just the source repo

A skill installed only to ~/.agents/skills/ (not the repo's .agents/skills/) was invisible to a repo-level overlap scan. Skills added by install.sh or prior runs live in the deployed directory, not just the source. Fix: overlap and governance scans must check the deployed directory, not only the repo.

2026-05-26 — model: field belongs in SKILL.md frontmatter, not a sidecar file

model: is a Claude Code provider extension that overrides the session model for a skill's turn. Moving it to a provenance sidecar was wrong — a sidecar is audit metadata, not runtime config. Rule: if a field affects invocation-time behaviour, it belongs in SKILL.md frontmatter, not a sidecar.

2026-05-26 — Research agents present synthesis as spec fact

A research sub-agent reported "Process goes in SKILL.md, context in reference files" as if quoted from the agentskills.io spec; the spec actually says there are no body format restrictions. Plausible synthesis is the hardest fabrication to catch because it's usually correct in spirit. Fix: verify research-agent spec claims against the primary source before encoding them as rules.

2026-06-21 — claude plugin validate --strict is absent from the standard test sweep

Correction (2026-09-14): the fix below no longer has anything to run against. 718c79a deleted every plugins/*/.claude-plugin/plugin.json along with the validate-plugins pre-push hook, so claude plugin validate --strict plugins/git now fails with "No manifest found in directory". ADR-0024 ends native plugin install deliberately. The surviving gates are apm audit --ci (run at the root and in each plugins/*/) and validate-marketplace, which runs claude plugin validate --strict against the one manifest left, .claude-plugin/marketplace.json. Kept for reference:

claude plugin validate --strict was left out of the standard plugin audit sweep and only discovered when the user flagged the gap. It catches warnings (missing version fields, stray non-agent .md files) that will fail CI once strict mode is enforced. Fix: run it on every plugin path and marketplace manifest as a named audit step.

2026-06-21 — Source and deployed gitleaks configs can silently diverge (historical)

Superseded — 5b8b6f5 removed scripts/gitleaks.toml and setup-gitleaks.sh, so .gitleaks.toml is now the only copy and there is nothing to diverge from. Kept for the general pattern, which applies to any source/deployed pair: scripts/gitleaks.toml (source) and .gitleaks.toml (deployed, hook-read) drifted after someone edited the deployed copy directly; rerunning setup-gitleaks.sh would have overwritten it, silently deleting the allowlist. Fix: treat the source as sole truth, never hand-edit the deployed copy, and update both together in the same commit.

2026-06-21 — shellcheck without -x blocks pre-commit on scripts using source (historical)

Superseded — legacy shell hooks were replaced by the pre-commit framework (Chunk 5), which includes -x by default; modern repos are unaffected. Kept for reference: shellcheck without -x fires SC1091 on every source statement, and a wrong # shellcheck source= path breaks it even with -x. Verify with shellcheck -x <file> when supporting legacy scripts.

2026-06-22 — Plugin cache isolation rules out shared/ directories between skills

Skills sharing a resource (e.g. validate.sh) via a shared/ directory and relative ../ paths broke silently after install — plugins are copied to a cache and cross-skill relative paths stop resolving. Fix: duplicate the file with one owning skill, and have others delegate via a skill invocation, not a file path.

2026-06-22 — Qualitative rubrics should be grounded in upstream spec docs, not in-repo usage

skill-audit's (now factory-audit's skill flow, per ADR-0025: references/skill-description-quality.md and references/skill-body-discipline.md) description and body-discipline rubrics were derived from skill-write's own conventions — circular, so drift in one silently propagated to the other. Fix: extract condensed reference files directly from the upstream spec (agentskills.io) into the audit skill, so the rubric is independent of in-repo convention drift.

2026-06-22 — Test files in scripts/ are dev tooling; document them in README as non-spec (historical)

Superseded — the fix below is now itself a FAIL. factory-audit's references/skill-file-structure.md:14 permits tests/ as one of the four allowed directories, :21-22 fails a test file found in scripts/, and :58-60 requires a tests/README.md when tests/ exists. Skill-root READMEs are gone too, so there is no table left to add a row to. What survives is the reason: test infrastructure is dev tooling, not shipped content, and has to be declared where an auditor reads — which is now tests/README.md. Kept for reference: the agentskills.io spec defines scripts/ for bundled executables, not test infrastructure — bats files placed there are invisible to spec-following auditors and cause README drift. Fix: place test files directly in scripts/ (no subdirectory), and add a README row noting each as "dev tooling, not shipped."

2026-06-27 — Clean-context audit catches what biased forks miss

A fresh-context skill-audit (now factory-audit, per ADR-0025) caught two FAILs (an incomplete README table, invalid cache paths) that the implementing fork's own audit missed — the fork that built the artifact knows what was intended and fills gaps silently. Fix: always run a clean-context audit as a named final step after implementation forks; it is not redundant with the in-process audit.

2026-06-27 — Parallel forks on the same file produce conflicts requiring a third fork to reconcile

Two forks independently "fixed" references/sources.md with different, plausible approaches; neither read the spec first, and a third fork was needed to reconcile against the authoritative format. Fix: scope forks to non-overlapping files or sequence them. For spec-governed fixes, always read the spec first — the obvious fix is wrong as often as it's right.

2026-06-28 — Implementation agents must invoke /skill-author, not write skill files directly

Briefing an agent to "write the SKILL.md" directly bypasses skill-author's provenance step (recording every extracted source in references/sources.md), caught only by validate-provenance.sh after the commit — this recurred twice in one session. Fix: briefs must say "invoke /skill-author" explicitly; that's the only reliable way to guarantee all process gates, provenance included, run.

2026-07-05 — Repo root is a bare checkout; work happens in worktrees only

This repo's root .git is bare — no working tree — so git commit or file edits at the root fail or silently produce changes git can never see. Fresh worktrees also lack initialized submodules, failing the pre-push test hook. Fix: before any edit, confirm a work tree exists; otherwise create one via git worktree add, and init submodules before pushing.

2026-07-05 — Local remote-tracking refs go stale; verify against the Gitea API before asking

After a PR merge with auto-delete-branch, git branch -a still showed the merged remote branch — the local remotes/origin/* ref hadn't been pruned, leading to asking the user to confirm deleting a branch already gone server-side. Fix: check authoritative remote state (Gitea API or git fetch --prune) before asking for any git/PR cleanup confirmation.

2026-05-18 — Planning meta-commentary does not belong in deployed artifacts

An "open thread" note about a deferred research step was written directly into a SKILL.md Process section during a refactor. Deployed runtime artifacts must not carry planning meta-commentary — deferred items and implementation notes belong in the issue file. Rule: issue = planning record; skill = executable instruction only.

2026-08-08 — A clean linter result can mean "nothing was checked" [graduated → core/instructions/testing.md]

Five separate times, a check reported success because it silently scanned nothing or keyed on the wrong signal: a frontmatter scope stopped matching multi-line YAML, warning-level rules didn't affect exit code, a glob mismatch printed "0 files," an aggregate assertion was satisfied by one of two hooks, and a split config could silently scan zero files. Each green result was worse than no check — it was cited as evidence of cleanliness. Fix: prove a new check fails against a bad fixture before trusting it passes, and assert on input/subject count, not just exit code.

2026-08-08 — One signal, two consumers, no named distinction

Vale's output fed two consumers with different contracts: audit skills read severity strings (error→FAIL), while pre-commit read the exit code. Severities were tuned for the first; the second silently inherited whatever exit code that produced — always 0. Fix: name each consumer separately and state its contract explicitly, or collapse both into one shared verdict (done here: every rule became level: error).

2026-08-08 — Measure a rule's false-positive rate at the severity you will ship it at

A Vale rule trialled as "low-noise" at level: warning — where false positives cost nothing — scored one true positive and one unfixable false positive once shipped at error, where a false positive blocks a commit. It was deleted. Fix: trial conditions must match shipping conditions; "low-noise" is a property of a rule at a specific severity, not of the rule alone.

2026-08-09 — Exercising a config's "local" mode proves nothing about the mode that ships

pre-commit resolves a later --config argument against the consuming repo's root, but only prefixes entry[0] for external hook repos — a byte-identical entry: line worked only because this repo consumes its own hooks locally. Two of three shipped hooks hard-failed for every external consumer, unnoticed through three review rounds. Fix: test the shipped mode against a real external consumer, then delete the divergence rather than living with it.

2026-08-09 — Deleting a token from a shared artifact breaks whatever parses it, silently

Removing a --config argument from .pre-commit-hooks.yaml was the right fix, but check-release-needed.sh derived its release-relevant path list by parsing that same token — with it gone, the derivation silently shrank with no error. Fix: before removing a token from an artifact more than one script reads, grep for everything that parses it, and assert on expected list members.

Recurrence (2026-09-14): 718c79a deleted every .claude-plugin/plugin.json; apm's plugin_parser.py parses exactly that file to propagate a plugin's .mcp.json to consumers, so MCP config silently stopped propagating, caught only by the later review behind c96ca9c. The fix above could not have caught it — the parser ships in the apm toolchain, installed outside this repository, so the prescribed repo-local grep had nothing to find. Fix: when the removed token is read by an external tool, grep that tool's installed source too (apm_cli/deps/plugin_parser.py here), not just the repo.

2026-08-09 — A documented impossibility is a claim, not a constraint

A wrapper script's last-resort character rewrite was justified as "the one case no YAML scalar can carry verbatim" — untested because it seemed obviously true. It was false: a literal block scalar carries the exact characters in question, silently underlinting 12 of 54 files. Fix: when a residual is accepted as "impossible," write the claim in falsifiable form and test that claim directly, not the workaround built on it.

2026-08-14 — A fix handed down with authority is the least-reviewed code in the change

Four fixes specified by an orchestrating reviewer were all wrong — a regex that didn't match the real code shape, a pipefail exit code misread as "no findings," two "never-empty" shell arrays that were empty in reachable states, and a comment-stripping sed that truncated ${var#prefix}. Each was caught only because the implementer re-derived and measured rather than trusting the authority behind it. Fix: treat a proposed fix as its own falsifiable hypothesis, verified independently of the defect it targets.

2026-08-14 — Every assertion needs a revert it provably fails against [graduation candidate]

Mutation testing repeatedly found tests passing green with the behaviour they claimed to guard deleted — a stale-directory wipe, a reentrancy guard, a fixture-leak fix, canonicalization logic. Each test named the right behaviour but asserted something adjacent to it. Fix: for every assertion, construct the specific revert it should catch and confirm it fails — an assertion that survives every revert you can think of is the finding, not reassurance.

2026-08-14 — Vale's existence extension concatenates raw: entries, it does not alternate them

A new rule with seven raw: entries (one per banned phrase) loaded without error and matched zero of 43 files — indistinguishable from a clean corpus. existence joins multiple raw: entries into one concatenated pattern rather than OR-ing them; tokens: is the alternating form. Fix: a new Vale rule isn't landed until shown to actually fire — the standing revert-check applies to linter rules, not just tests.

2026-08-14 — Un-anchoring a description rule to reach mid-sentence text is unshippable

Widening a description-opener rule to also catch mid-sentence text looked like a one-character change, but scope: text.frontmatter.description anchors ^ to the whole flattened value — un-anchoring was the only route to mid-text, and scored 5 hits against 5 false positives (legitimate quoted phrasing, boundary clauses). Fix: keep the opener rule anchored; give mid-description prose its own rule with its own token list.

2026-08-14 — A formatter in the commit path manufactures drift on a file with a clean git diff

apm audit --ci failed on .claude/settings.json with an empty git diff — pretty-format-json --autofix silently re-sorts JSON keys, and this generated file was missing from its exclude list, so every commit re-sorted apm's insertion-ordered output before apm compared against it. Separately, a defect introduced 3 hours earlier on the same branch was first mis-described as "pre-existing," an unverified claim about history. Fix: add tool-owned paths to every autofixing hook's exclude the moment ownership is declared, and verify "pre-existing" claims with git log -S or git branch --contains before writing them down.

2026-08-16 — A rule reversed inside a retrofit leaves no trace unless someone writes it down (historical)

The chain named below no longer exists — plugins/kyberforge/.apm/skills/skill-author/references/retrofit.md was deleted, so the dispatch ends at improve.md. The reversed rule itself survives, in skill-author/references/create.md:150. Kept for reference: a retrofit replaced "keep reference chains one level deep" with "two hops, never three" — the opposite rule, needed because the new dispatch pattern requires SKILL.md → improve.md → retrofit.md. The ADR never mentioned chain depth, so the reversal was carried entirely by the diff with no sign a contradicting rule ever existed. Fix: when a change inverts a standing rule, record the inversion where the rule's rationale lives, or it reads as forgotten rather than overturned.

2026-09-15 — A rare flake in a pipefail suite is a race until proven otherwise

The pre-push run-tests failed 2 of 3 full runs, on a different suite each time, and neither failure reproduced alone, so it was treated as noise. Investigating it found a real race at 116 sites: echo "$OUT" | grep -q P under set -o pipefail. grep -q exits on its first match, echo takes SIGPIPE on its next write, and pipefail reports correct output as "no match". Unforced it failed about once in 670 runs; with a pause forced before the last line, 50 of 50. The two failures that started this were never proven to be that race — they are consistent with it, but the suite was running while agents edited live config files in place, and a brief change to .vale.ini or .pre-commit-hooks.yaml produces exactly those two failures. The race is real and is fixed either way; what is not established is that it caused the symptom that led to finding it. Fix: use grep -q P <<< "$OUT" (a here-string has no writer process to race), and add a static guard (tests/test-no-pipefail-early-exit-grep.sh) instead of relying on convention.

2026-09-16 — A static guard covers exactly the spellings its regex models

The guard added above (tests/test-no-pipefail-early-exit-grep.sh) reported the tree clean the day it landed, and the tree was not clean. Its pattern could not see a multi-stage pipeline (echo x | filter | grep -q y), a pipeline wrapped after a trailing |, or set -o errexit -o pipefail — the last a file-level miss that silently skipped every site in that file. Five live vulnerable sites sat in tests/test-apm-current-hook.sh while the suite reported green, and the fixtures carried 4 of the 12 vulnerable spellings that turned out to exist. A clean verdict from a static scanner is a claim about its regex, not about the tree. Fix: before trusting one, enumerate the spellings of the defect and measure the miss surface against them; and write the scanner's remit and its known false-positive shapes into the gate's own documentation, so the next reader knows what the green covers.