chore(workflow): refine agents, skills, commands, and ADR structure for v5 (#70)

* chore(workflow): refine agents, skills, commands, and ADR structure for v5

- Consolidate ADR files into single adr.md (drop adr-NNN-<title>.md pattern)
- Add test-coverage and test-build tasks; fix test-fast/test flags
- Clarify design priority chain with full complexity ladder
- Remove self-selection language from agent instructions
- Add refactor and design-patterns skills to reviewer agent
- Add BASELINED guard to feature-selection skill
- Fix scope skill: drop silent pre-mortem requirement from Session 1
- Align function/class line-count wording to code-lines-only

* chore(workflow): document model v6 — 2-stage discovery, PO-owned feature moves, unified session protocol

- Split docs/discovery.md into three append-only files:
  docs/discovery_journal.md (raw Q&A), docs/discovery.md (synthesis),
  docs/architecture.md (architectural decisions)
- Remove docs/features/discovery.md (superseded)
- Replace Phase 1/Phase 2 model with 2-stage model: Stage 1 Discovery
  (unified iterative sessions) + Stage 2 Specification (stories + criteria)
- PO is sole owner of all .feature file moves; SE and reviewer never move
  or edit .feature files; explicit escalation when no in-progress feature
- Add bug handling protocol: PO adds @bug @id, SE writes both @id test
  and @given Hypothesis property test
- Add real-time split rule: >2 concerns or >8 Examples splits immediately
  within the same session
- Add session status markers to discovery_journal.md: IN-PROGRESS/COMPLETE
- Update all agent files, skills, workflow docs, and research docs to match

Author: nullhack

.opencode/agents/product-owner.md

@@ -25,16 +25,16 @@ Load `skill session-workflow` first — it reads TODO.md, orients you to the cur
 
 | Step | Action |
 |---|---|
-| **Step 1 — SCOPE** | Load `skill scope` — contains the full 4-phase discovery and criteria protocol |
+| **Step 1 — SCOPE** | Load `skill scope` — contains Stage 1 (Discovery sessions) and Stage 2 (Stories + Criteria) |
 | **Step 5 — ACCEPT** | See acceptance protocol below |
 
 ## Ownership Rules
 
-- You are the **sole owner** of `.feature` files and `docs/features/discovery.md`
+- You are the **sole owner** of `.feature` files, `docs/discovery_journal.md`, and `docs/discovery.md`
 - No other agent may edit these files
+- **You are the sole owner of all `.feature` file moves**: backlog → in-progress (before Step 2) and in-progress → completed (after Step 5 acceptance). No other agent moves `.feature` files.
 - Software-engineer escalates spec gaps to you; you decide whether to extend criteria
-- **You pick** the next feature from backlog — the software-engineer never self-selects
-- **NEVER move a feature to `in-progress/` unless its discovery section has `Status: BASELINED`** — if not baselined, complete Step 1 (Phase 2 + 3 + 4) first
+- **NEVER move a feature to `in-progress/` unless its `.feature` file has `Status: BASELINED`** — if not baselined, complete Step 1 (Stage 1 Discovery + Stage 2 Specification) first
 
 ## Step 5 — Accept
 
@@ -56,8 +56,17 @@ When a gap is reported (by software-engineer or reviewer):
 | Behavior contradicts an existing Example | Write a new Example with new `@id`. |
 | Post-merge defect | Move the `.feature` file back to `in-progress/`, add new Example with `@id`, resume at Step 3. |
 
+## Bug Handling
+
+When a defect is reported against any feature:
+
+1. Add a `@bug @id:<new-8-char-hex>` Example to the relevant `Rule:` block in the `.feature` file.
+2. Write the Example using the standard `Given/When/Then` format describing the correct behavior.
+3. Update TODO.md to note the new `@id` for the SE to implement.
+4. SE implements the `@id` test in `tests/features/` **and** a `@given` Hypothesis property test in `tests/unit/`. Both are required.
+
 ## Available Skills
 
 - `session-workflow` — session start/end protocol
 - `feature-selection` — when TODO.md is idle: score and select next backlog feature using WSJF
-- `scope` — Step 1: 3-session discovery (Phase 1 + 2), stories (Phase 3), and criteria (Phase 4)
+- `scope` — Step 1: Stage 1 (Discovery sessions with stakeholder) and Stage 2 (Stories + Criteria, PO alone)

.opencode/agents/reviewer.md

@@ -42,6 +42,14 @@ Load `skill session-workflow` first. Then load `skill verify` for Step 4 verific
 - **Never suggest `noqa`, `type: ignore`, or `pytest.skip` as a fix.** These are bypasses, not solutions.
 - **Report specific locations.** "`physics/engine.py:47`: unreachable return" not "there is dead code."
 - **Every PASS/FAIL cell must have evidence.** Empty evidence = UNCHECKED = REJECTED.
+- **Never move `.feature` files.** The PO is the sole owner of all feature file moves. After producing an APPROVED report, update TODO.md and stop — the PO accepts and moves the file.
+
+## After APPROVED
+
+When your report verdict is APPROVED:
+1. Write the report as described in `skill verify`.
+2. Update TODO.md `## Next` line: `Run @product-owner — accept feature <name> at Step 5.`
+3. Stop. Do not touch `.feature` files. The PO reviews the feature themselves and moves it to `completed/`.
 
 ## Gap Reporting
 
@@ -58,4 +66,6 @@ You never edit `.feature` files or add Examples yourself.
 ## Available Skills
 
 - `session-workflow` — session start/end protocol
+- `refactor` — Code refactoring heuristics
+- `design-patterns` — Reference for code smell and design patterns
 - `verify` — Step 4: full verification protocol with all tables, gates, and report template

.opencode/agents/software-engineer.md

@@ -45,7 +45,14 @@ Load `skill session-workflow` first — it reads TODO.md, orients you to the cur
 
 - You own all technical decisions: module structure, patterns, internal APIs, test tooling, linting config
 - **PO approves**: new runtime dependencies, changed entry points, scope changes
-- You are **never** the one to pick the next feature — only the PO picks from backlog
+- **You never move `.feature` files.** The PO is the sole owner of all feature file moves (backlog → in-progress → completed). If you find no `.feature` file in `docs/features/in-progress/`, **STOP** — do not self-select a feature. Write the gap in TODO.md and escalate to PO.
+
+## No In-Progress Feature
+
+If `docs/features/in-progress/` contains only `.gitkeep` (no `.feature` file):
+1. Do not pick a feature from backlog yourself.
+2. Update TODO.md: `Next: Run @product-owner — load skill feature-selection and pick the next BASELINED feature from backlog.`
+3. Stop. The PO must move the chosen feature into `in-progress/` before you can begin Step 2.
 
 ## Spec Gaps
 
@@ -61,4 +68,4 @@ If during implementation you discover behavior not covered by existing acceptanc
 - `design-patterns` — on-demand when smell detected during architecture or refactor
 - `pr-management` — Step 5: PRs with conventional commits
 - `git-release` — Step 5: calver versioning and themed release naming
-- `create-skill` — meta: create new skills when needed
+- `create-skill` — meta: create new skills when needed

.opencode/skills/feature-selection/SKILL.md

@@ -38,6 +38,10 @@ Read each `.feature` file in `docs/features/backlog/`. Check its discovery secti
 - Non-BASELINED features are not eligible — they need Step 1 (scope) first
 - If no BASELINED features exist: inform the stakeholder; run `@product-owner` with `skill scope` to baseline the most promising backlog item first
 
+**IMPORTANT**
+
+**NEVER move a feature to `in-progress/` unless its discovery section has `Status: BASELINED`**
+
 ### 3. Score Each Candidate
 
 For each BASELINED feature, fill this table:
@@ -96,7 +100,7 @@ Run @<agent-name> — <first concrete action for this feature>
 ```
 
 - If the feature has no `Rule:` blocks yet → Step 1 (SCOPE): `Run @product-owner — load skill scope and write stories`
-- If the feature has `Rule:` blocks but no `@id` Examples → Step 1 Phase 4 (Criteria): `Run @product-owner — load skill scope and write acceptance criteria`
+- If the feature has `Rule:` blocks but no `@id` Examples → Step 1 Stage 2 Step B (Criteria): `Run @product-owner — load skill scope and write acceptance criteria`
 - If the feature has `@id` Examples → Step 2 (ARCH): `Run @software-engineer — load skill implementation and write architecture stubs`
 
 ### 6. Commit

.opencode/skills/implementation/SKILL.md

@@ -15,20 +15,20 @@ Steps 2 (Architecture) and 3 (TDD Loop) combined into a single skill. The softwa
 
 During implementation, correctness priorities are (in order):
 
-1. **Design correctness** — YAGNI > KISS > DRY > SOLID > Object Calisthenics > appropriate design patterns
+1. **Design correctness** — YAGNI > KISS > DRY > SOLID > Object Calisthenics > appropriate design patterns > complex code > complicated code > failing code > no code
 2. **One @id green** — the specific test under work passes, plus `test-fast` still passes
 3. **Commit** — when a meaningful increment is green
 4. **Quality tooling** — `lint`, `static-check`, full `test` with coverage run at end-of-feature handoff
 
-Design correctness is far more important than lint/pyright/coverage compliance. Never run lint, static-check, or coverage during the TDD loop — those are handoff-only checks.
+Design correctness is far more important than lint/pyright/coverage compliance. Never run lint (ruff check, ruff format), static-check (pyright), or coverage during the TDD loop — those are handoff-only checks.
 
 ---
 
 ## Step 2 — Architecture
 
 ### Prerequisites (stop if any fail — escalate to PO)
 
-1. `docs/features/in-progress/` contains only `.gitkeep` (no `.feature` files). If another `.feature` file exists, **STOP** — another feature is already in progress.
+1. `docs/features/in-progress/` contains exactly one `.feature` file (not just `.gitkeep`). If none exists, **STOP** — update TODO.md `Next:` to `Run @product-owner — move the chosen feature to in-progress/` and stop. Never self-select or move a feature yourself.
 2. The feature file's discovery section has `Status: BASELINED`. If not, escalate to PO — Step 1 is incomplete.
 3. The feature file contains `Rule:` blocks with `Example:` blocks and `@id` tags. If not, escalate to PO — criteria have not been written.
 4. Package name confirmed: read `pyproject.toml` → locate `[tool.setuptools]` → confirm directory exists on disk.
@@ -37,26 +37,20 @@ Design correctness is far more important than lint/pyright/coverage compliance.
 
 1. Read `pyproject.toml` → locate `[tool.setuptools]` → record `packages = ["<name>"]`
 2. Confirm directory exists: `ls <name>/`
-3. All new source files go under `<name>/` — never under a template placeholder.
+3. All new source files go under `<name>/`
 
-### Move Feature File
-
-```bash
-mv docs/features/backlog/<name>.feature docs/features/in-progress/<name>.feature
-```
-
-Update `TODO.md` Source path from `backlog/` to `in-progress/`.
+**Note on feature file moves**: The PO moves `.feature` files between folders. The software-engineer never moves or edits `.feature` files. Update TODO.md `Source:` path to reflect `in-progress/` once the PO has moved the file.
 
 ### Read Phase (all before writing anything)
 
-1. Read `docs/features/discovery.md` (project-level)
+1. Read `docs/discovery.md` (project-level synthesis changelog) and optionally `docs/discovery_journal.md` (Q&A history for context)
 2. Read **ALL** `.feature` files in `docs/features/backlog/` (discovery + entities sections)
 3. Read in-progress `.feature` file (full: Rules + Examples + @id)
 4. Read **ALL** existing `.py` files in `<package>/` — understand what already exists before adding anything
 
 ### Domain Analysis
 
-From Entities table + Rules (Business) in `.feature` file:
+From the Domain Model table in `docs/discovery.md` + Rules (Business) in the `.feature` file:
 - **Nouns** → named classes, value objects, aggregates
 - **Verbs** → method names with typed signatures
 - **Datasets** → named types (not bare dict/list)
@@ -116,19 +110,20 @@ class UserRepository(Protocol):
 
 Place stubs where responsibility dictates — do not pre-create `ports/` or `adapters/` folders unless a concrete external dependency was identified in scope. Structure follows domain analysis, not a template.
 
-### Write ADR Files (significant decisions only)
+### Record Architectural Decisions
 
-For each significant architectural decision, create `docs/architecture/adr-NNN-<title>.md`:
+Append a new dated block to `docs/architecture.md` for each significant decision:
 
 ```markdown
-# ADR-NNN: <title>
+## YYYY-MM-DD — <feature-name>: <short title>
 
-**Decision:** <what was decided>
-**Reason:** <why, one sentence>
-**Alternatives considered:** <what was rejected and why>
+Decision: <what was decided>
+Reason: <why, one sentence>
+Alternatives considered: <what was rejected and why>
+Feature: <feature-name>
 ```
 
-Only write an ADR if the decision is non-obvious or has meaningful trade-offs. Routine YAGNI choices do not need an ADR.
+Only write a block for non-obvious decisions with meaningful trade-offs. Routine YAGNI choices do not need a record.
 
 ### Architecture Smell Check (hard gate)
 
@@ -153,25 +148,21 @@ Commit: `feat(<feature-name>): add architecture stubs`
 
 ### Prerequisites
 
+- [ ] Exactly one .feature `in_progress`. If not present, Load `skill feature-selection` 
 - [ ] Architecture stubs present in `<package>/` (committed by Step 2)
-- [ ] Read all `docs/architecture/adr-NNN-*.md` files — understand the architectural decisions before writing any test
-- [ ] Test stub files exist in `tests/features/<feature-name>/` — one file per `Rule:` block, all `@id` functions present with `@pytest.mark.skip`; if missing, write them now before entering RED
+- [ ] Read `docs/architecture.md` — understand all architectural decisions before writing any test
+- [ ] Test stub files exist in `tests/features/<feature-name>/<rule_slug>_test.py` — one file per `Rule:` block, all `@id` stub functions present with `@pytest.mark.skip`; if missing, write them now before entering RED
 
 ### Write Test Stubs (if not present)
 
-For each `Rule:` block in the in-progress `.feature` file, create `tests/features/<feature-name>/<rule-slug>_test.py` if it does not already exist. Write one function per `@id` Example, all skipped:
+For each `Rule:` block in the in-progress `.feature` file, create `tests/features/<feature-name>/<rule_slug>_test.py` if it does not already exist. Write one function per `@id` Example, all skipped:
 
 ```python
 @pytest.mark.skip(reason="not yet implemented")
-def test_<rule_slug>_<8char_hex>() -> None:
+def test_<feature_slug>_<@id>() -> None:
     """
-    Given: ...
-    When: ...
-    Then: ...
+    <@id steps raw text including new lines>
     """
-    # Given
-    # When
-    # Then
 ```
 
 Run `uv run task gen-todo` after writing stubs to sync `@id` rows into `TODO.md`.
@@ -192,17 +183,17 @@ For each pending `@id`:
 ```
 INNER LOOP
 ├── RED
-│   ├── Confirm stub for this @id exists in tests/features/<feature-name>/ with @pytest.mark.skip
+│   ├── Confirm stub for this @id exists in tests/features/<feature-name>/<rule_slug>.feature with @pytest.mark.skip
 │   ├── Read existing stubs in `<package>/` — base the test on the current data model and signatures
 │   ├── Write test body (Given/When/Then → Arrange/Act/Assert); remove @pytest.mark.skip
-│   ├── Update stub signatures as needed — edit the `.py` file directly
+│   ├── Update <package> stub signatures as needed — edit the `.py` file directly
 │   ├── uv run task test-fast
 │   └── EXIT: this @id FAILS
 │       (if it passes: test is wrong — fix it first)
 │
 ├── GREEN
 │   ├── Write minimum code — YAGNI + KISS only
-│   │   (no DRY, SOLID, OC here — those belong in REFACTOR)
+│   │   (no DRY, SOLID, OC, Docstring, type hint here — those belong in REFACTOR)
 │   ├── uv run task test-fast
 │   └── EXIT: this @id passes AND all prior tests pass
 │       (fix implementation only; do not advance to next @id)
@@ -221,7 +212,7 @@ Commit when a meaningful increment is green
 ```bash
 uv run task lint
 uv run task static-check
-uv run task test          # coverage must be 100%
+uv run task test-coverage          # coverage must be 100%
 timeout 10s uv run task run
 ```
 
@@ -231,7 +222,7 @@ All must pass before Self-Declaration.
 
 ### Self-Declaration (once, after all quality gates pass)
 
-Write into `TODO.md` under a `## Self-Declaration` block:
+Answer honestly the `## Self-Declaration` report:
 
 ```markdown
 ## Self-Declaration
@@ -256,6 +247,7 @@ As a software-engineer I declare:
 * OC-7: ≤20 lines per function, ≤50 per class — AGREE/DISAGREE | longest: file:line
 * OC-8: ≤2 instance variables per class (behavioural classes only; dataclasses, Pydantic models, value objects, and TypedDicts are exempt) — AGREE/DISAGREE | file:line
 * OC-9: no getters/setters — AGREE/DISAGREE | file:line
+* Patterns: I have no good reason to refactor parts of the code using OOP or Design Patterns — AGREE/DISAGREE | file:line
 * Patterns: no creational smell — AGREE/DISAGREE | file:line
 * Patterns: no structural smell — AGREE/DISAGREE | file:line
 * Patterns: no behavioral smell — AGREE/DISAGREE | file:line
@@ -268,7 +260,7 @@ A `DISAGREE` answer is not automatic rejection — state the reason inline and f
 
 Signal completion to the reviewer. Provide:
 - Feature file path
-- Self-Declaration from TODO.md
+- Self-Declaration report
 - Summary of what was implemented
 
 ---
@@ -278,40 +270,35 @@ Signal completion to the reviewer. Provide:
 ### Test File Layout
 
 ```
-tests/features/<feature-name>/<rule-slug>_test.py
+tests/features/<feature-name>/<rule_slug>_test.py
 ```
 
 - `<feature-name>` = the `.feature` file stem
-- `<rule-slug>` = the `Rule:` title slugified
+- `<rule_slug>` = the `Rule:` title slugified
 
 ### Function Naming
 
 ```python
-def test_<rule_slug>_<8char_hex>() -> None:
+def test_<feature_slug>_<@id>() -> None:
 ```
 
-- `rule_slug` = the `Rule:` title with spaces/hyphens replaced by underscores, lowercase
-- `8char_hex` = the `@id` from the `Example:` block
+- `feature_slug` = the `.feature` file stem with spaces/hyphens replaced by underscores, lowercase
+- `@id` = the `@id` from the `Example:` block
 
 ### Docstring Format (mandatory)
 
 New tests start as skipped stubs. Remove `@pytest.mark.skip` when implementing in the RED phase.
 
 ```python
 @pytest.mark.skip(reason="not yet implemented")
-def test_wall_bounce_a3f2b1c4() -> None:
+def test_<feature_slug>_<@id>() -> None:
     """
-    Given: A ball moving upward reaches y=0
-    When: The physics engine processes the next frame
-    Then: The ball velocity y-component becomes positive
+    <@id steps raw text including new lines>
     """
-    # Given
-    # When
-    # Then
 ```
 
 **Rules**:
-- Docstring contains `Given:/When:/Then:` on separate indented lines
+- Docstring contains `Gherkin steps` as raw text on separate indented lines
 - No extra metadata in docstring — traceability comes from function name `@id` suffix
 
 ### Markers
@@ -320,6 +307,7 @@ def test_wall_bounce_a3f2b1c4() -> None:
 - `@pytest.mark.deprecated` — auto-skipped by conftest; used for superseded Examples
 
 ```python
+@pytest.mark.deprecated
 def test_wall_bounce_a3f2b1c4() -> None:
     ...
 
@@ -350,11 +338,11 @@ def test_wall_bounce_c4d5e6f7(x: float) -> None:
 **Rules**:
 - `@pytest.mark.slow` is mandatory on every `@given`-decorated test
 - `@example(...)` is optional but encouraged
-- Never use Hypothesis for: I/O, side effects, network calls, database writes
+- Do not use Hypothesis for: I/O, side effects, network calls, database writes
 
 ### Semantic Alignment Rule
 
-The test's Given/When/Then must operate at the **same abstraction level** as the AC's Given/When/Then.
+The test's Given/When/Then must operate at the **same abstraction level** as the AC's Steps.
 
 | AC says | Test must do |
 |---|---|
@@ -369,7 +357,7 @@ If testing through the real entry point is infeasible, escalate to PO to adjust
 - No `isinstance()`, `type()`, or internal attribute (`_x`) checks in assertions
 - One assertion concept per test (multiple `assert` ok if they verify the same thing)
 - No `pytest.mark.xfail` without written justification
-- `pytest.mark.skip` is only valid on stubs (`reason="not yet implemented"`) — remove it when implementing
+- `pytest.mark.skip(reason="not yet implemented")` is only valid on stubs — remove it when implementing
 - Test data embedded directly in the test, not loaded from external files
 
 ### Test Tool Decision
@@ -396,7 +384,7 @@ Extra tests in `tests/unit/` are allowed freely (coverage, edge cases, etc.) —
 
 ## Signature Design
 
-Signatures are written during Step 2 (Architecture) and refined during Step 3 (RED). They live directly in the package `.py` files — never in the `.feature` file.
+<package> signatures are written during Step 2 (Architecture) and refined during Step 3 (RED). They live directly in the package `.py` files — never in the `.feature` file.
 
 Key rules:
 - Bodies are always `...` in the architecture stub
@@ -420,4 +408,4 @@ class EmailAddress:
 class UserRepository(Protocol):
     def save(self, user: "User") -> None: ...
     def find_by_email(self, email: EmailAddress) -> "User | None": ...
-```
+```