Add design and implement tasks to engineering-workflow skill

.agents/skills/engineering-workflow/SKILL.md

@@ -14,13 +14,16 @@ description: >
   writing new C# files, and before adding a new coding pattern not already
   established in the repo — even a request that just looks like "add a
   quick feature" or "fix this bug" should route through it first. Also
-  covers reviewing a PR/diff in this repo and responding to feedback left
-  on one (`tasks/` procedures, not just reference lookup) — see How to use
-  this skill below. Trigger on: "how should I structure this", "coding
-  standards", "is this secure", "how do we handle X in the database",
-  "where does this belong", "contributing", "code of conduct", "review
-  this PR", "review my changes", "address that feedback", "handle the
-  review comments", or any request to add a feature to SharpMud.Engine /
+  covers designing a feature/architecture decision end-to-end (ADR + plan),
+  implementing against an already-`Accepted` ADR, reviewing a PR/diff in
+  this repo, and responding to feedback left on one (`tasks/` procedures,
+  not just reference lookup) — see How to use this skill below. Trigger
+  on: "how should I structure this", "coding standards", "is this
+  secure", "how do we handle X in the database", "where does this
+  belong", "contributing", "code of conduct", "design this feature",
+  "let's build/implement this", "review this PR", "review my changes",
+  "address that feedback", "handle the review comments", "what's next on
+  the roadmap", or any request to add a feature to SharpMud.Engine /
   SharpMud.Ruleset.Classic / SharpMud.Persistence / SharpMud.Host /
   SharpMud.Adapters.*.
 ---
@@ -63,9 +66,17 @@ lives in the file itself.
 
 | When you're asked to... | Run |
 |---|---|
+| Design a feature/architecture decision (light or deep dive → ADR → plan) | `tasks/design.md` |
+| Implement code against an already-`Accepted` ADR/plan | `tasks/implement.md` |
 | Review a PR/diff in this repo | `tasks/pr-review.md` |
 | Address/respond to feedback already posted on a PR | `tasks/respond-to-pr-feedback.md` |
 
+`design.md` → `implement.md` → `pr-review.md` → `respond-to-pr-feedback.md`
+is the full lifecycle of a non-trivial change in this repo, in order —
+most requests only need one or two of these (e.g. a light-dive design
+folded straight into implementation), but a genuinely new feature or
+roadmap slice touches all four in sequence.
+
 ### References
 
 | When you're about to... | Read |

.agents/skills/engineering-workflow/tasks/design.md

@@ -0,0 +1,114 @@
+# Task: Design
+
+**Trigger:** asked to design a new feature, an architecture change, or
+answer "how should I structure this" — including picking up the next item
+off a roadmap (e.g. the next slice in `PLAN-0001`). Produces a decision
+(an ADR, and usually a plan) that `tasks/implement.md` then builds against.
+This task **does not write application code** — if you catch yourself
+editing a `.cs` file mid-design, stop; that belongs in `tasks/implement.md`
+once the ADR is `Accepted`.
+
+## Load these references first
+
+Always:
+- `references/design-decisions.md` — this task is that file's process,
+  turned into a checklist; read it in full, not just skimmed, since the
+  light/deep-dive distinction and the ADR/plan mechanics are the actual
+  content of this task
+- `references/documentation.md`
+
+Conditionally, based on what the design touches — load these *before*
+generating alternatives, not after, since they constrain which
+alternatives are even legal:
+- `references/coding-standards.md` — the design will produce code; know
+  the constraints (composition over inheritance, DI-only, no MEF/
+  singletons) before proposing a shape that violates them
+- `references/persistence.md` — touches `SharpMud.Persistence`, a new/
+  changed `Behavior`, or anything that needs to survive a restart
+- `references/security.md` — touches auth, secrets, config, or the Telnet
+  input surface
+- `references/testing.md` — the design has a non-obvious test strategy
+  worth deciding up front (e.g. how something inherently timing-/
+  concurrency-sensitive gets covered)
+
+## Procedure
+
+1. **Check for existing precedent before assuming this is new.** Grep for
+   the likely type name (`Behavior`, `Manager`, `Command`) and skim the
+   relevant `docs/*.md` — a request phrased as "add X" is often "X already
+   exists, here's where" (`design-decisions.md`'s opening rule). A design
+   session grounded in what's already there beats one that re-derives a
+   solved problem.
+2. **Decide light vs. deep**, per `design-decisions.md`:
+   - **Light** — problem and solution shape are already clear (adding a
+     `Behavior` following a pattern another `Behavior` already uses,
+     adopting an established org-wide tooling convention, etc). Skip
+     straight to step 4; most ADR sections compress to a sentence.
+   - **Deep** — the problem is ambiguous, spans multiple subsystems, or has
+     more than one plausible shape. Do step 3 first.
+3. **Deep dive only: brainstorm before drafting anything.** Restate the
+   problem in your own words. Surface the constraints that actually bind
+   (existing `Behavior` composition model, no-Ruleset-reference rule,
+   DI-only wiring, no migrations — pull the specifics from whichever
+   conditional references you loaded above, don't restate them generically
+   here). Generate **at least two genuinely different alternatives** — not
+   a strawman and a preferred option; if you can only think of one real
+   shape, it probably wasn't a deep dive. Research existing prior art if
+   relevant (e.g. how WheelMUD solved the analogous problem — see
+   `docs/research/wheelmud-findings.md` for the citation format) and record
+   what's adopted vs. deliberately changed. **Talk it through with the
+   user before committing to one** — if there's a genuine fork (e.g. "match
+   an external reference's shape 1:1" vs. "a leaner reimplementation fit to
+   this repo's actual needs"), surface it explicitly (a direct question
+   works better than silently picking one and writing the ADR around it) —
+   the point of this phase is to catch a disagreement before it's baked
+   into an `Accepted` ADR, not after.
+4. **Write the ADR.** Copy `docs/adr/0000-adr-template.md` to
+   `docs/adr/NNNN-kebab-case-title.md` (next sequential number — check
+   `docs/adr/README.md`'s index for the last one used). Fill in Context,
+   Decision Drivers, Considered Options, Decision Outcome, Consequences,
+   Links — compressed to a sentence/"N/A" per section for a light dive,
+   full detail for a deep one. Keep it to decision content only — no task
+   checklist, no file list (that's the plan, step 6). Set `Status:
+   Proposed` while still unsettled; once the user has actually confirmed
+   the direction (the deep dive's step 3 conversation, or an explicit "yes,
+   do that" for a light one), flip it to `Accepted` — that status change
+   *is* the record of the decision being made.
+5. **Index and cross-link — without describing unimplemented behavior as
+   current.** Add a row to `docs/adr/README.md`'s index table. Subsystem
+   docs describe *current* state (`documentation.md`), and this task
+   hasn't written any code yet, so don't rewrite a doc's prose to describe
+   the new decision as already true. The only doc touch that belongs here
+   is a forward-reference — an Open Items entry (or equivalent) noting the
+   gap now has a decision, linked to the ADR, e.g. "resolved by ADR-NNNN,
+   not yet implemented — see `tasks/implement.md`." Actually describing the
+   new current state is `tasks/implement.md`'s step 6, once the code
+   backing it exists. If this ADR supersedes an earlier one, set that
+   ADR's `Status` to `Superseded by ADR-XXXX` — don't edit its Context/
+   Decision/Options, an accepted ADR is a historical record.
+6. **Decide whether a plan is warranted.** Per `docs/plans/README.md`: not
+   every ADR needs one — a light dive whose outcome is an obvious one-file
+   change doesn't. Write one when the implementation is multi-file, spans
+   more than one sitting, or has enough real sequencing that a checklist
+   earns its keep (this covers most deep dives and some light ones). If
+   warranted: copy `docs/plans/0000-plan-template.md` to
+   `docs/plans/NNNN-kebab-case-title.md` (same number as the ADR it
+   implements), fill in Goal/Scope/Tasks/Critical files/Test plan/
+   Verification, `Status: Not Started`. A plan can be drafted alongside a
+   still-`Proposed` ADR (designing the how often surfaces questions about
+   the what) — but it stays `Not Started` until the ADR is `Accepted`; a
+   plan moving to `In Progress` against a `Proposed` ADR means code is
+   being written for an unsettled decision.
+7. **Index the plan** in `docs/plans/README.md` if one was created, and
+   — if this design is a slice of the WheelMUD reconciliation roadmap —
+   update `docs/plans/0001-wheelmud-reconciliation-roadmap.md`'s row for
+   that slice to link the new ADR/plan.
+
+## Output
+
+An `Accepted` ADR (or a `Proposed` one, if genuinely still under
+discussion and this task is being paused rather than finished), indexed,
+cross-linked from the affected subsystem doc(s), with a `Plan` alongside
+it if the implementation is non-trivial. That pair is the handoff to
+`tasks/implement.md` — implementation shouldn't start (a plan shouldn't
+move to `In Progress`) until the ADR reads `Accepted`.

.agents/skills/engineering-workflow/tasks/implement.md

@@ -0,0 +1,89 @@
+# Task: Implement
+
+**Trigger:** asked to build/implement a feature, write the code for a
+slice, or "let's build this now" — following up on `tasks/design.md`
+having produced an `Accepted` ADR (and usually a `Plan`). Distinct from
+`tasks/design.md`, which decides *what* and *why* and never touches code;
+this task is *how*, and assumes the decision is already made. If asked to
+implement something with no `Accepted` ADR behind it and the change is
+non-trivial (per `design-decisions.md`'s light/deep-dive bar), stop and
+run `tasks/design.md` first rather than writing code against an
+undocumented decision — retrofitting an ADR after the fact produces a
+worse record than writing it first.
+
+## Load these references first
+
+Always:
+- `references/coding-standards.md`
+- `references/testing.md`
+- `references/documentation.md`
+- `references/design-decisions.md` (just the ADR/plan status rules — you
+  already did the design thinking in `tasks/design.md`)
+
+Conditionally, based on what the ADR/plan actually touches:
+- `references/persistence.md` — anything under `SharpMud.Persistence`, a
+  new/changed `Behavior`, or an EF Core config class
+- `references/security.md` — auth, secrets, config, or anything on the
+  Telnet (untrusted) input path
+
+## Procedure
+
+1. **Confirm there's an `Accepted` ADR before writing any code.** Find it
+   (check `docs/adr/README.md`'s index, or the plan referenced by the
+   request) and read its Decision Outcome — that's the spec you're
+   building against, not your own read of the request. If a `Plan` exists,
+   read its current Tasks/Critical files/Test plan sections too; if its
+   `Status` is still `Not Started` and the ADR is now `Accepted`, flip it
+   to `In Progress` as you begin.
+2. **Implement per `coding-standards.md`** — naming, nullable annotations
+   (comment every `!` outside the `= null!` EF case), async patterns,
+   traditional constructors for new classes, DI-only wiring (no MEF, no
+   static `XManager.Instance`), 4-param-max methods, file layout. Prefer
+   composition over inheritance for game objects — a new `Behavior`, never
+   a new `Thing` subtype. XML doc comments on every new/substantially
+   -changed public member (`documentation.md`) — this is not optional
+   "nice to have" for new code, only pre-existing code gets the debt
+   carve-out.
+3. **Add tests as you build, not after.** New engine/ruleset/persistence
+   behavior gets test coverage per `testing.md` (xUnit v3 on Microsoft
+   Testing Platform, AAA with blank-line separation, per-project
+   `[XxxAutoData]` attributes, `TestLogger` over `NSubstitute.For<ILogger<T>>()`
+   for logging). Use the `dotnet-unit-testing-patterns` skill for the
+   mechanics (SpecimenBuilders, NSubstitute setup) if you need the deeper
+   reference.
+4. **Update the plan as reality meets it, not just at the end.** Check off
+   tasks as they're actually done; if the implementation diverges from
+   what the plan scoped (a file wasn't needed, an extra one was), edit the
+   plan's task list to match — a plan being wrong about *how* doesn't
+   require touching the ADR, which stays immutable (`design-decisions.md`).
+5. **Build and run the full test suite** before treating anything as done
+   — `dotnet build` then `dotnet test`, not just "it compiled."
+6. **Update `docs/*.md` in the same change**, not a follow-up
+   (`documentation.md`, `contributing.md`) — mark previously "not
+   implemented"/stub sections as done, close the Open Items this work
+   resolves, add "Verified" detail once you've actually checked the
+   behavior, link back to the ADR rather than re-deriving its reasoning.
+7. **Do real manual verification for anything network/session/persistence
+   -facing** — this repo's established pattern (see `testing.md`,
+   `contributing.md`) is that unit tests alone don't count as "verified"
+   for that surface. Actually run the host, connect a real client, exercise
+   the golden path and the edge case the ADR called out, and record what
+   you did in the doc's Verified section — not just "tests pass."
+8. **Close out the plan.** Once every task is checked off and verification
+   is done, set the plan's `Status: Done`. If this implements a slice of
+   `PLAN-0001` (the WheelMUD reconciliation roadmap), check off that
+   slice's row there too.
+9. **Commit, and open a PR if asked**, per `contributing.md`'s "keep PRs
+   scoped to one decision or one feature" rule — don't bundle an unrelated
+   fix or refactor into the same diff just because you're already touching
+   nearby code (`code-of-conduct.md`). Route review through
+   `tasks/pr-review.md` once a PR exists, rather than self-reviewing
+   informally.
+
+## Output
+
+Working, tested code matching the ADR's Decision Outcome; the plan (if one
+exists) at `Status: Done` with every task checked off; `docs/*.md` updated
+in the same change, not deferred; and, for anything network/session/
+persistence-facing, a real recorded manual verification — not just a
+green test suite.

.github/workflows/pr-build.yaml

@@ -2,6 +2,7 @@ name: PR Build
 
 on:
   pull_request:
+    types: [opened, synchronize, reopened, ready_for_review]
     branches:
       - main
   workflow_dispatch: