From 55a9d1b9fadc830ca844d1284f212746a20106a3 Mon Sep 17 00:00:00 2001 From: marquesds Date: Tue, 30 Jun 2026 10:07:47 -0300 Subject: [PATCH] chore(skills): remove caveman skill Agents kept loading a persona-style response skill even after the harness moved to concise normal prose, and existing installs could keep stale managed copies after reinstall. Remove the skill, catalog route, and routing fixture; document the decision in ADR-0013; and delete obsolete managed per-file outputs during normal install so old native skill files do not linger. AI assistance used; reviewed, tested, and verified with temp-root install proof. --- .ai-ack | 1 + ...ncise-technical-prose-for-agent-sources.md | 2 +- .../0013-remove-optional-response-skill.md | 40 ++++++ docs/contributing.md | 4 +- docs/style-overview.md | 9 +- scripts/adapters/__init__.py | 2 + scripts/adapters/base.py | 12 +- scripts/adapters/claude.py | 6 + scripts/adapters/codex.py | 5 + scripts/adapters/cursor.py | 6 + scripts/adapters/opencode.py | 6 + scripts/adapters/vibe.py | 6 + scripts/lint_source.py | 2 +- source/evals/skill-routing.yml | 2 - source/rules/skills-catalog.md | 1 - source/skills/caveman.md | 133 ------------------ tests/test_adapters_per_agent.py | 24 +++- 17 files changed, 114 insertions(+), 147 deletions(-) create mode 100644 .ai-ack create mode 100644 docs/adr/0013-remove-optional-response-skill.md delete mode 100644 source/skills/caveman.md diff --git a/.ai-ack b/.ai-ack new file mode 100644 index 0000000..41b9886 --- /dev/null +++ b/.ai-ack @@ -0,0 +1 @@ +I used AI assistance and reviewed the generated changes before submission. diff --git a/docs/adr/0012-concise-technical-prose-for-agent-sources.md b/docs/adr/0012-concise-technical-prose-for-agent-sources.md index f867114..c920533 100644 --- a/docs/adr/0012-concise-technical-prose-for-agent-sources.md +++ b/docs/adr/0012-concise-technical-prose-for-agent-sources.md @@ -2,7 +2,7 @@ Date: 2026-05-19 -Status: Accepted +Status: Superseded by [ADR-0013](0013-remove-optional-response-skill.md) Supersedes: [ADR-0003](0003-caveman-voice-for-rules.md) diff --git a/docs/adr/0013-remove-optional-response-skill.md b/docs/adr/0013-remove-optional-response-skill.md new file mode 100644 index 0000000..a58b683 --- /dev/null +++ b/docs/adr/0013-remove-optional-response-skill.md @@ -0,0 +1,40 @@ +# ADR-0013: Remove optional caveman skill + +Date: 2026-06-30 + +Status: Accepted + +Supersedes: [ADR-0012](0012-concise-technical-prose-for-agent-sources.md) + +## Context + +ADR-0012 moved source authoring to concise normal prose but left the optional +`caveman` skill installed for users who explicitly requested a terse response +mode. That optional skill still created an active route in the skills catalog and +native skill outputs. + +The harness goal is now narrower: install engineering practices, not response +personas. Concise prose remains a source-writing rule, but style-mode state is +better left to each agent or user session. + +## Decision + +Remove the `caveman` skill from `source/skills/`, the skills catalog, and routing +fixtures. Generated agent outputs must not install or advertise a `caveman` +skill. + +Historical ADRs remain unchanged except for status links. They record the path +from compressed source prose to concise normal prose, then to removing the +optional skill. + +## Consequences + +**Positive.** Installed harness output no longer activates or advertises a +persona-style skill. Skill routing stays focused on engineering tasks. + +**Negative.** Users who liked the packaged terse response mode must configure it +outside the harness. + +**Mitigation.** Keep concise technical prose in source files and generated rules. +If many users ask for the removed skill again, restore it through a new ADR and a +normal skill/catalog/eval change. diff --git a/docs/contributing.md b/docs/contributing.md index 534f6dd..c1dde7d 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -58,7 +58,9 @@ Implement `scripts/adapters/.py` with a class exposing `name: str` and `w ## Voice -Source files use concise technical prose. Keep them short, but do not force caveman grammar. README, this file, `docs/style-overview.md`, and ADRs also use normal prose so contributors have one writing style across the repo. +Source files use concise technical prose. Keep them short and use normal grammar +where it improves clarity. README, this file, `docs/style-overview.md`, and ADRs +use the same prose style so contributors have one writing style across the repo. ## ADRs diff --git a/docs/style-overview.md b/docs/style-overview.md index 3cabb95..97170ea 100644 --- a/docs/style-overview.md +++ b/docs/style-overview.md @@ -83,10 +83,7 @@ in front of you. ## Why concise prose Once an agent has the practices in context, every extra token of preamble costs -money and adds noise. The harness still removes filler, hedging, and throat-clearing, -but it no longer requires full caveman grammar. Concise normal prose keeps the token -budget low while preserving sequence, danger, and ownership where those details -matter. +money and adds noise. The harness removes filler, hedging, and throat-clearing +while keeping normal grammar where sequence, danger, and ownership matter. -See `docs/adr/0012-concise-technical-prose-for-agent-sources.md` for the current -decision. ADR-0003 records the earlier caveman default. +See `docs/adr/0013-remove-optional-response-skill.md` for the current decision. diff --git a/scripts/adapters/__init__.py b/scripts/adapters/__init__.py index 5751478..2898f69 100644 --- a/scripts/adapters/__init__.py +++ b/scripts/adapters/__init__.py @@ -8,6 +8,7 @@ WriteOp, apply_op, replace_managed_section, + stale_managed_delete_ops, strip_managed_section, walk_managed_files, ) @@ -38,6 +39,7 @@ "WriteOp", "apply_op", "replace_managed_section", + "stale_managed_delete_ops", "strip_managed_section", "walk_managed_files", ] diff --git a/scripts/adapters/base.py b/scripts/adapters/base.py index b6b4f72..0299864 100644 --- a/scripts/adapters/base.py +++ b/scripts/adapters/base.py @@ -6,7 +6,7 @@ from __future__ import annotations import contextlib -from collections.abc import Iterator +from collections.abc import Iterable, Iterator from dataclasses import dataclass, field from pathlib import Path from typing import Protocol @@ -145,3 +145,13 @@ def walk_managed_files(directory: Path) -> Iterator[Path]: yield path except OSError: continue + + +def stale_managed_delete_ops(directory: Path, keep_paths: Iterable[Path]) -> list[WriteOp]: + """Delete managed files under `directory` that current install no longer writes.""" + keep = set(keep_paths) + return [ + WriteOp(path=path, content="", action="delete") + for path in walk_managed_files(directory) + if path not in keep + ] diff --git a/scripts/adapters/claude.py b/scripts/adapters/claude.py index d113eda..7190f6b 100644 --- a/scripts/adapters/claude.py +++ b/scripts/adapters/claude.py @@ -17,6 +17,7 @@ render_command_markdown, replace_managed_section, skill_bundle_ops, + stale_managed_delete_ops, strip_managed_section, walk_managed_files, ) @@ -48,6 +49,11 @@ def write_all( if rules: report.add(self._claude_md_op(rules, root)) + keep = [op.path for op in report.ops] + for directory in (root / "skills", root / "commands"): + for op in stale_managed_delete_ops(directory, keep): + report.add(op) + if not dry_run: for op in report.ops: apply_op(op) diff --git a/scripts/adapters/codex.py b/scripts/adapters/codex.py index 50aaffe..a95bd74 100644 --- a/scripts/adapters/codex.py +++ b/scripts/adapters/codex.py @@ -18,6 +18,7 @@ apply_op, replace_managed_section, skill_bundle_ops, + stale_managed_delete_ops, strip_managed_section, walk_managed_files, ) @@ -90,6 +91,10 @@ def write_all( prune_merged_agents_md(target_root / "AGENTS.md", report) + keep = [op.path for op in report.ops] + for op in stale_managed_delete_ops(target_root / ".agents" / "skills", keep): + report.add(op) + if not dry_run: for op in report.ops: apply_op(op) diff --git a/scripts/adapters/cursor.py b/scripts/adapters/cursor.py index 8ce47cc..eef804b 100644 --- a/scripts/adapters/cursor.py +++ b/scripts/adapters/cursor.py @@ -18,6 +18,7 @@ apply_op, render_command_markdown, skill_bundle_ops, + stale_managed_delete_ops, walk_managed_files, ) from scripts.source import Source @@ -46,6 +47,11 @@ def write_all( elif s.kind == "command": report.add(self._command_op(s, root)) + keep = [op.path for op in report.ops] + for directory in (root / "rules", root / "skills", root / "commands"): + for op in stale_managed_delete_ops(directory, keep): + report.add(op) + if not dry_run: for op in report.ops: apply_op(op) diff --git a/scripts/adapters/opencode.py b/scripts/adapters/opencode.py index 7bb03d5..ecbeb8e 100644 --- a/scripts/adapters/opencode.py +++ b/scripts/adapters/opencode.py @@ -17,6 +17,7 @@ apply_op, replace_managed_section, skill_bundle_ops, + stale_managed_delete_ops, strip_managed_section, walk_managed_files, ) @@ -48,6 +49,11 @@ def write_all( if rules: report.add(self._agents_md_op(rules, root)) + keep = [op.path for op in report.ops] + for directory in (root / "skills", root / "commands"): + for op in stale_managed_delete_ops(directory, keep): + report.add(op) + if not dry_run: for op in report.ops: apply_op(op) diff --git a/scripts/adapters/vibe.py b/scripts/adapters/vibe.py index a62e697..8c87273 100644 --- a/scripts/adapters/vibe.py +++ b/scripts/adapters/vibe.py @@ -19,6 +19,7 @@ WriteOp, apply_op, render_aux_markdown, + stale_managed_delete_ops, walk_managed_files, ) from scripts.source import Source @@ -72,6 +73,11 @@ def write_all( for op in _vibe_ops(s, root / subdir): report.add(op) + keep = [op.path for op in report.ops] + for subdir in ("rules", "skills", "commands"): + for op in stale_managed_delete_ops(root / subdir, keep): + report.add(op) + if not dry_run: for op in report.ops: apply_op(op) diff --git a/scripts/lint_source.py b/scripts/lint_source.py index f8279cf..50bf3ea 100644 --- a/scripts/lint_source.py +++ b/scripts/lint_source.py @@ -11,7 +11,7 @@ - Skill bundles: kind skill, id matches directory, aux files plain markdown (see scripts/lint_bundle.py). -Caveman quality is NOT linted — that is a review concern. +Prose quality is NOT linted — that is a review concern. """ from __future__ import annotations diff --git a/source/evals/skill-routing.yml b/source/evals/skill-routing.yml index 45e6d03..68c26c5 100644 --- a/source/evals/skill-routing.yml +++ b/source/evals/skill-routing.yml @@ -27,8 +27,6 @@ cases: load: - data-privacy-and-retention - threat-modeling - do_not_load: - - caveman - id: slow-query prompt: This LEFT JOIN on orders is taking 3s; suggest indexes. diff --git a/source/rules/skills-catalog.md b/source/rules/skills-catalog.md index 7fee109..a35566e 100644 --- a/source/rules/skills-catalog.md +++ b/source/rules/skills-catalog.md @@ -49,7 +49,6 @@ guidance intact. Exception: skills whose topic is the language or format itself | capacity-and-cost-engineering | sizing infrastructure for launch; planning capacity for peak load; setting up cost budgets or guardrails; running a load test | | canonical-reference-in-docstrings | documenting an algorithm, protocol, or formula | | ci-pipeline-design | designing or improving a CI/CD pipeline; setting up or modifying GitHub Actions or CI configuration | -| caveman | optional terse response style when the user explicitly asks for caveman mode or extreme brevity | | centralized-ui-components | building UI components; enforcing one catalog source before page use | | code-review-and-quality | reviewing a diff or PR across six axes | | code-simplification | reducing complexity in tested code, one change at a time | diff --git a/source/skills/caveman.md b/source/skills/caveman.md deleted file mode 100644 index 65b902c..0000000 --- a/source/skills/caveman.md +++ /dev/null @@ -1,133 +0,0 @@ ---- -id: caveman -kind: skill -title: Caveman Voice -description: > - Optional terse response style for users who explicitly ask for caveman mode or - extreme brevity. Technical terms, code, and error strings stay exact. -applies_when: - - user says "caveman mode" / "talk like caveman" / "less tokens" / "be brief" - - compress agent prose for token efficiency -agents: - claude: { kind: skill } - cursor: { kind: skill } - codex: { section: skills } - goose: { section: skills } - openclaw: { section: skills } - opencode: { kind: skill } - pi: { section: skills } - vibe: { kind: skill } ---- - -# Caveman Voice - -Use this only as an optional response style when the user asks for it. Do not use it -as the default authoring voice for rules, skills, commands, README files, ADRs, PRs, -commits, or security guidance. - -## Persistence - -Caveman mode stays active after the user asks for it. Keep using the selected level -until the user says "stop caveman", "normal mode", or asks for more clarity. - -Default level: **full**. Switch with `/caveman lite|full|ultra`. - -## Rules - -Reduce: - -- Filler: `just`, `really`, `basically`, `actually`, `simply`. -- Pleasantries: `sure`, `certainly`, `of course`, `happy to`. -- Hedging: `maybe we could`, `it might be`, `probably`. -- Articles (`a`, `an`, `the`) only when removal does not create ambiguity. - -Keep: - -- Technical terms exact. -- Code blocks unchanged. -- Error strings quoted exact. -- Destructive-action warnings clear enough for a human to act safely. - -Useful pattern: `[thing] [action] [reason]. [next step].` - -## Intensity Levels - -| Level | What changes | -|---|---| -| **lite** | Remove filler and hedging. Keep articles and full sentences. | -| **full** | Use short fragments and remove articles when safe. | -| **ultra** | Abbreviate prose words (`DB`, `auth`, `config`, `req`, `res`, `fn`, `impl`). Use arrows for causality. Never abbreviate code symbols, function names, API names, or error strings. | - -### Sample — "Why LiveView component re-render?" - -- **lite**: "Your component re-renders because assigns changed. Only changed assigns trigger re-render. Use `assign_new` for expensive computations." -- **full**: "Assigns changed → re-render. Only changed assigns trigger it. Use `assign_new` for expensive stuff." -- **ultra**: "Assigns change → re-render. `assign_new` for expensive comp." - -### Sample — "Explain Ecto preloading" - -- **lite**: "Preloading fetches associated records in a separate query. Use `Repo.preload` after the main query, or `preload:` in the query itself to avoid N+1." -- **full**: "Preload = fetch assocs in separate query. `Repo.preload` after query or `preload:` in query. Avoid N+1." -- **ultra**: "Preload fetch assocs. `Repo.preload` or `preload:` in query. Kill N+1." - -## Auto-Clarity - -Drop caveman grammar when ambiguity has cost: - -- Security warnings. -- Irreversible action confirmations. -- Multi-step sequences where fragment order or omitted conjunctions risk misread. -- Compression itself creates technical ambiguity. -- User asks to clarify or repeats question. - -Resume caveman after the clear part is complete. - -```text -Warning: this will permanently delete all rows in `users` and cannot be undone. - -DROP TABLE users; - -Caveman resume. Verify backup first. -``` - -## Boundaries - -- Rules, skills, commands, README files, docs, ADRs, code, commits, and PRs: write normal concise prose. -- Identifiers, error strings, API names: verbatim. -- "stop caveman" / "normal mode": revert immediately. - -## GOOD - -Caveman mode used only after an explicit user request: - -```text -Bug in auth middleware. Token expiry check uses `<` not `<=`. Fix: change -the comparison in `verify_token` line 42. Test: -``` - -```python -def test_expiry_inclusive_at_boundary(): - token = make_token(expires_at=now()) - assert verify_token(token, at=now()) is True -``` - -Pattern matches: `[thing] [action] [reason]. [next step].` - -## BAD - -Using caveman as the default for safety-sensitive or normal project writing: - -```text -Destructive op. Deletes prod rows. Backup first. -``` - -This is too compressed for a destructive operation. Use normal prose that names -the blast radius and rollback path. - -## Red Flags - -- Caveman mode activates without a user request. -- Source files, ADRs, PRs, or commits are written in caveman grammar. -- Security, compliance, migration, or destructive-operation guidance is compressed - enough to hide sequence or blast radius. -- Caveman applied inside a code block, an error string, or an identifier. diff --git a/tests/test_adapters_per_agent.py b/tests/test_adapters_per_agent.py index 051eb18..8b878f3 100644 --- a/tests/test_adapters_per_agent.py +++ b/tests/test_adapters_per_agent.py @@ -2,10 +2,32 @@ from pathlib import Path +import pytest + from scripts.adapters import ADAPTERS -from scripts.adapters.base import BEGIN_MARKER, END_MARKER +from scripts.adapters.base import BEGIN_MARKER, END_MARKER, FILE_MARKER_HTML from scripts.source import load_all +STALE_SKILL_PATHS = [ + ("claude", ".claude/skills/caveman/SKILL.md"), + ("codex", ".agents/skills/caveman/SKILL.md"), + ("cursor", ".cursor/skills/caveman/SKILL.md"), + ("opencode", ".config/opencode/skills/caveman/SKILL.md"), + ("vibe", ".vibe/skills/caveman.md"), +] + + +@pytest.mark.parametrize(("agent_name", "rel_path"), STALE_SKILL_PATHS) +def test_install_deletes_obsolete_managed_skill( + agent_name: str, rel_path: str, fake_source_dir: Path, tmp_path: Path +) -> None: + out = tmp_path / "out" + stale = out / rel_path + stale.parent.mkdir(parents=True) + stale.write_text(f"{FILE_MARKER_HTML}\nold skill\n", encoding="utf-8") + ADAPTERS[agent_name].write_all(load_all(fake_source_dir), target_root=out, dry_run=False) + assert not stale.exists() + def test_claude_writes_skill_dir(fake_source_dir: Path, tmp_path: Path) -> None: out = tmp_path / "out"