fix(skills): document a session_log call the tool would actually accept

Nine skills documented the payload. None named `session_id`, which is the
tool's only required field. So every documented call was one the tool rejects —
an agent following any of them either failed the call or invented an id, and
invented ids do not group, which means the pass-rate the /pass-rate endpoint
computes was being assembled from entries that cannot be joined.

Found by the #19 pressure test against `debug`, which invented
`debug-signed-artifact-missing-2026-08-29` and said so: "my three entries are
only groupable by luck of naming". The grep afterwards showed it was fleet-wide,
not a `debug` bug.

Also removed: `duration_ms: <wall-clock>` in eight of them. An agent does not
observe a clock. The same run omitted it "rather than fabricate", which is the
good outcome; the bad outcome is the run that fabricates, and a made-up number
in a metrics stream is worse than a missing one. It is an optional field and is
now simply absent.

bin/check-session-log gates both, with tests. This is exactly the case #19 has
in mind for a small tool over prose: the rule is a grep, so it should be a gate,
not a note asking people to remember.

Closes part of #36
Bump-Type: patch

.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "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.",
-  "version": "0.12.0",
+  "version": "0.13.2",
   "author": {
     "name": "Mathias",
     "url": "https://git.d-ma.be/mathias"

.mcp.json

@@ -12,5 +12,22 @@
       "type": "http",
       "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}"
+      }
+    }
   }
-}
+}

Taskfile.yml

@@ -110,7 +110,11 @@ tasks:
         #    harness instead of erroring.
         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. Plugin manifest version must match the latest tag, or installers
         #    never see the update (Claude Code gates updates on this field).
         if [ -f '{{.PLUGIN_MANIFEST}}' ]; then
           manifest_v=$(sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' '{{.PLUGIN_MANIFEST}}' | head -1)

bin/check-session-log

@@ -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

skills/cad-babysit/SKILL.md

@@ -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.
 
 **At end of each phase:**
-- `session_log` with `{skill: "cad-babysit", phase: "<phase-name>", final_status: "pass" |
-  "fail" | "skip", message: "<one-line summary>", duration_ms: <wall-clock>, project_root:
+- `session_log` with `{session_id: "<this session's id>", skill: "cad-babysit", phase: "<phase-name>", final_status: "pass" |
+  "fail" | "skip", message: "<one-line summary>", project_root:
   "<absolute path>"}`
 
 **Phases for this skill:** query, read-comments, read-actions-logs, classify-fault-vs-outcome,

skills/clean-code/SKILL.md

@@ -281,5 +281,6 @@ During refactoring:
 
 - `references/clean-code.md` — detailed naming rules, object calisthenics
 - `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 `refactoring` skill for systematic refactoring techniques

skills/clean-code/references/python-adaptation.md

@@ -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

skills/code-review/SKILL.md

@@ -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.
 
 **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
 
@@ -279,6 +279,8 @@ treats `pass` as success, `fail` as failure, `skip` as neither.
 
 ## 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
 - Load `refactoring` skill when recommending fixes
 - Load `test-design` skill for reviewing test quality

skills/code-review/references/python-adaptation.md

@@ -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

skills/debug/SKILL.md

@@ -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.
 
 **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
 

skills/experiment-spec/SKILL.md

@@ -185,7 +185,7 @@ If silhouette ≤ 0.20:
 
 **After experiment concludes:**
 - `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
 

skills/feature-spec/SKILL.md

@@ -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.
 
 **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
 

skills/session-retrospective/SKILL.md

@@ -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.
 
 **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
 

skills/solid/SKILL.md

@@ -258,5 +258,6 @@ Domain types know nothing about HTTP, SQL, or external APIs.
 
 - `references/solid-principles.md` — canonical SOLID reference with TypeScript examples
 - `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 `code-review` skill for detecting violations during review

skills/solid/references/python-adaptation.md

@@ -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

skills/spec-driven-dev/SKILL.md

@@ -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.
 
 **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
 

skills/tdd/SKILL.md

@@ -100,13 +100,21 @@ go test -run TestRetryOperation_RetriesThreeTimes ./...
 ```
 
 Confirm:
-- Test fails (not compilation errors)
-- Failure message is expected
-- Fails because feature is missing (not typos)
+- Test fails, and fails because the feature is missing
+- The failure message is the one you expected
+- It is not failing on a typo in the test itself
 
 **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
 
@@ -166,6 +174,23 @@ Keep tests green. Don't add 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
 
 ### Table-Driven Tests (Preferred Pattern)
@@ -329,8 +354,14 @@ Before marking work complete:
 - [ ] Watched each test fail before implementing
 - [ ] Each test failed for expected reason (feature missing, not typo)
 - [ ] Wrote minimal code to pass each test
-- [ ] All tests pass: `go test ./...`
-- [ ] Race detector clean: `go test -race ./...`
+- [ ] All tests pass
+- [ ] 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)
 - [ ] Edge cases and errors covered
 
@@ -375,13 +406,13 @@ Pass-rate is computed downstream by the `/pass-rate` HTTP endpoint, which
 treats `pass` as success, `fail` as failure, `skip` as neither.
 
 **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:**
-- `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:**
-- `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:**
 - `pass` — the phase's intended outcome was reached (red: test fails as expected; green: test passes; refactor: tests still pass after refactor).

skills/tdd/references/java.md

@@ -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.

skills/tdd/references/python.md

@@ -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.

skills/tdd/references/rust.md

@@ -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.

skills/tdd/references/typescript.md

@@ -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.

skills/trainer/SKILL.md

@@ -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.
 
 **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:**
-- `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
 

tests/check_session_log_test.sh

@@ -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 ]