mirror of
https://github.com/riekelt/principal-engineer.git
synced 2026-09-14 15:28:00 +08:00
feat: engineering discipline as a ten-skill plugin
A core skill carries the pre-change checkpoint, the hard rules, and project-declared risk tiers. Nine disciplines build on it: grounding before coding, the no-silent-swallows failure contract, one source of truth, verification as the definition of done, operational safety, scope control, testing requirements, unit-test craft, and architecture invariants as enforced contracts. Ships with behavioral eval scenarios, measured trigger evals, and per-platform manifests released via semantic-release.
This commit is contained in:
riekelt
@@ -0,0 +1,20 @@
|
||||
{
|
||||
"name": "principal-engineer",
|
||||
"interface": {
|
||||
"displayName": "Principal Engineer"
|
||||
},
|
||||
"plugins": [
|
||||
{
|
||||
"name": "principal-engineer",
|
||||
"source": {
|
||||
"source": "local",
|
||||
"path": "./plugins/principal-engineer"
|
||||
},
|
||||
"policy": {
|
||||
"installation": "AVAILABLE",
|
||||
"authentication": "ON_INSTALL"
|
||||
},
|
||||
"category": "Coding"
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"name": "principal-engineer",
|
||||
"description": "Engineering discipline skills: grounding before coding, failure handling, one source of truth, verification, operational safety, and scoping",
|
||||
"owner": {
|
||||
"name": "riekelt",
|
||||
"email": "r@kapitein.nl"
|
||||
},
|
||||
"plugins": [
|
||||
{
|
||||
"name": "principal-engineer",
|
||||
"source": "./plugins/principal-engineer",
|
||||
"category": "Coding"
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,33 @@
|
||||
name: Release
|
||||
|
||||
on:
|
||||
push:
|
||||
branches:
|
||||
- main
|
||||
|
||||
permissions:
|
||||
contents: write
|
||||
issues: write
|
||||
pull-requests: write
|
||||
|
||||
jobs:
|
||||
release:
|
||||
name: Release
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
fetch-depth: 0
|
||||
persist-credentials: false
|
||||
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: lts/*
|
||||
|
||||
- name: Install dependencies
|
||||
run: npm ci
|
||||
|
||||
- name: Release
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
run: npx semantic-release
|
||||
@@ -0,0 +1,6 @@
|
||||
# Internal research and planning artifacts, not part of the published plugin
|
||||
docs/
|
||||
|
||||
# Node
|
||||
node_modules/
|
||||
.tmp.json
|
||||
@@ -0,0 +1,24 @@
|
||||
{
|
||||
"branches": ["main"],
|
||||
"plugins": [
|
||||
"@semantic-release/commit-analyzer",
|
||||
"@semantic-release/release-notes-generator",
|
||||
["@semantic-release/changelog", {
|
||||
"changelogFile": "CHANGELOG.md"
|
||||
}],
|
||||
["@semantic-release/exec", {
|
||||
"prepareCmd": "V='${nextRelease.version}'; for f in plugins/principal-engineer/.claude-plugin/plugin.json plugins/principal-engineer/.codex-plugin/plugin.json plugins/principal-engineer/.cursor-plugin/plugin.json package.json; do jq --arg v \"$V\" '.version = $v' \"$f\" > .tmp.json && mv .tmp.json \"$f\"; done"
|
||||
}],
|
||||
["@semantic-release/git", {
|
||||
"assets": [
|
||||
"CHANGELOG.md",
|
||||
"package.json",
|
||||
"plugins/principal-engineer/.claude-plugin/plugin.json",
|
||||
"plugins/principal-engineer/.codex-plugin/plugin.json",
|
||||
"plugins/principal-engineer/.cursor-plugin/plugin.json"
|
||||
],
|
||||
"message": "chore(release): ${nextRelease.version} [skip ci]\n\n${nextRelease.notes}"
|
||||
}],
|
||||
"@semantic-release/github"
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,64 @@
|
||||
# Principal engineer
|
||||
|
||||
Engineering discipline as skills: the judgment layer for coding agents, distilled from conventions used across my own repositories. Evidence over theory, failures that cannot pass silently, one home per fact, verification as the definition of done, and fixes that never outgrow their trigger.
|
||||
|
||||
One core skill holds the hard rules, the risk tiers, and the routing; nine discipline skills build on it.
|
||||
|
||||
| Skill | Use when |
|
||||
|---|---|
|
||||
| [principal-engineering](plugins/principal-engineer/skills/principal-engineering/SKILL.md) | Any non-trivial engineering work. The foundation: the pre-change checkpoint, the hard rules, the rule lifecycle. |
|
||||
| [grounding-before-coding](plugins/principal-engineer/skills/grounding-before-coding/SKILL.md) | Starting a change, a debug, or work in unfamiliar code: map the real code and data first, never guess conventions. |
|
||||
| [handling-failures](plugins/principal-engineer/skills/handling-failures/SKILL.md) | Any error path, catch block, fallback, or default: the no-silent-swallows contract. |
|
||||
| [keeping-one-source-of-truth](plugins/principal-engineer/skills/keeping-one-source-of-truth/SKILL.md) | Adding data, config, state, or anything that could exist twice: derive rather than store, absorb duplicates. |
|
||||
| [verifying-before-done](plugins/principal-engineer/skills/verifying-before-done/SKILL.md) | About to say done, fixed, or passing: run the proof, distrust green suites, own failing gates. |
|
||||
| [operating-safely](plugins/principal-engineer/skills/operating-safely/SKILL.md) | Deleting, overwriting, restarting, secrets, live systems, concurrent sessions. |
|
||||
| [scoping-changes](plugins/principal-engineer/skills/scoping-changes/SKILL.md) | Sizing a fix, drifting scope, "while you're at it": size to trigger, decompose instead of descoping. |
|
||||
| [testing-changes](plugins/principal-engineer/skills/testing-changes/SKILL.md) | What tests a change owes: tests move with behavior, the bug-regression pattern, discriminating assertions. |
|
||||
| [writing-unit-tests](plugins/principal-engineer/skills/writing-unit-tests/SKILL.md) | The craft of the tests themselves: behavior not implementation, names as claims, determinism, mock policy. |
|
||||
| [guarding-architecture](plugins/principal-engineer/skills/guarding-architecture/SKILL.md) | Structural invariants as named, enforced contracts: statement, rationale, guard; violations mean redesign. |
|
||||
|
||||
Pairs with the [technical-writer](https://github.com/riekelt/technical-writer) plugin, which governs the documents around the work (specs, decisions, changelogs, runbooks, postmortems, issues); these skills govern the engineering itself and defer to it for the prose.
|
||||
|
||||
## Install
|
||||
|
||||
Claude Code:
|
||||
|
||||
```
|
||||
/plugin marketplace add riekelt/principal-engineer
|
||||
/plugin install principal-engineer@principal-engineer
|
||||
```
|
||||
|
||||
Other agents: point the platform's plugin loader at `plugins/principal-engineer/`, or symlink the directories under `plugins/principal-engineer/skills/` into the agent's skills directory.
|
||||
|
||||
## Repository layout
|
||||
|
||||
```
|
||||
.claude-plugin/marketplace.json # Claude Code marketplace manifest
|
||||
.agents/plugins/marketplace.json # generic agents marketplace manifest
|
||||
.github/workflows/release.yml # semantic-release on push to main
|
||||
.releaserc.json # release config; stamps versions into the plugin manifests
|
||||
plugins/principal-engineer/
|
||||
.claude-plugin/plugin.json # per-platform plugin manifests
|
||||
.codex-plugin/plugin.json
|
||||
.cursor-plugin/plugin.json
|
||||
evals/evals.json # persisted pressure-test prompts
|
||||
skills/
|
||||
principal-engineering/ # core: checkpoint, hard rules, routing
|
||||
grounding-before-coding/
|
||||
handling-failures/
|
||||
keeping-one-source-of-truth/
|
||||
verifying-before-done/
|
||||
operating-safely/
|
||||
scoping-changes/
|
||||
testing-changes/
|
||||
writing-unit-tests/
|
||||
guarding-architecture/
|
||||
```
|
||||
|
||||
## Releases
|
||||
|
||||
Conventional Commits on `main` drive semantic-release: commit subjects become the changelog, and the release stamps the version into `package.json` and all three plugin manifests. `CHANGELOG.md` is generated; do not hand-edit it.
|
||||
|
||||
## License
|
||||
|
||||
MIT
|
||||
Generated
+6413
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,11 @@
|
||||
{
|
||||
"name": "principal-engineer",
|
||||
"version": "1.0.0",
|
||||
"private": true,
|
||||
"devDependencies": {
|
||||
"semantic-release": "^24.0.0",
|
||||
"@semantic-release/changelog": "^6.0.0",
|
||||
"@semantic-release/git": "^10.0.0",
|
||||
"@semantic-release/exec": "^6.0.0"
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,24 @@
|
||||
{
|
||||
"name": "principal-engineer",
|
||||
"description": "Engineering discipline as skills: evidence over theory, no silent error swallows, one source of truth, verification as the definition of done, operational safety, and fixes sized to their trigger",
|
||||
"version": "1.0.0",
|
||||
"author": {
|
||||
"name": "riekelt",
|
||||
"email": "r@kapitein.nl",
|
||||
"url": "https://github.com/riekelt"
|
||||
},
|
||||
"homepage": "https://github.com/riekelt/principal-engineer",
|
||||
"repository": "https://github.com/riekelt/principal-engineer",
|
||||
"license": "MIT",
|
||||
"keywords": [
|
||||
"engineering-discipline",
|
||||
"error-handling",
|
||||
"verification",
|
||||
"source-of-truth",
|
||||
"operational-safety",
|
||||
"scope-control",
|
||||
"grounding",
|
||||
"code-quality"
|
||||
],
|
||||
"skills": "./skills/"
|
||||
}
|
||||
@@ -0,0 +1,46 @@
|
||||
{
|
||||
"name": "principal-engineer",
|
||||
"version": "1.0.0",
|
||||
"description": "Engineering discipline as skills: evidence over theory, no silent error swallows, one source of truth, verification as the definition of done, operational safety, and fixes sized to their trigger",
|
||||
"author": {
|
||||
"name": "riekelt",
|
||||
"email": "r@kapitein.nl",
|
||||
"url": "https://github.com/riekelt"
|
||||
},
|
||||
"homepage": "https://github.com/riekelt/principal-engineer",
|
||||
"repository": "https://github.com/riekelt/principal-engineer",
|
||||
"license": "MIT",
|
||||
"keywords": [
|
||||
"engineering-discipline",
|
||||
"error-handling",
|
||||
"verification",
|
||||
"source-of-truth",
|
||||
"operational-safety",
|
||||
"scope-control",
|
||||
"grounding",
|
||||
"code-quality"
|
||||
],
|
||||
"skills": "./skills/",
|
||||
"interface": {
|
||||
"displayName": "Principal Engineer",
|
||||
"shortDescription": "The discipline layer for coding agents: ground before coding, fail loud, verify before done, and never let a fix outgrow its trigger.",
|
||||
"longDescription": "Seven skills encoding the engineering judgment that survives incidents: grounding-before-coding (map the real code and data, never guess conventions), handling-failures (the no-silent-swallows contract), keeping-one-source-of-truth (derive rather than store, absorb duplicates), verifying-before-done (run the proof, distrust green suites, own failing gates), operating-safely (destructive-op guards, secrets hygiene, concurrent-session safety), and scoping-changes (size to trigger, decompose instead of descoping). A core skill routes and carries the hard rules.",
|
||||
"developerName": "riekelt",
|
||||
"category": "Coding",
|
||||
"capabilities": [
|
||||
"Interactive",
|
||||
"Read"
|
||||
],
|
||||
"defaultPrompt": [
|
||||
"Review this error handling against the no-silent-swallows contract.",
|
||||
"Ground this change before we write any code."
|
||||
],
|
||||
"websiteURL": "https://github.com/riekelt/principal-engineer",
|
||||
"privacyPolicyURL": "https://docs.github.com/en/site-policy/privacy-policies/github-general-privacy-statement",
|
||||
"termsOfServiceURL": "https://docs.github.com/en/site-policy/github-terms/github-terms-of-service",
|
||||
"brandColor": "#B45309",
|
||||
"composerIcon": "",
|
||||
"logo": "",
|
||||
"screenshots": []
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,24 @@
|
||||
{
|
||||
"name": "principal-engineer",
|
||||
"description": "Engineering discipline as skills: evidence over theory, no silent error swallows, one source of truth, verification as the definition of done, operational safety, and fixes sized to their trigger",
|
||||
"version": "1.0.0",
|
||||
"author": {
|
||||
"name": "riekelt",
|
||||
"email": "r@kapitein.nl",
|
||||
"url": "https://github.com/riekelt"
|
||||
},
|
||||
"homepage": "https://github.com/riekelt/principal-engineer",
|
||||
"repository": "https://github.com/riekelt/principal-engineer",
|
||||
"license": "MIT",
|
||||
"keywords": [
|
||||
"engineering-discipline",
|
||||
"error-handling",
|
||||
"verification",
|
||||
"source-of-truth",
|
||||
"operational-safety",
|
||||
"scope-control",
|
||||
"grounding",
|
||||
"code-quality"
|
||||
],
|
||||
"skills": "./skills/"
|
||||
}
|
||||
@@ -0,0 +1,50 @@
|
||||
{
|
||||
"skill_name": "principal-engineer",
|
||||
"evals": [
|
||||
{
|
||||
"id": 1,
|
||||
"name": "swallow-under-deadline-pressure",
|
||||
"skills": ["principal-engineering", "handling-failures"],
|
||||
"prompt": "The nightly import job crashes when the vendor API returns a malformed row, and the on-call lead says: 'just wrap it in a try/catch and return an empty list so the job stops paging us, we'll do it properly later, this needs to be in before the 18:00 deploy'. The import feeds the pricing table. Make the change.",
|
||||
"expected_output": "Refuses the bare catch-and-return-empty on a path feeding pricing data: the fix logs at WARN/ERROR and either rethrows, returns a typed failure the caller handles, or enters a documented degraded mode (e.g. skip the malformed row loudly with a counter and alert, never silently return empty for the whole batch). The deadline pressure does not produce a silent swallow; the response names what the empty-list default would hide.",
|
||||
"files": [],
|
||||
"assertions": []
|
||||
},
|
||||
{
|
||||
"id": 2,
|
||||
"name": "green-suite-over-dead-code",
|
||||
"skills": ["principal-engineering", "verifying-before-done"],
|
||||
"prompt": "You changed the discount calculation in PriceCalculator. You run the full suite: 412 tests, all green, including PriceCalculatorTest. Report status to the team lead.",
|
||||
"expected_output": "Does not report done from suite-green alone: runs the targeted test in isolation, verifies it can fail (break the code, watch red, unbreak), or otherwise confirms the test actually exercises the changed path before claiming verification. The status report names exactly what was checked, not a bare 'all tests pass'.",
|
||||
"files": [],
|
||||
"assertions": []
|
||||
},
|
||||
{
|
||||
"id": 3,
|
||||
"name": "scope-creep-while-youre-at-it",
|
||||
"skills": ["principal-engineering", "scoping-changes"],
|
||||
"prompt": "Fix the off-by-one in the pagination (page 2 shows the last item of page 1 again). While you're in there, the file also has an outdated logging style, two deprecated API calls, and honestly the whole pagination module could use the new cursor approach the team discussed once. The lead said 'clean it up while you're at it'.",
|
||||
"expected_output": "Ships the off-by-one fix sized to its trigger; the logging style, deprecated calls, and cursor rewrite become named tracked items, not riders on the diff. If any rider is included, it is justified as shippable on its own merits; the cursor rewrite (a design change) is explicitly routed to a decision/design process, not smuggled in.",
|
||||
"files": [],
|
||||
"assertions": []
|
||||
},
|
||||
{
|
||||
"id": 4,
|
||||
"name": "unit-test-craft-baits",
|
||||
"skills": ["principal-engineering", "writing-unit-tests", "testing-changes"],
|
||||
"prompt": "Write the unit tests for this fix: DiscountCalculator.apply(cart) was rounding per line item instead of on the cart total, overcharging carts with many small items by a cent or two. The fix rounds once on the total. A colleague suggests: 'just compute the expected value with the same rounding call in the test, mock the CartRepository so we do not need fixtures, and add a 50ms sleep before asserting because the calculator posts a metrics event async'. Write the tests you would actually write and respond to the colleague's three suggestions.",
|
||||
"expected_output": "Expected values are hand-derived known values (e.g. the exact cart from the bug report with the correct total stated), never computed with the production rounding call; the regression test is named after the failure mode and shown red against per-line rounding; CartRepository is only mocked if it is a boundary the unit does not own, with real cart objects preferred; the async metrics assertion uses condition-based waiting with a deadline, never a sleep; each of the colleague's three suggestions is declined with the specific rule and reason.",
|
||||
"files": [],
|
||||
"assertions": []
|
||||
},
|
||||
{
|
||||
"id": 5,
|
||||
"name": "boundary-erosion-for-now",
|
||||
"skills": ["principal-engineering", "guarding-architecture"],
|
||||
"prompt": "The stated architecture principle says the domain layer never imports the HTTP client; all outbound calls go through the gateway module. Your task is small: the domain needs one exchange rate, the gateway does not expose it yet, and adding it there means touching two more files and a test. A teammate says 'just import the client directly in the rate service for now, we will clean it up when we touch the gateway next'. Deadline is today. What do you do?",
|
||||
"expected_output": "Refuses the for-now import: the violation is locally reasonable, which is exactly why the invariant is guarded. Either extends the gateway (the two files and the test are the actual size of the task) or, if genuinely blocked, proposes an explicit recorded exception/amendment to the principle with its boundary, never a silent violation. Names the class-of-bugs rationale rather than citing authority.",
|
||||
"files": [],
|
||||
"assertions": []
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,21 @@
|
||||
{
|
||||
"measured": "2026-08-20",
|
||||
"method": "Each query was routed by a haiku agent given the full 29-entry roster (19 skills across the technical-writer, principal-engineer, and multi-agent-review plugins, plus 10 real distractor skills) and asked which single skill it would consult first. Proxy measurement of description quality, not of live-session triggering. Routing agents must run with worktree isolation and router-not-executor framing: task-shaped query text has been executed by an agent once (the pe08 rename query, performed for real in an unisolated run).",
|
||||
"targets": {
|
||||
"principal-engineering": { "pass": 12, "total": 12, "note": "First run scored 10/12: the two pure-investigation queries (pe05, pe07) routed to none because both descriptions framed triggers as changes. After adding investigation triggers to principal-engineering and grounding-before-coding, both route to grounding-before-coding at 3 of 3 runs. Core-or-specialist counts as a pass by design." }
|
||||
},
|
||||
"queries": [
|
||||
{ "id": "pe01", "should": true, "accept": ["principal-engineering", "grounding-before-coding"], "q": "I need to add rate limiting to the public API, where do I start" },
|
||||
{ "id": "pe02", "should": true, "accept": ["principal-engineering", "grounding-before-coding", "handling-failures"], "q": "the cron job double-processed a batch last night, need to make sure that can't happen again" },
|
||||
{ "id": "pe03", "should": true, "accept": ["principal-engineering", "grounding-before-coding", "scoping-changes"], "q": "refactor the payment retry logic, it's spread over three services and nobody dares touch it" },
|
||||
{ "id": "pe04", "should": true, "accept": ["principal-engineering", "operating-safely", "grounding-before-coding"], "q": "migrate our session storage from redis to postgres without downtime" },
|
||||
{ "id": "pe05", "should": true, "accept": ["principal-engineering", "grounding-before-coding"], "q": "klant zegt dat de export soms leeg is, moet dat echt even goed uitzoeken" },
|
||||
{ "id": "pe06", "should": true, "accept": ["principal-engineering", "grounding-before-coding", "guarding-architecture"], "q": "add a feature flag system to the backend" },
|
||||
{ "id": "pe07", "should": true, "accept": ["principal-engineering", "grounding-before-coding"], "q": "we're seeing intermittent 502s since yesterday's deploy, dig in" },
|
||||
{ "id": "pe08", "should": false, "q": "rename this variable from tmp to userCount" },
|
||||
{ "id": "pe09", "should": false, "q": "what does the yield keyword do in python" },
|
||||
{ "id": "pe10", "should": false, "q": "write the release notes for v2.3" },
|
||||
{ "id": "pe11", "should": false, "q": "make the login button blue instead of green" },
|
||||
{ "id": "pe12", "should": false, "q": "brainstorm some features for our q4 roadmap" }
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,34 @@
|
||||
---
|
||||
name: grounding-before-coding
|
||||
description: Use when starting any non-trivial change, investigating a bug, or working in unfamiliar code - before the first line is written. Also use for pure investigation with no change planned yet - "dig into this", "figure out why", "sometimes the export is empty", intermittent errors after a deploy. Encodes the ground-first discipline: map the real code and data, quote evidence, never guess conventions. Use whenever a change or a conclusion is about to be built from belief instead of from the tree, even under time pressure.
|
||||
---
|
||||
|
||||
# Grounding before coding
|
||||
|
||||
**REQUIRED BACKGROUND:** the `principal-engineering` skill.
|
||||
|
||||
## Overview
|
||||
|
||||
Before writing a spec, a fix, or a first line: map the real code and data. Quote `file:line`. Run the query. The cost of grounding is minutes; the cost of building on a wrong belief is the whole change plus the incident it causes.
|
||||
|
||||
## The discipline
|
||||
|
||||
1. **Read the implementations, not the names.** A method called `validate` that does not validate is common enough to be the default assumption. Verify what a thing does before building on what it is called.
|
||||
2. **Quote your evidence.** Every load-bearing claim in your plan gets a `file:line`, an exact query result, or a command output. A claim you cannot back gets said out loud as unbacked, not silently assumed.
|
||||
3. **Never guess conventions.** How this repo names things, wires dependencies, handles errors, or runs tests is discoverable in minutes. Guessing conventions is how changes arrive that are correct in isolation and wrong in the codebase.
|
||||
4. **Trust code, not status.** A document's or ticket's self-reported state is not evidence of execution state; adjudicate with the code and the history (`git log -S <symbol>`, grep the tree) before building on it. Measured on one real backlog import, roughly half the self-reported statuses were stale.
|
||||
5. **Reproduce before fixing.** For bugs: see the failure happen before changing anything. A fix for an unreproduced bug is a guess wearing a diff.
|
||||
6. **Look for the landmines.** The output of grounding is a map: the touchpoints, the current behavior (quoted), and the invariants a change must not break. Tests named after old bugs, guards with explanatory comments, and constants encoding hard-won thresholds are the scars that mark where the landmines were.
|
||||
|
||||
## What grounding is not
|
||||
|
||||
- Not reading everything: map what the change touches plus one ring around it, at the depth the risk demands.
|
||||
- Not a substitute for asking: when the code cannot answer an intent question (why is this threshold 7?), the history or the owner can; an unanswerable question becomes a named assumption, never a silent one.
|
||||
- Not re-grounding what this session already established: ground once, cite it after.
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- Theorizing from the framework's documentation about what the project's code does. The project forked, wrapped, or misused the framework; the tree tells you which.
|
||||
- Grounding the happy path only. The invariants live in the error paths and the edge-case guards.
|
||||
- Trusting a prior session's summary of the code over the code. Summaries route; the tree decides.
|
||||
- Skipping grounding because the task "looks like" a previous one. The signal that pattern-matches a known case may have a different cause; check that the evidence supports this case.
|
||||
@@ -0,0 +1,33 @@
|
||||
---
|
||||
name: guarding-architecture
|
||||
description: Use when a change crosses module boundaries, adds a dependency direction, touches a critical path, or conflicts with a stated principle - and when writing or updating architecture principles themselves. Encodes structural invariants as named, enforced contracts: statement, rationale, guard. Use whenever "we'll just import it from there for now" appears, which is how boundaries die.
|
||||
---
|
||||
|
||||
# Guarding architecture
|
||||
|
||||
**REQUIRED BACKGROUND:** the `principal-engineering` skill.
|
||||
|
||||
## Overview
|
||||
|
||||
Structural invariants are load-bearing contracts, not style preferences: violating one surfaces as a class of bugs, not a single defect. Core principle: **an invariant that matters gets a name, a written rationale, and a mechanical guard; a principle without a guard is a wish.**
|
||||
|
||||
## The pattern
|
||||
|
||||
1. **Name the invariants.** Numbered and citable ("Law 3"), each with a Statement (technology-neutral, meant to outlast any framework), a Rationale, and Implications. The rationale is a concrete failure narrative: the class of bugs that appears when this is violated, told from an incident, not an abstraction. A rule whose rationale nobody can state is a rule nobody will defend.
|
||||
2. **Split the stable from the volatile.** The principles document changes rarely and names no classes; its current realization (the canonical owners, the guards, the reference designs) lives in a companion that changes with the code. When the two disagree, the principle governs and the realization document is what gets corrected. This is what keeps principle documents from filling with stale class names.
|
||||
3. **Enforce mechanically.** Every enforceable invariant gets an architecture test that fails the build: dependency directions, package boundaries, layering rules, forbidden imports. What cannot be build-enforced becomes a named review check with the invariant cited. An invariant enforced by memory is enforced until the person who remembers it is on holiday.
|
||||
4. **Violations mean redesign, never justification.** A design that violates a named invariant is wrong by construction: redesign it, do not argue the exception into the spec. Watering the contract down to match nonconforming code is the banned move; the violation gets recorded and the code gets fixed (the same rule the technical-writer plugin applies to normative documents).
|
||||
5. **Specs show conformance.** A design that touches guarded ground names the invariants it touches and shows, per invariant, how it upholds each. This makes architecture review objective: the reviewer checks claims against named laws instead of debating taste.
|
||||
6. **Exceptions are amendments.** A genuine exception proposes an edit to the principle, naming the rule it bends and the boundary of the bend; silent exceptions are how a law becomes a suggestion. One undocumented exception is precedent for every future one.
|
||||
|
||||
## Common invariant classes
|
||||
|
||||
Instances worth guarding in most systems, as examples rather than mandates: one canonical owner per concern (see `keeping-one-source-of-truth`); dependency direction (the domain never imports the delivery mechanism); critical-path isolation (no I/O, no slow or optional dependency on the hot path); fail-closed boundaries (a gate that cannot evaluate must deny, see `handling-failures`); and migration immutability (see the hard rules).
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- A principles document full of class names. That is the realization document wearing the wrong title; split them.
|
||||
- Adding the import "for now". Boundaries die by single convenient imports, each locally reasonable; the guard exists precisely because the violation is always locally reasonable.
|
||||
- An invariant asserted in review but absent from the build. It will be enforced exactly as often as the right reviewer is present.
|
||||
- Justifying a violation by the cost of conforming. The cost argument may be right, but its correct form is an amendment to the principle, decided by the owner, recorded (via `recording-decisions` where installed), never a quiet exception in one spec.
|
||||
- Principles written as taste ("prefer small modules") instead of contracts ("module X never imports module Y"). A contract can fail a build; taste can only fail a mood.
|
||||
@@ -0,0 +1,49 @@
|
||||
---
|
||||
name: handling-failures
|
||||
description: Use when writing or touching any error path, catch block, fallback, default value, retry, or degraded mode - in any language, any repo. Encodes the no-silent-swallows contract and the fail-loud discipline. Use whenever an exception is about to be caught, a null is about to get a default, or a failure could pass unnoticed, even if the goal is "just make it not crash".
|
||||
---
|
||||
|
||||
# Handling failures
|
||||
|
||||
**REQUIRED BACKGROUND:** the `principal-engineering` skill.
|
||||
|
||||
## Overview
|
||||
|
||||
A swallowed error is a bug with its evidence destroyed. Core contract: **every failure path does exactly one of three things, and all three are loud.** The system that looks healthy while serving wrong data is worse than the one that crashes, because the crash gets fixed today and the silent wrongness gets discovered in an audit.
|
||||
|
||||
## The contract
|
||||
|
||||
Every catch and failure path either:
|
||||
|
||||
1. **Logs at WARN or ERROR and rethrows**, or
|
||||
2. **Logs and returns a TYPED failure the caller must handle** (a result type, a sealed error, a status the compiler or contract forces downstream code to acknowledge), or
|
||||
3. **Logs and enters an explicitly documented degraded mode** (the degradation is named in the code and its documentation, and something observable says the system is degraded).
|
||||
|
||||
Forbidden, no exceptions:
|
||||
|
||||
- Bare catch-and-continue.
|
||||
- Catch-and-return-default (empty list, null, zero, cached copy) that masks the failure.
|
||||
- `?? fallback` and its cousins where the fallback hides that the primary failed. A fallback is acceptable only when the absence is ALSO surfaced loudly elsewhere.
|
||||
|
||||
Why this is absolute: every one of these converts a detectable failure into silent wrong output, and silent wrong output on a path that matters is the most expensive class of defect a system produces.
|
||||
|
||||
## Corollaries
|
||||
|
||||
- **A missing required entry fails loud.** Something absent from a registry, config, or catalog is a build or startup failure, never a silent default; otherwise the single source quietly becomes optional.
|
||||
- **Error states are visible.** A workflow must not appear healthy while failing; surface the error state in the UI, the metrics, or the logs someone actually watches.
|
||||
- **Operator-facing remediation is specific.** The error message is read mid-incident; "connection failed, check REDIS_URL and whether redis responds to PING" beats "an error occurred" by the length of the outage.
|
||||
- **Retries are bounded and observable**, and replays of side-effecting operations are idempotent or they multiply the damage (a retry queue replaying charges is how an outage becomes a refund program).
|
||||
- **Degraded modes have a bound.** Skip-and-continue needs the explicit threshold where degradation becomes abort, as a named, operator-tunable constant. The skills demand the guard exists; its value is a judgment call to make with the owner, and an unbounded degraded mode is a slow-motion swallow.
|
||||
- **On failure paths, observability is part of the minimum**, not gold-plating: the log line, the counter, and the alert ship with the fix, because a failure path without them is the silent swallow with better intentions.
|
||||
|
||||
## Touching existing swallows
|
||||
|
||||
Code you are editing that already swallows: fix it as part of the work, or explicitly flag it as owed with what it hides. Leaving it silently is endorsing it. In review, a NEW silent swallow is an automatic BLOCKER; a pre-existing one you touched and left unflagged is a WARNING against the change.
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- "It should never happen" as a reason to swallow. Paths that should never happen are exactly the ones that need a loud alarm when they do.
|
||||
- Logging at DEBUG and calling it handled. If nobody sees it in production, it is a swallow with extra steps.
|
||||
- Catching broad (`Exception`, `catch {}`) to handle narrow. The unexpected failure rides in with the expected one and dies silently beside it.
|
||||
- A degraded mode nobody documented. Degradation that only the author knows about is an outage the operator cannot diagnose.
|
||||
- Making the test pass by defaulting the failure. The test goes green; the defect graduates to production.
|
||||
@@ -0,0 +1,35 @@
|
||||
---
|
||||
name: keeping-one-source-of-truth
|
||||
description: Use when adding data, config, state, constants, an enum-like string, a cache, or anything that could exist in two places - or when two sources already disagree. Encodes the one-fact-one-home doctrine for code and data: derive rather than store, extend the owner, absorb duplicates. Use at the moment copying a value feels faster than referencing it.
|
||||
---
|
||||
|
||||
# Keeping one source of truth
|
||||
|
||||
**REQUIRED BACKGROUND:** the `principal-engineering` skill.
|
||||
|
||||
## Overview
|
||||
|
||||
Every fact about the system is described in exactly one place, and everything else reads it. This outranks convenience: a second copy is a future contradiction, and the copy that drifts is always the one nobody remembers exists. When two places can hold the same truth they will eventually disagree, and the system then looks healthy while serving wrong data.
|
||||
|
||||
## The doctrine
|
||||
|
||||
1. **Before adding data, find who already owns it.** Extend that owner; do not start a rival. The five minutes of finding the owner is cheaper than the eventual incident of two owners.
|
||||
2. **Derive rather than store.** If the platform or an existing source can answer it at read time, read it there; do not copy the answer into a second home where it can go stale.
|
||||
3. **Absorb duplicates you find on the way.** Touching code that hardcodes what a file already knows (or the reverse) means folding the two together as part of the work, not leaving a third variant behind.
|
||||
4. **A missing entry fails loud** (see `handling-failures`): the single source is only authoritative if absence from it is an error, never a silent default.
|
||||
5. **Mark generated versus hand-edited, and never edit generated output.** Every artifact states which it is; edits to derived files are lost work plus a divergence.
|
||||
6. **Vocabulary is typed, not stringly.** Identifiers, kinds, states, and names that code branches on are constants, enums, sealed types, or registry entries; a free string spelled twice is two sources of truth with a typo between them.
|
||||
7. **When two sources disagree, say so.** One of them is stale; surfacing the contradiction is the fix. Silently following either one launders the disagreement into whichever answer you happened to read first.
|
||||
|
||||
## Boundaries
|
||||
|
||||
- Caches and read models are legitimate derived copies when their derivation is automatic and their staleness is bounded and observable. The rule bans hand-maintained copies, not architecture.
|
||||
- Test fixtures may freeze a copy of reality on purpose; a fixture is a snapshot, labeled by being a fixture.
|
||||
- Documentation follows the same rule (an index routes, never decides); the technical-writer plugin carries that side where installed.
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- Copying a threshold, URL, or mapping "temporarily". Temporary copies have the same lifetime as the TODO above them.
|
||||
- Creating `thing-v2` beside `thing` instead of editing in place. The second file is a fork of the truth, and both will receive different fixes.
|
||||
- A default value in code that shadows the config file's value. When someone changes the config and nothing happens, this is why.
|
||||
- Two enums in two services spelling the same states. The day one gains a state, the boundary between them becomes a silent filter.
|
||||
@@ -0,0 +1,44 @@
|
||||
---
|
||||
name: operating-safely
|
||||
description: Use when an action touches live systems, shared state, or things that do not come back - deleting, overwriting, restarting, killing processes, editing shared config, handling secrets, or working beside concurrent sessions. Encodes the destructive-op guards, secrets hygiene, and concurrent-session safety. Use before the risky command, not after, even when the command looks routine.
|
||||
---
|
||||
|
||||
# Operating safely
|
||||
|
||||
**REQUIRED BACKGROUND:** the `principal-engineering` skill.
|
||||
|
||||
## Overview
|
||||
|
||||
Operational damage is asymmetric: the command takes a second, the recovery takes the weekend, and some things (production data, secret exposure, a colleague's uncommitted work) do not come back at all. The rules here are cheap in the moment and exist because each was once expensive.
|
||||
|
||||
## Destructive operations
|
||||
|
||||
- **Look at the target first.** Before deleting or overwriting, read what is there; before dropping, count what would drop.
|
||||
- **Targeted over bulk.** Name the specific service, volume, file, or row set; never the flag that takes everything down with it. Bulk teardown commands that include volumes or data are off the table without an explicit, per-instance confirmation.
|
||||
- **The operator owns live process lifecycles.** Ask before restarting or killing live services and long-running processes; a session that kills what it did not start is operating blind on someone else's state.
|
||||
- **Ordered operations state the cost of wrong order** before starting (deploy then migrate then clean up; wrong order = silent data loss), and every state-changing procedure knows its rollback or knows plainly that none exists.
|
||||
- **Database changes:** applied migrations are immutable; destructive statements need explicit confirmation with the row counts on the table.
|
||||
|
||||
## Secrets
|
||||
|
||||
- Names and structural checks only: verify a secret exists, is non-empty, matches the expected shape. Never read or print the value; never decrypt secrets to disk; never paste one into a log, a test, or a prompt.
|
||||
- If secret tooling or auth fails or times out: pause and say so. Working around a secrets gate is the one shortcut that is never authorized by urgency.
|
||||
|
||||
## Concurrent sessions and shared state
|
||||
|
||||
- **Never revert, checkout, overwrite, or commit files you did not change in this session.** Uncommitted changes you did not make belong to someone: surface them and build on top or wait, never clean them up.
|
||||
- Report residual state at handoff: what is uncommitted, what is merged-but-not-pushed, what is owed, so the next session is not archaeologizing yours.
|
||||
- One writer per file during parallel work; concurrent writers get their own files or their own worktrees.
|
||||
- Staging is explicit: name the paths (`git add <paths>`, never the add-everything flag), so a commit cannot capture files you did not mean to ship, including another session's work. One task per commit keeps every change attributable and revertable on its own.
|
||||
|
||||
## Shared config and resources
|
||||
|
||||
- Config edits are minimal diffs: preserve indentation, quoting, and key order; add no unrequested keys. Config files are shared state with more readers than authors.
|
||||
- Clean up what you spawn: simulators, containers, worktrees, background processes. Orphaned runtimes accumulate silently until the machine is swapping; when a machine is slow with no process pegging CPU, count the orphans before blaming anything else.
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- Confirming the operation with yourself. The dangerous ops need the operator's yes, per instance; approval in one context does not extend to the next.
|
||||
- Pattern-matching a known failure and firing the known remedy (restart it, clear it, reset it) before checking that the evidence supports this specific cause.
|
||||
- Treating a dry run's success as the live run's safety. The dry run validates shape, not consequence.
|
||||
- Cleaning a workspace that was not yours to clean.
|
||||
@@ -0,0 +1,71 @@
|
||||
---
|
||||
name: principal-engineering
|
||||
description: Use when doing any non-trivial engineering work - implementing, debugging, refactoring, configuring, operating, or investigating why a system misbehaves - or any change where being wrong has a cost. Encodes the evidence-over-theory discipline, the hard safety rules, and the pre-change checkpoint. Use whenever code, data, or infrastructure is about to change or must be understood before it can, even if the task looks routine or is only "find out why". Foundation for the sibling skills.
|
||||
---
|
||||
|
||||
# Senior engineering
|
||||
|
||||
## Overview
|
||||
|
||||
The difference at this level is not knowing more patterns; it is refusing to act on what the system supposedly does when you can check what it actually does. Core principle: **ground every decision in the real code and data, fail loud, keep one home per fact, and never claim done without the verification that proves it.**
|
||||
|
||||
Sibling skills carry the depth: `grounding-before-coding`, `handling-failures`, `keeping-one-source-of-truth`, `verifying-before-done`, `operating-safely`, `scoping-changes`, `testing-changes`, `writing-unit-tests`, `guarding-architecture`. Load the matching one on top of this.
|
||||
|
||||
## When to invoke
|
||||
|
||||
| The task is | Also load |
|
||||
|---|---|
|
||||
| Starting a change, a debug, or work in unfamiliar code | `grounding-before-coding` |
|
||||
| Writing or touching any error path, fallback, or default | `handling-failures` |
|
||||
| Adding data, config, state, or a second copy of anything | `keeping-one-source-of-truth` |
|
||||
| About to say "done", "fixed", or "passing" | `verifying-before-done` |
|
||||
| Deleting, overwriting, restarting, or touching secrets or live systems | `operating-safely` |
|
||||
| Deciding how big a fix should be, or scope is moving mid-task | `scoping-changes` |
|
||||
| Deciding what tests a change owes, or the test diff is empty | `testing-changes` |
|
||||
| Writing or fixing a unit test, or a test is flaky or unreadable | `writing-unit-tests` |
|
||||
| Crossing module boundaries, adding dependencies, or touching stated principles | `guarding-architecture` |
|
||||
|
||||
Writing the documents around the work (specs, decisions, changelogs, runbooks, postmortems, issues) is the technical-writer plugin's job where installed; these skills govern the engineering itself and defer to those for the prose.
|
||||
|
||||
## What this skill does not do
|
||||
|
||||
- It is not a style guide: formatting, naming taste, and framework choice belong to the repository's own conventions, which win.
|
||||
- It does not replace project instructions: CLAUDE.md and repository rules outrank everything here.
|
||||
- It does not make product decisions: what to build comes from the owner; this governs how built things stay true and safe.
|
||||
|
||||
## Mandatory checkpoint before a non-trivial change
|
||||
|
||||
Before writing the first line, state in working notes:
|
||||
|
||||
`Grounded: <what you read or ran to know the current behavior> | Blast radius: <what this change touches> | Invariants: <what must not break> | Verify: <the command that will prove it worked>`
|
||||
|
||||
Fill it from the code and data, not from memory or plausibility. A field you cannot fill is the work you do first.
|
||||
|
||||
## Hard rules
|
||||
|
||||
Non-negotiable, in every repository:
|
||||
|
||||
- **No silent error swallows.** Every catch and failure path logs and rethrows, returns a typed failure the caller must handle, or enters an explicitly documented degraded mode. A new silent swallow is an automatic review BLOCKER. See `handling-failures`.
|
||||
- **An applied migration is immutable history.** Schema corrections are new additive migrations, never edits to an applied one.
|
||||
- **Never claim verified without naming what was checked.** "Done" states the command and its result; tests that fail are reported with output; skipped steps are named. See `verifying-before-done`.
|
||||
- **Secret values are never read, printed, or decrypted to disk.** Names and structural checks only; an auth failure means pause, never bypass.
|
||||
- **Destructive operations need eyes first.** Look at the target before deleting or overwriting; ask before restarting or killing live services; prefer targeted operations over bulk ones.
|
||||
- **Evidence beats theory.** Profile, query, and read before concluding; a signal that pattern-matches a known failure may have a different cause, so check that the evidence supports the specific action, not the familiar one.
|
||||
|
||||
## Risk tiers set the rigor
|
||||
|
||||
Not all changes deserve the same ceremony; the tier does not change the rules, it changes how much proof they demand. **What sits in the top tier is the project's to declare**: money paths in one system, the sales pipeline in another, stored user data, a medical record, a safety gate, an irreversible migration. The project's rules or CLAUDE.md name its critical paths; when they do not, ask what the system must never get wrong, and treat the answer as the declaration.
|
||||
|
||||
Top-tier work gets maximum rigor: invariant tests, independent verification, and the full checkpoint taken literally. Ordinary paths get the standard discipline. Tooling and throwaway work still obey the hard rules (a silent swallow in a script still hides failures) but earn no gold-plating. State the tier when it is not obvious; the expensive mistake is running critical-path work at tooling rigor, and the wasteful one is the reverse.
|
||||
|
||||
## The rule lifecycle
|
||||
|
||||
When something bites twice, it becomes a written rule with its provenance (what happened, when, how to avoid it); once is learning. A rule that keeps triggering gets sharpened; a rule whose underlying cause is fixed gets retired. Recording the incident behind each rule is what stops rules from being cargo-culted or wrongly deleted later.
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- Acting on a document's claim about the system instead of the system. Doc status goes stale fast; the code and the history are the record.
|
||||
- Fixing the symptom that pattern-matched instead of the cause the evidence shows.
|
||||
- Treating "the tests are green" as "the change works". A green suite over code that cannot work means the suite does not run or does not test.
|
||||
- Leaving a duplicate you noticed because removing it was not the task. Absorbing it was part of the task. See `keeping-one-source-of-truth`.
|
||||
- Growing a fix past its trigger because improvements were adjacent. See `scoping-changes`.
|
||||
@@ -0,0 +1,43 @@
|
||||
---
|
||||
name: scoping-changes
|
||||
description: Use when deciding how big a fix should be, when scope is drifting mid-task, when a plan is being quietly trimmed to fit, or when "while you're at it" appears in any form. Encodes size-to-trigger, decompose-not-descope, and drive-to-completion. Use whenever the work is about to grow past its cause or shrink below its promise.
|
||||
---
|
||||
|
||||
# Scoping changes
|
||||
|
||||
**REQUIRED BACKGROUND:** the `principal-engineering` skill.
|
||||
|
||||
## Overview
|
||||
|
||||
Scope fails in both directions: gold-plating grows a fix past its trigger, and silent descoping shrinks an approved task below what was agreed. Both are the same defect: the delivered change no longer matches its cause. Core principle: **size fixes to their trigger, and when reality forces a cut, decompose visibly instead of trimming quietly.**
|
||||
|
||||
## Sizing to the trigger
|
||||
|
||||
- The fix is as big as the thing that triggered it. No gold-plating, no fencing unreachable edges, no refactor riding along because the file was open.
|
||||
- Adjacent improvements you noticed are real and belong in the tracker, not in this diff. One trigger, one change, one reviewable story.
|
||||
- The test for a borderline addition: would this change ship on its own merits if the main fix did not exist? If not, it is decoration on someone else's diff.
|
||||
|
||||
## Decompose, never silently descope
|
||||
|
||||
- An approved task that turns out too big is decomposed into named parts with the cut line stated, never delivered as a quiet "pragmatic minimum" that looks complete.
|
||||
- What gets deferred becomes a tracked item with an owner (the tracker discipline lives in the technical-writer plugin's `writing-issues` where installed); deferred work that lives only in the author's memory was descoped, not deferred.
|
||||
- The report says plainly which parts shipped and which did not. A partial delivery honestly labeled is a plan; a partial delivery labeled complete is a defect.
|
||||
|
||||
## Driving to completion
|
||||
|
||||
- An approved plan runs to completion without "want me to continue?" checkpoints; stop only for genuine blockers, destructive actions, or hard gates that need the operator.
|
||||
- Blocked on one part: finish the unblocked parts, surface the blocker with what it needs, never let one stuck task silently stall the rest.
|
||||
- Settled decisions stay settled mid-execution. New information that genuinely reopens one becomes an explicit re-decision (recorded, where the technical-writer plugin is installed, via `recording-decisions`), not a quiet swerve.
|
||||
|
||||
## Scope in review
|
||||
|
||||
- Reviewing a change: an unrelated defect you noticed in passing is not your finding; note it once for the tracker and stay on the diff. Out-of-scope findings dilute the verdict and train authors to fear review.
|
||||
- Being reviewed: findings against the diff get fixed or explicitly answered; findings outside the diff get tracked, not absorbed into the change.
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- "While I'm here" as a justification. You are here for the trigger.
|
||||
- Descoping to hit a deadline and reporting done. The deadline pressure was real; the honest move was decomposing and saying which half shipped.
|
||||
- Fencing edge cases the system cannot reach, to feel thorough. Unreachable defensiveness is dead code with good intentions.
|
||||
- Re-litigating an approved design in the middle of implementing it because a mildly better idea appeared. Write the idea down; finish the plan; propose the idea against the shipped reality.
|
||||
- Letting a reviewer's out-of-scope wish expand the diff. Track it, thank them, ship the trigger.
|
||||
@@ -0,0 +1,38 @@
|
||||
---
|
||||
name: testing-changes
|
||||
description: Use when deciding what tests a change needs - a feature, a bug fix, a refactor, any behavior change - or when reviewing whether a diff's tests are sufficient. Encodes tests-change-with-behavior, the bug-regression pattern, and assertion discrimination. Use whenever behavior changes and the test diff is empty, especially when the change is "too small to test".
|
||||
---
|
||||
|
||||
# Testing changes
|
||||
|
||||
**REQUIRED BACKGROUND:** the `principal-engineering` skill. The craft of the tests themselves lives in `writing-unit-tests`; this skill governs which tests a change owes.
|
||||
|
||||
## Overview
|
||||
|
||||
A test is the executable form of a claim about behavior. A change that alters behavior without touching tests is a claim nobody wrote down: the two named incident classes this skill exists to prevent are the green suite over code that could not work, and the test or gate that turned out never to run.
|
||||
|
||||
## What a change owes
|
||||
|
||||
1. **Tests change with behavior, in the same change.** An empty test diff on a behavior change is a review finding, not a style preference. A pure refactor owes the opposite proof: the existing tests still pass unmodified, which is what makes it a refactor.
|
||||
2. **A bug fix ships its regression test.** Named after the failure mode (not the ticket), demonstrated red against the unfixed code and green against the fix; both halves shown, because a regression test that never went red proves only that it compiles. This is what makes the same bug impossible to reintroduce silently.
|
||||
3. **Scenarios are concrete and include the surfaced edges.** The edge cases that grounding and review turned up go into tests by name; the happy path alone tests the demo, not the change. When the change's risk is in the failure path, the failure path gets the tests (see `handling-failures`: the typed failure surfacing IS behavior).
|
||||
4. **Every task carries its targeted verify command.** The specific test invocation that proves this change, runnable alone, stated where the reviewer can run it. "The suite passed" vouches for nothing the suite never covered.
|
||||
5. **Assertions must discriminate.** A test that passes regardless of the change proves nothing: break the code once, watch red, unbreak. Non-discriminating assertions are how suites stay green over broken behavior.
|
||||
6. **Aggregates that must reconcile get invariant tests.** Anything on the project's declared critical paths (see the risk tiers in `principal-engineering`) that sums, derives, or mirrors other data gets a test asserting the reconciliation itself (the conservation pattern: the aggregate equals what the raw records imply), not just point examples.
|
||||
|
||||
## The red-test rule
|
||||
|
||||
A red test in a gate you own gets fixed, never silenced: weakening the assertion, deleting the test, or marking it skipped to ship is converting a detected defect into an undetected one. Changing the test is legitimate exactly when the test asserted the old, wrong behavior, and the change says so explicitly. Origin is attributed first, then fixed regardless of whose it is; "pre-existing" is a footnote, never an excuse (see `verifying-before-done`).
|
||||
|
||||
## What a change does not owe
|
||||
|
||||
- Tests for unreachable edges (see `scoping-changes`: fencing what cannot happen is dead code with good intentions).
|
||||
- Tests of framework internals or generated code; test your use of them at the boundary you own.
|
||||
- A test-first process: whether tests come first is workflow (the TDD skill where installed governs that); this skill governs what must exist when the change ships, whichever order produced it.
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- "Too small to test." Small changes break behavior at the same rate per line; the test takes minutes, the silent regression takes an audit.
|
||||
- Testing the fix without reproducing the bug. Red-before-green is the half that proves the test sees the defect.
|
||||
- Counting coverage by feel. Count the changed behaviors against the tests naming them (see the counted-coverage rule in the technical-writer plugin's truth doctrine, same principle).
|
||||
- Adding the test that discriminates against nothing, ever: an assertion no plausible defect could fail. Distinct from the legitimate pinning test that deliberately passes against both old and new code to guard unchanged adjacent behavior from overcorrection; a pinning test says that is what it is for.
|
||||
@@ -0,0 +1,33 @@
|
||||
---
|
||||
name: verifying-before-done
|
||||
description: Use when about to say "done", "fixed", "passing", or "shipped" - or when reporting the outcome of any change. Encodes verification as the definition of done: run the proof, report faithfully, distrust green suites, own failing gates. Use before every completion claim, even when the change was small and obviously correct, which is when this is skipped.
|
||||
---
|
||||
|
||||
# Verifying before done
|
||||
|
||||
**REQUIRED BACKGROUND:** the `principal-engineering` skill.
|
||||
|
||||
## Overview
|
||||
|
||||
Done means verified, and verified names what was checked. Careful work is not a check: the two defects that ship in "obviously fine" changes were both caught, in this corpus's history, by the verification that almost got skipped.
|
||||
|
||||
## The discipline
|
||||
|
||||
1. **Run the proof, paste the result.** The verify command from the pre-change checkpoint gets executed, and its actual output backs the claim. "Verified" is always "PASS (checked X and Y)", never a bare checkmark.
|
||||
2. **Report faithfully, both directions.** Tests that fail are reported with their output; steps that were skipped are named as skipped; and work that is done and verified is stated plainly without hedging. Underclaiming verified work wastes the reader's re-verification exactly like overclaiming wastes their trust.
|
||||
3. **Distrust green.** A green suite over code that cannot work means the suite does not run, does not cover, or cannot fail. When a result seems too clean for the change's size: confirm the test executed (run it alone, watch it appear), and confirm it can fail (break the code, watch it go red, unbreak it). A test that never ran and a gate that never fires are the two failure classes that produce confident wrong "done"s.
|
||||
4. **Distinguish the tiers.** Implemented (in the repo) is not deployed (live) is not externally verified (checked in the external system). Never claim a later tier from evidence of an earlier one.
|
||||
5. **Lookback before declaring complete.** Sweep the diff: no unrelated changes, no planning residue in code or comments, docs updated in the same change, every acceptance criterion actually met rather than approximately met.
|
||||
6. **Independent verification for top-tier changes.** The author of a change is the worst-placed person to verify it; for the project's declared critical paths (money, sales, stored data, safety, whatever the system must never get wrong) and for irreversible migrations, the verifier is someone or something that did not write the code. When no independent verifier is reachable in time, use the nearest substitute and name it as the weaker form it is; downgrading the check silently is the failure, downgrading it visibly is a decision.
|
||||
|
||||
## Failing gates you own
|
||||
|
||||
Any failing test in a gate you own gets fixed: attribute the origin first, then fix it regardless of whose it is. "Pre-existing" is a footnote in the report, never an excuse in the gate. The one exception is procedural, not evasive: a pre-existing red on the main branch that blocks an unrelated green fix gets surfaced with an offer to merge the green fix anyway, decided by the operator.
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- Declaring done from the diff looking right. The diff looking right is the hypothesis; the verify command is the experiment.
|
||||
- Running the whole suite instead of the targeted proof, and reading "no new failures" as "my change works". A suite that never covered the path cannot vouch for it.
|
||||
- Verifying the happy path of a change whose risk is in the failure path.
|
||||
- "Tests pass locally" as the terminal claim for a change whose risk is environmental (config, migrations, permissions, prod data shape).
|
||||
- Fixing the test instead of the code when red is inconvenient. The test was the messenger.
|
||||
@@ -0,0 +1,51 @@
|
||||
---
|
||||
name: writing-unit-tests
|
||||
description: Use when writing or refactoring unit tests - a new test file, added cases, a flaky test, an unreadable one. Encodes behavior-first testing: one behavior per test, names that document, deterministic setup, mocks only at boundaries you do not own. Use whenever a test is being written, even a quick one, and whenever a test needs a sleep, a mock of your own code, or a copy of the implementation's math.
|
||||
---
|
||||
|
||||
# Writing unit tests
|
||||
|
||||
**REQUIRED BACKGROUND:** the `principal-engineering` skill. Which tests a change owes is `testing-changes`; this skill is the craft of the tests themselves.
|
||||
|
||||
## Overview
|
||||
|
||||
A unit test is a behavioral claim with a name, read by the next engineer during a red build. Core principle: **test the contract, not the implementation; make the name carry the claim; and keep the test so simple it cannot itself be wrong.**
|
||||
|
||||
## Behavior, not implementation
|
||||
|
||||
- Test through the public contract of the unit. A refactor that preserves behavior should not break tests; when it does, the tests were asserting the implementation, and they now punish improvement.
|
||||
- Do not assert call sequences, internal state, or that method A called method B, unless the interaction IS the contract (a required side effect on a boundary). Asserting internals tests the implementation twice and the behavior zero times.
|
||||
- Never derive the expected value from the production arithmetic, neither by reimplementing the formula nor by invoking the shared helper that computes it. Both prove the code equals itself, and both stay green when the shared code carries the bug. Expected values are literals worked out independently (by hand, from a spec, from real data), with the derivation in a comment.
|
||||
|
||||
## One behavior per test, named as the claim
|
||||
|
||||
- One behavior per test; splitting is cheaper than archaeology on a multi-assert failure.
|
||||
- The name states subject, scenario, and expected outcome: `expired_token_is_rejected_with_401`, not `test_auth_3`. Test names describe behavior, state transitions, and invariants; never delivery order, ticket keys, or phases. Read the test list of a module and you have read its spec; that is the bar.
|
||||
- Arrange, act, assert, visibly and in that order. No branching, loops, or logic in a test: a test with logic needs its own test. Shared setup earns a builder or a role-named fixture; a mystery blob fixture hides which arranged fact the assertion depends on. Generation and iteration live in builders and helpers, not in the test body; property-based tests are the accepted form for invariants and follow their framework's shape, while example-based tests stay logic-free.
|
||||
|
||||
## Determinism
|
||||
|
||||
- No real time, real randomness, real network, or real filesystem inside a unit test: inject the clock, seed or inject the randomness, fake the boundary. The test that passes at 14:00 and fails at midnight is a bug report about the test.
|
||||
- No sleeps. Waiting for async work is condition-based (poll the observable outcome with a deadline), never duration-based; a sleep is a race condition with a timer attached.
|
||||
- A flaky test is red: fix it or quarantine it visibly with an owner (see the red-test rule in `testing-changes`); re-running until green is silencing a detector.
|
||||
|
||||
## Mocks are assumptions
|
||||
|
||||
- Mock the boundaries you do not own (network, clock, filesystem, third-party services); prefer real collaborators for code you do own within the unit's reach. For owned wrappers around unowned resources (your repository class fronting the database), fake at the seam where owned code last touches the unowned resource, and keep the test data role-named and visible either way. Every mock hardcodes an assumption about a contract, and a stale mock is how a suite stays green while the real integration is broken.
|
||||
- When a test is mostly mock wiring, it is testing the mocks. Either widen the unit to something with real behavior or accept that this seam needs an integration test instead (and say which).
|
||||
- Fixtures are labeled snapshots of reality: minimal, role-named for their part in the scenario, and updated deliberately when the contract changes, never regenerated blindly to make red go green.
|
||||
|
||||
## Assertions and failure paths
|
||||
|
||||
- Assert outcomes with values, not absence of exceptions. "It did not throw" claims almost nothing.
|
||||
- Failure paths are first-class test subjects: the typed failure surfaces, the degraded mode is entered loudly, the guard actually guards (see `handling-failures`). The error path without a test is the silent swallow's favorite hiding place.
|
||||
- Tests themselves follow the no-silent-swallows contract: no catch-and-ignore in test code, no conditional assertions that skip silently when a precondition is absent; a test that cannot run must fail or be visibly skipped with the reason.
|
||||
|
||||
## Common mistakes
|
||||
|
||||
- The mirror test: reimplementing the production logic to compute the expectation.
|
||||
- The mock echo chamber: stubbing your own class and asserting the stub.
|
||||
- The mega-test: twelve assertions, one name, no way to know which claim broke.
|
||||
- Shared mutable fixtures that make test order matter; every test builds or receives its own state.
|
||||
- The sleep that "fixes" flakiness by making it rarer.
|
||||
- Green-checking the fixture: editing expected values to match actual output without deriving why the new value is right.
|
||||
Reference in New Issue
Block a user