diff --git a/.agent_instructions/skills_overview.md b/.agent_instructions/skills_overview.md index 87d8de1aed..14c9a14a8d 100644 --- a/.agent_instructions/skills_overview.md +++ b/.agent_instructions/skills_overview.md @@ -15,6 +15,19 @@ Skills are slash commands that automate multi-step workflows and enforce Brighte | `/test-first` | TDD with mandatory approval | `/test-first ` | | `/tidy-first` | Separate refactoring from features | `/tidy-first ` | | `/adr` | Create Architecture Decision Record | `/adr ` | +| `/bugfix:*` | Diagnosis-first bug workflow (Confirm gate) | `/bugfix:triage [issue \| description]` | + +### Bugfix Workflow Skills + +| Skill | Purpose | Usage | +|-------|---------|-------| +| `/bugfix:triage` | Restate symptom, locate code, form hypothesis | `/bugfix:triage [issue-number \| description]` | +| `/bugfix:confirm` | ✋ Prove the root cause before any fix | `/bugfix:confirm` | +| `/bugfix:test` | Failing regression test (via `/test-first`) | `/bugfix:test` | +| `/bugfix:fix` | Minimal fix scoped to the confirmed cause | `/bugfix:fix` | +| `/bugfix:verify` | Run suite; capture root cause + `Fixes #N` | `/bugfix:verify` | +| `/bugfix:status` | Show all bugs and their phase | `/bugfix:status` | +| `/bugfix:switch` | Switch the active bug | `/bugfix:switch <NNNN-slug>` | ### Specification Workflow Skills @@ -39,6 +52,12 @@ Skills are slash commands that automate multi-step workflows and enforce Brighte │ Quick Reference │ └────────────────────────────────────────────────────────────────┘ +🐞 BUGFIX (DIAGNOSIS-FIRST) + /bugfix:triage → :confirm → :test → :fix → :verify + • Triage → ✋Confirm gate → Test-first → Fix → Verify + • Proves the root cause BEFORE writing a fix + • Example: /bugfix:triage 4054 + 🧪 TEST-DRIVEN DEVELOPMENT /test-first <behavior> • Write test → Approve → Implement → Refactor @@ -74,24 +93,28 @@ Skills are slash commands that automate multi-step workflows and enforce Brighte ## Decision Tree: Which Skill Should I Use? ``` - Need to document decision? + Need to document a decision? │ - ┌─────────┴─────────┐ - YES NO - │ │ - /adr ↓ - Adding behavior? - │ - ┌──────────┴──────────┐ - YES NO - │ │ - ↓ ↓ - Code needs cleanup? Just refactoring? - │ │ - ┌───────┴───────┐ │ - YES NO │ - │ │ │ - /tidy-first /test-first /tidy-first + ┌───────────┴───────────┐ + YES NO + │ │ + /adr Fixing a bug? + │ + ┌────────────────┴────────────────┐ + YES NO + │ │ + Root cause proven? Adding behavior? + │ │ + ┌─────────┴─────────┐ ┌─────────┴─────────┐ + NO YES YES NO + │ │ │ │ + /bugfix:triage /test-first Code needs Just refactoring + (Confirm gate) cleanup? → /tidy-first + │ + ┌─────────┴─────────┐ + YES NO + │ │ + /tidy-first /test-first ``` ## Enforcement of Brighter Practices @@ -134,6 +157,25 @@ Each skill enforces specific practices from `.agent_instructions/`: ## Common Workflows +### Workflow 0: Fix a Bug (Diagnosis-First) + +```bash +# 1. Triage — restate symptom, locate code, form hypothesis (suggested fix is UNVERIFIED) +/bugfix:triage 4054 + +# 2. ✋ Confirm — prove the root cause BEFORE any fix (may widen the scope) +/bugfix:confirm + +# 3. ✋ Test-first — failing regression test (delegates to /test-first) +/bugfix:test + +# 4. Fix — minimal change to green, scoped to the confirmed cause +/bugfix:fix + +# 5. Verify — run the suite; commit captures root cause + Fixes #4054 +/bugfix:verify +``` + ### Workflow 1: Add New Feature (Clean Code) ```bash diff --git a/.claude/commands/README.md b/.claude/commands/README.md index 89490e2865..1676e77e95 100644 --- a/.claude/commands/README.md +++ b/.claude/commands/README.md @@ -111,11 +111,45 @@ Skills are invoked using slash commands in Claude Code: --- +### 4. Bugfix - Diagnosis-First Bug Workflow + +**Commands**: `/bugfix:triage`, `/bugfix:confirm`, `/bugfix:test`, `/bugfix:fix`, `/bugfix:verify` (plus `/bugfix:status`, `/bugfix:switch`) + +**Purpose**: A lightweight, diagnosis-first workflow for fixing bugs. It is `/test-first` wrapped with an explicit **Confirm** gate up front — because a bug's root cause is a hypothesis until proven. + +**When to use**: +- A defect whose root cause is not yet proven +- An issue that arrived with a suggested fix (including agent-authored) you should verify before trusting +- Anywhere `/test-first` alone would jump to a test for an *assumed* cause + +**Workflow**: +1. **Triage** (`/bugfix:triage [issue|description]`) - Restate the symptom, locate the code, form a root-cause hypothesis (any suggested fix is UNVERIFIED) +2. ✋ **Confirm** (`/bugfix:confirm`) - Prove the hypothesis by code-trace and/or red repro before any fix; surfaces scope changes / extra defects +3. ✋ **Test-first** (`/bugfix:test`) - Delegates to `/test-first` for the failing regression test +4. **Fix** (`/bugfix:fix`) - Minimal change to green, scoped to the confirmed cause +5. **Verify** (`/bugfix:verify`) - Run the suite; capture the root cause and `Fixes #N` in the commit/PR + +**Example**: +```bash +/bugfix:triage 4054 # ASB SessionId case-sensitivity +/bugfix:confirm # proves CamelCase round-trip cause; finds a 2nd defect +/bugfix:test # red regression test (via /test-first) +/bugfix:fix # minimal fix scoped to the confirmed cause +/bugfix:verify # suite green; fix: commit with Fixes #4054 +``` + +**Why it matters**: The Confirm gate stops you fixing a symptom or trusting a wrong suggested fix — and frequently changes the scope of the fix. It deliberately omits the ADR/requirements/review rounds that `/spec` mandates. + +📖 **Documentation**: [.claude/commands/bugfix/README.md](bugfix/README.md) + +--- + ## Skill Categories ### Development Workflow Skills - **`/test-first`** - TDD with approval gate - **`/tidy-first`** - Safe refactoring workflow +- **`/bugfix:*`** - Diagnosis-first bug workflow (Triage → Confirm → Test-first → Fix → Verify) ### Documentation Skills - **`/adr`** - Architecture Decision Records @@ -142,7 +176,14 @@ Do you need to document an architectural decision? ├─ Yes → /adr <title> └─ No ↓ -Are you adding new behavior or fixing a bug? +Are you fixing a bug? +├─ Yes ↓ +│ └─ Is the root cause already proven/obvious? +│ ├─ No → /bugfix:triage (Triage → Confirm gate → Test-first → Fix → Verify) +│ └─ Yes → /test-first <behavior> (cause is clear; just need the test) +└─ No ↓ + +Are you adding new behavior? ├─ Yes ↓ │ └─ Does existing code need refactoring first? │ ├─ Yes → /tidy-first <description> @@ -401,6 +442,7 @@ Three new skills enforce Brighter's mandatory engineering practices: | `/test-first` | TDD with approval | Tests → Implementation → Refactoring | | `/adr` | Documented decisions | Numbered ADR files | | `/tidy-first` | Structural/behavioral separation | Two commits: refactor + feat | +| `/bugfix:*` | Confirm root cause before fixing | Bug record + regression test + scoped `fix:` commit | **Key insight**: These skills make the **correct approach the easy path** by automating multi-step workflows and enforcing approval gates. diff --git a/.claude/commands/bugfix/README.md b/.claude/commands/bugfix/README.md new file mode 100644 index 0000000000..b6aac5bcfa --- /dev/null +++ b/.claude/commands/bugfix/README.md @@ -0,0 +1,172 @@ +# Bugfix Workflow Commands + +This directory contains Claude Code commands that implement a **lightweight, diagnosis-first +workflow for fixing bugs**. It is the bug-shaped counterpart to the heavyweight `/spec:*` +workflow: where `/spec` runs Requirements → ADR → adversarial review → tasks → implement (right +for *features*), `/bugfix` runs **Triage → Confirm → Test-first → Fix → Verify** and deliberately +omits the ADR/requirements/review rounds. + +## Why a separate workflow? + +Bugs differ from features in one important way: **the root cause is a hypothesis until proven.** +Issues often arrive with a suggested fix (sometimes from an automated agent), but a suggested fix +can be wrong, incomplete, or address a *symptom* rather than the cause. `/test-first` jumps +straight to writing a test for an *assumed* behaviour — it has no gate for *confirming the +diagnosis first*. + +`/bugfix` is essentially `/test-first` wrapped with an explicit **Confirm** gate up front, plus a +triage step that records the hypothesis. + +**Worked example — #4054 (ASB `SessionId` case-sensitivity).** The issue arrived with a plausible +suggested fix. Before writing any code, Confirm proved the root cause by reading +`JsonSerialisationOptions` (`PropertyNamingPolicy = JsonNamingPolicy.CamelCase`) — keys are +camelCased on serialization round-trips — and surfaced a *second* defect the suggested fix only +partially addressed (reserved-header stripping is also case-sensitive, so reserved keys leak into +`ApplicationProperties`). Confirming the theory **changed the scope of the fix.** That is exactly +what the Confirm gate exists to catch. + +## Workflow + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ Bugfix Workflow │ +└─────────────────────────────────────────────────────────────────┘ + + GitHub Issue / bug report + │ + ▼ + Triage ──────────────► /bugfix:triage [issue-number | description] + │ (restate symptom, locate code, form hypothesis; + │ any suggested fix is UNVERIFIED) + ▼ + Confirm ─────────────► /bugfix:confirm + │ ✋ Confirm gate — prove the hypothesis (code-trace + │ and/or red repro) before any fix; .confirm-approved + │ (refuted → back to triage) + ▼ + Test-first ──────────► /bugfix:test + │ ✋ delegates to /test-first (approve test in IDE) + ▼ + Fix ─────────────────► /bugfix:fix + │ (minimal change to green, scoped to confirmed cause; + │ /tidy-first if structural cleanup needed first) + ▼ + Verify ──────────────► /bugfix:verify + │ (run suite; regression test stays; capture root cause, + │ Fixes #N — no auto-close) + ▼ + Pull Request +``` + +## The two gates + +| Gate | Where | What it guarantees | +|------|-------|--------------------| +| ✋ **Confirm** | `/bugfix:confirm` | The root-cause diagnosis is **proven**, not assumed, and the fix's scope (including any extra defects) is agreed before code is written. The one on-disk marker: `.confirm-approved`. | +| ✋ **Test approval** | `/bugfix:test` → `/test-first` | The failing regression test is reviewed in the IDE before any implementation (inherited from `/test-first`). | + +### Confirm evidence standard — red repro preferred, code-trace allowed + +Confirm passes on **either**: +- an executable **red reproduction** (preferred when the bug reproduces cheaply — pure logic, no + live infra), **or** +- a documented **code-trace** that proves the cause (the accepted path when a repro would need a + real broker/database/etc., as in #4054). + +The durable, executable regression test is written later by `/bugfix:test` (via `/test-first`) — +Confirm itself stays read-only and writes no source. + +## Sub-agents & model policy + +Modelled on `/spec` (see `.claude/commands/spec/README.md` → "Sub-agents & model policy"): +reasoning-heavy, read-only steps delegate to a **`Plan`** sub-agent (no file-editing tool, no +`AskUserQuestion`) which RETURNS its findings as text; the **main agent** owns all user +interaction (the Confirm gate) and all bookkeeping (writing `bugfix.md`, markers, `gh`, git). + +| Command | Sub-agent (type) | Model | Rationale | +|---------|------------------|-------|-----------| +| `/bugfix:triage` | Yes — `Plan` (read-only) | **opus** | Diagnostic analysis / locating code | +| `/bugfix:confirm` | Yes — `Plan` (read-only) | **opus** | Adversarial root-cause proof | +| `/bugfix:test` | No — delegates to `/test-first` | **sonnet** | Implementation (interactive approval gate) | +| `/bugfix:fix` | No (main agent) | **sonnet** | Implementation work | +| `/bugfix:verify` | No (main agent) | — | Mechanical: run suite, prepare commit/PR | +| `/bugfix:status`, `/bugfix:switch` | No | — | Bookkeeping | + +`triage` and `confirm` use `Plan` so the "return as text, don't write the file" rule is hard to +violate accidentally (no `Write`/`Edit`). `test`/`fix` run in the **main agent** because the +test-approval gate is interactive and the fix writes source — set the session model to **sonnet** +for those, matching the implementation policy. + +## State model (lightweight — single record + one marker) + +``` +bugfixes/ +├── .current-bug # tracks the active bug (like specs/.current-spec) +└── 0001-asb-sessionid-case/ + ├── .issue-number # linked GitHub issue (optional) + ├── .confirm-approved # the ONE gate marker + └── bugfix.md # symptom / hypothesis / root cause / evidence / fix +``` + +Unlike `/spec` (three approval markers), `/bugfix` has a **single** marker (`.confirm-approved`). +Every other phase is inferred from the content of `bugfix.md`, keeping the workflow light. + +### `bugfix.md` sections (filled progressively) + +| Section | Filled by | +|---------|-----------| +| Symptom, Suspected Location, Root-Cause Hypothesis | `/bugfix:triage` | +| Confirmed Root Cause, Evidence, Scope Notes | `/bugfix:confirm` | +| Regression Test | `/bugfix:test` | +| Fix | `/bugfix:fix` | +| Status (Triaged → Confirmed → Tested → Fixed → Verified) | each step | + +## Commands + +### `/bugfix:triage [issue-number | description]` +Start a bug. Sets up `bugfixes/NNNN-slug/` and `.current-bug`, pulls the issue via `gh`, and (via +a `Plan` sub-agent) restates the symptom, locates the suspect code, and forms a root-cause +hypothesis. Any suggested fix is recorded as **UNVERIFIED**. Offers a `bugfix/N-slug` branch. + +### `/bugfix:confirm` +The ✋ Confirm gate. A `Plan` sub-agent proves or refutes the hypothesis by tracing the code +(and/or describing a red repro), citing `file:line` evidence, and hunts for scope changes / +additional defects. The main agent writes the findings into `bugfix.md` and asks the user to +approve the diagnosis; on approval it creates `.confirm-approved`. A refuted hypothesis loops back +to triage. + +### `/bugfix:test` +Requires `.confirm-approved`. Delegates to `/test-first` to write the failing regression test for +the confirmed behaviour (and one per additional defect in the Scope Notes). The `/test-first` +IDE-approval gate applies. + +### `/bugfix:fix` +Requires `.confirm-approved` + a red test. Makes the test green with the **minimal** change scoped +to the confirmed cause — no speculative edits, no default changes. If structural cleanup is needed +first, defers to `/tidy-first` (separate `refactor:` commit, then the `fix:` commit). + +### `/bugfix:verify` +Runs the regression test(s) and the broader suite; the regression test stays. Suggests a `fix:` +commit and PR body that capture the confirmed root cause and link the issue with `Fixes #N` — +without closing the issue directly (merge closes it). + +### `/bugfix:status` +Lists all bugs and derives each phase from `bugfix.md` + `.confirm-approved`. + +### `/bugfix:switch <bug-slug>` +Switches the active bug (updates `bugfixes/.current-bug`). + +## When to use `/bugfix` vs other workflows + +- **`/bugfix`** — a defect whose root cause is not yet proven, or that arrived with a suggested fix + you should verify before trusting. The Confirm gate is the value-add. +- **`/test-first`** — a small change whose behaviour/cause is already obvious; you just need the + test-first discipline. +- **`/tidy-first`** — the change is (or includes) structural cleanup; keep refactor and behaviour + in separate commits. `/bugfix:fix` calls this when a fix needs cleanup first. +- **`/spec:*`** — a *feature* or capability needing requirements + ADR design, not a bug fix. + +## Related Commands +- **`/test-first`** — `.claude/commands/tdd/test-first.md` (the test step delegates here) +- **`/tidy-first`** — `.claude/commands/refactor/tidy-first.md` (the fix step defers here when needed) +- **`/spec:*`** — `.claude/commands/spec/README.md` (the heavyweight feature workflow this mirrors) diff --git a/.claude/commands/bugfix/confirm.md b/.claude/commands/bugfix/confirm.md new file mode 100644 index 0000000000..ff1cdc3a7e --- /dev/null +++ b/.claude/commands/bugfix/confirm.md @@ -0,0 +1,118 @@ +--- +allowed-tools: Bash(cat:*), Bash(test:*), Bash(touch:*), Bash(ls:*), Bash(echo:*), Bash(grep:*), Read, Write, Edit, Glob, Grep, Agent, AskUserQuestion +description: Confirm the root-cause hypothesis before any fix (the ✋ Confirm gate) +argument-hint: (no arguments — operates on the active bug) +--- + +## Context + +Current bug: !`cat bugfixes/.current-bug 2>/dev/null || echo "No active bug"` + +## Your Task + +**Workflow**: Issue → Triage → **Confirm (✋ gate)** → Test-first (✋ gate) → Fix → Verify + +This is the step that makes `/bugfix` different from a plain `/test-first`. **Prove the root-cause +hypothesis before writing any fix.** A suggested fix — even an agent-authored one — is unverified +until this passes. Confirming the theory frequently changes the *scope* of the fix (e.g. issue +#4054: confirming the camelCase serialization cause surfaced a *second* defect that the suggested +fix only partially addressed). + +**Sub-agent**: The diagnostic proof (tracing the code) is delegated to a sub-agent +(`subagent_type: "Plan"`, **`model: "opus"`**). It is read-only — it traces, it does not fix, and +it RETURNS its findings as text. The **main agent** writes the findings into `bugfix.md` and owns +the user-facing **Confirm gate** (`Plan` has no `AskUserQuestion`, so it cannot run the gate). See +`.claude/commands/bugfix/README.md` → "Sub-agents & model policy". + +### Step 1: Load State + +1. Read `bugfixes/.current-bug`. If absent, tell the user to run `/bugfix:triage` first and stop. +2. Read `bugfixes/{current-bug}/bugfix.md`. If the Root-Cause Hypothesis is empty, tell the user + to complete `/bugfix:triage` first and stop. + +### Step 2: Launch Sub-Agent to Prove or Refute + +Launch an `Agent` with `subagent_type: "Plan"` and **`model: "opus"`**. The prompt MUST include: + +1. The full `bugfix.md` text (Symptom, Suspected Location, Root-Cause Hypothesis). +2. The instruction below and the confirmation criteria. +3. An explicit instruction: **RETURN the findings as markdown text. Do NOT write any file. Do NOT + apply a fix. Do NOT ask the user any questions.** + +**Confirmation criteria for the sub-agent:** + +- **Trace the code** from the symptom to the suspected cause using `Read`, `Glob`, `Grep`. Cite + concrete `file:line` evidence at every step. The goal is to either **prove** or **refute** the + hypothesis — be adversarial, do not rubber-stamp it. +- **Evidence standard (red repro preferred, code-trace allowed):** + - A documented **code-trace** that proves the cause is sufficient — this is the accepted path + when an executable reproduction would need live infrastructure (a real broker, database, etc.). + - If the bug *can* be reproduced cheaply (pure logic, no live infra), describe the **red repro**: + the exact assertion that fails today and why. The durable executable test is written later by + `/bugfix:test` — Confirm does not write source. +- **Check the suggested fix** (if the issue/triage proposed one): does the proven cause match it? + Is it complete, or does it address only a symptom? State explicitly whether it is + CONFIRMED / PARTIAL / WRONG. +- **Hunt for scope changes**: does the proven cause imply *additional* defects, related call + sites, or cross-backend parity gaps the suggested fix misses? List them — these change the fix's + scope. + +**Sub-agent output format:** + +```markdown +## Verdict +CONFIRMED | REFUTED — one line on whether the hypothesis holds. + +## Confirmed Root Cause +The proven cause, in precise terms. + +## Evidence +- [x] Code-trace: step-by-step with `file:line` references, OR +- [ ] Red repro: the assertion that fails today and why +(tick whichever applies; a trace alone is valid for infra-bound bugs) + +## Suggested-Fix Assessment +CONFIRMED / PARTIAL / WRONG — and why. + +## Scope Notes +Additional defects, related sites, or cross-backend parity gaps the fix must also cover (or +"none found"). Be specific with `file:line`. +``` + +### Step 3: Write Findings Into bugfix.md + +After the sub-agent returns, use `Edit` to fill the **Confirmed Root Cause**, **Evidence**, and +**Scope Notes** sections of `bugfixes/{current-bug}/bugfix.md` with the validated findings. + +- If the verdict is **REFUTED**: update the Root-Cause Hypothesis to reflect what was learned, set + Status back to `Triaged`, tell the user the hypothesis did not hold, and recommend re-running + `/bugfix:triage` (or refining the hypothesis) — do **not** create the approval marker. Stop here. + +### Step 4: ✋ Confirm Gate (MAIN agent — CRITICAL) + +**You MUST get explicit user approval of the diagnosis before any test or fix.** Present: +- The Confirmed Root Cause and the Evidence +- The Suggested-Fix Assessment +- Any Scope Notes (especially additional defects — these may widen the fix) + +Use `AskUserQuestion`: + +``` +Question: "I've confirmed the root cause: {one-line}. Approve this diagnosis and proceed to write + the regression test?" +Options: +1. "Yes, the diagnosis is confirmed" — proceed +2. "Revise the diagnosis" — user explains; loop back to Step 2 +3. "Cancel" — stop +``` + +**On approval only:** `touch bugfixes/{current-bug}/.confirm-approved` and set Status to +`Confirmed` in `bugfix.md`. + +### Step 5: Next Steps + +Remind the user: +- The diagnosis is approved. Run `/bugfix:test` to write the failing regression test (it delegates + to `/test-first`, which has its own approval gate). +- If the Scope Notes widened the fix, the regression test(s) should cover the additional defects + too. diff --git a/.claude/commands/bugfix/fix.md b/.claude/commands/bugfix/fix.md new file mode 100644 index 0000000000..7d408c1b0c --- /dev/null +++ b/.claude/commands/bugfix/fix.md @@ -0,0 +1,68 @@ +--- +allowed-tools: Bash(cat:*), Bash(test:*), Bash(ls:*), Bash(echo:*), Bash(dotnet:*), Bash(git:*), Read, Write, Edit, Glob, Grep, AskUserQuestion +description: Make the regression test green with the minimal change scoped to the confirmed cause +argument-hint: (no arguments — operates on the active bug) +--- + +## Context + +Current bug: !`cat bugfixes/.current-bug 2>/dev/null || echo "No active bug"` +Confirm gate: !`test -f bugfixes/$(cat bugfixes/.current-bug 2>/dev/null)/.confirm-approved && echo "✓ approved" || echo "✗ NOT approved — run /bugfix:confirm first"` + +## Your Task + +**Workflow**: Issue → Triage → Confirm (✋ gate) → Test-first (✋ gate) → **Fix** → Verify + +Make the red regression test pass with the **minimum** change required by the confirmed cause — +nothing more. This is the GREEN phase from `/test-first`, scoped by the confirmed diagnosis. + +> **Recommended model: `sonnet`.** This is implementation work in the main agent (writes source, +> runs `dotnet`). See `.claude/commands/bugfix/README.md` → "Sub-agents & model policy". + +### Step 1: Enforce Prerequisites + +1. Read `bugfixes/.current-bug`. If absent, tell the user to run `/bugfix:triage` and stop. +2. Verify `bugfixes/{current-bug}/.confirm-approved` exists. If not, stop and point to + `/bugfix:confirm`. +3. Read `bugfixes/{current-bug}/bugfix.md`. Confirm the **Regression Test** section names a test + (a red test must exist). If not, stop and point to `/bugfix:test`. + +### Step 2: Decide — Straight Fix or Tidy-First + +Read the suspect code (Confirmed Root Cause + Suspected Location). + +- **If the fix is a clean, localized change**: proceed to Step 3 (a single `fix:` commit). +- **If the code needs structural cleanup *before* the fix is safe** (e.g. a large method must be + broken up, duplicated logic consolidated): do the cleanup via **`/tidy-first`** so the + structural change lands in its own `refactor:` commit first, *then* the behavioural `fix:` + commit. Never mix structural and behavioural changes in one commit + (`.agent_instructions/code_style.md`). + +### Step 3: GREEN — Minimal Fix + +1. Write the **minimum** code to make the regression test(s) pass. Scope is bounded by the + **Confirmed Root Cause** and the **Scope Notes** — no speculative edits, no unrequested + "improvements", no default changes (CLAUDE.md → "Change Scope"). +2. Follow `.agent_instructions/code_style.md` (naming, RDD, nullable, XML docs on public members, + MIT header on new files) and `.agent_instructions/documentation.md`. +3. If the Scope Notes listed additional defects, fix those too — they share the confirmed cause + and each has its own red test from `/bugfix:test`. +4. **Cross-backend parity**: if the cause touches one backend (MSSQL/PostgreSQL/MySQL/SQLite/ + Spanner/Dynamo) and the Scope Notes flagged parity, apply the equivalent fix to the others. + +### Step 4: Run the Targeted Test + +Run the regression test(s): +`dotnet test {test-project} --filter "FullyQualifiedName~When_{...}"` +Confirm they now pass (green). Report the result faithfully — if still red, iterate; do not +declare success. + +### Step 5: Record the Fix + +Use `Edit` to fill the **Fix** section of `bugfixes/{current-bug}/bugfix.md` with the files +changed and a one-line summary, and set Status to `Fixed`. + +### Step 6: Next Steps + +Remind the user to run `/bugfix:verify` to run the full suite (no regressions) before +committing/PR. diff --git a/.claude/commands/bugfix/status.md b/.claude/commands/bugfix/status.md new file mode 100644 index 0000000000..5087934906 --- /dev/null +++ b/.claude/commands/bugfix/status.md @@ -0,0 +1,65 @@ +--- +allowed-tools: Bash(cat:*), Bash(grep:*), Bash(test:*), Bash(find:*), Bash(ls:*), Bash(echo:*), Read, Glob +description: Show all bugfixes and their phase +argument-hint: (no arguments) +--- + +## Gather Status Information + +Bug tracking directory: bugfixes/ + +**Workflow**: Issue → Triage → Confirm (✋ gate) → Test-first (✋ gate) → Fix → Verify + +## Your Task + +### Step 1: Gather Information + +1. Read `bugfixes/.current-bug` to determine the active bug (if any). +2. List all bug directories: `ls -d bugfixes/*/ 2>/dev/null`. If none, report "No bugfixes yet — + start one with `/bugfix:triage [issue-number | description]`" and stop. +3. For each bug directory, check: + - `.issue-number` (linked GitHub issue, if any) + - `.confirm-approved` (the one approval marker) + - `bugfix.md` — read the `**Status**` line and which sections are filled + +### Step 2: Derive Each Phase + +Infer the phase for each bug from its `bugfix.md` + marker (lightweight — only one marker exists): + +- **Triaged** — `bugfix.md` exists with a Root-Cause Hypothesis, no `.confirm-approved` +- **Confirmed** — `.confirm-approved` exists (Confirmed Root Cause + Evidence filled) +- **Tested** — Regression Test section names a test +- **Fixed** — Fix section names changed files +- **Verified** — Status line reads `Verified` + +A section still holding its `_(left blank — filled by …)_` placeholder counts as **empty** — it is +not a real entry, so do not treat it as reaching that phase. + +### Step 3: Present the Report + +``` +Bugfix Status Report +==================== + +Active bug: bugfixes/{current-bug}/ (marked with *) + +* bugfixes/0001-asb-sessionid-case/ + Issue: #4054 + Phase: Confirmed ✓ (Confirm gate: ✓ approved) + Next: /bugfix:test + + bugfixes/0002-... + Issue: (none) + Phase: Triaged + Next: /bugfix:confirm +``` + +### Step 4: Legend and Next Action + +``` +Phases: Triaged → Confirmed → Tested → Fixed → Verified +Gate: ✋ Confirm gate (.confirm-approved) must pass before Test-first +* Active bug +``` + +Finish with the single recommended next command for the active bug. diff --git a/.claude/commands/bugfix/switch.md b/.claude/commands/bugfix/switch.md new file mode 100644 index 0000000000..def7bf403f --- /dev/null +++ b/.claude/commands/bugfix/switch.md @@ -0,0 +1,24 @@ +--- +allowed-tools: Bash(cat:*), Bash(test:*), Bash(ls:*), Bash(echo:*), Read, Glob +description: Switch the active bugfix +argument-hint: <NNNN-slug> +--- + +## Available Bugfixes + +!`ls -d bugfixes/*/ 2>/dev/null | sort` + +## Your Task + +Switch the active bug to: $ARGUMENTS + +`$ARGUMENTS` is the full directory name (`NNNN-slug`, e.g. `0001-asb-sessionid-case`) as shown in +**Available Bugfixes** above — not the bare slug. + +1. Verify `bugfixes/$ARGUMENTS/` exists. If not (e.g. only a bare slug was given), list the + available bugs and stop. +2. Update the active bug: `echo $ARGUMENTS > bugfixes/.current-bug` +3. Read `bugfixes/$ARGUMENTS/bugfix.md` and show its current phase and recommended next command + (see `/bugfix:status` for the phase derivation). + +If no argument is provided, list all available bugs. diff --git a/.claude/commands/bugfix/test.md b/.claude/commands/bugfix/test.md new file mode 100644 index 0000000000..c3329cc615 --- /dev/null +++ b/.claude/commands/bugfix/test.md @@ -0,0 +1,62 @@ +--- +allowed-tools: Bash(cat:*), Bash(test:*), Bash(ls:*), Bash(echo:*), Bash(dotnet:*), Read, Write, Edit, Glob, Grep, AskUserQuestion +description: Write the failing regression test for a confirmed bug (delegates to /test-first) +argument-hint: (no arguments — operates on the active bug) +--- + +## Context + +Current bug: !`cat bugfixes/.current-bug 2>/dev/null || echo "No active bug"` +Confirm gate: !`test -f bugfixes/$(cat bugfixes/.current-bug 2>/dev/null)/.confirm-approved && echo "✓ approved" || echo "✗ NOT approved — run /bugfix:confirm first"` + +## Your Task + +**Workflow**: Issue → Triage → Confirm (✋ gate) → **Test-first (✋ gate)** → Fix → Verify + +Write the **failing regression test** that pins the confirmed bug. This step is a thin wrapper +around `/test-first` — it carries the confirmed diagnosis in, and `/test-first`'s own mandatory +IDE-approval gate governs the test before any implementation. + +> **Recommended model: `sonnet`.** This step writes a test in the main agent and runs `dotnet`, +> so it is implementation work — match the model policy for implementation +> (`.claude/commands/bugfix/README.md` → "Sub-agents & model policy"). If your session is on +> opus, consider switching to sonnet first. + +### Step 1: Enforce the Confirm Gate + +1. Read `bugfixes/.current-bug`. If absent, tell the user to run `/bugfix:triage` and stop. +2. Verify `bugfixes/{current-bug}/.confirm-approved` exists. **If it does not, stop** and tell the + user to run `/bugfix:confirm` first — a regression test must pin a *confirmed* cause, not an + assumed one. +3. Read `bugfixes/{current-bug}/bugfix.md` for the Confirmed Root Cause, Evidence, and **Scope + Notes** (the test set must also cover any additional defects surfaced at Confirm). + +### Step 2: Delegate to /test-first + +Hand the confirmed behaviour to the `/test-first` workflow. Frame the behaviour from the +**Confirmed Root Cause**, not the raw symptom — e.g. for #4054: +`/test-first when a message with a SessionId header is round-tripped through the ASB +serializer the reserved header is read back case-insensitively and not leaked into +ApplicationProperties`. + +Follow `/test-first` exactly: +- 🔴 RED: write the test per `.agent_instructions/testing.md` conventions + (`When_[condition]_should_[behavior]`, one test per file, Arrange/Act/Assert, Evident Data, + InMemory* for I/O, public exports only) and run it to confirm it fails for the right reason. +- ✋ **APPROVAL GATE**: `/test-first` requires explicit user approval of the test before any + implementation. Honour it — do not write the fix here. + +If the Scope Notes listed additional defects, write a failing test for each (run `/test-first` +once per behaviour) so the fix can't leave a known defect uncovered. + +### Step 3: Record the Test + +Use `Edit` to fill the **Regression Test** section of `bugfixes/{current-bug}/bugfix.md` with the +test file path(s), and set Status to `Tested`. + +### Step 4: Next Steps + +Remind the user: +- The regression test is red and approved. Run `/bugfix:fix` to make it green with the **minimal** + change scoped to the confirmed cause. +- Do **not** broaden the change beyond what the confirmed cause (and Scope Notes) require. diff --git a/.claude/commands/bugfix/triage.md b/.claude/commands/bugfix/triage.md new file mode 100644 index 0000000000..591c49cc1e --- /dev/null +++ b/.claude/commands/bugfix/triage.md @@ -0,0 +1,130 @@ +--- +allowed-tools: Bash(cat:*), Bash(test:*), Bash(touch:*), Bash(ls:*), Bash(mkdir:*), Bash(echo:*), Bash(gh:*), Bash(git:*), Read, Write, Glob, Grep, Agent, AskUserQuestion +description: Triage a bug — restate the symptom, locate the code, form a root-cause hypothesis +argument-hint: [issue-number | bug description] +--- + +## Context + +Current bug: !`cat bugfixes/.current-bug 2>/dev/null || echo "No active bug"` +Current branch: !`git branch --show-current` + +## Your Task + +**Workflow**: Issue → **Triage** → Confirm (✋ gate) → Test-first (✋ gate) → Fix → Verify + +This is the **first** step of the lightweight `/bugfix` workflow. Its job is to record the +*symptom*, *locate the suspect code*, and *form a root-cause hypothesis* — **without** fixing +anything yet. A bug's root cause is a hypothesis until proven; that proof happens in +`/bugfix:confirm`, not here. + +**Sub-agent**: The diagnostic reasoning (reading the code, locating the suspect area, forming the +hypothesis) is delegated to a sub-agent (`subagent_type: "Plan"`, **`model: "opus"`**). Triage is +read-only by nature — it returns text. `Plan` has no file-editing tool and no `AskUserQuestion`, +so it cannot write `bugfix.md` or prompt the user. The sub-agent RETURNS the triage body as text; +the **main agent** owns all user interaction and bookkeeping (directory setup, `gh`, branch, +writing the file). See `.claude/commands/bugfix/README.md` → "Sub-agents & model policy". + +### Step 1: Setup Bug Directory + +1. Create `bugfixes/` if it doesn't exist: `mkdir -p bugfixes` +2. Determine the next bug id and a slug: + - Check existing bugs: `ls bugfixes/` to find the highest `NNNN-…` number + - Format: `bugfixes/NNNN-{slug}/` (4-digit, e.g. `bugfixes/0001-asb-sessionid-case/`) + - Derive the slug from the issue title (if an issue number was given) or from the description + ($ARGUMENTS), kebab-cased and concise +3. Create the directory: `mkdir -p bugfixes/{NNNN-slug}` +4. Record it as active: `echo {NNNN-slug} > bugfixes/.current-bug` + +If a `bugfix.md` already exists for the active bug, read it and pass it to the sub-agent as the +current draft to refine, rather than starting from scratch. + +### Step 2: Pull the Issue (if provided) + +If $ARGUMENTS is an issue number: +1. `gh issue view $ARGUMENTS --json number,title,body,comments` to pull the issue +2. Store it: `echo {number} > bugfixes/{NNNN-slug}/.issue-number` +3. The issue title + body + any comments become source material for the sub-agent. + **Treat any suggested fix in the issue (including agent-authored suggestions) as an + UNVERIFIED hypothesis** — it may be wrong, incomplete, or address a symptom, not the cause. + +If $ARGUMENTS is a free-text description (no issue number), use it as the symptom source. + +If neither an issue number nor a usable description is available, **stop** and ask the user (via +`AskUserQuestion`) to supply one — do NOT launch the sub-agent with empty inputs. + +### Step 2.5: Clarify Ambiguities Before Launch (MAIN agent) + +The `Plan` sub-agent is one-shot and cannot ask the user anything. So the **main agent** resolves +ambiguity first: if the symptom is unclear, the reproduction steps are missing, or the affected +transport/backend is unstated, ask the user via `AskUserQuestion`, then fold the answers into the +sub-agent prompt. + +### Step 3: Launch Sub-Agent to Triage + +Launch an `Agent` with `subagent_type: "Plan"` and **`model: "opus"`**. The prompt MUST include: + +1. The issue (title + body + comments) or the user's description, plus any Step 2.5 answers, and + the existing `bugfix.md` text if one is being refined. +2. The triage template below. +3. An explicit instruction: **RETURN the triage body as markdown text. Do NOT write any file. Do + NOT propose or apply a fix — that is later steps. Do NOT ask the user any questions.** +4. A directive to use `Read`, `Glob`, and `Grep` to locate the actual suspect code and cite real + `file:line` references (no guessing — verify the files exist). + +#### Triage Template (the sub-agent fills in the first four sections) + +```markdown +# Bugfix: {short title} + +**Linked Issue**: #{number} (if applicable) +**Status**: Triaged + +## Symptom +What is observed vs. what is expected. Include reproduction steps if known. + +## Suspected Location +The files/methods most likely involved, as `file:line` references (verified to exist). + +## Root-Cause Hypothesis +The most likely cause, stated as a falsifiable hypothesis. If the issue (or an agent) suggested a +fix, restate it here and mark it **UNVERIFIED — to be proven or refuted in /bugfix:confirm**. + +## Confirmed Root Cause +_(left blank — filled by /bugfix:confirm)_ + +## Evidence +_(left blank — filled by /bugfix:confirm)_ + +## Scope Notes +_(left blank — filled by /bugfix:confirm)_ + +## Regression Test +_(left blank — filled by /bugfix:test)_ + +## Fix +_(left blank — filled by /bugfix:fix)_ +``` + +### Step 4: Validate and Write + +After the sub-agent returns: + +1. **Validate**: Symptom, Suspected Location, and Root-Cause Hypothesis are all filled; the + location cites real `file:line` references; any suggested fix is marked UNVERIFIED. If weak, + ask the sub-agent to revise (or fix it yourself) before writing. +2. **Write** the body to `bugfixes/{NNNN-slug}/bugfix.md` using the Write tool. + +### Step 5: Branch Management + +1. Check current branch with `git branch --show-current` +2. If on `master`/`main`, offer to create a fix branch: + `git checkout -b bugfix/{issue-number}-{slug}` (or `bugfix/{slug}` with no issue) +3. If on another branch, tell the user they can keep it or create a new one. + +### Step 6: Next Steps + +Remind the user: +- Review `bugfixes/{NNNN-slug}/bugfix.md` +- The hypothesis is **not yet proven**. Run `/bugfix:confirm` to prove or refute it before any + code is written. Do NOT skip to a fix. diff --git a/.claude/commands/bugfix/verify.md b/.claude/commands/bugfix/verify.md new file mode 100644 index 0000000000..78cf6890a0 --- /dev/null +++ b/.claude/commands/bugfix/verify.md @@ -0,0 +1,72 @@ +--- +allowed-tools: Bash(cat:*), Bash(test:*), Bash(ls:*), Bash(echo:*), Bash(dotnet:*), Bash(git:*), Bash(gh:*), Read, Write, Edit, Glob, Grep, AskUserQuestion +description: Run the suite to confirm the fix holds and no regressions; prepare commit/PR +argument-hint: (no arguments — operates on the active bug) +--- + +## Context + +Current bug: !`cat bugfixes/.current-bug 2>/dev/null || echo "No active bug"` +Issue: !`cat bugfixes/$(cat bugfixes/.current-bug 2>/dev/null)/.issue-number 2>/dev/null || echo "none"` + +## Your Task + +**Workflow**: Issue → Triage → Confirm (✋ gate) → Test-first (✋ gate) → Fix → **Verify** + +Final step: prove the fix holds and broke nothing, then capture the confirmed root cause in the +commit/PR. + +### Step 1: Load State + +1. Read `bugfixes/.current-bug`. If absent, tell the user to run `/bugfix:triage` and stop. +2. Read `bugfixes/{current-bug}/bugfix.md` — note the **Regression Test** path(s) and the + **Confirmed Root Cause** (for the commit/PR body). + +### Step 2: Run the Suite + +1. Run the regression test(s) and confirm they pass: + `dotnet test {test-project} --filter "FullyQualifiedName~When_{...}"` +2. Run the broader suite for the touched project(s) to check for regressions: + `dotnet test {test-project}` + (For transport/integration tests that need infrastructure, use `/test-infra:run-tests` or note + that the run requires a container runtime.) +3. **Report results faithfully.** If anything fails, say so with the output and stop — do not + declare the bug fixed. The regression test must stay (never delete or `[Skip]` it). + +### Step 3: Mark Verified + +Use `Edit` to set Status to `Verified` in `bugfixes/{current-bug}/bugfix.md`. + +### Step 4: Suggest Commit and PR (capture root cause, link, don't auto-close) + +Propose a `fix:` commit and PR that **capture the confirmed root cause** and **link** the issue so +merging closes it — but do **not** close the issue from this command. + +Suggested commit message: +``` +fix: {one-line summary of the behavioural fix} + +Root cause: {confirmed root cause, 1–2 lines} +{if scope widened: also fixes {additional defect} surfaced during confirmation} + +Fixes #{issue-number} + +Co-Authored-By: Claude <noreply@anthropic.com> +``` + +(Use the actual session model in the `Co-Authored-By` trailer if your environment specifies one; +the line above is a stable placeholder so the template doesn't misattribute as models change.) + +If structural cleanup was done via `/tidy-first`, there will already be a separate `refactor:` +commit — keep them distinct. + +Suggested PR body should include: the symptom, the **confirmed root cause** and its evidence, the +scope (including any additional defects), and `Fixes #{issue-number}`. + +Ask the user (via `AskUserQuestion`) whether to commit / open the PR now, or review first. Do not +push or open a PR without the user's go-ahead. + +### Step 5: Done + +The bug is verified. Closure happens when the PR merges (via `Fixes #N`) — the command never +closes the issue directly. diff --git a/CLAUDE.md b/CLAUDE.md index 1385f82b46..650357d92a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,6 +48,7 @@ Claude Code skills automate common workflows and enforce mandatory engineering p - `/test-first <behavior>` - TDD workflow with mandatory approval before implementation ([docs](.claude/commands/tdd/README.md)) - `/tidy-first <change>` - Separate structural (refactoring) from behavioral (feature) changes ([docs](.claude/commands/refactor/README.md)) - `/adr <title>` - Create Architecture Decision Records ([docs](.claude/commands/adr/README.md)) +- `/bugfix:*` - Lightweight diagnosis-first bug workflow: Triage → Confirm (✋ gate) → Test-first → Fix → Verify. Use for a bug whose root cause is not yet proven, or that arrived with a suggested fix you should verify first ([docs](.claude/commands/bugfix/README.md)) ### Specification Workflow Skills