diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..97901c5 --- /dev/null +++ b/.gitignore @@ -0,0 +1,8 @@ +# Vim swap files +*.swp +*.swo +*~ + +# OS files +.DS_Store +Thumbs.db diff --git a/.reflection-state.json b/.reflection-state.json new file mode 100644 index 0000000..5858405 --- /dev/null +++ b/.reflection-state.json @@ -0,0 +1,9 @@ +{ + "version": 1, + "last_run": "2026-03-15T09:25:01Z", + "processed": { + "log/2026-03-12.233744.md": "ca482f03914a3b4dec89c2dbf6e0f2e5", + "log/2026-03-13.100758.md": "65c5b65fdd5984c036f1de53f8c82f2a", + "log/2026-03-13.115251.md": "56a27d18deeb89c8ce8b112b099ac7cf" + } +} diff --git a/BESTPRACTICES.md b/BESTPRACTICES.md index 1a8e205..e818477 100644 --- a/BESTPRACTICES.md +++ b/BESTPRACTICES.md @@ -16,3 +16,5 @@ Generalised best practices extracted from real project work. Each topic file is - [Debugging Methodology](best-practices/debugging.md) — Systematic diagnosis, full-chain testing, common pitfalls - [Claude Code Skills](best-practices/skills-development.md) — Skill authoring, context injection, tool restrictions - [Linting & Formatting](best-practices/linting.md) — Tool choices per language, PostToolUse hook, pre-commit integration, formatter contract +- [Spec-Driven Development](best-practices/spec-driven-development.md) — Spec structure, requirement numbering, test-first workflow, context tiers, anti-patterns +- [Test-Driven Development](best-practices/test-driven-development.md) — Edge case discovery, property-based testing, mutation testing, AI agent testing patterns, test architecture diff --git a/CLAUDE.md b/CLAUDE.md index ac050a3..69a0251 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,7 +2,7 @@ ## Session Start -1. **Immediately** (without waiting for user input) list the project directories under `~/dev/claude/` (excluding `secrets/`) and present a numbered menu like: +1. **Immediately** (without waiting for user input) use the directory tree already provided by `context-load` (the `TREE` section in your context) to identify project directories under `~/dev/claude/` (excluding `secrets/`). Present a numbered menu like: > What are we working on today? > @@ -12,7 +12,7 @@ > N-1. **No project right now** — just chat > N. **New project!** — start something new - Scan the directories at runtime so the list is always current. Include a brief description if the project has a CLAUDE.md or README.md you can glean one from. + Do **not** run shell commands to list directories — the tree is already in your context. Include a brief description if the project has a CLAUDE.md or README.md you can glean one from. 2. Based on the user's choice: - **Existing project**: `cd` into the directory, read all `.md` files, and read `~/dev/claude/secrets/` (read-only reference — review every file to refresh context). Then read `BESTPRACTICES.md` (loaded automatically by `context-load`) and load any topic files relevant to the selected project's technology stack. Ask clarifying questions if anything is unclear or incomplete, and note context in MEMORY.md. @@ -74,7 +74,7 @@ The primary reference for Claude sessions. Should contain: ### MEMORY.md (Tiered Memory System) Long-running projects accumulate significant context. To keep MEMORY.md useful rather than bloated, use a **tiered memory** structure: -**MEMORY.md** is a **thin index only** — one-line descriptions with links to topic files in `memory/`. No content lives in MEMORY.md itself. Think of it as a card catalog. Keep it under ~50 lines. +**MEMORY.md** is a **thin index only** — one-line descriptions with links to topic files in `memory/`. No content lives in MEMORY.md itself. Think of it as a card catalog. **memory/** contains the actual content, split by topic: - `memory/project-status.md` — Current milestone, what's next, blockers @@ -165,6 +165,28 @@ Session logs use structured markdown with parseable section headers: Summary, De - Unreflected logs older than `log.warn_unreflected_days` (default: 14) trigger a warning instead of deletion - `/reflect-logs` flags stale memory entries (version-specific bugs that have been fixed, manual processes that have been automated) +## Plan Mode + +When working in plan mode (`permission_mode: plan`): + +**Before calling `ExitPlanMode`**, always write the complete plan to a file in the project root: + +- **Filename:** `[MILESTONE]-[PURPOSE]-PLAN.md` — e.g. `M2-auth-PLAN.md`, `M3-monitoring-PLAN.md` + - MILESTONE: the milestone identifier (e.g. `M2`) or a short label if not milestone-scoped (e.g. `initial`) + - PURPOSE: a short kebab-case description of what the plan covers +- **Contents:** the full plan as developed in the planning conversation — steps, decisions, rationale, open questions +- **Location:** project root (same directory as CLAUDE.md) + +This file becomes the implementation reference for the session that follows plan mode. + +**Keep the plan updated during implementation.** After completing each phase or significant step, update the PLAN.md file: +- Mark the phase/step status as **Complete** +- Add key commits, references, or artifacts produced +- Document deviations from the original plan (what changed and why) +- Note verification results + +This ensures the plan stays accurate as a living document — useful for resuming across sessions, reflecting on the milestone, and understanding what actually happened vs. what was planned. + ## Milestones Break projects into numbered milestones (M1, M2, ...). Every milestone completion MUST include: @@ -201,6 +223,7 @@ Break projects into numbered milestones (M1, M2, ...). Every milestone completio - Use colour output for pass/fail indicators in verification scripts - Verification scripts should check for default/insecure credentials and print remediation instructions on failure - Scripts should exit non-zero on failure so `&&` chains work naturally +- **Never hardcode secrets, tokens, or access keys in scripts.** Accept them via environment variables, stdin, or `@file` references. If a script needs a secret at runtime, read it from `~/dev/claude/secrets/` or accept it as a parameter — never embed it. ## Process Principles diff --git a/MEMORY.md b/MEMORY.md index 12a6fe1..1330557 100644 --- a/MEMORY.md +++ b/MEMORY.md @@ -1,5 +1,9 @@ # claude-foundations — Memory Index +## Runbooks + +- [New ArgoCD App](memory/runbook-new-argocd-app.md) — Full procedure: manifests, KSOPS secrets, ArgoCD Application, Caddy reverse proxy, DNS, verification, common pitfalls + ## Scripts - [context-load](memory/script-context-load.md) — Walks cwd upward collecting CLAUDE.md, CONTEXT.md, MEMORY.md, BESTPRACTICES.md, trees, and git status for system prompt injection @@ -7,6 +11,7 @@ - [install-hooks](memory/script-install-hooks.md) — Symlinks hooks into ~/.claude/hooks/ and prints settings.json config - [setup-formatters](memory/script-setup-formatters.md) — Opts a project into auto-formatting by symlinking formatter scripts - [git-status-report](memory/script-git-status-report.md) — Scans directories for git repos, reports uncommitted changes and remote sync (lives in small-scripts) +- [require-plan-file](memory/script-require-plan-file.md) — PreToolUse hook: blocks ExitPlanMode unless a *-PLAN.md file exists in the project root ## Skills @@ -15,3 +20,10 @@ - [/reflect](memory/skill-reflect.md) — Milestone reflection: reviews conversation + git log, produces structured artifacts - [/distill-best-practices](memory/skill-distill-best-practices.md) — Cross-project distillation of memory files into best-practices/ topic files - [/linter](memory/skill-linter.md) — Audit and manage project formatters/linters (scan, run, cleanup modes) +- [/context-load](memory/skill-context-load.md) — Reload project context after /clear or mid-session context loss + +## Topic Files + +- [Decisions](memory/decisions.md) — Architecture and design decisions: pipeline design, linting system, context-load, CLAUDE.md structure +- [Process Lessons](memory/process-lessons.md) — Working rules: git-status at session start, plan mode for architecture, hook exit codes, ssh-agent +- [Skills Gotchas](memory/gotchas-skills.md) — Broken symlinks under set -e, mid-session skill discovery limitation diff --git a/README.md b/README.md index 20de509..ff5295d 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ claude-foundations/ post-edit-lint.sh # PostToolUse: auto-format/lint on Edit/Write pre-commit-lint.sh # Git pre-commit: lint staged files pre-compact-backup.sh # PreCompact: backup transcript before compaction + require-plan-file.sh # PreToolUse/ExitPlanMode: enforce *-PLAN.md exists before leaving plan mode scripts/ install-hooks.sh # Symlink hooks into ~/.claude/hooks/ setup-formatters.sh # Set up formatters for a project diff --git a/best-practices/.distill-state.json b/best-practices/.distill-state.json new file mode 100644 index 0000000..0370e95 --- /dev/null +++ b/best-practices/.distill-state.json @@ -0,0 +1,41 @@ +{ + "version": 1, + "last_run": "2026-03-15T10:24:05Z", + "projects": { + "agent-runtimes": { + "path": "/home/paul/dev/claude/projects/agent-runtimes", + "last_sha": "2adcc5aea4af520d407b062dcf5c81ff5fd69a73", + "last_run": "2026-03-15T10:24:05Z" + }, + "claude-foundations": { + "path": "/home/paul/dev/claude/projects/claude-foundations", + "last_sha": "c3a151a87bf726a239532e6739c58e8c41c45f4e", + "last_run": "2026-03-15T10:24:05Z" + }, + "cluster-apps/octopus-deploy": { + "path": "/home/paul/dev/claude/projects/cluster-apps/octopus-deploy", + "last_sha": "5989ef737cc5bf0885136259aecbb3726c2bf1c1", + "last_run": "2026-03-15T10:24:05Z" + }, + "cluster-bootstrap": { + "path": "/home/paul/dev/claude/projects/cluster-bootstrap", + "last_sha": "ab0460488a1251c6c1afdc7df929b3b7cd147243", + "last_run": "2026-03-15T10:24:05Z" + }, + "custom-claude-skills": { + "path": "/home/paul/dev/claude/projects/custom-claude-skills", + "last_sha": "bb6fb7270590296aa7c1594053cde1bcae114019", + "last_run": "2026-03-15T10:24:05Z" + }, + "hugo-accelerator": { + "path": "/home/paul/dev/claude/projects/hugo-accelerator", + "last_sha": "b300687256b70ef9f82781840863534a814496d3", + "last_run": "2026-03-15T10:24:05Z" + }, + "small-scripts": { + "path": "/home/paul/dev/claude/small-scripts", + "last_sha": "5230e542360f8b07b7375c943ffb99501f4aacd8", + "last_run": "2026-03-15T10:24:05Z" + } + } +} diff --git a/best-practices/debugging.md b/best-practices/debugging.md index 62dfdc5..e0e4dad 100644 --- a/best-practices/debugging.md +++ b/best-practices/debugging.md @@ -36,6 +36,41 @@ Before starting any OIDC integration, research: 2. Which claims are in the ID token vs userinfo endpoint 3. How the consumer matches RBAC identities (groups? email? username?) +## Log-First Diagnosis + +- **CrashLoopBackOff: check logs first.** Error messages in pod logs usually point directly to the fix. Don't tweak configuration or security contexts blindly — `kubectl logs ` first. +- **Discriminate transient from persistent errors.** CSI lock contention, etcd timeouts during first install, and brief connectivity blips are self-healing. Don't spend time debugging errors that resolve on retry. If you see retry/backoff patterns in logs, wait before intervening. +- **Trust controller retry logic.** CSI controllers, operators, and reconciliation loops have built-in retry. Transient failures during rapid provisioning are expected, not bugs. + +## Reproduce Before Fixing + +When a bug is discovered or reported, **do not start by trying to fix it.** The first step is always to write a test that reproduces the failure: + +1. **Write a failing test.** Capture the bug as a test case that demonstrates the broken behaviour. This forces you to understand the bug precisely — what input triggers it, what the wrong output is, and what the correct output should be. +2. **Fix the bug in isolation.** Use a subagent or a separate session to write the fix. The fixing agent gets the failing test as its success criterion — it's done when the test passes. This separation prevents the fixer from unconsciously weakening the test to match a broken implementation. +3. **The test stays forever.** The reproduction test becomes a permanent regression test. It proves the fix works and prevents the bug from returning. + +This workflow has several advantages: +- **Forces precise understanding.** Writing a test means you know exactly what's broken, not just "it doesn't work." +- **Prevents partial fixes.** The test defines "done" objectively — the fix either passes or it doesn't. +- **Parallelises work.** While one agent fixes the bug, you can continue other work. +- **Catches regressions.** The test remains in the suite, guarding against the same class of failure. + +```python +# Step 1: Write the failing test FIRST +def test_regression_issue_427_empty_payload_crashes(): + """Bug #427: Empty payload causes unhandled TypeError in dispatcher. + Should return a 400 validation error, not crash.""" + response = client.post("/dispatch", json={}) + assert response.status_code == 400 # Currently crashes with 500 + +# Step 2: Hand to a subagent/session: "Make this test pass without breaking others" +``` + +## Pattern Mining Before Authoring + +Before building a new service, component, or script, read existing patterns in the codebase first. This matches conventions on the first attempt and avoids rework on naming, structure, and integration points. Applies to K8s manifests, CI pipelines, skill authoring, and script structure. + ## Grep Your Own Docs Known issues documented in CLAUDE.md or MEMORY.md but not applied to new scripts/configs waste debugging time. Search your own documentation before writing automation that touches areas with known gotchas. diff --git a/best-practices/git-source-control.md b/best-practices/git-source-control.md index 853ef58..d10e65a 100644 --- a/best-practices/git-source-control.md +++ b/best-practices/git-source-control.md @@ -26,6 +26,11 @@ - Remote URL format: `git@:/.git` - Optionally push-mirror to GitHub for public visibility +## Access and Clone Gotchas + +- **Org repos require explicit collaborator grants.** Don't assume organizational membership implies write access — verify permissions before setting up automation or CI/CD. +- **Shallow clones break push operations.** `git clone --depth 1` is fine for read-only CI jobs, but pipelines that push artifacts, tags, or mirror to other remotes need full clones. + ## Version Management - Use the latest stable version of dependencies unless pinned for a reason diff --git a/best-practices/kubernetes.md b/best-practices/kubernetes.md index 78061ca..dba2d51 100644 --- a/best-practices/kubernetes.md +++ b/best-practices/kubernetes.md @@ -29,6 +29,33 @@ Manual bootstrap secrets (encryption keys, OIDC client secrets) must be document - Cross-namespace policies need explicit namespace matching (e.g., `matchExpressions` on namespace label). - Always test from the actual consumer namespace, not same-namespace test pods. +## Probe Strategy + +- **Liveness vs readiness probes serve different purposes.** TCP checks confirm the process is listening (liveness). Exec/command checks confirm the application is ready to serve (readiness). Don't conflate them. +- **Probes must match application host validation.** Applications that validate Host headers (e.g., Next.js `ALLOWED_HOSTS`) will reject probes sent to the pod IP. Set `httpGet.httpHeaders` with the expected Host value. +- **Don't load credentials into liveness probes.** If readiness requires an authenticated check (e.g., `sqlcmd`), use a simple TCP check for liveness and reserve the authenticated check for readiness only. + +## Init Container Patterns + +- **Writable config via init container + emptyDir.** When apps require writable directories but ConfigMaps are read-only, use an init container to copy config into an emptyDir volume that the main container mounts read-write. +- **Privilege separation.** Init containers can run as root to create directories or set ownership, while the main container runs as a non-root UID. Prefer this over running the entire workload as root. +- **Non-root images have hidden filesystem requirements.** Many modern images (e.g., MSSQL 2022, UID 10001) need writable directories beyond the obvious ones. Always check image documentation or `docker inspect` before writing manifests. + +## StatefulSet Edge Cases + +- **CrashLoopBackOff pods won't auto-replace on spec update.** The StatefulSet controller won't delete and recreate a crashing pod when you update the spec — manual `kubectl delete pod` is required to force recreation. +- **Immutable field diffs can deadlock auto-sync.** StatefulSet fields like `volumeClaimTemplates` are immutable after creation. GitOps controllers (ArgoCD) will show permanent OutOfSync if the desired state differs from the live immutable fields. Force sync or recreate the StatefulSet. + +## GitOps: Imperative vs Declarative + +- **Never use imperative operations on GitOps-managed resources.** `kubectl rollout restart` adds annotations that conflict with the GitOps controller's desired state, causing permanent OutOfSync. Use declarative paths instead — update a configmap hash annotation in Git, or change a pod template label. +- **ArgoCD reconciliation has latency.** New Application manifests don't appear immediately due to polling intervals. Use manual refresh annotations when automation needs immediate reconciliation. + +## PodSecurity Alignment + +- **Namespace PodSecurity labels must match container security contexts.** DinD, CSI drivers, and other privileged workloads need `pod-security.kubernetes.io/enforce: privileged` on their namespace. A `baseline` or `restricted` namespace silently blocks privileged pods. +- **Document privileged namespace requirements.** When a workload needs elevated privileges, document the specific requirement (e.g., "Docker-in-Docker for CI builds") alongside the namespace label. + ## Miscellaneous - `enableServiceLinks: false` may be needed when K8s-injected service env vars conflict with app config (e.g., Authelia interprets `AUTHELIA_*` service vars as configuration). diff --git a/best-practices/scripting.md b/best-practices/scripting.md index 6f86f56..7a039be 100644 --- a/best-practices/scripting.md +++ b/best-practices/scripting.md @@ -22,8 +22,25 @@ If you run the same 3+ commands in sequence more than once, it should become a s - Steps requiring careful ordering - Multi-step manual processes that are error-prone +## Error Handling by Tool Purpose + +Not all scripts need the same error handling strategy: +- **Destructive scripts** (deploy, configure, delete) should use `set -euo pipefail` — fail fast on any error. +- **Reporting/read-only scripts** (status dashboards, aggregation, monitoring) should start without `set -e` — complex data collection from multiple sources is hard to debug under errexit. Use explicit conditional checks instead. +- **The choice depends on the tool's purpose.** A script that writes to production needs strict error handling. A script that reads from 10 sources and aggregates results needs resilience. + +## Dryrun Mode + +Every script that modifies state should support `--dryrun` / `-n`: +- Makes the script self-documenting about its side effects +- Enables safe testing and review before execution +- Enables test harnesses that verify output without executing changes +- Dryrun output should show exactly what would happen, not a summary + ## Shell Gotchas - `((PASS++))` fails under `set -e` when PASS=0 — the expression evaluates to 0 (false), triggering errexit. Use `PASS=$((PASS + 1))` instead. +- `set -e` silently terminates complex pipelines and subshells with no output — makes debugging extremely difficult. Also kills command substitutions that capture non-zero exit codes (e.g., `result=$(grep "pattern" file)` exits if grep finds nothing). +- `grep` interprets option-like strings (starting with `-`) as flags — use `--` terminator before patterns or input that may start with dashes. - Always quote variables in conditionals and file paths - Use `trap` for cleanup of temp files and credentials diff --git a/best-practices/secrets-management.md b/best-practices/secrets-management.md index 7252965..7323b2f 100644 --- a/best-practices/secrets-management.md +++ b/best-practices/secrets-management.md @@ -36,6 +36,12 @@ Each Ansible project needs `vars_plugins_enabled = host_group_vars,community.sop Some secrets are chicken-and-egg (e.g., the age decryption key for ArgoCD's KSOPS). These must be created manually as a bootstrap step and documented clearly. +## Credential Lifecycle Management + +- **Track credential expiry dates.** OAuth client secrets, API tokens, and certificates have expiry dates that can cause silent failures. Document expiry dates when creating credentials. +- **Set alerts before expiry.** For long-lived credentials (e.g., 720-day OAuth client secrets), set calendar reminders or automated monitoring alerts well before they expire. +- **Rotation plan.** Know the rotation procedure before you need it — some credential types (e.g., Azure app registrations) require coordinated updates across multiple systems. + ## Backup Considerations Backup plans must include encryption keys (age private keys, etc.) so that encrypted data in Git repos remains recoverable. diff --git a/best-practices/skills-development.md b/best-practices/skills-development.md index f3255fd..973dd69 100644 --- a/best-practices/skills-development.md +++ b/best-practices/skills-development.md @@ -15,6 +15,17 @@ - **Use $ARGUMENTS** for user input, `$0`, `$1` etc. for positional args - Dynamic commands in `!`command`` run at skill load time, not during Claude's execution +## Portable Path Resolution + +- **Use `CLAUDE_PROJECT_ROOT` env var** for cross-project path references in `!`command`` blocks. Hardcoded absolute paths (e.g., `~/dev/claude/...`) are user-specific. Relative paths (`../`) break depending on CWD and can trigger sandbox violations when they resolve outside allowed directories. +- **Add a detection fallback.** Include a "Step 0" in skill instructions that detects the project root by walking up the directory tree to find the highest `CLAUDE.md` if the env var isn't set. This makes skills work even without prior setup. +- **Keep config paths relative to the root.** Settings files should use paths relative to `CLAUDE_PROJECT_ROOT` (e.g., `projects_dir: projects`) rather than absolute paths, so they're portable across machines. + +## Skill Discovery Timing + +- **Skills are discovered at session start, not dynamically.** Creating or symlink a new skill mid-session requires restarting Claude Code to use it as a slash command. +- **Broken symlinks cause silent failures under `set -e`.** `readlink -f` on a broken symlink returns empty string. The install script should validate symlinks and remove stale ones. + ## `!`command`` Gotchas - **No `$()` command substitution** — the permission checker rejects commands containing `$()` @@ -22,3 +33,4 @@ - **`allowed-tools` patterns must match the command binary** — each binary used in `!`command`` blocks needs its own pattern - **Prefer specific tool patterns over broad ones** — `Bash(git log *)` is safer than `Bash(git *)` - **Fallback to tool instructions for dynamic paths** — if a command needs `$ARGUMENTS` to compute a path, use a plain-text instruction telling Claude to use the Read tool instead +- **Env var expansion works** — `${CLAUDE_PROJECT_ROOT}` expands in `!`command`` blocks because they run as shell commands. This is the recommended pattern for portable cross-project paths. diff --git a/best-practices/spec-driven-development.md b/best-practices/spec-driven-development.md new file mode 100644 index 0000000..9c9f947 --- /dev/null +++ b/best-practices/spec-driven-development.md @@ -0,0 +1,201 @@ +# Spec-Driven Development with AI Agents + +Best practices for using structured specifications to coordinate AI agent implementation work. Extracted from real project experience (agent-runtimes) and industry research (OpenSpec, Codified Context paper, Addy Osmani's workflow guides). + +## Why Specs Matter for AI Agents + +AI agents trust documentation absolutely. A well-written spec gives an agent everything it needs to implement a subsystem without reading the entire codebase. A stale or vague spec causes silent failures where agents generate code that is structurally valid but architecturally wrong. + +Specs serve three functions that CLAUDE.md alone cannot: +1. **Compressed context** — an agent reads one spec, not 300 lines of mixed concerns +2. **Testable contracts** — numbered requirements and scenarios translate directly to pytest +3. **Handoff boundaries** — an agent working on the dispatcher doesn't need to understand the entrypoint internals, just the interface between them + +## Spec Structure + +Each spec follows a consistent template. Sections are ordered so an agent can read top-down and build understanding progressively. + +### Required Sections + +1. **Overview** — What this subsystem does. 2-3 sentences. An agent should know if this spec is relevant after reading this. +2. **Responsibilities** — What this subsystem owns and what it delegates. Prevents scope creep during implementation. +3. **Dependencies** — Which other specs to read first. Keeps the reading list minimal. +4. **Data Model** — Types, schemas, state machines, interfaces. The concrete contract. +5. **Requirements** — Numbered functional requirements (e.g., E-1, E-2). Each must be independently testable. +6. **Scenarios** — Concrete given/when/then examples. These become test functions. + +### Optional Sections + +7. **Interface** — API surface, function signatures, HTTP endpoints. Include when the subsystem has an external-facing API. +8. **Extension Points** — How to add new capabilities without modifying existing code. Step-by-step instructions. +9. **Error Handling** — Failure modes and expected behaviour. Prevents agents from inventing their own error strategies. + +### Writing Guidelines + +- **Be specific, not comprehensive.** A spec that says "handle errors appropriately" is useless. A spec that says "return exit code 124 on timeout" is testable. +- **Include the why.** Design intent and constraints prevent agents from making structurally valid but architecturally wrong changes. Requirements without rationale are followed mechanically — agents can't judge edge cases or make trade-offs. Every constraint needs a "Why:" line. Example: `"Secrets never in payload"` needs `"because payloads may be logged and stored in task history"`. +- **Use concrete examples.** Every data model section should include a realistic JSON/code example, not just a schema. +- **Cross-reference, don't duplicate.** If two specs share a concept (e.g., the payload schema), one spec owns it and the other links to it. +- **Keep each spec self-contained.** An agent should be able to implement a subsystem by reading the target spec plus its listed dependencies. If it needs to read CLAUDE.md, the spec is incomplete. + +## Requirement Numbering + +Each spec uses a short prefix derived from its name, followed by a sequential number: + +| Spec | Prefix | Example | +|---|---|---| +| payload.md | P | P-1, P-2 | +| entrypoint.md | E | E-1, E-2 | +| actions.md | A | A-1, A-2 | +| runners.md | R | R-1, R-2 | +| dispatcher.md | D | D-1, D-2 | +| control-plane.md | CP | CP-1, CP-2 | + +Requirements must be: +- **Independently testable** — each maps to one or more test functions +- **Unambiguous** — an agent can determine pass/fail without human judgement +- **Stable** — changing a requirement number invalidates tests, so avoid renumbering + +## Scenarios as Test Blueprints + +Every scenario in a spec should be directly translatable to a test function. Use this format: + +```markdown +### Scenario: Pre-action failure +**Given:** Payload with clone pre-action (invalid repo URL) +**When:** Clone fails (git returns non-zero) +**Then:** on_error actions run, container exits 1. Runner never executes. +``` + +This becomes: + +```python +def test_scenario_preaction_failure_runs_on_error_and_exits_1(...): + """Given clone fails, on_error runs and exits 1. Runner never executes.""" +``` + +Guidelines: +- Each scenario tests one behaviour, not a combination +- Include both happy paths and error paths +- Name the scenario descriptively — it becomes the test function's docstring +- Include enough setup detail that an agent can write the test without guessing + +## The Spec → Test → Code Workflow + +This is the core development loop. Tests are written from the spec before code exists. + +### 1. Write or Update the Spec + +Define requirements and scenarios. Get them reviewed. The spec is the source of truth for what the system should do. + +### 2. Write Tests from the Spec + +Translate requirements and scenarios into pytest functions. Tests should: +- Map to requirement IDs in their names: `test_e4_preaction_failure_skips_remaining` +- Use the scenario's given/when/then as the test body structure +- Mock external dependencies (subprocess, HTTP, filesystem) +- Run fast (no Docker, no network, no real APIs) + +### 3. Run the Tests — They Should All Fail + +This confirms the tests are actually testing something. If a test passes before implementation, it's either testing the wrong thing or the feature already exists. + +### 4. Implement Until Tests Pass + +Write the minimum code to make tests pass. The spec defines what, the tests verify it, the code implements it. + +### 5. Update Spec if Implementation Reveals Issues + +Sometimes implementation reveals that a requirement is unworkable or incomplete. Update the spec, update the test, then update the code. The spec stays authoritative. + +## Spec Maintenance + +### Preventing Drift + +Specs drift from code when they're treated as planning documents that are "done" after implementation. They must be treated as living contracts. + +**Rules:** +- **Spec changes require test changes.** If a requirement changes, its test must change in the same commit. +- **Code changes that affect interfaces require spec changes.** If a function signature, API endpoint, or data schema changes, the relevant spec must be updated in the same commit. +- **New features require spec-first.** Add the requirement and scenario to the spec, write the test, then implement. + +### CI Enforcement + +Enforce spec hygiene with automated checks: + +1. **Pre-commit hook** — run pytest, block commit on failure (already implemented) +2. **Spec coverage check** — a script that verifies every numbered requirement has at least one test function referencing it +3. **Orphan test detection** — tests referencing requirement IDs that no longer exist in specs + +### Review Checklist + +When reviewing a PR that touches a spec subsystem: +- [ ] Spec updated if interface or behaviour changed +- [ ] Test added/updated for new/changed requirements +- [ ] Cross-references still valid +- [ ] No requirements removed without deprecation note + +## Context Architecture for Agents + +Based on the Codified Context paper (108k-line system, 283 sessions), structure project knowledge in three tiers: + +### Tier 1: Hot Context (Always Loaded) + +CLAUDE.md — conventions, env vars, repo structure, scripts. Loaded every session. Keep under ~300 lines by linking to details elsewhere. + +### Tier 2: Spec Context (Per-Task) + +`spec/` files — loaded based on what the agent is working on. An agent implementing a new action reads `spec/actions.md` + `spec/payload.md`. An agent working on the dispatcher reads `spec/dispatcher.md` + `spec/container-backends.md`. + +The spec index (SPEC.md) has a "read this when..." column to guide selection. + +### Tier 3: Cold Context (On-Demand) + +`memory/` files — gotchas, reflections, decisions. Loaded only when relevant. An agent hitting a weird Cilium issue checks `memory/gotchas-cilium.md`. + +### Routing Context to Agents + +When launching an agent to work on a subsystem: +1. Point it at the relevant spec(s) via its prompt +2. Include CLAUDE.md for conventions +3. Let it pull from memory/ on-demand if it hits issues + +Don't load everything — agents perform better with focused context than with a 50-page dump. + +## Testing Depth + +The spec→test→code workflow defines *when* to write tests. For *how* to write comprehensive tests — edge case discovery, property-based testing, mutation testing, AI agent testing patterns — see [Test-Driven Development](test-driven-development.md). + +## Post-Write Spec Audit + +After writing specs, audit them against best practices before implementation. Common gap categories: + +1. **Missing rationale** — Constraints without "Why:" lines. Agents follow them mechanically but can't judge edge cases. +2. **Missing error/failure scenarios** — Happy paths are covered but failure modes aren't specified. +3. **Cross-spec interface misalignment** — Two specs describe the same interface differently. +4. **Vague requirements** — "Handle errors appropriately" instead of specific error codes and behaviours. +5. **Missing specs for discovered subsystems** — Implementation reveals components that weren't planned for. + +Write-then-audit is more productive than trying to get specs perfect on the first pass. The audit step catches systematic gaps across all specs at once. + +## Anti-Patterns + +### Specs as documentation, not contracts +**Symptom:** Specs describe what was built, updated after the fact. Tests don't reference spec IDs. +**Fix:** Write specs before code. Tests reference requirement IDs. Specs are the input, not the output. + +### Mega-spec +**Symptom:** One large spec covering the entire system. Agents must read thousands of lines to find what they need. +**Fix:** Split by subsystem. Each spec should be readable in under 5 minutes. + +### Spec without scenarios +**Symptom:** Requirements are abstract ("handle errors gracefully"). No concrete examples. +**Fix:** Every requirement needs at least one given/when/then scenario with specific inputs and outputs. + +### Implementation details in specs +**Symptom:** Spec dictates variable names, algorithm choices, internal data structures. +**Fix:** Specs define what and why, not how. The interface is specified; the implementation is free. + +### Untested requirements +**Symptom:** Requirements exist in the spec but no test references them. They drift without anyone noticing. +**Fix:** Spec coverage check in CI. Every requirement ID must appear in at least one test function name. diff --git a/best-practices/test-driven-development.md b/best-practices/test-driven-development.md new file mode 100644 index 0000000..52ffc57 --- /dev/null +++ b/best-practices/test-driven-development.md @@ -0,0 +1,456 @@ +# Test-Driven Development for Spec-Based Projects + +Best practices for writing comprehensive, regression-catching tests in projects that use structured specifications. Focuses on maximising test value (catching real bugs) rather than test volume (inflating coverage numbers). Extracted from industry research, academic papers (TDAD, Codified Context), and practitioner experience. + +## Core Principle: Tests Are the Spec's Enforcement Layer + +In a spec-driven project, the spec defines *what* and the tests *prove it*. A requirement without a test is an aspiration. A test without a requirement is undocumented behaviour. Keep them tightly coupled: + +- Every numbered requirement (P-1, E-3) has at least one test +- Every test function name includes its requirement ID: `test_e3_preaction_failure_exits_1` +- Spec changes and test changes ship in the same commit + +## Deriving Tests from Specs + +### Requirements to Tests + +Each spec requirement becomes one or more test functions. The mapping isn't always 1:1 — a requirement like "must respect timeout" needs tests for: default timeout, explicit timeout, timeout=0 (no limit), timeout exceeded. + +```python +# From spec: R-4: Runners must respect runtime.timeout. +# Default 3600s. Value of 0 means no timeout. + +def test_r4_default_timeout_is_3600(): + """R-4: When timeout not specified, default is 3600s.""" + +def test_r4_explicit_timeout_is_honoured(): + """R-4: When timeout=60, process killed after 60s.""" + +def test_r4_zero_timeout_means_no_limit(): + """R-4: When timeout=0, no timeout is applied.""" + +def test_r4_timeout_returns_exit_code_124(): + """R-4 + R-5: Timeout produces exit code 124.""" +``` + +### Scenarios to Tests + +GIVEN/WHEN/THEN scenarios translate directly to Arrange/Act/Assert: + +```python +def test_scenario_preaction_failure_runs_on_error(): + """Given clone fails, on_error runs and exits 1. Runner never executes.""" + # GIVEN — arrange + payload = make_payload(pre_actions=[{"action": "clone", "repo": "bad-url"}]) + mock_clone = Mock(side_effect=subprocess.CalledProcessError(128, "git")) + + # WHEN — act + exit_code = run_entrypoint(payload, clone_handler=mock_clone) + + # THEN — assert + assert exit_code == 1 + mock_runner.assert_not_called() + mock_on_error.assert_called_once() +``` + +### Parameterised Tests from Spec Enumerations + +When a spec lists multiple valid values, use `@pytest.mark.parametrize`: + +```python +# From spec: task states are pending, assigned, running, succeeded, failed, timed_out, cancelled +@pytest.mark.parametrize("terminal_state", ["succeeded", "failed", "timed_out", "cancelled"]) +def test_cp_terminal_state_cannot_be_overwritten(terminal_state): + """CP: Terminal states reject further transitions with 409.""" +``` + +## Systematic Edge Case Discovery + +~80% of bugs cluster at boundaries. Use these techniques to find edge cases systematically rather than by intuition. + +### Boundary Value Analysis + +For every input parameter, test at the edges of its valid range: + +| Input type | Test values | +|---|---| +| Integer (range 1-100) | 0, 1, 2, 99, 100, 101, -1, MAX_INT | +| String | `""`, `"a"`, max-length string, max+1, unicode (`"\u0000"`, emoji), whitespace-only | +| List/Array | `[]`, `[single]`, many items, duplicates, `None` | +| Dict/Map | `{}`, missing required keys, extra unknown keys, `None` values | +| Timeout (seconds) | 0, 1, -1, very large (999999), `None`/missing | +| Base64 | valid, invalid chars, empty, padding variants (`=`, `==`, none) | + +### Equivalence Partitioning + +Group inputs into classes where all members should behave identically. Test one from each class: + +```python +# Payload validation: prompt field +# Class 1: valid string → accepted +# Class 2: empty string → rejected (spec says prompt is required) +# Class 3: missing key → rejected +# Class 4: wrong type (int, list, None) → rejected +# Class 5: very long string → accepted (no length limit in spec) + +@pytest.mark.parametrize("prompt,should_pass", [ + ("Fix the bug", True), # Class 1: valid + ("", False), # Class 2: empty + (None, False), # Class 3: missing/None + (42, False), # Class 4: wrong type + ("x" * 100_000, True), # Class 5: long string +]) +def test_p_prompt_validation(prompt, should_pass): + ... +``` + +### State Transition Coverage + +For state machines (task states, dispatcher states), test: + +1. **Every valid transition:** `pending → assigned → running → succeeded` +2. **Every invalid transition:** `succeeded → running` (should be rejected) +3. **Initial state:** newly created tasks start in `pending` +4. **Terminal states:** `succeeded`, `failed`, `timed_out`, `cancelled` cannot transition further +5. **Re-entrant transitions:** same state → same state (should be idempotent or rejected, per spec) + +```python +VALID_TRANSITIONS = [ + ("pending", "assigned"), + ("assigned", "running"), + ("running", "succeeded"), + ("running", "failed"), + ("running", "timed_out"), + ("assigned", "cancelled"), + ("running", "cancelled"), +] + +INVALID_TRANSITIONS = [ + ("succeeded", "failed"), + ("failed", "running"), + ("cancelled", "pending"), + ("timed_out", "running"), +] + +@pytest.mark.parametrize("from_state,to_state", VALID_TRANSITIONS) +def test_valid_state_transition(from_state, to_state): + ... + +@pytest.mark.parametrize("from_state,to_state", INVALID_TRANSITIONS) +def test_invalid_state_transition_rejected(from_state, to_state): + ... +``` + +### The Edge Case Checklist + +Walk through this for every function under test: + +1. **Empty/null inputs** — what happens when required fields are missing? +2. **Boundary values** — min, max, zero, negative, off-by-one +3. **Type mismatches** — string where int expected, list where dict expected +4. **Malformed input** — invalid JSON, bad base64, truncated data +5. **Concurrent operations** — two tasks claiming the same resource +6. **Ordering** — actions that depend on sequence (pre-action before runner) +7. **Idempotency** — calling the same operation twice (kill an already-killed container) +8. **Resource exhaustion** — at capacity, disk full, timeout expired +9. **Partial failure** — first action succeeds, second fails (cleanup?) + +## Property-Based Testing with Hypothesis + +Instead of specifying individual test cases, define *properties* that must hold for all inputs. Hypothesis generates hundreds of inputs including edge cases you'd never think of. + +### When to Use Property-Based Testing + +- **Serialisation roundtrips:** encode → decode returns original +- **Parsers:** should never crash on any input +- **Data transformations:** invariants that hold regardless of input +- **Validators:** valid inputs accepted, invalid inputs rejected (never crash) + +### When NOT to Use It + +- Tests where generating valid inputs is harder than the code itself +- Tests where the "property" just restates the implementation +- UI or integration tests + +### Patterns + +```python +from hypothesis import given, strategies as st, assume, settings +from hypothesis import example + +# Roundtrip: base64 encode/decode preserves payload +@given(st.text()) +def test_base64_roundtrip(payload_str): + encoded = base64.b64encode(payload_str.encode()).decode() + decoded = base64.b64decode(encoded).decode() + assert decoded == payload_str + +# Invariant: payload validation never crashes (may reject, never exception) +@given(st.dictionaries(st.text(), st.text() | st.integers() | st.none())) +def test_payload_validation_never_crashes(raw_payload): + # Should return True/False or raise ValidationError — never unhandled exception + try: + validate_payload(raw_payload) + except ValidationError: + pass # Expected for invalid input + +# Pin known edge cases alongside random generation +@example("") # empty string +@example("\x00") # null byte +@example("a" * 10**6) # very long +@given(st.text()) +def test_prompt_handling(prompt): + ... + +# Composite strategies for domain objects +@st.composite +def valid_payloads(draw): + return { + "task_id": draw(st.uuids()).hex, + "prompt": draw(st.text(min_size=1)), + "runtime": {"cli": draw(st.sampled_from(["claude", "codex"]))}, + } + +@given(valid_payloads()) +def test_valid_payload_always_accepted(payload): + assert validate_payload(payload) is True +``` + +### Stateful Testing for State Machines + +Hypothesis can generate sequences of operations and check invariants after each step: + +```python +from hypothesis.stateful import RuleBasedStateMachine, rule, precondition + +class TaskStateMachine(RuleBasedStateMachine): + def __init__(self): + super().__init__() + self.task = Task(state="pending") + + @rule() + @precondition(lambda self: self.task.state == "pending") + def assign(self): + self.task.transition("assigned") + assert self.task.state == "assigned" + + @rule() + @precondition(lambda self: self.task.state == "running") + def complete(self): + self.task.transition("succeeded") + assert self.task.state == "succeeded" + + # Invariant: terminal states never change + @invariant() + def terminal_states_are_final(self): + if self.task.state in ("succeeded", "failed", "cancelled"): + with pytest.raises(InvalidTransition): + self.task.transition("running") + +TestTaskStates = TaskStateMachine.TestCase +``` + +## Mutation Testing + +Mutation testing answers: "If someone introduced a bug, would our tests catch it?" + +Tools make small code changes (replacing `>` with `>=`, `True` with `False`, deleting statements) and check if tests still pass. Surviving mutants = test gaps. + +### Setup with mutmut + +```toml +# pyproject.toml +[tool.mutmut] +paths_to_mutate = "entrypoint/" +tests_dir = "tests/" +runner = "python -m pytest tests/ -x -q" +``` + +```bash +# Run mutation testing +mutmut run + +# See surviving mutants +mutmut results + +# Inspect a specific mutant +mutmut show 42 +``` + +### Practical Guidance + +- **Target: mutation score above 80%.** Scores above 90% have diminishing returns (equivalent mutants). +- **Focus on business logic** — validators, state machines, parsers. Skip glue code. +- **Use mutation testing to audit AI-generated tests.** This is the most powerful combination: AI writes tests from spec, mutation testing verifies those tests catch real faults. +- **Run on changed files only in CI** (full suite is slow). Full run nightly or pre-release. + +## Test Architecture + +### The Testing Pyramid for Spec-Driven Projects + +| Layer | Proportion | Speed | What it catches | +|---|---|---|---| +| Unit tests | 60-70% | <1ms each | Logic errors, boundary violations, state machine bugs | +| Property-based | 10-15% | ~10ms each | Edge cases humans miss, roundtrip failures, crash inputs | +| Integration | 15-20% | ~100ms each | Component interaction bugs, mock/reality divergence | +| E2E / acceptance | 5-10% | ~1s+ each | Full-chain failures, deployment config issues | + +### Test Isolation Principles + +- **No test depends on another test's state.** Each test sets up its own preconditions. +- **No test depends on execution order.** `pytest-randomly` catches order dependencies. +- **No test touches the real filesystem outside `tmp_path`.** Monkeypatch paths that default to production locations (like `/workspace`). +- **No test makes network calls.** Mock HTTP, subprocess, and socket calls. +- **Integration tests are marked** (`@pytest.mark.integration`) and excluded by default. + +### Fixture Architecture + +```python +# conftest.py — shared fixtures, not test logic + +@pytest.fixture +def minimal_payload(): + """Smallest valid payload — tests shouldn't need more unless testing specific fields.""" + return {"task_id": "test-123", "prompt": "do something", "runtime": {"cli": "claude"}} + +@pytest.fixture +def encode_payload(): + """Helper: dict → base64 string (how the dispatcher passes payloads).""" + def _encode(d): + return base64.b64encode(json.dumps(d).encode()).decode() + return _encode + +# Per-module conftest for module-specific fixtures +# tests/test_dispatcher/conftest.py +@pytest.fixture +def mock_backend(): + """Fake container backend that records calls without Docker.""" + ... +``` + +### Negative Tests Are as Important as Positive Tests + +For every "this works" test, write at least one "this fails correctly" test: + +```python +# Positive: valid payload accepted +def test_p1_valid_payload_loads(): + ... + +# Negative: missing required field rejected +def test_p3_missing_prompt_raises(): + ... + +# Negative: wrong type rejected +def test_p_prompt_wrong_type_raises(): + ... + +# Negative: extra unknown fields are ignored (not rejected) +def test_p_unknown_fields_ignored(): + ... +``` + +## AI Agent Testing Patterns + +### The Two-Phase Rule + +**Never let the same agent write both tests and implementation in one pass.** An agent that writes tests and code together will unconsciously write tests that verify its own broken assumptions. + +The workflow: +1. **Phase 1:** Agent reads spec → writes tests. Human reviews tests against spec. +2. **Phase 2:** Agent (or different agent) reads spec + tests → writes implementation until tests pass. + +### Hidden Test Splits + +Hold back some tests that the implementing agent never sees. Use them as a final validation: + +```python +# tests/test_payload.py — agent sees these during development +def test_p1_load_from_env_var(): ... +def test_p2_missing_payload_exits_1(): ... + +# tests/test_payload_hidden.py — agent never sees these, run post-implementation +# (Marked with a custom marker, excluded from default run) +@pytest.mark.hidden +def test_p1_load_from_file_fallback(): ... +@pytest.mark.hidden +def test_p_concurrent_payload_loads(): ... +``` + +### Regression Tests from Real Bugs + +Every bug found in production or during integration testing becomes a permanent test case: + +```python +def test_regression_crlf_corruption(): + """Regression: smtp-oauth-relay converted \\r\\n to \\n, breaking quoted-printable. + Fixed by as_bytes(policy=email_policy.SMTP). See memory/gotchas-email-relay.md.""" + ... +``` + +These are the highest-value tests because they catch proven failure modes. + +## Test Quality Metrics + +### What to Measure + +| Metric | Target | Why | +|---|---|---| +| Spec coverage | 100% | Every numbered requirement has at least one test | +| Mutation score | >80% | Tests catch real faults, not just inflate coverage | +| Line coverage | >90% | Baseline hygiene (necessary but not sufficient) | +| Test speed | <10s total | Fast enough for pre-commit hooks | +| Assertion density | >1 per test | Tests that don't assert don't catch anything | + +### What NOT to Measure + +- **100% line coverage as a goal.** Chasing 100% leads to tests that exercise code paths without meaningful assertions. +- **Test count.** 50 well-targeted tests beat 200 shallow ones. +- **Test-to-code ratio.** The ratio depends on the module's complexity, not a universal number. + +## CI Integration + +### Pre-commit (Every Commit) +```bash +pytest tests/ -x -q --tb=short -m "not integration" +``` + +### PR Validation (Every Push) +```bash +# Unit + property-based tests +pytest tests/ -q --tb=short -m "not integration" +# Mutation testing on changed files only +mutmut run --paths-to-mutate="$(git diff --name-only main... | grep '.py$' | tr '\n' ',')" +``` + +### Nightly +```bash +# Full mutation testing +mutmut run +# Integration tests (requires Docker) +pytest tests/ -m integration +# Hidden test validation +pytest tests/ -m hidden +``` + +## Anti-Patterns + +### Tests that mirror implementation +**Symptom:** Test asserts that function calls happen in a specific order, using mock.assert_has_calls with exact sequences. Breaks on any refactor. +**Fix:** Test behaviour (inputs → outputs), not implementation details. + +### Tests without assertions +**Symptom:** `test_it_runs()` calls the function and checks it doesn't crash. No assertion on the result. +**Fix:** Every test must assert something specific about the output, side effects, or raised exceptions. + +### Overmocking +**Symptom:** Every dependency is mocked. Tests pass but integration fails because mocks don't match real behaviour. +**Fix:** Mock at the boundary (subprocess, HTTP, filesystem), not between your own modules. Use real objects for internal dependencies. + +### Fragile tests +**Symptom:** Tests break when unrelated code changes. Usually caused by asserting on implementation details, shared mutable state, or execution order. +**Fix:** Test the public interface. Use fixtures for setup. Isolate each test completely. + +### Testing private methods +**Symptom:** Tests import `_internal_helper` and test it directly. These break on any refactor. +**Fix:** Test through the public API. If a private method is complex enough to need its own tests, it should probably be a separate module with a public interface. diff --git a/hooks/require-plan-file.sh b/hooks/require-plan-file.sh new file mode 100755 index 0000000..34e8876 --- /dev/null +++ b/hooks/require-plan-file.sh @@ -0,0 +1,18 @@ +#!/bin/bash +# Blocks ExitPlanMode unless a *-PLAN.md file exists in the current working directory. +# Fired via PreToolUse hook on ExitPlanMode. +# Input: JSON on stdin with session context including "cwd". + +INPUT=$(cat) +CWD=$(echo "$INPUT" | python3 -c "import sys, json; print(json.load(sys.stdin).get('cwd', ''))" 2>/dev/null) + +if [ -z "$CWD" ]; then + exit 0 # Can't determine cwd, allow through +fi + +if ls "$CWD"/*-PLAN.md 2>/dev/null | grep -q .; then + exit 0 # Plan file exists, allow exit +fi + +echo "No PLAN.md file found in $CWD. Before exiting plan mode, write the complete plan to a file named [MILESTONE]-[PURPOSE]-PLAN.md in the project root (e.g. M2-auth-PLAN.md)." >&2 +exit 2 diff --git a/memory/decisions.md b/memory/decisions.md new file mode 100644 index 0000000..a8c62f2 --- /dev/null +++ b/memory/decisions.md @@ -0,0 +1,53 @@ +# Architecture & Design Decisions + +## Knowledge Pipeline: Separate /reflect-logs from /reflect + +`/reflect-logs` handles continuous log processing; `/reflect` handles milestone reflections. Different purpose, different cadence — combining them would overcomplicate the milestone skill. + +## Knowledge Pipeline: MD5 for reflection state, git SHAs for distill state + +Log files may not be committed when reflected on, so MD5 of file content is the right identity. `/distill-best-practices` works across committed repos, so git SHAs are appropriate there. + +## Knowledge Pipeline: Pruning happens in /log, not /reflect-logs + +`/log` runs most frequently (every session end), so it naturally keeps the log directory clean as a side effect. + +## Knowledge Pipeline: /distill-best-practices is interactive + +Cross-project convention changes need human judgment. The skill proposes updates and waits for approval before writing. + +## Session Logs: Timestamp-based IDs (HHMMSS) + +Human-readable, naturally sorted, no external dependencies. Format: `YYYY-MM-DD.HHMMSS.md`. + +## Linting: Formatter exit codes vs hook exit codes + +Formatter scripts exit 1 on lint errors. The dispatcher hook decides the final exit code (exit 2 for PostToolUse feedback). This separates formatter logic from hook semantics — same scripts work for both PostToolUse and pre-commit. + +## Linting: Checkpoint via git hash-object with .pre-lint sidecar + +Fast (~1ms), no commits or stash needed, orphan blobs auto-GC'd. Falls back to `cp` outside git repos. + +## Linting: Project opt-in via formatter symlinks + +Projects opt in by having a `formatters/` directory with symlinks back to canonical scripts. Zero-config, visible in `ls`, no parsing needed. The hook walks up the directory tree to find `formatters/`. + +## CLAUDE.md: Remove technology-specific sections from root + +Ansible and Helm sections removed from root CLAUDE.md — already covered with more detail in `best-practices/ansible.md` and `best-practices/helm.md`. Technology-specific practices belong in best-practices, not root guidelines. + +## context-load: Walk upward collecting context files + +Walks from cwd upward collecting CLAUDE.md, CONTEXT.md, MEMORY.md, BESTPRACTICES.md at each level. Gives hierarchical context inheritance — highest ancestor provides global guidelines, project dir provides specifics. + +## context-load: Dedup via readlink -f + +Root CLAUDE.md is a symlink to claude-foundations. Without dedup it would load twice. `readlink -f` resolves all symlinks before comparison. + +## context-load: Tree depth 3 + +Deep enough to show project structure without overwhelming output. Applied at every CLAUDE.md location. + +## CONTEXT.md follows MEMORY.md pattern + +Thin index + `context/` folder. Consistency with MEMORY.md. CONTEXT.md focuses on active work for agent orientation; MEMORY.md on accumulated learnings. diff --git a/memory/gotchas-skills.md b/memory/gotchas-skills.md new file mode 100644 index 0000000..fcbf0fb --- /dev/null +++ b/memory/gotchas-skills.md @@ -0,0 +1,13 @@ +# Skills Gotchas + +## Broken symlinks cause silent failures under set -e + +**Symptom:** install.sh fails with exit 1 on a broken symlink. +**Cause:** `readlink -f` on a broken symlink returns an empty string, causing comparison failure under `set -e`. +**Fix:** Remove stale symlinks before re-running install. When skills move directories (e.g., from `~/dev/claude/custom-claude-skills/` to `~/dev/claude/projects/custom-claude-skills/`), old symlinks break. + +## Skills created mid-session are not available as slash commands + +**Symptom:** A newly created skill doesn't appear when you type `/skillname`. +**Cause:** Skills are discovered at session start, not dynamically during the session. +**Fix:** Start a new Claude Code session to pick up newly created skills. diff --git a/memory/log/2026-03-13.115251.md b/memory/log/2026-03-13.115251.md new file mode 100644 index 0000000..b914fdb --- /dev/null +++ b/memory/log/2026-03-13.115251.md @@ -0,0 +1,25 @@ +# Session Log — 2026-03-13 + +## Summary +Cleaned up the root CLAUDE.md (removed duplicated Ansible/Helm sections, consolidated validation guidance, fixed best-practices references) and built a `context-load` / `start-claude` script pair for automated session context gathering. Introduced the CONTEXT.md pattern for future independent agent work. + +## Decisions +- Decision: Remove Ansible and Helm sections from root CLAUDE.md — Rationale: already covered with more detail in `best-practices/ansible.md` and `best-practices/helm.md`; technology-specific practices belong in best-practices, not the root guidelines +- Decision: Fold "Validate Before Deploying" into Process Principles — Rationale: was duplicated content; the examples fit naturally in the existing bullet point +- Decision: context-load walks upward from cwd collecting CLAUDE.md, CONTEXT.md, MEMORY.md, BESTPRACTICES.md — Rationale: gives hierarchical context inheritance; highest ancestor provides global guidelines, project dir provides specifics +- Decision: Tree depth 3 from every CLAUDE.md location — Rationale: deep enough to show project structure without overwhelming output +- Decision: Dedup loaded files via `readlink -f` — Rationale: root CLAUDE.md is a symlink to claude-foundations; without dedup it would load twice +- Decision: CONTEXT.md follows MEMORY.md pattern (thin index + `context/` folder) — Rationale: consistency; CONTEXT.md focuses on active work for agent orientation, MEMORY.md on accumulated learnings + +## Key Context +- `~/dev/claude/CLAUDE.md` is a symlink to `~/dev/claude/projects/claude-foundations/CLAUDE.md` — this is intentional, claude-foundations is the canonical source +- `context-load` output is passed via `--append-system-prompt` by the `start-claude` wrapper +- Both scripts symlinked into `~/sbin/` +- CONTEXT.md is intended for future Docker-based independent agent operation — each agent gets full context load, starting prompt points to relevant CONTEXT.md entry +- git-status-report output is ANSI-stripped before inclusion in context +- Also committed the previously uncommitted linting system (18 files) from the earlier session + +## Process Notes +- Session was efficient — cleanups and script creation done in parallel with minimal iteration +- The context-load smoke tests from different directories caught the symlink dedup working correctly +- Previous session's linting work was uncommitted — worth running `git-status-report` at session start to catch this pattern diff --git a/memory/log/2026-03-15.225345.md b/memory/log/2026-03-15.225345.md new file mode 100644 index 0000000..db3fede --- /dev/null +++ b/memory/log/2026-03-15.225345.md @@ -0,0 +1,22 @@ +# Session Log — 2026-03-15 + +## Summary +Fixed the `/distill-best-practices` skill which was broken due to hardcoded and relative paths in `!`command`` blocks. Replaced all paths with `CLAUDE_PROJECT_ROOT` env var for portability, added a `find-project-root` helper script, and updated `settings.yaml` to use relative paths. Also fixed the same relative-path issue in `/log` and `/reflect-logs` skills. + +## Decisions +- Decision: Use `CLAUDE_PROJECT_ROOT` env var for all cross-project path resolution in skills — Rationale: Makes skills shareable with colleagues; hardcoded `~/dev/claude/` paths are user-specific and relative `../` paths break depending on CWD +- Decision: Add Step 0 (detect project root) as runtime fallback in distill skill — Rationale: Skills should degrade gracefully if env var isn't set; Claude can walk up the directory tree to find highest CLAUDE.md +- Decision: `settings.yaml` paths relative to CLAUDE_PROJECT_ROOT, not absolute — Rationale: Portability; `projects_dir: projects` instead of `~/dev/claude/projects` +- Decision: Added `extra_projects` section to settings.yaml for projects outside `projects_dir` — Rationale: `small-scripts` lives at root level, not under `projects/` + +## Gotchas Discovered +- **[skills]** Symptom: `/distill-best-practices` failed with sandbox error — `cat ../claude-foundations/...` resolved to `/home/paul/dev/claude-foundations/` (outside sandbox) when CWD was `~/dev/claude/` — Fix: Replace all relative and hardcoded paths with `${CLAUDE_PROJECT_ROOT}` env var +- **[skills]** Symptom: `!`command`` blocks can't use `$()` command substitution — Fix: Use env var expansion (`${CLAUDE_PROJECT_ROOT}`) which works, and fall back to runtime detection in skill instructions +- **[skills]** Symptom: Claude Code Bash tool doesn't persist `export` across `;`-separated commands in the same invocation when the variable is used in file path arguments — Fix: Use `bash -c '...'` wrapper or ensure var is in shell profile + +## Key Context +- `settings.yaml` now tracks 6 projects under `distill.projects` plus `small-scripts` under `extra_projects` +- Added projects: `agent-runtimes`, `claude-foundations`, `cluster-apps/octopus-deploy`, `hugo-accelerator` +- `find-project-root` script created at `claude-foundations/scripts/find-project-root` — walks up from CWD to find highest CLAUDE.md +- `CLAUDE_PROJECT_ROOT` export added to `~/.bashrc` +- Three skills updated: `distill-best-practices` (full rewrite of paths), `log` and `reflect-logs` (settings fallback path) diff --git a/memory/process-lessons.md b/memory/process-lessons.md new file mode 100644 index 0000000..7b22da0 --- /dev/null +++ b/memory/process-lessons.md @@ -0,0 +1,29 @@ +# Process Lessons + +## Always run git-status-report at session start + +Previous sessions may leave uncommitted work. Running `git-status-report` (or checking git status) at session start catches this pattern before it compounds. + +## Use plan mode for architectural tasks + +Plan mode (Shift+Tab twice) works well for designing systems before implementing. Explore existing patterns, design the architecture, get approval, then execute. Implementation is straightforward when the plan is thorough. + +## PostToolUse exit code 2 feeds errors back to Claude + +Exit code 2 from a PostToolUse hook sends stderr content back to Claude as feedback without blocking the edit. Exit 0 = silent success. Exit 1 = hard block. + +## Agent-type hooks are read-only + +Hooks with `type: "command"` cannot use Edit/Write tools. Lint fixing from hooks must happen via the Agent tool subagent, not directly in hooks. + +## All hooks in a matcher array run in parallel + +Multiple hooks registered for the same matcher execute concurrently, not sequentially. Design hooks to be independent. + +## SSH key for ai_enablement is password-protected + +Needs ssh-agent loaded before git push to Gitea. If push hangs, check that the key is added to the agent. + +## Batch parallel file creation for efficiency + +Creating many independent files in a single Write batch (e.g., 11 best-practices files at once) is significantly faster than sequential creation. diff --git a/memory/runbook-new-argocd-app.md b/memory/runbook-new-argocd-app.md new file mode 100644 index 0000000..2c3e34d --- /dev/null +++ b/memory/runbook-new-argocd-app.md @@ -0,0 +1,222 @@ +# Runbook: Adding a New ArgoCD-Driven App to the Cluster + +Step-by-step procedure for deploying a new application to the homelab Kubernetes cluster via ArgoCD GitOps. Derived from deploying Octopus Deploy (2026-03-13) and existing apps (Authelia, Homepage, Email Relay). + +## Decision: In-repo vs Separate Repo + +| Approach | When to use | Example | +|----------|-------------|---------| +| **In cluster-bootstrap** (`platform//`) | Tightly coupled to cluster lifecycle, simple apps | Authelia, Homepage, Email Relay | +| **Separate Gitea repo** | Independent lifecycle, external contributors, large/complex apps | Octopus Deploy | + +Most apps go in cluster-bootstrap under `platform/`. Use a separate repo only when there's a clear reason. + +## Step 1: Create Manifests + +### Directory structure (in-repo) + +``` +platform// +├── kustomization.yaml +├── namespace.yaml +├── statefulset.yaml or deployment.yaml +├── service.yaml +├── ingressroute.yaml # If externally accessible +├── ksops-generator.yaml # If app has secrets +└── -secret.sops.yaml # SOPS-encrypted secrets +``` + +### Directory structure (separate repo) + +``` +/ +├── .sops.yaml # SOPS encryption rules (same age key as cluster-bootstrap) +├── .gitignore # local_secrets/ +├── kustomization.yaml +├── namespace.yaml +├── / # Subdirectories per component +│ ├── statefulset.yaml +│ └── service.yaml +├── ksops-generator.yaml +├── *-secret.sops.yaml +├── local_secrets/ # Gitignored plaintext secrets for setup +└── scripts/ + └── setup.sh # Secret generation + SOPS encryption +``` + +### Kustomization pattern + +```yaml +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: + - namespace.yaml + - /statefulset.yaml + - /service.yaml + - ingressroute.yaml +generators: + - ksops-generator.yaml # Only if secrets exist +``` + +### KSOPS generator pattern + +```yaml +apiVersion: viaduct.ai/v1 +kind: ksops +metadata: + name: -secret-generator + annotations: + config.kubernetes.io/function: | + exec: + path: ksops +files: + - ./-secret.sops.yaml +``` + +### SOPS config (separate repo only) + +```yaml +creation_rules: + - path_regex: .*secret.*\.yaml$ + encrypted_regex: "^(data|stringData)$" + age: >- + age1edc9agzzs8cngd2rsvfhm8aeucnlq2clmj36jh0rrkwuj073fyssr0u4x9 +``` + +In-repo apps inherit from the cluster-bootstrap root `.sops.yaml`. + +## Step 2: Namespace Checklist + +```yaml +apiVersion: v1 +kind: Namespace +metadata: + name: + labels: + # Only if app needs privileged containers (DinD, host networking, etc): + pod-security.kubernetes.io/enforce: privileged + pod-security.kubernetes.io/audit: privileged + pod-security.kubernetes.io/warn: privileged +``` + +**Always check:** Does any container need `securityContext.privileged: true` or host-level access? If yes, add PodSecurity labels. Forgetting this causes silent pod creation failures. + +## Step 3: Secrets + +1. Create plaintext secret YAML in `local_secrets/` (gitignored) +2. Encrypt with SOPS: `sops --encrypt local_secrets/-secret.yaml > -secret.sops.yaml` +3. Reference in `ksops-generator.yaml` +4. Secret filenames **must** contain `secret` (triggers SOPS rules) +5. Non-secret files must **not** contain `secret` in their name + +## Step 4: Traefik IngressRoute (if externally accessible) + +```yaml +apiVersion: traefik.io/v1alpha1 +kind: IngressRoute +metadata: + name: + namespace: +spec: + entryPoints: + - websecure + routes: + - match: Host(`.oreillyit.nz`) + kind: Rule + services: + - name: + port: 80 +``` + +If the app needs Authelia protection, add a middleware reference. If the app handles its own auth, omit it. + +## Step 5: ArgoCD Application + +Create `bootstrap/apps/.yaml` in cluster-bootstrap: + +```yaml +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: + namespace: argocd + finalizers: + - resources-finalizer.argocd.argoproj.io +spec: + project: default + source: + # In-repo: + repoURL: http://gitea.bootstrap.homelab.internal/homelab/cluster-bootstrap.git + targetRevision: main + path: platform/ + # Separate repo: + # repoURL: http://gitea.bootstrap.homelab.internal/homelab/.git + # targetRevision: main + # path: . + destination: + server: https://kubernetes.default.svc + namespace: + syncPolicy: + automated: + prune: true + selfHeal: true + syncOptions: + - ServerSideApply=true + - CreateNamespace=true +``` + +After pushing, the root app may not pick it up immediately. Force refresh: +```bash +kubectl annotate application root -n argocd argocd.argoproj.io/refresh=normal --overwrite +``` + +## Step 6: Caddy Reverse Proxy (if externally accessible) + +Add to `external/vps/inventory.yml` under `caddy_sites`: + +```yaml +- domain: ".oreillyit.nz" + type: public + upstream: "https://10.111.1.100:443" # Traefik VIP + upstream_tls_insecure: true # Traefik uses self-signed certs + comment: "" +``` + +Deploy: +```bash +ansible-playbook -i external/vps/inventory.yml external/vps/playbook.yml --tags compose +``` + +Caddy automatically obtains a Let's Encrypt certificate on first request. + +## Step 7: DNS + +Add a Cloudflare A record for `.oreillyit.nz` pointing to the VPS IP (`43.224.182.153`). Use orange cloud (proxied) for most services. + +## Step 8: Verify + +```bash +# ArgoCD status +kubectl get application -n argocd + +# Pod health +kubectl get pods -n + +# PVC status (if applicable) +kubectl get pvc -n + +# External access +curl -sk -o /dev/null -w "%{http_code}" https://.oreillyit.nz/ +``` + +## Common Pitfalls + +| Pitfall | Symptom | Prevention | +|---------|---------|------------| +| Missing PodSecurity labels | `violates PodSecurity "baseline"` in StatefulSet events | Always check if any container needs privileged mode | +| Container runs as non-root | Permission denied on writable dirs | Check image docs / `docker inspect` before writing manifests | +| ArgoCD polling delay | New app doesn't appear after push | Annotate root app with `refresh=normal` | +| StatefulSet CrashLoopBackOff | Updated spec not applied to pod | Delete the crashlooping pod manually | +| Gitea repo permissions | `not authorized to write` on push | Grant collaborator access before pushing | +| Proxmox CSI lock contention | PVC provisioning failures with lock timeout | Transient — CSI retries automatically | +| Caddy not configured | Connection refused or SSL error on domain | Add site to inventory.yml and deploy with Ansible | diff --git a/memory/script-install-hooks.md b/memory/script-install-hooks.md index 025897f..097f51b 100644 --- a/memory/script-install-hooks.md +++ b/memory/script-install-hooks.md @@ -19,7 +19,7 @@ One-time setup. Re-run after adding new hooks. 1. Iterates over `hooks/*.sh` 2. Creates symlinks in `~/.claude/hooks/` (force-overwrites existing) -3. Prints the JSON config for `~/.claude/settings.json` covering PreCompact and PostToolUse matchers +3. Prints the JSON config for `~/.claude/settings.json` covering PreCompact, PostToolUse, and PreToolUse matchers ## Gotchas diff --git a/memory/script-require-plan-file.md b/memory/script-require-plan-file.md new file mode 100644 index 0000000..7a390af --- /dev/null +++ b/memory/script-require-plan-file.md @@ -0,0 +1,45 @@ +--- +name: require-plan-file +description: PreToolUse hook that blocks ExitPlanMode unless a *-PLAN.md file exists in the current project root +type: reference +--- + +# script: require-plan-file + +**Location:** `hooks/require-plan-file.sh` +**Symlinked to:** `~/.claude/hooks/require-plan-file.sh` + +## Purpose + +Enforces the plan-file convention: Claude cannot exit plan mode until it has written a `[MILESTONE]-[PURPOSE]-PLAN.md` file in the project root. + +## How it works + +Fires as a `PreToolUse` hook on `ExitPlanMode`. Reads `cwd` from the JSON input on stdin, checks for any `*-PLAN.md` file in that directory: + +- **File found** → exits 0 (allows ExitPlanMode to proceed) +- **No file found** → exits 2 (blocks ExitPlanMode, stderr message injected into Claude's context) + +The exit 2 message tells Claude exactly what to do, so it self-corrects and writes the file before retrying. + +## Settings.json config + +```json +"PreToolUse": [ + { + "matcher": "ExitPlanMode", + "hooks": [{ "type": "command", "command": "~/.claude/hooks/require-plan-file.sh" }] + } +] +``` + +## Naming convention + +`[MILESTONE]-[PURPOSE]-PLAN.md` — e.g. `M2-auth-PLAN.md`, `M3-monitoring-PLAN.md`, `initial-setup-PLAN.md` + +Plan files are committed to the project repo as a persistent record of planning decisions. + +## Gotchas + +- `cwd` in the hook input is the Claude session's working directory, not necessarily the repo root — works correctly as long as Claude is `cd`'d into the project root (the standard pattern). +- If `cwd` cannot be parsed from stdin, the hook exits 0 (fails open) to avoid blocking legitimate use. diff --git a/memory/skill-context-load.md b/memory/skill-context-load.md new file mode 100644 index 0000000..5155d5e --- /dev/null +++ b/memory/skill-context-load.md @@ -0,0 +1,28 @@ +# skill: /context-load + +**Location:** `custom-claude-skills/skills/context-load/SKILL.md` +**Symlinked to:** `~/.claude/skills/context-load` + +## Purpose + +Reloads project context into the conversation after `/clear` or when context has been lost mid-session. Equivalent to the context that `start-claude` injects at session start via `--append-system-prompt`. + +## Usage + +``` +/context-load +``` + +Run from any project directory. The skill will gather context from cwd upward, just like the `context-load` script does at launch. + +## How it works + +- Uses `!`context-load`` to run the `scripts/context-load` script at skill load time +- The script output (CLAUDE.md files, trees, CONTEXT.md, MEMORY.md, BESTPRACTICES.md, git status) is injected directly into the skill prompt +- Claude reads and internalizes the output, then confirms what it loaded + +## Gotchas + +- Depends on `context-load` being on `$PATH` (symlinked to `~/sbin/context-load`) +- Output size scales with the number of projects in the directory hierarchy — deep nesting or large index files may use significant tokens +- Only loads index files, not topic files from `memory/` or `context/` — Claude must use Read tool for those if needed diff --git a/scripts/find-project-root b/scripts/find-project-root new file mode 100755 index 0000000..ef627c7 --- /dev/null +++ b/scripts/find-project-root @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +# Walk up from CWD to find the highest directory containing CLAUDE.md. +# Usage: +# eval "$(scripts/find-project-root)" # sets CLAUDE_PROJECT_ROOT +# export CLAUDE_PROJECT_ROOT="$(scripts/find-project-root --print)" +# +# With --print: outputs just the path (for subshell capture). +# Without flags: outputs an export statement (for eval). + +set -euo pipefail + +root="" +dir="$(pwd)" +while [[ "$dir" != "/" ]]; do + if [[ -f "$dir/CLAUDE.md" ]]; then + root="$dir" + fi + dir="$(dirname "$dir")" +done + +if [[ -z "$root" ]]; then + echo "ERROR: No CLAUDE.md found in any parent directory of $(pwd)" >&2 + exit 1 +fi + +if [[ "${1:-}" == "--print" ]]; then + echo "$root" +else + echo "export CLAUDE_PROJECT_ROOT=\"$root\"" +fi diff --git a/scripts/install-hooks.sh b/scripts/install-hooks.sh index 6da3a8d..b2f716f 100755 --- a/scripts/install-hooks.sh +++ b/scripts/install-hooks.sh @@ -40,6 +40,12 @@ cat <<'EOF' "matcher": "Edit|Write|MultiEdit", "hooks": [{ "type": "command", "command": "~/.claude/hooks/post-edit-lint.sh", "timeout": 30, "statusMessage": "Formatting and linting..." }] } + ], + "PreToolUse": [ + { + "matcher": "ExitPlanMode", + "hooks": [{ "type": "command", "command": "~/.claude/hooks/require-plan-file.sh" }] + } ] } } diff --git a/settings.yaml b/settings.yaml index 9b37c93..aa645ba 100644 --- a/settings.yaml +++ b/settings.yaml @@ -8,7 +8,13 @@ reflect: max_logs_per_run: 10 # Process at most N logs per invocation distill: - projects_dir: ~/dev/claude/projects + projects_dir: projects # Relative to CLAUDE_PROJECT_ROOT projects: # Projects to scan for memory changes + - agent-runtimes + - claude-foundations + - cluster-apps/octopus-deploy - cluster-bootstrap - custom-claude-skills + - hugo-accelerator + extra_projects: # Projects outside projects_dir (paths relative to CLAUDE_PROJECT_ROOT) + - path: small-scripts