Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1d140ec525 | ||
|
|
2fcc15e9e4 | ||
|
|
b2af4e1129 | ||
|
|
3c8d2f5613 | ||
|
|
1d368d4b5d | ||
|
|
a565b04e91 | ||
|
|
b616c51a2c | ||
|
|
b8bacfa7b3 | ||
|
|
c377c23e21 | ||
|
|
8f9d5dd8bd | ||
|
|
7ba01abdbe | ||
|
|
9a81a707af | ||
|
|
5070a738e8 | ||
|
|
8da7c9c45f | ||
|
|
6e5024d498 |
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"name": "skills",
|
"name": "skills",
|
||||||
"description": "Mathias's engineering skills library, plus the hooks and MCP servers that make it enforceable: TDD, clean code, debugging, specs, discovery, CI, and secret hygiene.",
|
"description": "Mathias's engineering skills library, plus the hooks and MCP servers that make it enforceable: TDD, clean code, debugging, specs, discovery, CI, and secret hygiene.",
|
||||||
"version": "0.12.0",
|
"version": "0.14.0",
|
||||||
"author": {
|
"author": {
|
||||||
"name": "Mathias",
|
"name": "Mathias",
|
||||||
"url": "https://git.d-ma.be/mathias"
|
"url": "https://git.d-ma.be/mathias"
|
||||||
|
|||||||
@@ -12,5 +12,22 @@
|
|||||||
"type": "http",
|
"type": "http",
|
||||||
"url": "https://koala.tail30472e.ts.net:8444/mcp"
|
"url": "https://koala.tail30472e.ts.net:8444/mcp"
|
||||||
}
|
}
|
||||||
|
},
|
||||||
|
"_parked": {
|
||||||
|
"_reason": "cad and gitea-mcp are proxied, reachable and AUTHENTICATING: tailscale serve 8445 -> 127.0.0.1:31877 and 8446 -> 127.0.0.1:30340; an MCP initialize with the correct bearer returns 200 from both (serverInfo gitea-mcp 0.1.0, cad aed2aaf). They stay parked only because nothing puts those two tokens into the session environment. Both live in the 1Password AgentSecrets vault, and by deliberate design (infra#224, closed) no single identity on koala reads both vaults: koala-homelab is HomeLab-only and resolves ~/.op-env, koala-agentsecrets-write is AgentSecrets-only and lives in ~/.op-agent-write.env. Adding an AgentSecrets reference to ~/.op-env does NOT degrade gracefully -- `op run` fails wholesale and every other token, BRAIN_MCP_TOKEN included, goes unresolved with it. Enabling needs a read-scoped AgentSecrets identity in the session-env path; see mathias/skills#32, #33.",
|
||||||
|
"cad": {
|
||||||
|
"type": "http",
|
||||||
|
"url": "https://koala.tail30472e.ts.net:8445/mcp",
|
||||||
|
"headers": {
|
||||||
|
"Authorization": "Bearer ${CAD_MCP_TOKEN}"
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"gitea": {
|
||||||
|
"type": "http",
|
||||||
|
"url": "https://koala.tail30472e.ts.net:8446/mcp",
|
||||||
|
"headers": {
|
||||||
|
"Authorization": "Bearer ${GITEA_MCP_STATIC_TOKEN}"
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -110,10 +110,25 @@ Antigravity and Mistral Vibe targets were removed — unused.
|
|||||||
task doctor # or bin/doctor
|
task doctor # or bin/doctor
|
||||||
```
|
```
|
||||||
|
|
||||||
Reports dangling links, links resolving into a foreign checkout, skill-count
|
Reports dangling links, links into a source nobody configured, skills in the
|
||||||
mismatches, stray entries, and leftover per-skill Claude symlinks. Non-zero
|
checkout with no link, stray entries, leftover per-skill Claude symlinks, and a
|
||||||
exit on drift. It also runs at session start via the plugin, so a drifting
|
Claude Code plugin sitting on an older generation than the host already has.
|
||||||
host says so rather than waiting to be discovered.
|
Non-zero exit on drift. It also runs at session start via the plugin, so a
|
||||||
|
drifting host says so rather than waiting to be discovered.
|
||||||
|
|
||||||
|
All of that compares copies already on the machine — deliberately, since it
|
||||||
|
runs in a session hook where latency shows. That leaves one case invisible: a
|
||||||
|
host where the checkout, the marketplace clone and the installed plugin all
|
||||||
|
agree, and all three are a release behind origin.
|
||||||
|
|
||||||
|
```bash
|
||||||
|
task doctor -- --online # or bin/doctor --online
|
||||||
|
```
|
||||||
|
|
||||||
|
One `git ls-remote` against origin, opt-in. Belongs in a scheduled per-host job,
|
||||||
|
not the session hook. An unreachable remote is reported as its own fault rather
|
||||||
|
than as staleness — "I could not check" and "you are behind" are different
|
||||||
|
answers and only one of them is about the host.
|
||||||
|
|
||||||
## Versioning
|
## Versioning
|
||||||
|
|
||||||
@@ -130,13 +145,48 @@ before `task skills:install`; default is `main` for always-current.
|
|||||||
|
|
||||||
## Contributing a skill
|
## Contributing a skill
|
||||||
|
|
||||||
1. Add a directory `<short-kebab-name>/`.
|
1. Add a directory `skills/<short-kebab-name>/`.
|
||||||
2. Create `<short-kebab-name>/SKILL.md` — start with one-paragraph
|
2. Create `skills/<short-kebab-name>/SKILL.md` with this frontmatter — all four
|
||||||
purpose, then triggers ("use when …"), then mechanics.
|
keys are gated by `task check`, and the index table is generated from the
|
||||||
3. Add a row to `SKILLS_INDEX.md` (description + use-when).
|
last three:
|
||||||
4. Commit directly to `main`. Conventional-commit prefix
|
|
||||||
|
```yaml
|
||||||
|
---
|
||||||
|
name: <short-kebab-name>
|
||||||
|
surfaces: [claude-code, library] # any of claude-code | library | chat
|
||||||
|
summary: "<one line for the index table>"
|
||||||
|
use_when: "<the trigger, for the index table>"
|
||||||
|
description: <the paragraph the model selects on>
|
||||||
|
---
|
||||||
|
```
|
||||||
|
|
||||||
|
Then: one-paragraph purpose, triggers ("use when …"), mechanics.
|
||||||
|
3. Run `task index` and commit the regenerated `SKILLS_INDEX.md`. Do **not**
|
||||||
|
hand-edit the table — `task check` regenerates it and fails on a diff.
|
||||||
|
4. Pressure-test it before shipping: run the scenario against a subagent that
|
||||||
|
does *not* have the skill, write the skill against what that run actually
|
||||||
|
did, then re-run with it. See `superpowers-writing-skills` and #19.
|
||||||
|
5. Commit directly to `main`. Conventional-commit prefix
|
||||||
(`feat:` / `fix:` / `docs:`). Add `Bump-Type: minor` footer when
|
(`feat:` / `fix:` / `docs:`). Add `Bump-Type: minor` footer when
|
||||||
adding a new skill or expanding scope; default patch covers edits.
|
adding a new skill or expanding scope; default patch covers edits.
|
||||||
|
|
||||||
This repo is markdown-only and public — never include secrets, client
|
This repo is markdown-only and public — never include secrets, client
|
||||||
names, or anything under NDA in a skill.
|
names, or anything under NDA in a skill.
|
||||||
|
|
||||||
|
## Mode 2 routing candidates
|
||||||
|
|
||||||
|
These notes used to sit in each SKILL.md as a `## Mode 2 Routing Note` section.
|
||||||
|
They changed nothing about how the skill is invoked — several said so outright —
|
||||||
|
and cost context on every load, so they live here instead. Nothing in the repo
|
||||||
|
depends on this table; it is a record of which skills looked mechanical enough
|
||||||
|
to route to a local model, should a routing layer ever exist.
|
||||||
|
|
||||||
|
| Skill | Note |
|
||||||
|
|---|---|
|
||||||
|
| `cad-babysit` | Phases 2-4 (reading comments/logs, classifying fault vs outcome) are judgment-heavy and should stay Mode 1. Phase 1 (running `cad stuck` and grouping its output) is mechanical and a Mode 2 routing candidate once Plan 6 ships the routing pod. |
|
||||||
|
| `code-review` | This skill is invoked identically in Mode 1 (cloud Claude) and Mode 2 (client-local with supervisor routing). The routing layer (Plan 6) does not exist yet; treat this skill as Mode 1 only for now. Findings format and severity classification are unchanged across modes. |
|
||||||
|
| `debug` | This skill produces high-volume mechanical output (hypothesis enumeration) and is a candidate for Mode 2 routing to a local model in the future. Until Plan 6 ships the routing pod, treat as Mode 1 only. The hypothesis format and discipline are identical regardless of which model generates them. |
|
||||||
|
| `feature-spec` | Spec writing requires interpretation of intent and tradeoff reasoning — work where Claude generally outperforms a 9-13B local model. This skill is intentionally **not** a Mode 2 routing candidate. In Mode 2, the routing layer should leave `feature-spec` calls on Claude. |
|
||||||
|
| `session-retrospective` | Reading long session logs and pulling themes is high-volume, mechanical pattern-recognition work — a strong Mode 2 routing candidate. Until Plan 6 ships the routing pod, treat as Mode 1 only. |
|
||||||
|
| `tdd` | This skill is invoked identically whether the agent is running in Mode 1 (cloud Claude, no routing) or Mode 2 (client-local, supervisor routing layer). The routing pod (Plan 6) does not exist yet; until it does, treat this skill as Mode 1 only. The discipline does not change between modes — only the model behind the call. |
|
||||||
|
| `trainer` | Trainer is the same shape as `session-retrospective` — long-context reading, mechanical filtering — and is a Mode 2 routing candidate. Until Plan 6, treat as Mode 1 only. |
|
||||||
|
|||||||
+10
-1
@@ -110,7 +110,16 @@ tasks:
|
|||||||
# harness instead of erroring.
|
# harness instead of erroring.
|
||||||
sh '{{.TASKFILE_DIR}}/bin/check-surfaces' '{{.TASKFILE_DIR}}' || fail=1
|
sh '{{.TASKFILE_DIR}}/bin/check-surfaces' '{{.TASKFILE_DIR}}' || fail=1
|
||||||
|
|
||||||
# 5. Plugin manifest version must match the latest tag, or installers
|
# 5. A documented session_log call must be one the tool would accept:
|
||||||
|
# session_id is its only required field, and nine skills omitted it.
|
||||||
|
sh '{{.TASKFILE_DIR}}/bin/check-session-log' '{{.TASKFILE_DIR}}' || fail=1
|
||||||
|
|
||||||
|
# 6. Every skill says when it stops and hands back (#5). Skills whose
|
||||||
|
# rule is not written yet are listed in bin/escalation-pending.txt,
|
||||||
|
# which only shrinks.
|
||||||
|
sh '{{.TASKFILE_DIR}}/bin/check-escalation' '{{.TASKFILE_DIR}}' || fail=1
|
||||||
|
|
||||||
|
# 7. Plugin manifest version must match the latest tag, or installers
|
||||||
# never see the update (Claude Code gates updates on this field).
|
# never see the update (Claude Code gates updates on this field).
|
||||||
if [ -f '{{.PLUGIN_MANIFEST}}' ]; then
|
if [ -f '{{.PLUGIN_MANIFEST}}' ]; then
|
||||||
manifest_v=$(sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' '{{.PLUGIN_MANIFEST}}' | head -1)
|
manifest_v=$(sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' '{{.PLUGIN_MANIFEST}}' | head -1)
|
||||||
|
|||||||
Executable
+64
@@ -0,0 +1,64 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# check-escalation — every skill states when it stops and hands back.
|
||||||
|
#
|
||||||
|
# #5 decided this belongs in the body, not the frontmatter: no harness reads
|
||||||
|
# frontmatter beyond name/description, so a `failure_criteria:` key would be a
|
||||||
|
# guarantee nothing honours. The model reads the body.
|
||||||
|
#
|
||||||
|
# #37 then had to define which skills need one, and every obvious answer was
|
||||||
|
# bad: a hardcoded list rots, sniffing the text for "does it write anything" is
|
||||||
|
# unfalsifiable, and a frontmatter flag is the rejected option wearing a hat.
|
||||||
|
#
|
||||||
|
# So there is no acting set. EVERY skill answers the question. A read-only skill
|
||||||
|
# answers it in one line — "read-only; findings are the output, nothing is
|
||||||
|
# handed back" — which turns an absence into a claim someone can disagree with.
|
||||||
|
#
|
||||||
|
# bin/escalation-pending.txt is a ratchet for skills whose real rule has not
|
||||||
|
# been written yet (a stop condition nobody thought about is not worth
|
||||||
|
# enforcing). It is self-cleaning: a listed skill that HAS a section fails, so
|
||||||
|
# the list cannot outlive its reason.
|
||||||
|
#
|
||||||
|
# Usage: check-escalation [checkout-root]
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
ROOT=${1:-$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)}
|
||||||
|
PENDING="$ROOT/bin/escalation-pending.txt"
|
||||||
|
fail=0
|
||||||
|
|
||||||
|
listed() {
|
||||||
|
[ -f "$PENDING" ] || return 1
|
||||||
|
sed 's/#.*//' "$PENDING" | grep -qx "[[:space:]]*$1[[:space:]]*"
|
||||||
|
}
|
||||||
|
|
||||||
|
for dir in "$ROOT"/skills/*/; do
|
||||||
|
[ -f "$dir/SKILL.md" ] || continue
|
||||||
|
name=$(basename "$dir")
|
||||||
|
|
||||||
|
if grep -q '^## Escalation' "$dir/SKILL.md"; then
|
||||||
|
if listed "$name"; then
|
||||||
|
echo "ERROR: skills/$name has an Escalation section but is still in bin/escalation-pending.txt — remove it from the list" >&2
|
||||||
|
fail=1
|
||||||
|
fi
|
||||||
|
continue
|
||||||
|
fi
|
||||||
|
|
||||||
|
listed "$name" && continue
|
||||||
|
echo "ERROR: skills/$name has no '## Escalation' section — say when it stops and hands back, or say it never does" >&2
|
||||||
|
fail=1
|
||||||
|
done
|
||||||
|
|
||||||
|
# A pending entry for a skill that no longer exists is a stale exemption, and a
|
||||||
|
# stale exemption is how a gate quietly stops covering something. Looped without
|
||||||
|
# a pipe on purpose: a `while read` on the right of a pipe runs in a subshell,
|
||||||
|
# so `fail=1` would be set in a process that exits before it is read.
|
||||||
|
if [ -f "$PENDING" ]; then
|
||||||
|
entries=$(sed 's/#.*//' "$PENDING" | tr -d '[:blank:]' | grep -v '^$' || true)
|
||||||
|
for entry in $entries; do
|
||||||
|
[ -f "$ROOT/skills/$entry/SKILL.md" ] && continue
|
||||||
|
echo "ERROR: bin/escalation-pending.txt lists '$entry', which has no skills/$entry/SKILL.md" >&2
|
||||||
|
fail=1
|
||||||
|
done
|
||||||
|
fi
|
||||||
|
|
||||||
|
exit $fail
|
||||||
Executable
+39
@@ -0,0 +1,39 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# check-session-log — a skill that documents a session_log call must document a
|
||||||
|
# call the tool would actually accept.
|
||||||
|
#
|
||||||
|
# Nine skills documented the payload and none named `session_id`, which is the
|
||||||
|
# tool's only REQUIRED field. An agent following any of them either failed the
|
||||||
|
# call or invented an id — and invented ids do not group, so the pass-rate the
|
||||||
|
# endpoint computes was being assembled from entries that could not be joined
|
||||||
|
# (#36, found by a pressure test against `debug`).
|
||||||
|
#
|
||||||
|
# Also rejected: `duration_ms: <wall-clock>`. An agent does not observe a clock.
|
||||||
|
# A documented field nobody can honestly populate gets fabricated or silently
|
||||||
|
# dropped, and fabricated numbers in a metrics stream are worse than no numbers.
|
||||||
|
#
|
||||||
|
# Usage: check-session-log [checkout-root]
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
ROOT=${1:-$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)}
|
||||||
|
fail=0
|
||||||
|
|
||||||
|
for dir in "$ROOT"/skills/*/; do
|
||||||
|
[ -f "$dir/SKILL.md" ] || continue
|
||||||
|
name=$(basename "$dir")
|
||||||
|
|
||||||
|
grep -q 'session_log' "$dir/SKILL.md" || continue
|
||||||
|
|
||||||
|
if ! grep -q 'session_id' "$dir/SKILL.md"; then
|
||||||
|
echo "ERROR: skills/$name documents session_log without session_id — the tool's only required field" >&2
|
||||||
|
fail=1
|
||||||
|
fi
|
||||||
|
|
||||||
|
if grep -q 'duration_ms:[[:space:]]*<' "$dir/SKILL.md"; then
|
||||||
|
echo "ERROR: skills/$name documents duration_ms as a placeholder an agent cannot measure — omit it or pass a real measurement" >&2
|
||||||
|
fail=1
|
||||||
|
fi
|
||||||
|
done
|
||||||
|
|
||||||
|
exit $fail
|
||||||
+39
@@ -10,6 +10,7 @@
|
|||||||
# doctor human report, non-zero exit on drift
|
# doctor human report, non-zero exit on drift
|
||||||
# doctor --quiet print only when something is wrong (SessionStart hook)
|
# doctor --quiet print only when something is wrong (SessionStart hook)
|
||||||
# doctor --json machine-readable findings
|
# doctor --json machine-readable findings
|
||||||
|
# doctor --online ALSO ask origin for the latest tag (one network call)
|
||||||
#
|
#
|
||||||
# POSIX sh on purpose: it runs on macOS and Linux, before any toolchain is
|
# POSIX sh on purpose: it runs on macOS and Linux, before any toolchain is
|
||||||
# guaranteed present, and inside a session-start hook where latency shows.
|
# guaranteed present, and inside a session-start hook where latency shows.
|
||||||
@@ -21,10 +22,12 @@ set -u
|
|||||||
|
|
||||||
QUIET=0
|
QUIET=0
|
||||||
JSON=0
|
JSON=0
|
||||||
|
ONLINE=0
|
||||||
for arg in "$@"; do
|
for arg in "$@"; do
|
||||||
case "$arg" in
|
case "$arg" in
|
||||||
--quiet) QUIET=1 ;;
|
--quiet) QUIET=1 ;;
|
||||||
--json) JSON=1 ;;
|
--json) JSON=1 ;;
|
||||||
|
--online) ONLINE=1 ;;
|
||||||
-h|--help)
|
-h|--help)
|
||||||
sed -n '2,20p' "$0" | sed 's/^# \{0,1\}//'
|
sed -n '2,20p' "$0" | sed 's/^# \{0,1\}//'
|
||||||
exit 0 ;;
|
exit 0 ;;
|
||||||
@@ -288,6 +291,42 @@ if [ -n "$installed_v" ] && [ -n "$available_v" ] && [ "$installed_v" != "$avail
|
|||||||
add FAIL plugin-version "Claude Code has skills@mathias $installed_v installed but $available_v is available on this host — /plugin update skills@mathias (a host stale enough to matter is running the old doctor, so the first detection may need this run by hand)"
|
add FAIL plugin-version "Claude Code has skills@mathias $installed_v installed but $available_v is available on this host — /plugin update skills@mathias (a host stale enough to matter is running the old doctor, so the first detection may need this run by hand)"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# --- optional: the release this host has never fetched ----------------------
|
||||||
|
# Every check above compares copies already present, so a host whose checkout,
|
||||||
|
# marketplace clone and installed plugin all agree reads as clean while sitting
|
||||||
|
# a release behind origin (#38). Closing that needs a network call, which the
|
||||||
|
# default run must not make — hence opt-in, for a scheduled per-host job rather
|
||||||
|
# than the SessionStart hook.
|
||||||
|
#
|
||||||
|
# $DOCTOR_LATEST_TAG_CMD is the test seam; it prints the newest remote tag.
|
||||||
|
if [ "$ONLINE" -eq 1 ]; then
|
||||||
|
if [ -n "${DOCTOR_LATEST_TAG_CMD:-}" ]; then
|
||||||
|
remote_tag=$(eval "$DOCTOR_LATEST_TAG_CMD" 2>/dev/null | tail -1 | tr -d '[:space:]')
|
||||||
|
else
|
||||||
|
# The URL is never printed: a remote rewritten by url.insteadOf can expand
|
||||||
|
# into a live token.
|
||||||
|
origin=$(git -C "$CHECKOUT" config --get remote.origin.url 2>/dev/null || true)
|
||||||
|
if [ -n "$origin" ]; then
|
||||||
|
remote_tag=$(git ls-remote --tags --refs "$origin" 2>/dev/null \
|
||||||
|
| sed 's#.*refs/tags/##' | sort -t. -k1,1n -k2,2n -k3,3n | tail -1 | tr -d '[:space:]')
|
||||||
|
else
|
||||||
|
remote_tag=''
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ -z "$remote_tag" ]; then
|
||||||
|
# Not drift. A host that cannot reach origin has an unanswered question,
|
||||||
|
# not a stale library, and saying otherwise would make the two look alike.
|
||||||
|
add WARN offline "could not reach origin for the latest tag — this run proves nothing about staleness"
|
||||||
|
else
|
||||||
|
remote_v=${remote_tag#v}
|
||||||
|
if [ -n "$available_v" ] && [ "$remote_v" != "$available_v" ] \
|
||||||
|
&& [ "$(newer_version "$available_v" "$remote_v")" = "$remote_v" ]; then
|
||||||
|
add FAIL behind "this host has $available_v; origin has released $remote_v — run 'task update' and '/plugin update skills@mathias'"
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
|
||||||
# --- output -----------------------------------------------------------------
|
# --- output -----------------------------------------------------------------
|
||||||
if [ "$JSON" -eq 1 ]; then
|
if [ "$JSON" -eq 1 ]; then
|
||||||
printf '{"checkout":"%s","expected":%s,"plugin":{"installed":"%s","available":"%s"},"findings":[%s]}\n' \
|
printf '{"checkout":"%s","expected":%s,"plugin":{"installed":"%s","available":"%s"},"findings":[%s]}\n' \
|
||||||
|
|||||||
@@ -0,0 +1,42 @@
|
|||||||
|
# Skills whose escalation rule has not been written yet (#37).
|
||||||
|
#
|
||||||
|
# This list only shrinks. A skill here is exempt from the '## Escalation'
|
||||||
|
# requirement until someone writes its real stop conditions; a skill that HAS a
|
||||||
|
# section and is still listed fails the gate, so the list cannot outlive its
|
||||||
|
# reason.
|
||||||
|
#
|
||||||
|
# The ones already done were the advisory and read-only skills, where the honest
|
||||||
|
# answer is a line. What is left is the acting set — skills that write files,
|
||||||
|
# dispatch agents, touch credentials or move releases — and each of those needs
|
||||||
|
# a rule someone thought about, not a rote paragraph. Per #19, one at a time.
|
||||||
|
|
||||||
|
atdd
|
||||||
|
dream
|
||||||
|
experiment-spec
|
||||||
|
feature-spec
|
||||||
|
gitea-ci
|
||||||
|
own-the-task
|
||||||
|
planning
|
||||||
|
playwright
|
||||||
|
problem-analysis
|
||||||
|
refactoring
|
||||||
|
regulatory-risk-assessment
|
||||||
|
secret-hygiene
|
||||||
|
session-retrospective
|
||||||
|
spec-driven-dev
|
||||||
|
superpowers-brainstorming
|
||||||
|
superpowers-dispatching-parallel-agents
|
||||||
|
superpowers-executing-plans
|
||||||
|
superpowers-finishing-a-development-branch
|
||||||
|
superpowers-receiving-code-review
|
||||||
|
superpowers-requesting-code-review
|
||||||
|
superpowers-subagent-driven-development
|
||||||
|
superpowers-using-git-worktrees
|
||||||
|
superpowers-using-superpowers
|
||||||
|
superpowers-verification-before-completion
|
||||||
|
superpowers-writing-plans
|
||||||
|
superpowers-writing-skills
|
||||||
|
tdd
|
||||||
|
trainer
|
||||||
|
user-stories
|
||||||
|
web-shot
|
||||||
@@ -234,8 +234,8 @@ wrong" — it means "nothing on the known list is wrong."
|
|||||||
Call `session_log` once at the end of every phase to record the outcome.
|
Call `session_log` once at the end of every phase to record the outcome.
|
||||||
|
|
||||||
**At end of each phase:**
|
**At end of each phase:**
|
||||||
- `session_log` with `{skill: "cad-babysit", phase: "<phase-name>", final_status: "pass" |
|
- `session_log` with `{session_id: "<this session's id>", skill: "cad-babysit", phase: "<phase-name>", final_status: "pass" |
|
||||||
"fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root:
|
"fail" | "skip", message: "<one-line summary>", project_root:
|
||||||
"<absolute path>"}`
|
"<absolute path>"}`
|
||||||
|
|
||||||
**Phases for this skill:** query, read-comments, read-actions-logs, classify-fault-vs-outcome,
|
**Phases for this skill:** query, read-comments, read-actions-logs, classify-fault-vs-outcome,
|
||||||
@@ -247,12 +247,6 @@ decide
|
|||||||
- `fail` — the phase could not be completed (e.g. `cad stuck` errored, Gitea was unreachable).
|
- `fail` — the phase could not be completed (e.g. `cad stuck` errored, Gitea was unreachable).
|
||||||
- `skip` — phase intentionally skipped (e.g. Phase 3 skipped because Phase 2 was conclusive).
|
- `skip` — phase intentionally skipped (e.g. Phase 3 skipped because Phase 2 was conclusive).
|
||||||
|
|
||||||
## Mode 2 Routing Note
|
|
||||||
|
|
||||||
Phases 2-4 (reading comments/logs, classifying fault vs outcome) are judgment-heavy and should
|
|
||||||
stay Mode 1. Phase 1 (running `cad stuck` and grouping its output) is mechanical and a Mode 2
|
|
||||||
routing candidate once Plan 6 ships the routing pod.
|
|
||||||
|
|
||||||
## Cross-References
|
## Cross-References
|
||||||
|
|
||||||
- Depends on `mathias/cad#14`'s `cad stuck` query — this skill is the ritual around it, not a
|
- Depends on `mathias/cad#14`'s `cad stuck` query — this skill is the ritual around it, not a
|
||||||
@@ -264,3 +258,13 @@ routing candidate once Plan 6 ships the routing pod.
|
|||||||
- `mathias/cad-atlas#11` renders the same `cad stuck` output as a pipeline overlay — read that
|
- `mathias/cad-atlas#11` renders the same `cad stuck` output as a pipeline overlay — read that
|
||||||
instead of this skill's text output when a visual "where is everything stuck" view is wanted
|
instead of this skill's text output when a visual "where is everything stuck" view is wanted
|
||||||
over working one row in depth.
|
over working one row in depth.
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Read-only by construction, and the Iron Laws above are the escalation rule: never close
|
||||||
|
an issue, never dispatch, never write a label. Everything that is the operator's call
|
||||||
|
leaves as a `DECISION NEEDED` block (question / options / recommendation / blocks).
|
||||||
|
|
||||||
|
Stop and hand back when: the stuck queue is empty; a diagnosis needs a write you are not
|
||||||
|
allowed to make; or two runs in a row produce the same finding, which means the finding
|
||||||
|
is not being acted on and repeating it is noise.
|
||||||
|
|||||||
@@ -281,5 +281,14 @@ During refactoring:
|
|||||||
|
|
||||||
- `references/clean-code.md` — detailed naming rules, object calisthenics
|
- `references/clean-code.md` — detailed naming rules, object calisthenics
|
||||||
- `references/code-smells.md` — comprehensive smell catalog with examples
|
- `references/code-smells.md` — comprehensive smell catalog with examples
|
||||||
|
- `references/python-adaptation.md` — Python conventions: error chaining, dataclasses, the truthiness trap
|
||||||
- Load `solid` skill for SOLID principles with Go examples
|
- Load `solid` skill for SOLID principles with Go examples
|
||||||
- Load `refactoring` skill for systematic refactoring techniques
|
- Load `refactoring` skill for systematic refactoring techniques
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Advisory during the REFACTOR phase. Suggests; does not restructure behaviour.
|
||||||
|
|
||||||
|
Stop and hand back when: the cleanup you are proposing changes behaviour. That is not
|
||||||
|
refactoring, it is a change, and it needs its own failing test first — hand back to
|
||||||
|
`tdd`.
|
||||||
|
|||||||
@@ -0,0 +1,104 @@
|
|||||||
|
# Clean Code: Python Adaptation
|
||||||
|
|
||||||
|
The principles in `SKILL.md` are language-agnostic. This file covers the places where
|
||||||
|
Python's idioms differ from the Go examples, and where following the Go shape produces
|
||||||
|
bad Python.
|
||||||
|
|
||||||
|
The canonical stack (ruff, mypy --strict, pytest, uv, pydantic-settings, src-layout)
|
||||||
|
lives in `~/dev/.context/AGENT.md` and is not restated here — one source of truth, and
|
||||||
|
it is not this file.
|
||||||
|
|
||||||
|
## Reveal intent with types, not comments
|
||||||
|
|
||||||
|
`mypy --strict` is the Python analogue of Go's compiler, and it is non-negotiable for
|
||||||
|
the same reason: a type is a claim the machine checks, and a comment is a claim nobody
|
||||||
|
checks. Two consequences for clean code specifically:
|
||||||
|
|
||||||
|
- **`Any` is a hole in the argument.** Each one says "the reader is on their own here".
|
||||||
|
If a value is genuinely polymorphic, say so precisely — `Protocol`, a union, a
|
||||||
|
`TypeVar` — rather than surrendering.
|
||||||
|
- **A `# type: ignore` needs the error code and a reason**: `# type: ignore[arg-type] #
|
||||||
|
boto3 stubs lag the API`. A bare ignore is an unexplained exception to a rule the rest
|
||||||
|
of the file follows.
|
||||||
|
|
||||||
|
## Errors: chain them, never swallow them
|
||||||
|
|
||||||
|
Go's `fmt.Errorf("operation: %w", err)` is `raise` with `from`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Bad — the original cause is gone, and so is the stack that produced it
|
||||||
|
try:
|
||||||
|
row = fetch(invoice_id)
|
||||||
|
except KeyError:
|
||||||
|
raise InvoiceNotFound(invoice_id)
|
||||||
|
|
||||||
|
# Good — the chain survives; the traceback shows both
|
||||||
|
try:
|
||||||
|
row = fetch(invoice_id)
|
||||||
|
except KeyError as e:
|
||||||
|
raise InvoiceNotFound(invoice_id) from e
|
||||||
|
```
|
||||||
|
|
||||||
|
Two rules with no exceptions:
|
||||||
|
|
||||||
|
- **Never bare `except:`.** It catches `KeyboardInterrupt` and `SystemExit` too, so it
|
||||||
|
turns Ctrl-C into a swallowed error. `except Exception` if you truly mean everything.
|
||||||
|
- **Never a silent `pass`.** An exception handler that does nothing is a decision to
|
||||||
|
ignore a failure; if that is genuinely right, the handler needs a comment saying why,
|
||||||
|
and usually a log line. Most of the time it is a bug being stored for later.
|
||||||
|
|
||||||
|
## Data classes over dicts
|
||||||
|
|
||||||
|
A `dict[str, Any]` passed between functions is Go's `map[string]interface{}` — every
|
||||||
|
reader has to reconstruct the shape from the code that writes it.
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Bad: shape known only by reading every producer
|
||||||
|
def total(invoice: dict[str, Any]) -> int: ...
|
||||||
|
|
||||||
|
# Good: shape declared once, checked everywhere
|
||||||
|
@dataclass(frozen=True, slots=True)
|
||||||
|
class Invoice:
|
||||||
|
id: InvoiceId
|
||||||
|
lines: tuple[Line, ...]
|
||||||
|
currency: str # ISO 4217
|
||||||
|
|
||||||
|
def total(invoice: Invoice) -> Money: ...
|
||||||
|
```
|
||||||
|
|
||||||
|
`frozen=True` where the value is conceptually immutable — it makes accidental mutation
|
||||||
|
an error rather than a surprise at a distance. Use `pydantic` models at the *boundary*
|
||||||
|
(env config, HTTP payloads) where validation is the point, and dataclasses inside,
|
||||||
|
where validation has already happened.
|
||||||
|
|
||||||
|
## Two Python-specific traps that read as clean code
|
||||||
|
|
||||||
|
**Mutable default arguments** are evaluated once, at definition:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def add_line(line: Line, lines: list[Line] = []) -> list[Line]: # shared across calls
|
||||||
|
```
|
||||||
|
|
||||||
|
Use `None` and build inside. Ruff catches this (`B006`); the point of naming it here is
|
||||||
|
that the broken version looks tidier than the correct one.
|
||||||
|
|
||||||
|
**Truthiness is not presence.** `if not amount:` is true for `0`, `0.0`, `""`, `[]` and
|
||||||
|
`None` alike. For anything where zero is a legitimate value — money, counts, offsets —
|
||||||
|
write `if amount is None:`. This is the single most common way a Python function
|
||||||
|
silently mishandles a valid input.
|
||||||
|
|
||||||
|
## Structure
|
||||||
|
|
||||||
|
- `src/` layout, `pyproject.toml` only. A flat layout lets tests import the working
|
||||||
|
directory instead of the installed package, so the suite passes on a package that
|
||||||
|
cannot actually be installed.
|
||||||
|
- Module names are the unit of cohesion, as Go packages are. `utils.py` is the Python
|
||||||
|
spelling of a god package: it has no reason to change because it has no subject.
|
||||||
|
- Prefer functions to classes with one method. A class whose only job is to hold
|
||||||
|
constructor arguments and expose `run()` is a closure with ceremony.
|
||||||
|
|
||||||
|
## See also
|
||||||
|
|
||||||
|
- `references/clean-code.md` — the language-agnostic practices
|
||||||
|
- `references/code-smells.md` — the smell catalogue, all of which apply here
|
||||||
|
- `tdd/references/python.md` — the test-side conventions
|
||||||
@@ -253,7 +253,7 @@ Pass-rate is computed downstream by the `/pass-rate` HTTP endpoint, which
|
|||||||
treats `pass` as success, `fail` as failure, `skip` as neither.
|
treats `pass` as success, `fail` as failure, `skip` as neither.
|
||||||
|
|
||||||
**At end of each phase:**
|
**At end of each phase:**
|
||||||
- `session_log` with `{skill: "code-review", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "code-review", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**Phases for this skill:** phase-0-iron-laws, phase-1-context, phase-2-architectural-analysis, phase-3-go-checklist, phase-4-code-smell-detection, phase-5-test-quality
|
**Phases for this skill:** phase-0-iron-laws, phase-1-context, phase-2-architectural-analysis, phase-3-go-checklist, phase-4-code-smell-detection, phase-5-test-quality
|
||||||
|
|
||||||
@@ -262,7 +262,7 @@ treats `pass` as success, `fail` as failure, `skip` as neither.
|
|||||||
- `fail` — the phase's intended outcome was NOT reached.
|
- `fail` — the phase's intended outcome was NOT reached.
|
||||||
- `skip` — phase was skipped intentionally.
|
- `skip` — phase was skipped intentionally.
|
||||||
|
|
||||||
**Why this matters:** the routing pod (Plan 6) reads pass-rate to decide whether to route a future call to a local model. If your skill never logs, the routing pod sees no data.
|
**Why this matters:** pass-rate is what a future routing decision would be based on — whether a given call can be served by a local model instead of a cloud one. A skill that never logs contributes no evidence either way.
|
||||||
|
|
||||||
## Common Go Anti-Patterns
|
## Common Go Anti-Patterns
|
||||||
|
|
||||||
@@ -279,11 +279,18 @@ treats `pass` as success, `fail` as failure, `skip` as neither.
|
|||||||
|
|
||||||
## References
|
## References
|
||||||
|
|
||||||
|
- `references/python-adaptation.md` — the Python anti-pattern table, and what ruff/mypy
|
||||||
|
should have caught before review
|
||||||
- `clean-code/references/code-smells.md` — comprehensive smell catalog
|
- `clean-code/references/code-smells.md` — comprehensive smell catalog
|
||||||
- Load `refactoring` skill when recommending fixes
|
- Load `refactoring` skill when recommending fixes
|
||||||
- Load `test-design` skill for reviewing test quality
|
- Load `test-design` skill for reviewing test quality
|
||||||
- Load `solid` skill for SOLID principle violations
|
- Load `solid` skill for SOLID principle violations
|
||||||
|
|
||||||
## Mode 2 Routing Note
|
## Escalation
|
||||||
|
|
||||||
This skill is invoked identically in Mode 1 (cloud Claude) and Mode 2 (client-local with supervisor routing). The routing layer (Plan 6) does not exist yet; treat this skill as Mode 1 only for now. Findings format and severity classification are unchanged across modes.
|
Read-only analysis. Findings are the output; nothing is handed back mid-review.
|
||||||
|
|
||||||
|
Stop and hand back when: the diff is large enough that a review would be a skim — say so
|
||||||
|
and ask for it in parts, rather than producing confident coverage of a fraction of it; or
|
||||||
|
the problem is structural rather than pointwise, in which case name it and hand off to
|
||||||
|
`solid` or `refactoring` instead of filing twenty line-level comments about one cause.
|
||||||
|
|||||||
@@ -0,0 +1,60 @@
|
|||||||
|
# Code Review: Python Anti-Patterns
|
||||||
|
|
||||||
|
`SKILL.md` carries the Go anti-pattern table. This is the Python one. The review
|
||||||
|
process, severity classification and output format above it are language-neutral and
|
||||||
|
apply unchanged.
|
||||||
|
|
||||||
|
Canonical stack in `~/dev/.context/AGENT.md`. Two gates decide most of this file:
|
||||||
|
`ruff` and `mypy --strict`. **If a finding is something ruff or mypy already catches,
|
||||||
|
the review comment is not "fix this line" — it is "why is the gate not running?"**
|
||||||
|
|
||||||
|
## The table
|
||||||
|
|
||||||
|
| Anti-pattern | What to look for | Why it matters |
|
||||||
|
|---|---|---|
|
||||||
|
| Bare `except:` | `except:` with no exception class | Swallows `KeyboardInterrupt` and `SystemExit`; Ctrl-C stops working |
|
||||||
|
| Silent `pass` | `except X: pass` with no comment | A decision to ignore a failure, recorded nowhere |
|
||||||
|
| Lost cause | `raise Y(...)` inside `except X as e` with no `from e` | Traceback loses the original failure — the Python analogue of a naked `fmt.Errorf` |
|
||||||
|
| Mutable default | `def f(x, acc=[])` or `={}` | Evaluated once at definition; state leaks between calls |
|
||||||
|
| Truthiness for presence | `if not amount:` where `0` is valid | Silently mishandles zero, empty string, empty list |
|
||||||
|
| `Any` as an escape | `Any` in a signature that could be `Protocol`/union/`TypeVar` | Removes exactly the checking `--strict` was adopted for |
|
||||||
|
| Unexplained ignore | bare `# type: ignore` | No error code, no reason; permanent by default |
|
||||||
|
| `assert` in production code | `assert user.is_admin` outside tests | `python -O` strips asserts — the check vanishes in a tuned deployment |
|
||||||
|
| Import-time side effects | connections, env reads, or I/O at module scope | Cannot be injected, cannot be tested, ordering-dependent |
|
||||||
|
| Sync I/O in `async def` | `requests.get`, `time.sleep`, blocking file reads | Blocks the whole event loop; looks concurrent, is not |
|
||||||
|
| Un-awaited coroutine | a call to an `async def` with no `await` | Silently does nothing; "coroutine was never awaited" is a warning, not an error |
|
||||||
|
| `float` for money | `float` amounts, `round()` on currency | `0.1 + 0.2 != 0.3`; use `Decimal` or int minor units |
|
||||||
|
| Broad `Exception` re-raise | `except Exception` around a whole function body | Turns a specific failure into an unclassifiable one |
|
||||||
|
| Mutating a shared default/global | module-level `list`/`dict` mutated by functions | The Python spelling of shared mutable state |
|
||||||
|
| `dict[str, Any]` as a domain type | dicts passed between layers | Shape known only by reading every producer; use a dataclass |
|
||||||
|
| Star imports | `from x import *` | Breaks `--strict`, hides shadowing, defeats grep |
|
||||||
|
| `os.path` string surgery | manual path joining, `+ "/"` | `pathlib` exists and is correct on every platform |
|
||||||
|
| `subprocess(..., shell=True)` with interpolation | f-strings into a shell command | Injection; pass a list, no shell |
|
||||||
|
|
||||||
|
## What to check that a linter cannot
|
||||||
|
|
||||||
|
- **Does the test suite assert the failure mode, or only the happy path?** `pytest.raises`
|
||||||
|
with no `match=` accepts any message, including one from an unrelated bug.
|
||||||
|
- **Are fixtures doing real work?** A `conftest.py` fixture with a `session` scope and
|
||||||
|
mutable state couples every test that touches it; failures then depend on ordering.
|
||||||
|
- **Is the mock mocking a boundary or a collaborator you own?** The latter tests the mock.
|
||||||
|
- **Does the type say what the code means?** `-> dict` and `-> tuple[int, int]` both pass
|
||||||
|
`--strict` and both tell the reader nothing. A named type is a review comment worth
|
||||||
|
making.
|
||||||
|
- **Would `--strict` actually run on this file?** Check for a per-module override in
|
||||||
|
`pyproject.toml` silently exempting it. An exemption nobody revisits is a hole with a
|
||||||
|
changelog entry.
|
||||||
|
|
||||||
|
## Severity in this context
|
||||||
|
|
||||||
|
- **Blocking:** anything in the table above under security or correctness — `shell=True`
|
||||||
|
interpolation, `assert` as an authorization check, `float` money, sync I/O in async.
|
||||||
|
- **Should fix:** typing escapes (`Any`, unexplained ignores), mutable defaults,
|
||||||
|
truthiness-for-presence.
|
||||||
|
- **Consider:** structural comments — dataclass instead of dict, narrower Protocol.
|
||||||
|
|
||||||
|
## See also
|
||||||
|
|
||||||
|
- `clean-code/references/python-adaptation.md` — the style-level conventions
|
||||||
|
- `solid/references/python-adaptation.md` — Protocol over ABC, DIP in Python
|
||||||
|
- `tdd/references/python.md` — the test-side conventions this review checks against
|
||||||
@@ -347,3 +347,12 @@ type UserWriter interface {
|
|||||||
- Load `refactoring` skill for systematic techniques to reduce identified hotspots
|
- Load `refactoring` skill for systematic techniques to reduce identified hotspots
|
||||||
- Load `solid` skill for structural fixes (coupling, cohesion)
|
- Load `solid` skill for structural fixes (coupling, cohesion)
|
||||||
- Load `clean-code` skill for naming improvements
|
- Load `clean-code` skill for naming improvements
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Read-only measurement. The score is the output.
|
||||||
|
|
||||||
|
Stop and hand back when: the measurement disagrees with the reader's lived experience of
|
||||||
|
the code. A metric that says "fine" about code everyone finds hard is evidence about the
|
||||||
|
metric, not about the code, and continuing to defend the number is how a measure becomes
|
||||||
|
a target.
|
||||||
|
|||||||
+12
-6
@@ -154,7 +154,7 @@ Pass-rate is computed downstream by the `/pass-rate` HTTP endpoint, which
|
|||||||
treats `pass` as success, `fail` as failure, `skip` as neither.
|
treats `pass` as success, `fail` as failure, `skip` as neither.
|
||||||
|
|
||||||
**At end of each phase:**
|
**At end of each phase:**
|
||||||
- `session_log` with `{skill: "debug", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "debug", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**Phases for this skill:** read-failure, generate-hypotheses, output
|
**Phases for this skill:** read-failure, generate-hypotheses, output
|
||||||
|
|
||||||
@@ -163,11 +163,7 @@ treats `pass` as success, `fail` as failure, `skip` as neither.
|
|||||||
- `fail` — the phase's intended outcome was NOT reached.
|
- `fail` — the phase's intended outcome was NOT reached.
|
||||||
- `skip` — phase was skipped intentionally.
|
- `skip` — phase was skipped intentionally.
|
||||||
|
|
||||||
**Why this matters:** the routing pod (Plan 6) reads pass-rate to decide whether to route a future call to a local model. If your skill never logs, the routing pod sees no data.
|
**Why this matters:** pass-rate is what a future routing decision would be based on — whether a given call can be served by a local model instead of a cloud one. A skill that never logs contributes no evidence either way.
|
||||||
|
|
||||||
## Mode 2 Routing Note
|
|
||||||
|
|
||||||
This skill produces high-volume mechanical output (hypothesis enumeration) and is a candidate for Mode 2 routing to a local model in the future. Until Plan 6 ships the routing pod, treat as Mode 1 only. The hypothesis format and discipline are identical regardless of which model generates them.
|
|
||||||
|
|
||||||
## Cross-References
|
## Cross-References
|
||||||
|
|
||||||
@@ -186,3 +182,13 @@ Techniques kept in `debug/references/`, inherited from the deleted
|
|||||||
including as one of the fixes already tried. A bumped timeout is a symptom.
|
including as one of the fixes already tried. A bumped timeout is a symptom.
|
||||||
- `find-polluter.sh` — run when a test passes alone and fails in the suite, or when
|
- `find-polluter.sh` — run when a test passes alone and fails in the suite, or when
|
||||||
something in the tree appears that no test admits to creating.
|
something in the tree appears that no test admits to creating.
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Diagnose only — Iron Law 3 is already an escalation rule: the fix is a separate TDD cycle,
|
||||||
|
not this skill's output.
|
||||||
|
|
||||||
|
Stop and hand back when: the report contains no failure text and no locator, in which case
|
||||||
|
name the specific artifact you need (run id, job log, stack trace) and stop rather than
|
||||||
|
hypothesising into a vacuum; or the evidence needed to discriminate between your top two
|
||||||
|
hypotheses requires access you do not have.
|
||||||
|
|||||||
@@ -152,3 +152,12 @@ hand-wavy. Crisp solution + fuzzy problem = reverse-engineered justification. St
|
|||||||
|
|
||||||
> Named instances of both war stories are client-tagged and held out of this public skill
|
> Named instances of both war stories are client-tagged and held out of this public skill
|
||||||
> pending client-store decision. The transferable lesson lives here; the wrapper stays out.
|
> pending client-store decision. The transferable lesson lives here; the wrapper stays out.
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Advisory. Produces a problem brief; commits to nothing.
|
||||||
|
|
||||||
|
Stop and hand back when: the evidence needed to size a problem requires talking to
|
||||||
|
customers you cannot reach. Sizing a problem from assumptions and presenting it as
|
||||||
|
discovery is worse than saying the number is unknown — it launders a guess into a brief
|
||||||
|
that later decisions will cite.
|
||||||
|
|||||||
@@ -185,7 +185,7 @@ If silhouette ≤ 0.20:
|
|||||||
|
|
||||||
**After experiment concludes:**
|
**After experiment concludes:**
|
||||||
- `brain_write` to `jepa-fx/failures/` with any new failure modes discovered
|
- `brain_write` to `jepa-fx/failures/` with any new failure modes discovered
|
||||||
- `session_log` with outcome
|
- `session_log` with `{session_id: "<this session's id>", skill: "experiment-spec", phase: "conclude", final_status: "pass" | "fail" | "skip", message: "<one-line outcome>"}`
|
||||||
|
|
||||||
## Cross-References
|
## Cross-References
|
||||||
|
|
||||||
|
|||||||
@@ -245,7 +245,7 @@ Pass-rate is computed downstream by the `/pass-rate` HTTP endpoint, which
|
|||||||
treats `pass` as success, `fail` as failure, `skip` as neither.
|
treats `pass` as success, `fail` as failure, `skip` as neither.
|
||||||
|
|
||||||
**At end of each phase:**
|
**At end of each phase:**
|
||||||
- `session_log` with `{skill: "feature-spec", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "feature-spec", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**Phases for this skill:** draft
|
**Phases for this skill:** draft
|
||||||
|
|
||||||
@@ -254,11 +254,7 @@ treats `pass` as success, `fail` as failure, `skip` as neither.
|
|||||||
- `fail` — the phase's intended outcome was NOT reached (spec rejected, blocked on unresolved questions).
|
- `fail` — the phase's intended outcome was NOT reached (spec rejected, blocked on unresolved questions).
|
||||||
- `skip` — phase was skipped intentionally.
|
- `skip` — phase was skipped intentionally.
|
||||||
|
|
||||||
**Why this matters:** the routing pod (Plan 6) reads pass-rate to decide whether to route a future call to a local model. If your skill never logs, the routing pod sees no data.
|
**Why this matters:** pass-rate is what a future routing decision would be based on — whether a given call can be served by a local model instead of a cloud one. A skill that never logs contributes no evidence either way.
|
||||||
|
|
||||||
## Mode 2 Routing Note
|
|
||||||
|
|
||||||
Spec writing requires interpretation of intent and tradeoff reasoning — work where Claude generally outperforms a 9-13B local model. This skill is intentionally **not** a Mode 2 routing candidate. In Mode 2, the routing layer should leave `feature-spec` calls on Claude.
|
|
||||||
|
|
||||||
## Cross-References
|
## Cross-References
|
||||||
|
|
||||||
|
|||||||
@@ -148,3 +148,12 @@ This skill maps to the **convergence moments** in the Double Diamond workflow:
|
|||||||
- **Mid-Diamond 2 (Develop):** Use Quick Poke when an agent's experiment result is ambiguous and you're deciding whether to continue or pivot.
|
- **Mid-Diamond 2 (Develop):** Use Quick Poke when an agent's experiment result is ambiguous and you're deciding whether to continue or pivot.
|
||||||
|
|
||||||
The skill does NOT replace human judgment at convergence points — it sharpens the plan so that judgment is better informed.
|
The skill does NOT replace human judgment at convergence points — it sharpens the plan so that judgment is better informed.
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Advisory. Produces sharper questions and a better plan; decides nothing.
|
||||||
|
|
||||||
|
Stop and hand back when: the plan's weak point is a decision that is the operator's to
|
||||||
|
make rather than a gap in reasoning — surface it as a question with options and a
|
||||||
|
recommendation, and do not grill past it. Also stop when a round produces no new
|
||||||
|
objection: interrogation past that point manufactures doubt rather than finding it.
|
||||||
|
|||||||
@@ -116,7 +116,7 @@ Pass-rate is computed downstream by the `/pass-rate` HTTP endpoint, which
|
|||||||
treats `pass` as success, `fail` as failure, `skip` as neither.
|
treats `pass` as success, `fail` as failure, `skip` as neither.
|
||||||
|
|
||||||
**At end of each phase:**
|
**At end of each phase:**
|
||||||
- `session_log` with `{skill: "session-retrospective", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "session-retrospective", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**Phases for this skill:** surfaced
|
**Phases for this skill:** surfaced
|
||||||
|
|
||||||
@@ -125,11 +125,7 @@ treats `pass` as success, `fail` as failure, `skip` as neither.
|
|||||||
- `fail` — the phase's intended outcome was NOT reached (retrospective could not produce coherent candidates).
|
- `fail` — the phase's intended outcome was NOT reached (retrospective could not produce coherent candidates).
|
||||||
- `skip` — phase was skipped intentionally (no novel learnings worth surfacing).
|
- `skip` — phase was skipped intentionally (no novel learnings worth surfacing).
|
||||||
|
|
||||||
**Why this matters:** the routing pod (Plan 6) reads pass-rate to decide whether to route a future call to a local model. If your skill never logs, the routing pod sees no data.
|
**Why this matters:** pass-rate is what a future routing decision would be based on — whether a given call can be served by a local model instead of a cloud one. A skill that never logs contributes no evidence either way.
|
||||||
|
|
||||||
## Mode 2 Routing Note
|
|
||||||
|
|
||||||
Reading long session logs and pulling themes is high-volume, mechanical pattern-recognition work — a strong Mode 2 routing candidate. Until Plan 6 ships the routing pod, treat as Mode 1 only.
|
|
||||||
|
|
||||||
## Cross-References
|
## Cross-References
|
||||||
|
|
||||||
|
|||||||
@@ -258,5 +258,15 @@ Domain types know nothing about HTTP, SQL, or external APIs.
|
|||||||
|
|
||||||
- `references/solid-principles.md` — canonical SOLID reference with TypeScript examples
|
- `references/solid-principles.md` — canonical SOLID reference with TypeScript examples
|
||||||
- `references/go-adaptation.md` — this workspace's Go-specific rewrite of each principle
|
- `references/go-adaptation.md` — this workspace's Go-specific rewrite of each principle
|
||||||
|
- `references/python-adaptation.md` — the Python rewrite: Protocol over ABC, and what does not carry over from Go
|
||||||
- Load `clean-code` skill for naming and structure
|
- Load `clean-code` skill for naming and structure
|
||||||
- Load `code-review` skill for detecting violations during review
|
- Load `code-review` skill for detecting violations during review
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Advisory. Names principle violations and suggests structure; writes no code.
|
||||||
|
|
||||||
|
Stop and hand back when: applying a principle would require a change larger than the one
|
||||||
|
under discussion. "This wants dependency inversion" on a two-line fix is a design
|
||||||
|
conversation, not a review comment, and forcing it through as one is how a small change
|
||||||
|
acquires an architecture.
|
||||||
|
|||||||
@@ -0,0 +1,104 @@
|
|||||||
|
# SOLID Principles: Python Adaptation
|
||||||
|
|
||||||
|
`references/go-adaptation.md` rewrites each principle for Go. This file does the same
|
||||||
|
for Python, and marks the places where the Go rewrite's advice does *not* carry over.
|
||||||
|
|
||||||
|
The canonical Python stack lives in `~/dev/.context/AGENT.md`; it is not restated here.
|
||||||
|
|
||||||
|
## The one idea that changes everything: `Protocol`
|
||||||
|
|
||||||
|
Go's interfaces are structural — a type satisfies one by having the methods, with no
|
||||||
|
declaration. Python has the same thing since 3.8, and it is the right default:
|
||||||
|
|
||||||
|
```python
|
||||||
|
from typing import Protocol
|
||||||
|
|
||||||
|
class InvoiceStore(Protocol):
|
||||||
|
def get(self, invoice_id: InvoiceId) -> Invoice: ...
|
||||||
|
def save(self, invoice: Invoice) -> None: ...
|
||||||
|
```
|
||||||
|
|
||||||
|
- **Define the Protocol where it is *consumed*, not where it is implemented.** This is
|
||||||
|
Go's "accept interfaces, return structs" and it is what makes DIP real: the high-level
|
||||||
|
module owns the abstraction, and the adapter depends on it rather than the reverse.
|
||||||
|
- **Prefer `Protocol` to `ABC`.** An ABC forces implementers to inherit, which couples
|
||||||
|
them to your package and makes a third-party class impossible to adapt without a
|
||||||
|
wrapper. Use an ABC only when you genuinely need shared implementation, not just a
|
||||||
|
contract.
|
||||||
|
- Add `@runtime_checkable` only if you actually call `isinstance` — and if you are
|
||||||
|
calling `isinstance` against a Protocol, check whether the real problem is a missing
|
||||||
|
polymorphic method (an LSP smell).
|
||||||
|
|
||||||
|
## S — Single Responsibility
|
||||||
|
|
||||||
|
Same as Go: the unit is the module, not just the class. `utils.py` is the Python god
|
||||||
|
package — it cannot have one reason to change because it has no subject.
|
||||||
|
|
||||||
|
Python-specific tell: a class whose `__init__` stores arguments and whose only other
|
||||||
|
method is `run()` or `execute()`. That is a function with ceremony. Write the function.
|
||||||
|
|
||||||
|
## O — Open/Closed
|
||||||
|
|
||||||
|
Extension in Python usually means **passing behaviour in**, not subclassing:
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Closed for modification: adding a rule does not touch this function
|
||||||
|
def apply_rules(invoice: Invoice, rules: Sequence[Rule]) -> Invoice: ...
|
||||||
|
```
|
||||||
|
|
||||||
|
`functools.singledispatch` is the idiomatic open extension point when dispatch is on
|
||||||
|
type and the set of types grows outside your module. Beware the alternative that looks
|
||||||
|
Pythonic and is not: monkey-patching is modification pretending to be extension, and it
|
||||||
|
is invisible at the call site.
|
||||||
|
|
||||||
|
## L — Liskov
|
||||||
|
|
||||||
|
The Python-specific violation is **signature widening/narrowing that the type checker
|
||||||
|
would catch if you let it**. Under `mypy --strict`, an override that narrows a parameter
|
||||||
|
type or widens a return type is an error — which means most LSP violations in Python are
|
||||||
|
type errors you chose not to see. This is the strongest argument for `--strict` in a
|
||||||
|
design document rather than a style one.
|
||||||
|
|
||||||
|
Raising a *new* exception type in an override is the violation `mypy` will not catch:
|
||||||
|
callers wrote `except StoreError`, and your subclass raises `TimeoutError`. Exceptions
|
||||||
|
are part of the contract; Python just will not check them for you.
|
||||||
|
|
||||||
|
## I — Interface Segregation
|
||||||
|
|
||||||
|
Protocols make this cheap: define the narrow one you need at the call site.
|
||||||
|
|
||||||
|
```python
|
||||||
|
class InvoiceReader(Protocol):
|
||||||
|
def get(self, invoice_id: InvoiceId) -> Invoice: ...
|
||||||
|
```
|
||||||
|
|
||||||
|
A function that only reads should accept `InvoiceReader`, not the full `InvoiceStore`.
|
||||||
|
The test double then implements one method instead of eight, which is the practical
|
||||||
|
payoff and the reason this principle survives contact with real code.
|
||||||
|
|
||||||
|
## D — Dependency Inversion
|
||||||
|
|
||||||
|
Constructor injection, exactly as in Go. Two Python-specific hazards:
|
||||||
|
|
||||||
|
- **Import-time side effects are hidden dependencies.** A module that opens a database
|
||||||
|
connection or reads an env var at import has a dependency nothing can inject and no
|
||||||
|
test can replace. Configuration belongs in `pydantic-settings` objects passed in.
|
||||||
|
- **Default arguments are compile-time bindings.** `def send(client: Client = Client())`
|
||||||
|
constructs the default once, at definition, and welds it to every caller. Default to
|
||||||
|
`None` and construct inside, or require it.
|
||||||
|
|
||||||
|
## What does not carry over from the Go rewrite
|
||||||
|
|
||||||
|
- **"Return structs, accept interfaces"** — the returning half is about Go's zero values
|
||||||
|
and nil interfaces. In Python, returning a Protocol type is fine and often better.
|
||||||
|
- **Small interfaces because of embedding** — Python has no embedding; composition is
|
||||||
|
explicit delegation, which is more verbose but reads honestly.
|
||||||
|
- **Package-level responsibility as the unit of deployment** — Python packaging is much
|
||||||
|
looser. The module boundary is still the right unit for *reasoning*, but do not expect
|
||||||
|
it to be enforced.
|
||||||
|
|
||||||
|
## See also
|
||||||
|
|
||||||
|
- `references/solid-principles.md` — the canonical, language-neutral reference
|
||||||
|
- `references/go-adaptation.md` — the Go rewrite; contrast is instructive
|
||||||
|
- `clean-code/references/python-adaptation.md` — style-level Python conventions
|
||||||
@@ -332,7 +332,7 @@ Pass-rate is computed downstream by the `/pass-rate` HTTP endpoint, which
|
|||||||
treats `pass` as success, `fail` as failure, `skip` as neither.
|
treats `pass` as success, `fail` as failure, `skip` as neither.
|
||||||
|
|
||||||
**At end of each phase:**
|
**At end of each phase:**
|
||||||
- `session_log` with `{skill: "spec-driven-dev", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "spec-driven-dev", phase: "<phase-name>", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**Phases for this skill:** specify, plan, tasks, implement
|
**Phases for this skill:** specify, plan, tasks, implement
|
||||||
|
|
||||||
@@ -341,7 +341,7 @@ treats `pass` as success, `fail` as failure, `skip` as neither.
|
|||||||
- `fail` — the phase's intended outcome was NOT reached (gate blocked, rework required).
|
- `fail` — the phase's intended outcome was NOT reached (gate blocked, rework required).
|
||||||
- `skip` — phase was skipped intentionally.
|
- `skip` — phase was skipped intentionally.
|
||||||
|
|
||||||
**Why this matters:** the routing pod (Plan 6) reads pass-rate to decide whether to route a future call to a local model. If your skill never logs, the routing pod sees no data.
|
**Why this matters:** pass-rate is what a future routing decision would be based on — whether a given call can be served by a local model instead of a cloud one. A skill that never logs contributes no evidence either way.
|
||||||
|
|
||||||
## Cross-References
|
## Cross-References
|
||||||
|
|
||||||
|
|||||||
@@ -151,3 +151,12 @@ stays in the client boundary**:
|
|||||||
- Council/forum names, internal stage labels, internal tooling, named contacts, strategy
|
- Council/forum names, internal stage labels, internal tooling, named contacts, strategy
|
||||||
specifics, deal terms → client dir/org only, local models only, never the public brain.
|
specifics, deal terms → client dir/org only, local models only, never the public brain.
|
||||||
- This skill carries the **pattern**; the client carries the **wrapper**. Never merge them.
|
- This skill carries the **pattern**; the client carries the **wrapper**. Never merge them.
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Advisory. Prepares or chairs a gate; the council decides, not this skill.
|
||||||
|
|
||||||
|
Stop and hand back when: a discipline's evidence is missing rather than weak. A gap on a
|
||||||
|
gating discipline means do not bring the ask — bring a feedback-only ask instead. Marking
|
||||||
|
it "cleared with a note" is precisely the gate theatre the anti-patterns section is
|
||||||
|
about.
|
||||||
|
|||||||
+41
-14
@@ -100,13 +100,21 @@ go test -run TestRetryOperation_RetriesThreeTimes ./...
|
|||||||
```
|
```
|
||||||
|
|
||||||
Confirm:
|
Confirm:
|
||||||
- Test fails (not compilation errors)
|
- Test fails, and fails because the feature is missing
|
||||||
- Failure message is expected
|
- The failure message is the one you expected
|
||||||
- Fails because feature is missing (not typos)
|
- It is not failing on a typo in the test itself
|
||||||
|
|
||||||
**Test passes?** You're testing existing behavior. Fix test.
|
**Test passes?** You're testing existing behavior. Fix test.
|
||||||
|
|
||||||
**Compilation errors?** Fix errors, re-run until it fails correctly.
|
**Failing because the thing does not exist yet?** That is a legitimate red — a Go
|
||||||
|
compile error for an undefined function, a Python `ImportError` for a module you have
|
||||||
|
not written, a TypeScript "cannot find module". Read the message, confirm it names the
|
||||||
|
unit you are about to build, and proceed to GREEN. **Do not write a stub to make the
|
||||||
|
runner happier.** A stub is production code written before a failing test, which is the
|
||||||
|
Iron Law with extra steps.
|
||||||
|
|
||||||
|
**Failing on a typo or a broken import elsewhere?** Different thing. Fix it and re-run
|
||||||
|
until the failure is the one above.
|
||||||
|
|
||||||
### GREEN - Minimal Code
|
### GREEN - Minimal Code
|
||||||
|
|
||||||
@@ -166,6 +174,23 @@ Keep tests green. Don't add behavior.
|
|||||||
|
|
||||||
Next failing test for next behavior.
|
Next failing test for next behavior.
|
||||||
|
|
||||||
|
## Other languages
|
||||||
|
|
||||||
|
The discipline above is language-neutral; the examples below it are Go, because Go is
|
||||||
|
this fleet's default stack and one good example beats five mediocre ones.
|
||||||
|
|
||||||
|
For anything else, the per-language conventions live in reference files — load the one
|
||||||
|
you need rather than translating Go idioms by eye. Translation is where the mistakes
|
||||||
|
happen: the Go table-driven exemplar below branches on a `wantErr` column inside the
|
||||||
|
loop, which is idiomatic Go and an anti-pattern in pytest, where it produces a
|
||||||
|
conditional assertion and folds two behaviours into one test.
|
||||||
|
|
||||||
|
- `references/python.md` — pytest, `@pytest.mark.parametrize`, `mypy --strict` in the
|
||||||
|
RED gate, `pytest.raises(..., match=)`, fixtures vs helpers
|
||||||
|
- `references/typescript.md` — vitest/jest, `it.each`, `expect(...).rejects`
|
||||||
|
- `references/rust.md` — `#[cfg(test)]` modules, table-driven via arrays, `#[should_panic]`
|
||||||
|
- `references/java.md` — JUnit 5, `@ParameterizedTest`, `assertThrows`
|
||||||
|
|
||||||
## Go-Specific TDD Notes
|
## Go-Specific TDD Notes
|
||||||
|
|
||||||
### Table-Driven Tests (Preferred Pattern)
|
### Table-Driven Tests (Preferred Pattern)
|
||||||
@@ -329,8 +354,14 @@ Before marking work complete:
|
|||||||
- [ ] Watched each test fail before implementing
|
- [ ] Watched each test fail before implementing
|
||||||
- [ ] Each test failed for expected reason (feature missing, not typo)
|
- [ ] Each test failed for expected reason (feature missing, not typo)
|
||||||
- [ ] Wrote minimal code to pass each test
|
- [ ] Wrote minimal code to pass each test
|
||||||
- [ ] All tests pass: `go test ./...`
|
- [ ] All tests pass
|
||||||
- [ ] Race detector clean: `go test -race ./...`
|
- [ ] The language's static gate is clean — `go vet`, `mypy --strict`, `tsc --noEmit`,
|
||||||
|
`cargo clippy`. A red type-check is a red build; it does not wait for GREEN.
|
||||||
|
- [ ] The language's concurrency gate is clean **where one exists and applies** —
|
||||||
|
`go test -race`, `cargo test` under Miri for unsafe code. There is no equivalent
|
||||||
|
for a pure Python function, and a checklist box that can never be ticked teaches
|
||||||
|
people to skip checklists. Tick it or strike it deliberately; do not leave it
|
||||||
|
hanging.
|
||||||
- [ ] Tests use real code (mocks only if crossing a system boundary)
|
- [ ] Tests use real code (mocks only if crossing a system boundary)
|
||||||
- [ ] Edge cases and errors covered
|
- [ ] Edge cases and errors covered
|
||||||
|
|
||||||
@@ -375,20 +406,20 @@ Pass-rate is computed downstream by the `/pass-rate` HTTP endpoint, which
|
|||||||
treats `pass` as success, `fail` as failure, `skip` as neither.
|
treats `pass` as success, `fail` as failure, `skip` as neither.
|
||||||
|
|
||||||
**At end of `red` phase:**
|
**At end of `red` phase:**
|
||||||
- `session_log` with `{skill: "tdd", phase: "red", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "tdd", phase: "red", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**At end of `green` phase:**
|
**At end of `green` phase:**
|
||||||
- `session_log` with `{skill: "tdd", phase: "green", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "tdd", phase: "green", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**At end of `refactor` phase:**
|
**At end of `refactor` phase:**
|
||||||
- `session_log` with `{skill: "tdd", phase: "refactor", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "tdd", phase: "refactor", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**Status semantics:**
|
**Status semantics:**
|
||||||
- `pass` — the phase's intended outcome was reached (red: test fails as expected; green: test passes; refactor: tests still pass after refactor).
|
- `pass` — the phase's intended outcome was reached (red: test fails as expected; green: test passes; refactor: tests still pass after refactor).
|
||||||
- `fail` — the phase's intended outcome was NOT reached (test compiled when it shouldn't, test still fails after green attempt, refactor broke tests).
|
- `fail` — the phase's intended outcome was NOT reached (test compiled when it shouldn't, test still fails after green attempt, refactor broke tests).
|
||||||
- `skip` — phase was skipped intentionally (e.g. refactor not warranted).
|
- `skip` — phase was skipped intentionally (e.g. refactor not warranted).
|
||||||
|
|
||||||
**Why this matters:** the routing pod (Plan 6) reads pass-rate to decide whether to route a future `tdd` call to a local model. If your skill never logs, the routing pod sees no data and may default-route or default-not-route in a way that doesn't reflect real performance.
|
**Why this matters:** pass-rate is what a future routing decision would be based on — whether a `tdd` call can be served by a local model instead of a cloud one. A skill that never logs contributes no evidence either way, and a decision made on no evidence is a coin flip wearing a metric.
|
||||||
|
|
||||||
## Final Rule
|
## Final Rule
|
||||||
|
|
||||||
@@ -398,7 +429,3 @@ Otherwise → not TDD
|
|||||||
```
|
```
|
||||||
|
|
||||||
No exceptions without Mathias's permission.
|
No exceptions without Mathias's permission.
|
||||||
|
|
||||||
## Mode 2 Routing Note
|
|
||||||
|
|
||||||
This skill is invoked identically whether the agent is running in Mode 1 (cloud Claude, no routing) or Mode 2 (client-local, supervisor routing layer). The routing pod (Plan 6) does not exist yet; until it does, treat this skill as Mode 1 only. The discipline does not change between modes — only the model behind the call.
|
|
||||||
|
|||||||
@@ -0,0 +1,78 @@
|
|||||||
|
# TDD in Java
|
||||||
|
|
||||||
|
The Iron Law and Red-Green-Refactor in `SKILL.md` apply unchanged. This file replaces
|
||||||
|
the Go-Specific TDD Notes for Java. JUnit 5 (Jupiter) throughout.
|
||||||
|
|
||||||
|
## The first red is a compile error, and that is correct
|
||||||
|
|
||||||
|
`cannot find symbol: method splitVat` is a legitimate red. Do not stub the method with
|
||||||
|
`throw new UnsupportedOperationException()` to make the module compile — that is
|
||||||
|
production code before a failing test.
|
||||||
|
|
||||||
|
## Table-driven: `@ParameterizedTest`
|
||||||
|
|
||||||
|
```java
|
||||||
|
@ParameterizedTest(name = "{0}")
|
||||||
|
@MethodSource("vatCases")
|
||||||
|
void roundsHalfToEven(String name, long amountMinor, BigDecimal rate, long net, long vat) {
|
||||||
|
assertEquals(new Split(net, vat), VatSplitter.split(amountMinor, rate));
|
||||||
|
}
|
||||||
|
|
||||||
|
static Stream<Arguments> vatCases() {
|
||||||
|
return Stream.of(
|
||||||
|
Arguments.of("tie rounds down to even", 14L, new BigDecimal("0.12"), 12L, 2L),
|
||||||
|
Arguments.of("tie rounds up to even", 42L, new BigDecimal("0.12"), 38L, 4L),
|
||||||
|
Arguments.of("exact division", 12500L, new BigDecimal("0.25"), 10000L, 2500L)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`name = "{0}"` puts the case label in the report — the `t.Run` analogue. `@CsvSource` is
|
||||||
|
terser for primitives; `@MethodSource` is the one to use as soon as a case carries a
|
||||||
|
`BigDecimal` or a domain object.
|
||||||
|
|
||||||
|
## Error cases get their own test
|
||||||
|
|
||||||
|
```java
|
||||||
|
@Test
|
||||||
|
void rejectsNegativeAmount() {
|
||||||
|
var e = assertThrows(IllegalArgumentException.class,
|
||||||
|
() -> VatSplitter.split(-1, new BigDecimal("0.25")));
|
||||||
|
assertTrue(e.getMessage().contains("non-negative"));
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Assert on the message, not only the type — `IllegalArgumentException` is thrown by half
|
||||||
|
the JDK, and a test that accepts any of them will keep passing for the wrong reason.
|
||||||
|
|
||||||
|
## Money is `BigDecimal` or `long` minor units, never `double`
|
||||||
|
|
||||||
|
`0.1 + 0.2 != 0.3` is not a rounding curiosity in a payments system, it is a defect.
|
||||||
|
Where a rounding mode matters, state it: `setScale(0, RoundingMode.HALF_EVEN)`. Banker's
|
||||||
|
rounding is the default expectation in Swedish accounting and is *not* Java's default.
|
||||||
|
|
||||||
|
## Assertions
|
||||||
|
|
||||||
|
Prefer AssertJ (`assertThat(x).isEqualTo(y)`) where the project already has it: the
|
||||||
|
failure messages name the actual value. Where it does not, plain JUnit assertions are
|
||||||
|
fine — do not add a dependency for aesthetics.
|
||||||
|
|
||||||
|
`assertAll` is the closest analogue to Go's soft `assert`: it reports every failure in
|
||||||
|
the group instead of stopping at the first.
|
||||||
|
|
||||||
|
## Mocks
|
||||||
|
|
||||||
|
Mockito is standard, and easy to overuse. Per `SKILL.md`, mock only across a system
|
||||||
|
boundary. A mocked class you own tests the mock; a `@SpringBootTest` that mocks the
|
||||||
|
repository tests neither the wiring nor the query.
|
||||||
|
|
||||||
|
## Gates
|
||||||
|
|
||||||
|
```bash
|
||||||
|
./gradlew test # the failing test
|
||||||
|
./gradlew check # static gate: compiler warnings, spotless, errorprone
|
||||||
|
```
|
||||||
|
|
||||||
|
Java has no `-race`. For concurrent code the honest substitutes are a stress test with
|
||||||
|
`CountDownLatch` to force the interleaving, and jcstress for lock-free work. A pure
|
||||||
|
function should strike that checklist box deliberately.
|
||||||
@@ -0,0 +1,109 @@
|
|||||||
|
# TDD in Python
|
||||||
|
|
||||||
|
The Iron Law, Red-Green-Refactor and the rationalization table in `SKILL.md` apply
|
||||||
|
unchanged. This file replaces the Go-Specific TDD Notes for Python work.
|
||||||
|
|
||||||
|
Canonical stack (`~/dev/.context/AGENT.md`): pytest + pytest-cov, `mypy --strict`, ruff,
|
||||||
|
uv, src-layout. Not restated here — that table has one home.
|
||||||
|
|
||||||
|
## The first red is an ImportError, and that is correct
|
||||||
|
|
||||||
|
For a function that does not exist yet, the first run fails at collection:
|
||||||
|
|
||||||
|
```
|
||||||
|
E ModuleNotFoundError: No module named 'invoicing.vat'
|
||||||
|
```
|
||||||
|
|
||||||
|
That is a legitimate red. Read it, confirm it names the module you are about to write,
|
||||||
|
and go to GREEN. **Do not create a stub raising `NotImplementedError` to make pytest
|
||||||
|
collect.** A stub is production code written before a failing test.
|
||||||
|
|
||||||
|
## Table-driven: `parametrize`, one behaviour per function
|
||||||
|
|
||||||
|
```python
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("amount_minor", "rate", "expected"),
|
||||||
|
[
|
||||||
|
pytest.param(14, Decimal("0.12"), (12, 2), id="tie-rounds-down-to-even"),
|
||||||
|
pytest.param(42, Decimal("0.12"), (38, 4), id="tie-rounds-up-to-even"),
|
||||||
|
pytest.param(12500, Decimal("0.25"), (10000, 2500), id="exact-division"),
|
||||||
|
pytest.param(0, Decimal("0.25"), (0, 0), id="zero-is-not-an-error"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_split_vat_rounds_half_to_even(
|
||||||
|
amount_minor: int, rate: Decimal, expected: tuple[int, int]
|
||||||
|
) -> None:
|
||||||
|
assert split_vat(amount_minor, rate) == expected
|
||||||
|
```
|
||||||
|
|
||||||
|
`id=` is the analogue of Go's `t.Run` name: it is what appears in the failure output and
|
||||||
|
what you pass to `pytest -k`.
|
||||||
|
|
||||||
|
**Do not port Go's `wantErr` column.** The Go exemplar branches inside the loop —
|
||||||
|
`if tt.wantErr { assert.Error } else { assert.NoError }`. Translated literally that
|
||||||
|
gives a conditional assertion and folds two behaviours into one test, which `SKILL.md`'s
|
||||||
|
own quality table forbids ("One thing. 'and' in name? Split it."). Error cases get
|
||||||
|
their own function:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_split_vat_rejects_negative_amount() -> None:
|
||||||
|
with pytest.raises(ValueError, match="amount_minor must be non-negative"):
|
||||||
|
split_vat(-1, Decimal("0.25"))
|
||||||
|
```
|
||||||
|
|
||||||
|
`match=` pins the message, not just the type. Go's `assert.Error` does not, and the
|
||||||
|
stricter version is available here for free — use it.
|
||||||
|
|
||||||
|
## `mypy --strict` is part of RED, not a chore after GREEN
|
||||||
|
|
||||||
|
A red type-check is a red build.
|
||||||
|
|
||||||
|
```bash
|
||||||
|
uv run pytest tests/test_vat.py -x -q # the failing test
|
||||||
|
uv run mypy --strict src tests # same gate, same phase
|
||||||
|
```
|
||||||
|
|
||||||
|
Under `--strict` every test function needs `-> None`, or it is an untyped def and the
|
||||||
|
type-checker rejects the file — including the file that holds your failing test.
|
||||||
|
|
||||||
|
## Layout and imports
|
||||||
|
|
||||||
|
- `src/` layout, tests in a sibling `tests/`. This is the *opposite* of Go's
|
||||||
|
same-directory convention, and it buys black-box testing structurally: `from
|
||||||
|
invoicing.vat import split_vat` can only resolve against the installed package, so
|
||||||
|
the preference Go has to state explicitly is enforced here by the import system.
|
||||||
|
- No `__init__.py` in `tests/` unless you need shared test packages; pytest's rootdir
|
||||||
|
handling copes, and an accidental package shadows names.
|
||||||
|
- `conftest.py` holds fixtures shared across a directory. A fixture in a single test
|
||||||
|
file that only one test uses should be a plain function call instead — fixtures are
|
||||||
|
indirection, and indirection is only worth it at reuse.
|
||||||
|
|
||||||
|
## No `require` vs `assert`
|
||||||
|
|
||||||
|
Every pytest `assert` is fatal to its test; there is no continue-on-failure form. Where
|
||||||
|
Go would use `assert` to collect several soft failures, either split the test or use
|
||||||
|
`pytest.approx`/a single structural comparison:
|
||||||
|
|
||||||
|
```python
|
||||||
|
assert result == Invoice(id=InvoiceId("42"), net=10000, vat=2500) # one assertion, whole shape
|
||||||
|
```
|
||||||
|
|
||||||
|
Comparing whole values beats three field assertions: the failure output shows the entire
|
||||||
|
diff rather than the first field that differed.
|
||||||
|
|
||||||
|
## Fixtures vs helpers
|
||||||
|
|
||||||
|
- **Helper function** — when the setup is cheap and local. It reads top-to-bottom.
|
||||||
|
- **Fixture** — when the setup is shared, or needs teardown, or needs a scope
|
||||||
|
(`session`, `module`) because it is expensive.
|
||||||
|
- Set `__tracebackhide__ = True` in an assertion helper so failures point at the caller,
|
||||||
|
the way `t.Helper()` does in Go.
|
||||||
|
|
||||||
|
## Coverage is not the concurrency gate
|
||||||
|
|
||||||
|
`SKILL.md`'s checklist asks for the language's concurrency gate. Python has no `-race`
|
||||||
|
equivalent, and pretending `pytest --cov` fills that slot is dishonest — coverage says
|
||||||
|
which lines ran, not whether two threads can interleave badly. For a pure function,
|
||||||
|
strike the box deliberately. For genuinely concurrent code (`asyncio`, threads), the
|
||||||
|
substitute is a test that runs the interleaving you fear, plus `pytest-timeout` so a
|
||||||
|
deadlock fails rather than hangs.
|
||||||
@@ -0,0 +1,86 @@
|
|||||||
|
# TDD in Rust
|
||||||
|
|
||||||
|
The Iron Law and Red-Green-Refactor in `SKILL.md` apply unchanged. This file replaces
|
||||||
|
the Go-Specific TDD Notes for Rust.
|
||||||
|
|
||||||
|
## The first red is a compile error, and that is correct
|
||||||
|
|
||||||
|
```
|
||||||
|
error[E0425]: cannot find function `split_vat` in this scope
|
||||||
|
```
|
||||||
|
|
||||||
|
Legitimate — the compiler is the test runner's first phase. Read it, confirm it names
|
||||||
|
the item you are about to write, and go to GREEN. Do not write a `todo!()` stub first;
|
||||||
|
that is production code before a failing test. (`todo!()` is fine *during* GREEN as a
|
||||||
|
branch you have not reached yet.)
|
||||||
|
|
||||||
|
## Where tests live
|
||||||
|
|
||||||
|
- **Unit tests** in the same file, in `mod tests` behind `#[cfg(test)]`. They can reach
|
||||||
|
private items — the white-box option Go spells `package foo`.
|
||||||
|
- **Integration tests** in `tests/`, which link the crate as an external consumer and see
|
||||||
|
only the public API. This is the black-box default `SKILL.md` prefers; use it unless
|
||||||
|
the behaviour under test is genuinely private.
|
||||||
|
|
||||||
|
```rust
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn tie_rounds_down_to_even() {
|
||||||
|
assert_eq!(split_vat(14, dec!(0.12)).unwrap(), (12, 2));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Table-driven: an array of cases
|
||||||
|
|
||||||
|
```rust
|
||||||
|
#[test]
|
||||||
|
fn rounds_half_to_even() {
|
||||||
|
let cases = [
|
||||||
|
("tie rounds down", 14u64, dec!(0.12), (12, 2)),
|
||||||
|
("tie rounds up", 42, dec!(0.12), (38, 4)),
|
||||||
|
("exact division", 12500, dec!(0.25), (10000, 2500)),
|
||||||
|
];
|
||||||
|
for (name, amount, rate, want) in cases {
|
||||||
|
assert_eq!(split_vat(amount, rate).unwrap(), want, "case: {name}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
The `"case: {name}"` message is the `t.Run` analogue — without it a failure names a line
|
||||||
|
number and leaves you counting rows. If you want each case to be its own reported test,
|
||||||
|
use `rstest`'s `#[case]`; that is a dependency, so justify it before adding it.
|
||||||
|
|
||||||
|
## Error cases
|
||||||
|
|
||||||
|
`Result` makes the Go `wantErr` column unnecessary — assert on the variant:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
#[test]
|
||||||
|
fn rejects_negative_amount() {
|
||||||
|
let err = split_vat_signed(-1, dec!(0.25)).unwrap_err();
|
||||||
|
assert!(matches!(err, VatError::NegativeAmount));
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`#[should_panic(expected = "...")]` is for genuine panics only — invariant violations,
|
||||||
|
not expected error paths. A function whose contract includes "rejects negative input"
|
||||||
|
should return `Err`, not panic, and a test that accepts a panic hides that design choice.
|
||||||
|
|
||||||
|
## Gates
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cargo test # the failing test
|
||||||
|
cargo clippy -- -D warnings # static gate, same phase as RED
|
||||||
|
cargo fmt --check
|
||||||
|
```
|
||||||
|
|
||||||
|
`cargo test` compiles in debug, so integer overflow panics rather than wrapping — a
|
||||||
|
release build will not. Where overflow matters, test the checked path explicitly
|
||||||
|
(`checked_mul`), or you are testing debug-only behaviour.
|
||||||
|
|
||||||
|
For unsafe code, `cargo +nightly miri test` is the nearest thing to Go's `-race`.
|
||||||
|
Safe-only crates have no equivalent and should strike that checklist box deliberately.
|
||||||
@@ -0,0 +1,85 @@
|
|||||||
|
# TDD in TypeScript
|
||||||
|
|
||||||
|
The Iron Law and Red-Green-Refactor in `SKILL.md` apply unchanged. This file replaces
|
||||||
|
the Go-Specific TDD Notes for TypeScript.
|
||||||
|
|
||||||
|
Vitest is the default; Jest differs only in imports and a few matcher names, both noted
|
||||||
|
where it matters.
|
||||||
|
|
||||||
|
## The first red is a resolution error, and that is correct
|
||||||
|
|
||||||
|
```
|
||||||
|
Error: Failed to resolve import "./vat" from "src/vat.test.ts"
|
||||||
|
```
|
||||||
|
|
||||||
|
Legitimate. Do not create an empty module or a stub throwing `new Error("not
|
||||||
|
implemented")` to get past collection — that is production code before a failing test.
|
||||||
|
|
||||||
|
## Table-driven: `it.each`
|
||||||
|
|
||||||
|
```ts
|
||||||
|
describe('splitVat', () => {
|
||||||
|
it.each([
|
||||||
|
{ amountMinor: 14, rate: 0.12, expected: [12, 2], name: 'tie rounds down to even' },
|
||||||
|
{ amountMinor: 42, rate: 0.12, expected: [38, 4], name: 'tie rounds up to even' },
|
||||||
|
{ amountMinor: 12500, rate: 0.25, expected: [10000, 2500], name: 'exact division' },
|
||||||
|
])('$name', ({ amountMinor, rate, expected }) => {
|
||||||
|
expect(splitVat(amountMinor, rate)).toEqual(expected)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
```
|
||||||
|
|
||||||
|
`$name` interpolates into the test title, which is the `t.Run` analogue. Prefer the
|
||||||
|
object form over the array form — a positional tuple with five entries is unreadable at
|
||||||
|
the point of failure.
|
||||||
|
|
||||||
|
**Do not port Go's `wantErr` column.** Error cases get their own test:
|
||||||
|
|
||||||
|
```ts
|
||||||
|
it('rejects a negative amount', () => {
|
||||||
|
expect(() => splitVat(-1, 0.25)).toThrow(/must be non-negative/)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('rejects a negative amount (async)', async () => {
|
||||||
|
await expect(splitVat(-1, 0.25)).rejects.toThrow(/must be non-negative/)
|
||||||
|
})
|
||||||
|
```
|
||||||
|
|
||||||
|
The `await` on the async form is load-bearing. Without it the assertion returns a
|
||||||
|
promise nobody checks, the test passes, and it will keep passing after you break the
|
||||||
|
code. This is the single most common false-green in a TypeScript suite.
|
||||||
|
|
||||||
|
## `tsc --noEmit` is part of RED
|
||||||
|
|
||||||
|
A red type-check is a red build:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
npx vitest run src/vat.test.ts # the failing test
|
||||||
|
npx tsc --noEmit # same gate, same phase
|
||||||
|
```
|
||||||
|
|
||||||
|
Vitest transpiles without type-checking, so a suite can be green while the code does not
|
||||||
|
compile. `tsc --noEmit` is not optional here — it is the half of the gate the runner
|
||||||
|
does not perform.
|
||||||
|
|
||||||
|
## Equality matchers are not interchangeable
|
||||||
|
|
||||||
|
- `toBe` — `Object.is`. For primitives and identity.
|
||||||
|
- `toEqual` — structural, and **ignores `undefined` properties**: `{a: 1, b: undefined}`
|
||||||
|
equals `{a: 1}`. That is a real hole when the bug you are hunting is a missing field.
|
||||||
|
- `toStrictEqual` — structural, respects `undefined`, and checks class identity. Default
|
||||||
|
to this one; reach for `toEqual` deliberately.
|
||||||
|
|
||||||
|
## Mocks
|
||||||
|
|
||||||
|
`vi.mock` (Jest: `jest.mock`) is hoisted above imports, so a factory referencing an
|
||||||
|
outer variable throws at runtime. Use `vi.hoisted` when the mock needs shared state.
|
||||||
|
Per `SKILL.md`, mock only across a system boundary — a mocked module you own is a test
|
||||||
|
of your mock.
|
||||||
|
|
||||||
|
## Async and fake timers
|
||||||
|
|
||||||
|
- `vi.useFakeTimers()` needs a matching `vi.useRealTimers()` in teardown, or the next
|
||||||
|
file inherits frozen time and fails somewhere unrelated.
|
||||||
|
- Prefer condition polling (`await vi.waitFor(...)`) over a fixed `setTimeout` — an
|
||||||
|
arbitrary wait is a flake with a schedule.
|
||||||
@@ -82,3 +82,11 @@ created in a CAD session should trace to a TELOS GOAL. Every GOAL
|
|||||||
traces to a PROBLEM.
|
traces to a PROBLEM.
|
||||||
|
|
||||||
Traceability chain: PROBLEM → GOAL → SPEC → TICKET → IMPL → TEST
|
Traceability chain: PROBLEM → GOAL → SPEC → TICKET → IMPL → TEST
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Read-only. Loads intention context; changes nothing.
|
||||||
|
|
||||||
|
Stop and hand back when: the brain is unreachable or returns nothing for the wing. Working
|
||||||
|
from a remembered TELOS is the failure this skill exists to prevent — say the load failed
|
||||||
|
rather than reconstructing it from memory.
|
||||||
|
|||||||
@@ -309,3 +309,11 @@ Apply this checklist to every new test:
|
|||||||
- Load `tdd` skill for the full TDD workflow
|
- Load `tdd` skill for the full TDD workflow
|
||||||
- Load `code-review` skill for test quality review during pre-merge review
|
- Load `code-review` skill for test quality review during pre-merge review
|
||||||
- See `clean-code/references/code-smells.md` for testing-specific smells
|
- See `clean-code/references/code-smells.md` for testing-specific smells
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Read-only scoring of existing tests.
|
||||||
|
|
||||||
|
Stop and hand back when: the tests are fine and the *design under test* is what makes them
|
||||||
|
awkward. Hard-to-test is a design finding, not a test finding, and rewriting the tests
|
||||||
|
around it hides the thing worth knowing.
|
||||||
|
|||||||
@@ -231,10 +231,10 @@ Pass-rate is computed downstream by the `/pass-rate` HTTP endpoint, which
|
|||||||
treats `pass` as success, `fail` as failure, `skip` as neither.
|
treats `pass` as success, `fail` as failure, `skip` as neither.
|
||||||
|
|
||||||
**At end of `read` phase:**
|
**At end of `read` phase:**
|
||||||
- `session_log` with `{skill: "trainer", phase: "read", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "trainer", phase: "read", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**At end of `write` phase:**
|
**At end of `write` phase:**
|
||||||
- `session_log` with `{skill: "trainer", phase: "write", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root: "<absolute path>"}`
|
- `session_log` with `{session_id: "<this session's id>", skill: "trainer", phase: "write", final_status: "pass" | "fail" | "skip", message: "<one-line summary>", project_root: "<absolute path>"}`
|
||||||
|
|
||||||
**Phases for this skill:** read, write
|
**Phases for this skill:** read, write
|
||||||
|
|
||||||
@@ -243,11 +243,7 @@ treats `pass` as success, `fail` as failure, `skip` as neither.
|
|||||||
- `fail` — the phase's intended outcome was NOT reached.
|
- `fail` — the phase's intended outcome was NOT reached.
|
||||||
- `skip` — phase was skipped intentionally (e.g. no candidates passed the gate, write skipped).
|
- `skip` — phase was skipped intentionally (e.g. no candidates passed the gate, write skipped).
|
||||||
|
|
||||||
**Why this matters:** the routing pod (Plan 6) reads pass-rate to decide whether to route a future call to a local model. If your skill never logs, the routing pod sees no data.
|
**Why this matters:** pass-rate is what a future routing decision would be based on — whether a given call can be served by a local model instead of a cloud one. A skill that never logs contributes no evidence either way.
|
||||||
|
|
||||||
## Mode 2 Routing Note
|
|
||||||
|
|
||||||
Trainer is the same shape as `session-retrospective` — long-context reading, mechanical filtering — and is a Mode 2 routing candidate. Until Plan 6, treat as Mode 1 only.
|
|
||||||
|
|
||||||
## Cross-References
|
## Cross-References
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,85 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# Tests for bin/check-escalation.
|
||||||
|
#
|
||||||
|
# #5 asked whether SKILL.md needs first-class failure/escalation fields and
|
||||||
|
# decided: not frontmatter (nothing reads it), a body section (the model reads
|
||||||
|
# that). #37 then had to define WHICH skills need one, and every obvious answer
|
||||||
|
# was bad — a hardcoded list rots, content sniffing is unfalsifiable, a
|
||||||
|
# frontmatter flag is the rejected option through the back door.
|
||||||
|
#
|
||||||
|
# So there is no acting set. EVERY skill answers the question; a read-only one
|
||||||
|
# answers it in a line. The classification problem disappears, and "this skill
|
||||||
|
# never hands back" becomes a stated claim rather than an absence.
|
||||||
|
#
|
||||||
|
# bin/escalation-pending.txt is a ratchet for skills whose real rule has not
|
||||||
|
# been written yet. It is self-cleaning: a skill that has a section AND is still
|
||||||
|
# listed fails, so the list cannot silently outlive its reason.
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
|
||||||
|
SUT="$ROOT/bin/check-escalation"
|
||||||
|
|
||||||
|
pass=0
|
||||||
|
fail=0
|
||||||
|
WORK=${TMPDIR:-/tmp}/check_escalation_test.$$
|
||||||
|
|
||||||
|
if [ ! -f "$SUT" ]; then
|
||||||
|
printf 'FAIL: %s does not exist\n' "$SUT"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
cleanup() { rm -rf "$WORK"; }
|
||||||
|
trap cleanup EXIT INT TERM
|
||||||
|
|
||||||
|
# fixture <name> — an empty checkout with a bin/ for the pending list.
|
||||||
|
fixture() {
|
||||||
|
_f="$WORK/$1"
|
||||||
|
mkdir -p "$_f/skills" "$_f/bin"
|
||||||
|
: > "$_f/bin/escalation-pending.txt"
|
||||||
|
printf '%s' "$_f"
|
||||||
|
}
|
||||||
|
|
||||||
|
# skill <root> <name> <has-section>
|
||||||
|
skill() {
|
||||||
|
mkdir -p "$1/skills/$2"
|
||||||
|
{
|
||||||
|
printf -- '---\nname: %s\ndescription: d\n---\n\n# %s\n' "$2" "$2"
|
||||||
|
[ "$3" = yes ] && printf '\n## Escalation\n\nStop and hand back when X. Hand back by: saying so.\n'
|
||||||
|
} > "$1/skills/$2/SKILL.md"
|
||||||
|
}
|
||||||
|
|
||||||
|
# expect <rc> <label> <fixture> [needle]
|
||||||
|
expect() {
|
||||||
|
_want=$1; _label=$2; _fx=$3; _needle=${4:-}
|
||||||
|
_out=$(sh "$SUT" "$_fx" 2>&1); _rc=$?
|
||||||
|
if [ "$_rc" -ne "$_want" ]; then
|
||||||
|
fail=$((fail + 1)); printf ' FAIL %s (want rc=%s got %s)\n%s\n' "$_label" "$_want" "$_rc" "$_out"; return
|
||||||
|
fi
|
||||||
|
if [ -n "$_needle" ] && ! printf '%s' "$_out" | grep -q "$_needle"; then
|
||||||
|
fail=$((fail + 1)); printf ' FAIL %s (rc ok, report never says %s)\n%s\n' "$_label" "$_needle" "$_out"; return
|
||||||
|
fi
|
||||||
|
pass=$((pass + 1)); printf ' ok %s\n' "$_label"
|
||||||
|
}
|
||||||
|
|
||||||
|
f=$(fixture ok); skill "$f" alpha yes
|
||||||
|
expect 0 'a skill with an Escalation section passes' "$f"
|
||||||
|
|
||||||
|
f=$(fixture missing); skill "$f" alpha no
|
||||||
|
expect 1 'a skill with no section fails, naming it' "$f" 'alpha'
|
||||||
|
|
||||||
|
f=$(fixture pending); skill "$f" alpha no; printf 'alpha\n' > "$f/bin/escalation-pending.txt"
|
||||||
|
expect 0 'a listed skill is allowed through the ratchet' "$f"
|
||||||
|
|
||||||
|
f=$(fixture done); skill "$f" alpha yes; printf 'alpha\n' > "$f/bin/escalation-pending.txt"
|
||||||
|
expect 1 'a listed skill that now HAS a section fails — remove it from the list' "$f" 'escalation-pending'
|
||||||
|
|
||||||
|
f=$(fixture stale); skill "$f" alpha yes; printf 'ghost\n' > "$f/bin/escalation-pending.txt"
|
||||||
|
expect 1 'a listed skill that does not exist fails' "$f" 'ghost'
|
||||||
|
|
||||||
|
f=$(fixture comments); skill "$f" alpha no
|
||||||
|
printf '# a comment, and a blank line follow\n\nalpha\n' > "$f/bin/escalation-pending.txt"
|
||||||
|
expect 0 'comments and blank lines in the list are ignored' "$f"
|
||||||
|
|
||||||
|
printf '\ncheck_escalation: %d passed, %d failed\n' "$pass" "$fail"
|
||||||
|
[ "$fail" -eq 0 ]
|
||||||
@@ -0,0 +1,67 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# Tests for bin/check-session-log.
|
||||||
|
#
|
||||||
|
# The regression: nine skills documented a session_log payload and not one of
|
||||||
|
# them named `session_id` — the tool's ONLY required field. Every documented
|
||||||
|
# call was incomplete, so an agent following any of them either failed the call
|
||||||
|
# or invented an id, and invented ids do not group. Found by a pressure test
|
||||||
|
# against `debug`, then found to be fleet-wide (#36).
|
||||||
|
#
|
||||||
|
# Also gated: `duration_ms: <wall-clock>`, which an agent cannot measure. A
|
||||||
|
# documented field nobody can honestly populate gets fabricated or silently
|
||||||
|
# dropped; both are worse than omitting it.
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
|
||||||
|
SUT="$ROOT/bin/check-session-log"
|
||||||
|
|
||||||
|
pass=0
|
||||||
|
fail=0
|
||||||
|
WORK=${TMPDIR:-/tmp}/check_session_log_test.$$
|
||||||
|
|
||||||
|
if [ ! -f "$SUT" ]; then
|
||||||
|
printf 'FAIL: %s does not exist\n' "$SUT"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
cleanup() { rm -rf "$WORK"; }
|
||||||
|
trap cleanup EXIT INT TERM
|
||||||
|
|
||||||
|
# fixture <name> <body> — a checkout with one skill whose SKILL.md is <body>.
|
||||||
|
fixture() {
|
||||||
|
_f="$WORK/$1"
|
||||||
|
mkdir -p "$_f/skills/alpha"
|
||||||
|
printf -- '---\nname: alpha\ndescription: a\n---\n\n%s\n' "$2" > "$_f/skills/alpha/SKILL.md"
|
||||||
|
printf '%s' "$_f"
|
||||||
|
}
|
||||||
|
|
||||||
|
# expect <rc> <label> <fixture> [needle]
|
||||||
|
expect() {
|
||||||
|
_want=$1; _label=$2; _fx=$3; _needle=${4:-}
|
||||||
|
_out=$(sh "$SUT" "$_fx" 2>&1); _rc=$?
|
||||||
|
if [ "$_rc" -ne "$_want" ]; then
|
||||||
|
fail=$((fail + 1)); printf ' FAIL %s (want rc=%s got %s)\n%s\n' "$_label" "$_want" "$_rc" "$_out"; return
|
||||||
|
fi
|
||||||
|
if [ -n "$_needle" ] && ! printf '%s' "$_out" | grep -q "$_needle"; then
|
||||||
|
fail=$((fail + 1)); printf ' FAIL %s (rc ok, report never says %s)\n%s\n' "$_label" "$_needle" "$_out"; return
|
||||||
|
fi
|
||||||
|
pass=$((pass + 1)); printf ' ok %s\n' "$_label"
|
||||||
|
}
|
||||||
|
|
||||||
|
expect 0 'a skill that never mentions session_log is fine' \
|
||||||
|
"$(fixture quiet 'Just a skill. No logging here.')"
|
||||||
|
|
||||||
|
expect 0 'a documented call naming session_id passes' \
|
||||||
|
"$(fixture good 'Call `session_log` with `{session_id: "<id>", skill: "alpha", phase: "red", final_status: "pass"}`')"
|
||||||
|
|
||||||
|
expect 1 'a documented call omitting session_id fails' \
|
||||||
|
"$(fixture bad 'Call `session_log` with `{skill: "alpha", phase: "red", final_status: "pass"}`')" \
|
||||||
|
'session_id'
|
||||||
|
|
||||||
|
expect 1 'documenting duration_ms as wall-clock fails' \
|
||||||
|
"$(fixture clock 'Call `session_log` with `{session_id: "<id>", skill: "alpha", duration_ms: <wall-clock>}`')" \
|
||||||
|
'duration_ms'
|
||||||
|
|
||||||
|
printf '\ncheck_session_log: %d passed, %d failed\n' "$pass" "$fail"
|
||||||
|
[ "$fail" -eq 0 ]
|
||||||
@@ -224,5 +224,72 @@ manifest "$f/ck" 0.7.0
|
|||||||
manifest "$f/home/.claude/plugins/marketplaces/mathias" 0.9.0
|
manifest "$f/home/.claude/plugins/marketplaces/mathias" 0.9.0
|
||||||
assert_fails "$f" '0.9.0' 'a newer marketplace clone is reported'
|
assert_fails "$f" '0.9.0' 'a newer marketplace clone is reported'
|
||||||
|
|
||||||
|
# --- 9. --online: the release this host has never fetched ------------------
|
||||||
|
# Everything above compares copies already on the host, so a host where the
|
||||||
|
# checkout, the marketplace clone and the installed plugin all agree reads as
|
||||||
|
# clean while being a release behind origin. Offline by design (#30); this is
|
||||||
|
# the opt-in that closes it (#38).
|
||||||
|
#
|
||||||
|
# The remote lookup is stubbed through $DOCTOR_LATEST_TAG_CMD so no test needs
|
||||||
|
# a network or a real remote.
|
||||||
|
|
||||||
|
online_fixture() {
|
||||||
|
_f=$(new_fixture "$1"); wire_clean "$_f"
|
||||||
|
installed_state "$_f" "$2"
|
||||||
|
manifest "$_f/ck" "$2"
|
||||||
|
printf '%s' "$_f"
|
||||||
|
}
|
||||||
|
|
||||||
|
# assert_online <fixture> <stub-tag> <want-rc> <needle> <label>
|
||||||
|
assert_online() {
|
||||||
|
_out=$(HOME="$1/home" DOCTOR_CHECKOUT="$1/ck" \
|
||||||
|
DOCTOR_LATEST_TAG_CMD="printf %s\\n $2" sh "$DOCTOR" --online 2>&1); _rc=$?
|
||||||
|
if [ "$3" -eq 0 ] && [ "$_rc" -ne 0 ]; then
|
||||||
|
fail=$((fail + 1)); printf ' FAIL %s\n exited %s on a current host\n got: %s\n' "$5" "$_rc" "$_out"; return
|
||||||
|
fi
|
||||||
|
if [ "$3" -ne 0 ] && [ "$_rc" -eq 0 ]; then
|
||||||
|
fail=$((fail + 1)); printf ' FAIL %s\n exited 0; staleness went unreported\n' "$5"; return
|
||||||
|
fi
|
||||||
|
if [ -n "$4" ] && ! printf '%s' "$_out" | grep -q -- "$4"; then
|
||||||
|
fail=$((fail + 1)); printf ' FAIL %s\n report never mentions "%s"\n got: %s\n' "$5" "$4" "$_out"; return
|
||||||
|
fi
|
||||||
|
pass=$((pass + 1)); printf ' ok %s\n' "$5"
|
||||||
|
}
|
||||||
|
|
||||||
|
f=$(online_fixture behind 0.8.0)
|
||||||
|
assert_online "$f" v0.9.0 1 '0.9.0' '--online reports a release the host has not fetched'
|
||||||
|
|
||||||
|
f=$(online_fixture current 0.9.0)
|
||||||
|
assert_online "$f" v0.9.0 0 '' '--online stays quiet when the host is current'
|
||||||
|
|
||||||
|
# A host AHEAD of the newest tag is the normal state between a release commit
|
||||||
|
# and its tag; that is not drift (same reasoning as check's ordering rule).
|
||||||
|
f=$(online_fixture ahead 0.9.1)
|
||||||
|
assert_online "$f" v0.9.0 0 '' '--online does not complain about being ahead'
|
||||||
|
|
||||||
|
# Without the flag, no lookup happens at all — the stub would report a newer
|
||||||
|
# tag and must be ignored.
|
||||||
|
f=$(online_fixture offline 0.8.0)
|
||||||
|
_out=$(HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" \
|
||||||
|
DOCTOR_LATEST_TAG_CMD="printf v9.9.9\\n" sh "$DOCTOR" 2>&1); _rc=$?
|
||||||
|
if [ "$_rc" -eq 0 ]; then
|
||||||
|
pass=$((pass + 1)); printf ' ok the default run performs no remote lookup\n'
|
||||||
|
else
|
||||||
|
fail=$((fail + 1)); printf ' FAIL the default run performs no remote lookup\n got: %s\n' "$_out"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# A remote that cannot be reached is not drift and must not be reported as it.
|
||||||
|
# An unreachable remote produces no output at all, so it gets its own run
|
||||||
|
# rather than an empty stub argument — "prints nothing" and "prints an empty
|
||||||
|
# string" are different things and only the first is the case under test.
|
||||||
|
f=$(online_fixture unreachable 0.9.0)
|
||||||
|
_out=$(HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" \
|
||||||
|
DOCTOR_LATEST_TAG_CMD="true" sh "$DOCTOR" --online 2>&1); _rc=$?
|
||||||
|
if [ "$_rc" -ne 0 ] && printf '%s' "$_out" | grep -q 'could not'; then
|
||||||
|
pass=$((pass + 1)); printf ' ok --online reports an unreachable remote as its own fault\n'
|
||||||
|
else
|
||||||
|
fail=$((fail + 1)); printf ' FAIL --online reports an unreachable remote as its own fault\n rc=%s got: %s\n' "$_rc" "$_out"
|
||||||
|
fi
|
||||||
|
|
||||||
printf '\ndoctor: %d passed, %d failed\n' "$pass" "$fail"
|
printf '\ndoctor: %d passed, %d failed\n' "$pass" "$fail"
|
||||||
[ "$fail" -eq 0 ]
|
[ "$fail" -eq 0 ]
|
||||||
|
|||||||
Reference in New Issue
Block a user