Refine kotlin-engineer and related skills

.agents/_TOC.md

@@ -15,11 +15,14 @@
 13. [Common tasks](common-tasks.md)
 14. [Team memory](memory/MEMORY.md)
 15. [Task plans](tasks/README.md)
-16. [Java to Kotlin conversion](skills/java-to-kotlin/SKILL.md)
-17. [Dependency update](skills/dependency-update/SKILL.md)
-18. [Documentation review](skills/review-docs/SKILL.md)
-19. [Pre-PR checklist](skills/pre-pr/SKILL.md)
-20. [Kotlin code review](skills/kotlin-review/SKILL.md)
-21. [Dependency audit](skills/dependency-audit/SKILL.md)
-22. [Gradle review](skills/gradle-review/SKILL.md)
-23. [Raise test coverage](skills/raise-coverage/SKILL.md)
+16. [Kotlin engineering](skills/kotlin-engineer/SKILL.md) — implementation
+    policy for Kotlin, coroutines, Flow, null-safety, and API design
+17. [Java to Kotlin conversion](skills/java-to-kotlin/SKILL.md)
+18. [Dependency update](skills/dependency-update/SKILL.md)
+19. [Documentation review](skills/review-docs/SKILL.md)
+20. [Pre-PR checklist](skills/pre-pr/SKILL.md)
+21. [Code review](skills/spine-code-review/SKILL.md) — repo-specific Kotlin
+    and Java review; defers general Kotlin standards to `kotlin-engineer`
+22. [Dependency audit](skills/dependency-audit/SKILL.md)
+23. [Gradle review](skills/gradle-review/SKILL.md)
+24. [Raise test coverage](skills/raise-coverage/SKILL.md)

.agents/skills/gradle-review/SKILL.md

@@ -14,7 +14,8 @@ description: >
 
 You are the Gradle reviewer for a Spine Event Engine project. You review
 Gradle build logic and plugin production code; you do **not** duplicate
-`kotlin-review` (Kotlin idioms, safety rules, tests, version-gate) or
+`spine-code-review` (repo-specific safety rules, tests, version-gate),
+`kotlin-engineer` (general Kotlin language standards), or
 `dependency-audit` (artifact declarations under
 `buildSrc/src/main/kotlin/io/spine/dependency/`).
 
@@ -140,7 +141,7 @@ If after filtering nothing in the diff falls in any scope, return
 
 ## Output format
 
-Three sections, in this order, matching `kotlin-review`,
+Three sections, in this order, matching `spine-code-review`,
 `review-docs`, and `dependency-audit`:
 
 - **Must fix** — Spine mandate violations (missing `group` or

.agents/skills/java-to-kotlin/SKILL.md

@@ -8,6 +8,10 @@ description: >
 
 # 🪄 Converting Java code to Kotlin
 
+Use `.agents/skills/kotlin-engineer/SKILL.md` as the Kotlin implementation
+baseline for every conversion. Its null-safety, API-design, coroutine, Flow,
+and idiom rules apply to the Kotlin code produced by this skill.
+
 * Java code API comments are Javadoc format.
 * Kotlin code API comments are in KDoc format. 
 

.agents/skills/kotlin-engineer/SKILL.md

@@ -1,24 +1,43 @@
 ---
 name: kotlin-engineer
 description: >
-  Kotlin 2.x policy and pitfalls. Use when writing, reviewing, or refactoring
-  Kotlin code — enforces coroutine-safety, Flow correctness, null-safety, and
-  API-design rules that LLMs frequently get wrong.
+  Kotlin 2.x implementation policy and pitfalls. Use whenever writing,
+  modifying, refactoring, or explaining Kotlin code, especially coroutines,
+  Flow, null-safety, Java interop, Gradle Kotlin DSL, and public API design.
 ---
 
 # Kotlin — policy & pitfalls
 
 Baseline Kotlin knowledge (data/sealed/value classes, scope functions, null-safety operators, extension functions, `suspend`, `Flow`, `when` exhaustiveness) is assumed. This skill does not teach the language — it encodes the project policy and the traps that keep appearing in code review.
 
+## When to Use
+
+Use `kotlin-engineer` for implementation work in Kotlin or Kotlin DSL:
+
+- Writing or changing `.kt`, `.kts`, or Kotlin Multiplatform source.
+- Refactoring Java-style Kotlin into idiomatic Kotlin.
+- Designing public Kotlin APIs, result/state types, or domain identifiers.
+- Touching coroutines, `Flow`, `StateFlow`, `SharedFlow`, suspend functions, or
+  cancellation behavior.
+- Reviewing Kotlin code for correctness.
+
+## Fast Path for Agents
+
+1. Run the setup check below only for non-trivial Kotlin changes or when the
+   module's Kotlin/JDK/tooling baseline is unclear.
+2. Apply the MUST / MUST NOT rules while editing.
+3. Load only the reference file that matches the risky part of the task:
+   coroutines, API idioms, or build setup.
+4. Verify with the narrowest relevant Gradle compile/test task.
+
 ## Setup Check (run first)
 
 Before writing non-trivial code:
 
 1. **Kotlin version** — target 2.x when possible. Check `build.gradle(.kts)` (`kotlin("jvm") version "2.x"`) or `libs.versions.toml`.
-2. **JDK target** — `kotlin { jvmToolchain(21) }` or `compileOptions { targetCompatibility = JavaVersion.VERSION_21 }`. Matters for virtual threads (21+) and records interop (17+).
-3. **Compiler plugins** — `kotlin("plugin.spring")`, `kotlin("plugin.jpa")`, `kotlinx-serialization`, `kotlin("kapt")` vs `com.google.devtools.ksp`. Missing `plugin.spring` → final Spring classes can't be proxied. Missing `plugin.jpa` → `InstantiationException: No default constructor`.
-4. **Lint** — `detekt` / `ktlint` configured? Follow the existing rules; don't introduce new violations.
-5. **Build wrapper** — use `./gradlew`
+2. **JDK target** — Use JDK 21+ for virtual threads or JDK 17+ for record interop.
+3. **Lint** — `detekt` / `ktlint` configured? Follow the existing rules; don't introduce new violations.
+4. **Build wrapper** — use `./gradlew`
 
 ## MUST DO
 
@@ -48,11 +67,11 @@ Before writing non-trivial code:
 
 ## Reference Guide
 
-| Load when | File |
-|---|---|
-| Async / reactive code — coroutines, Flow, StateFlow/SharedFlow, cancellation, testing | `references/coroutines.md` |
-| API design — scope functions, value/data/sealed classes, extension functions, inline/reified, delegates, `Result<T>` | `references/idioms.md` |
-| Gradle / tooling — Kotlin DSL, version catalogs, KSP vs kapt, multi-module layout, compiler plugins | `references/build-setup.md` |
+| Load when                                                                                                            | File                        |
+|----------------------------------------------------------------------------------------------------------------------|-----------------------------|
+| Async / reactive code — coroutines, Flow, StateFlow/SharedFlow, cancellation, testing                                | `references/coroutines.md`  |
+| API design — scope functions, value/data/sealed classes, extension functions, inline/reified, delegates, `Result<T>` | `references/idioms.md`      |
+| Gradle / tooling — Kotlin DSL, version catalogs, KSP vs kapt, multi-module layout, compiler plugins                  | `references/build-setup.md` |
 
 ## Output Format
 

.agents/skills/kotlin-engineer/agents/openai.yaml

@@ -1,4 +1,4 @@
 interface:
   display_name: "Kotlin Engineer"
-  short_description: "Kotlin 2.x policy and pitfalls"
-  default_prompt: "Use $kotlin-engineer when writing, reviewing, or refactoring Kotlin code to enforce coroutine-safety, Flow correctness, null-safety, and API-design rules."
+  short_description: "Kotlin implementation policy and pitfalls"
+  default_prompt: "Use $kotlin-engineer when writing, modifying, refactoring, or explaining Kotlin code, including coroutines, Flow, null-safety, Java interop, Gradle Kotlin DSL, and API design."

.agents/skills/kotlin-engineer/references/coroutines.md

@@ -85,12 +85,12 @@ Assumes you know `suspend`, `launch` / `async`, `Flow` / `StateFlow` / `SharedFl
 
 ## Common anti-patterns
 
-| Anti-pattern | Correct |
-|---|---|
-| `GlobalScope.launch { ... }` | Inject `CoroutineScope` or use framework scope |
-| `runBlocking { suspendCall() }` inside a suspend function | Just `suspendCall()` — remove `runBlocking` |
-| `MutableStateFlow` returned from a public API | `val state: StateFlow<X> = _state.asStateFlow()` |
-| `.value = state.value.copy(x = y)` | `state.update { it.copy(x = y) }` |
-| `flow { withContext(IO) { emit(...) } }` | `flow { emit(...) }.flowOn(IO)` |
-| `try { work() } catch (e: Exception) { log(e) }` | Same plus `if (e is CancellationException) throw e` first |
+| Anti-pattern                                                                           | Correct                                                                       |
+|----------------------------------------------------------------------------------------|-------------------------------------------------------------------------------|
+| `GlobalScope.launch { ... }`                                                           | Inject `CoroutineScope` or use framework scope                                |
+| `runBlocking { suspendCall() }` inside a suspend function                              | Just `suspendCall()` — remove `runBlocking`                                   |
+| `MutableStateFlow` returned from a public API                                          | `val state: StateFlow<X> = _state.asStateFlow()`                              |
+| `.value = state.value.copy(x = y)`                                                     | `state.update { it.copy(x = y) }`                                             |
+| `flow { withContext(IO) { emit(...) } }`                                               | `flow { emit(...) }.flowOn(IO)`                                               |
+| `try { work() } catch (e: Exception) { log(e) }`                                       | Same plus `if (e is CancellationException) throw e` first                     |
 | Parallel fan-out with `.map { async { it.fetch() } }.map { it.await() }` inside `List` | Wrap in `coroutineScope { ... awaitAll() }` for proper cancellation semantics |

.agents/skills/pre-pr/SKILL.md

@@ -5,8 +5,8 @@ description: >
   the repository has a root `version.gradle.kts`, run a scope-dependent
   build/check command per `.agents/running-builds.md` (docs-only → `dokka`;
   code/deps → `build`; proto → `clean build`; no documented command → skipped),
-  and invoke the relevant reviewers (`kotlin-review`, `review-docs`,
-  `dependency-audit`,
+  and invoke the relevant reviewers (`kotlin-engineer`, `spine-code-review`,
+  `review-docs`, `dependency-audit`,
   `check-links`) against the branch diff. On success, write a sentinel file at
   `.git/pre-pr.ok` so the `gh pr create` hook can verify the checklist ran
   for the current HEAD. Use before opening a PR, or when CI rejected a
@@ -115,7 +115,11 @@ Before running a reviewer, check that the skill directory exists under
 `.agents/skills/`; if a skill is absent, skip it with a note "not applicable
 for this repo" rather than failing.
 
-- **code** changed → `kotlin-review`
+- **code** changed → `kotlin-engineer` (general Kotlin language standards) and
+  `spine-code-review` (repo-specific rules). Dispatch both; they cover
+  disjoint concerns and do not double-report. `kotlin-engineer` applies only
+  when `.kt` / `.kts` files changed; `spine-code-review` covers `.kt`, `.kts`,
+  and `.java`.
 - **docs** or KDoc changed → `review-docs`
 - **deps** changed → `dependency-audit`
 - **site** changed → `check-links` (unless the sentinel short-circuit below
@@ -133,9 +137,9 @@ missing version bump.
 
 **Auto-fix policy for reviewer findings:**
 
-- Findings from `kotlin-review`, `review-docs`, or `dependency-audit` → record
-  as Must-fix or Should-fix; do **not** auto-apply. Surface them and wait for
-  user action.
+- Findings from `kotlin-engineer`, `spine-code-review`, `review-docs`, or
+  `dependency-audit` → record as Must-fix or Should-fix; do **not** auto-apply.
+  Surface them and wait for user action.
 - If a reviewer reports a missing version bump after Step 2 already ran, the
   auto-fix did not take — record a Must-fix and do not silently re-apply.
 - `dependency-audit` reports a **version rollback** → do **not** auto-fix.
@@ -182,7 +186,8 @@ prefixed with the source reviewer or check:
 Pre-PR: FAIL (<branch> vs <base>)
 
 Must fix:
-- [kotlin-review] <item>
+- [kotlin-engineer] <item>
+- [spine-code-review] <item>
 - [review-docs] <item>
 
 Should fix:

.agents/skills/raise-coverage/SKILL.md

@@ -27,6 +27,9 @@ and wait for approval. The mechanical recipe lives in
 
 The authoritative standards live in `.agents/`:
 
+- `.agents/skills/kotlin-engineer/SKILL.md` — Kotlin implementation baseline
+  for every test you write, including null-safety, API design, coroutines,
+  Flow, and idioms.
 - `.agents/testing.md` — stubs not mocks; Kotest assertions; cover API edge
   cases; scaffold `when`/sealed-class branches.
 - `.agents/coding-guidelines.md` — Kotlin/Java idioms for the tests you write.

.agents/skills/kotlin-review/SKILL.md → .agents/skills/spine-code-review/SKILL.md

@@ -1,21 +1,48 @@
 ---
-name: kotlin-review
+name: spine-code-review
 description: >
-  Review Kotlin (and Java) changes in this repo against the Spine coding
-  guidelines, safety rules, and testing policy. Use after any non-trivial
-  code edit, before opening a PR, or when asked for a code review.
+  Review Kotlin, Java, and build changes in this repo against repo-specific Spine
+  rules: the AGENTS.md code-review filter, safety rules, testing policy, and
+  the version gate. Defers general Kotlin language, API, coroutine/Flow, and
+  null-safety standards to the `kotlin-engineer` skill. Use after any
+  non-trivial code edit, before opening a PR, or when asked for a code review.
   Read-only; does not run builds.
 ---
 
-# Kotlin code review (repo-specific)
+# Code review (repo-specific)
 
-You are the Kotlin reviewer for this repository. The authoritative standards
-live in `.agents/`:
+You are the repository code reviewer for this Spine project, covering both
+Kotlin and Java changes. You enforce the *repo-specific* rules; you do **not**
+re-teach or re-check the general Kotlin standards that `kotlin-engineer` owns.
 
-- `.agents/coding-guidelines.md` — Kotlin idioms, formatting, what to prefer/avoid.
-- `.agents/safety-rules.md` and `.agents/advanced-safety-rules.md` — hard constraints
-  (no reflection without approval, no analytics/telemetry, no blocking calls in
-  coroutines, no auto-updating external dependencies).
+## Division of responsibility
+
+`kotlin-engineer` is the authority for general Kotlin language and design
+standards and takes priority wherever the two overlap. **Do not duplicate its
+checks.** It owns:
+
+- Null-safety operators and `!!` justification.
+- Coroutine safety (`runBlocking`, `GlobalScope`, `CancellationException`,
+  dispatcher choice, blocking work inside `suspend`).
+- `Flow` / `StateFlow` / `SharedFlow` correctness and read-only exposure.
+- Idiomatic API design — sealed/data/value classes, scope/extension functions,
+  immutability defaults, named arguments, platform-type leaks.
+
+When a Kotlin change needs that lens, assume `kotlin-engineer` is reviewing it
+in parallel (the `pre-pr` skill dispatches both). If you run standalone and
+spot a clear `kotlin-engineer`-owned violation, note it briefly as a pointer to
+`kotlin-engineer` rather than re-deriving its rules.
+
+This skill owns everything below.
+
+## Authoritative standards
+
+The standards live in `.agents/`:
+
+- `.agents/coding-guidelines.md` — repo-specific idioms and formatting.
+- `.agents/safety-rules.md` and `.agents/advanced-safety-rules.md` — hard
+  constraints (no reflection without approval, no analytics/telemetry, no
+  unsafe code, no auto-updating external dependencies).
 - `.agents/testing.md` — Kotest assertions preferred, stubs not mocks.
 - `.agents/project-structure-expectations.md` — module/source-set layout.
 - `.agents/version-policy.md` — version bumps are required only when the
@@ -35,17 +62,16 @@ live in `.agents/`:
      `APPROVE — all changes are config-distributed files.` and stop.
 2. Read each affected file fully, not just the diff hunks. Smart casts,
    nullability, and idiomatic refactors require surrounding context.
-3. Check against `.agents/coding-guidelines.md`:
-   - Kotlin idioms (extension functions, `when`, smart casts, data/sealed classes).
+3. Check the repo-specific guidelines from `.agents/coding-guidelines.md`
+   (leave general Kotlin idioms to `kotlin-engineer`):
    - Kotlin Protobuf DSL (`message { ... }`) preferred over Java builders (`newBuilder()`, `toBuilder()`) in Kotlin.
-   - Immutability by default.
-   - No `!!` without justification.
    - No type names in variable names.
    - No string duplication — use companion-object constants.
    - No mixing Groovy/Kotlin DSL in build logic.
    - No double empty lines (collapse to a single empty line); no trailing whitespace.
-4. Check safety rules: reflection, telemetry, blocking-in-coroutines, dependency
-   bumps that weren't requested.
+4. Check the repo safety rules: reflection, telemetry, unsafe code, and
+   dependency bumps that weren't requested. (Coroutine-blocking and other
+   Kotlin concurrency safety are covered by `kotlin-engineer`.)
 5. Check tests: every functional change should have tests using Kotest assertions
    and stubs (not mocks).
 6. Check the version gate:
@@ -61,8 +87,8 @@ Return three sections, in this order:
 
 - **Must fix** — violations of safety rules, broken builds, missing version
   bump when the version gate applies, missing tests for functional changes.
-- **Should fix** — coding-guideline violations and clearer idiomatic alternatives.
-  Cite the specific guideline.
+- **Should fix** — repo coding-guideline violations and clearer repo-idiomatic
+  alternatives. Cite the specific guideline.
 - **Nits** — style and naming suggestions.
 
 For each item, quote the file and line, show the current code, and show the

.agents/skills/spine-code-review/agents/openai.yaml

@@ -0,0 +1,4 @@
+interface:
+  display_name: "Spine Code Review"
+  short_description: "Repo-specific Kotlin and Java code review"
+  default_prompt: "Use $spine-code-review to review Kotlin and Java changes against repo-specific Spine rules (AGENTS.md filter, safety rules, testing, version gate); it defers general Kotlin language standards to $kotlin-engineer."