implement-issue

name	implement-issue
description	Use when asked to implement, fix, or resolve a bess-manager GitHub issue end-to-end from the command line, especially when local verification (not just CI) is wanted before the PR opens.

Implement Issue

Overview

Drive a bess-manager GitHub issue from diagnosis to a locally-verified draft PR against main. This is the CLI counterpart to the @claude-bot analyze + @claude-bot fix pipeline (docs/agents/workflow.md) — same diagnose-then-fix shape, but with the one thing the bot pipeline structurally cannot do: run the app locally and observe the fix working before the PR opens. That local verification step is the entire reason to run this from the command line instead of the bot, so it is never optional.

This skill orchestrates other skills — it does not re-implement them: superpowers:using-git-worktrees, superpowers:test-driven-development, superpowers:finishing-a-development-branch, code-review, verify, and the bess-analyst sub-agent.

When to Use

• User gives you a bess-manager issue number/URL and asks you to implement, fix, or resolve it locally.
• Not for the feature-lifecycle multi-release integration flow (new inverter/price-provider platforms) — that skill owns experimental→stable graduation across multiple beta cycles. Use implement-issue for single-PR bug fixes and small enhancements.

Process

1. Fetch & scope

gh issue view <n> --json title,body,labels,comments

Read chronologically for the CURRENT problem — issues evolve, don't fix a stale complaint. Branch prefix from label: bug → fix/, enhancement → feat/. Branch name: <prefix>/issue-<n>-<slug>.

2. Diagnose (conditional)

Check the issue comments for an existing Stage 2 diagnosis: a bot comment with ## Root cause / ## Evidence / ## Proposed fix sections (label analyzed). This is the common case — issues are usually run through @claude-bot analyze first.

• Comment present: use it as the diagnosis. Independently verify by reading the cited file:line locations against current code — quote real code, don't just trust the summary. Do NOT re-run bess-analyst from scratch.
• Comment absent: dispatch bess-analyst as a sub-agent (Agent tool, subagent_type: bess-analyst) for a full independent diagnosis — pass it the issue title, body, and comment history, and the task: "diagnose independently; the reporter's explanation is a hypothesis, not a conclusion."

3. Confirm gate

Present the root cause and proposed fix to the user. Wait for explicit go-ahead before touching code. One message — cheap insurance against building an entire implementation on a wrong diagnosis.

4. Worktree + branch

Invoke superpowers:using-git-worktrees.

5. TDD implementation

Invoke superpowers:test-driven-development. Write a test that reproduces the bug (from the diagnosis's evidence — the specific period/scenario/input) and watch it fail, then write the minimal fix. No refactors outside the bug — match docs/agents/patterns.md.

Required test shape — checked against the diff, not optional:

If the fix touches the DP (dp_battery_algorithm.py), intent classification (decision_intelligence.py), or control/rate mapping (inverter_controller.py / battery_system_manager.py), the PRIMARY RED test — not an extra test alongside it, the one that proves the bug — is a plan-faithfulness scenario, not a unit test calling the changed function with hand-built arguments. Write it as:

from core.bess.tests.helpers import run_scenario_realized
# scenario is a full DP-optimized schedule, not a hand-built period/decision
result, realized_cost = run_scenario_realized(scenario)
assert realized_cost == pytest.approx(result.total_cost, ...)  # R == P

A unit test on the changed function directly (e.g. calling _apply_period_schedule or intra_period_discharge_gate with stubbed arguments) can pass while the new branch is unreachable by any real DP-produced schedule — that is exactly the coverage gap that shipped undetected in PR #385 (docs/agents/simulator.md). Add such a unit test only as a supplement, never as the sole RED test, for this category of fix.

If the diagnosis's evidence is a user-supplied debug log/bundle, build the scenario from that real data instead of a hand-assembled fixture:

python scripts/mock_ha/scenarios/from_debug_log.py <bundle.md>

(docs/agents/testing.md → Bug Reproduction with Mock HA). Do this before writing the RED test — the bundle already contains the exact conditions that reproduced the bug.

6. Quality gate + code review (background)

Every PR must pass both the fast and slow suites, plus code review. This is the long-wait step (slow suite is ~30min) — per CLAUDE.md's Cost Discipline, do NOT hold the session open watching it run. Dispatch a background Agent (no isolation — it must operate in the Step 4 worktree, not spawn a new one; run_in_background: true, the default) with a self-contained prompt covering:

1. ./scripts/quality-check.sh (fast suite) — if it fails, fix and re-run, do not proceed with failures.
2. .venv/bin/pytest -m slow (slow suite) — same failure handling.
3. If the diff touches the DP, intent classification, or control/rate mapping (the Step 5 table): confirm the diff's new/changed tests include a run_scenario_realized / verify_plan_faithfulness call, not only a unit test on the changed function with hand-built arguments. If missing, this is a required-before-continuing gap, not a nice-to-have — report it as a blocking finding alongside the suite results, same severity as a failing test.
4. Invoke the code-review skill on the diff.
5. Report back: pass/fail on both suites, whether check 3 passed, and any CONFIRMED code-review findings verbatim (everything else goes to TODO.md).

Do not poll — you'll be notified on completion. This is a hard session boundary: don't keep re-touching the diagnosis/TDD context while it runs.

7. Confirm gate 2 (manual — mandatory)

Present the background agent's report to the user: suite results and any CONFIRMED findings. Fix CONFIRMED findings in the same worktree before continuing. Wait for explicit go-ahead before Step 8 — same reasoning as Step 3, cheap insurance against shipping a finding-blocked or slow-suite- broken change.

8. Local run & observe (never skip this)

Invoke the verify skill: actually exercise the fix and capture real output — the reproducing mock-HA scenario via docker compose -f docker-compose.ci.yml, a dev-server flow for frontend changes, or the relevant CLI/pytest path with output inspected, not just its exit code. A green test suite is necessary, not sufficient — this step is what makes this skill worth running instead of the bot pipeline, and it is not satisfied by re-stating that quality-check.sh passed.

9. Commit + draft PR

Add a CHANGELOG.md entry under ## [Unreleased] (create that heading at the top if it's not already there), in the matching ### Added / ### Changed / ### Fixed subsection, following the existing entries' style (bold summary line, link to the issue/PR). This is a normal part of the PR, not a release step — per docs/superpowers/specs/2026-07-09-release-workflow-design.md, Unreleased entries accumulate as each PR merges; the release skill only ever renames or copies that section, it never authors it. Skipping this here means the release skill has to backfill it later from a colder context.

Documentation check (mandatory, not optional): if the fix changed a mechanism, formula, threshold, or code path that docs/agents/bess-knowledge.md or docs/SOFTWARE_DESIGN.md describes, update the affected section in this same PR. These two files are read as ground truth by the AI chat, the GitHub analysis agent, and future implementers — a removed/changed mechanism left undocumented silently rots into a wrong answer later (found in practice: a "Bellman-optimality guardrail removal" refactor left both docs describing a profit-threshold gate and a reward floor that no longer existed, and a FIFO cost-basis claim that was never true). Concretely: grep both files for any function/field/formula your diff touches before opening the PR, not after. If neither file mentions anything your change touches, say so explicitly in the PR description rather than silently skipping — a reviewer shouldn't have to guess whether it was checked.

Commit per docs/agents/workflow.md format (subject + blank line + body explaining WHY). Open a draft PR against main via superpowers:finishing-a-development-branch (Option 2: push + PR), body:

## Summary
- <bullet>

## Root cause
<quote from the Step 2 diagnosis>

## Fix
<what changed and why>

## Test plan
- [ ] `./scripts/quality-check.sh` passes locally (already done)
- [ ] <what you actually observed in Step 8 — be concrete>

Closes #<n>

10. Hard constraints

• Draft PR only. Never auto-merge.
• Never push directly to main.
• Do NOT modify the version in bess_manager/config.yaml — bumping it is a release-time step, not a per-PR one. DO add a CHANGELOG.md entry under ## [Unreleased] per Step 9 — this is the one CHANGELOG.md edit expected in every PR.
• If quality-check.sh keeps failing after 3 fix attempts, or Step 8 can't demonstrate the fix actually works, stop, push the branch as-is, and report what failed — don't force a PR through.
• If this work went through superpowers:writing-plans (a docs/superpowers/plans/ file exists for it), delete that plan file before the Step 9 commit. Keep the spec (if any); the plan is execution scaffolding that only drifts once the code is the source of truth. Never commit a plan doc into the PR.

After Merge

A separate, later invocation — often a different session, sometimes days later once CI is green and the user has reviewed. Not part of the numbered flow above, which stops at draft-PR-open per the Step 10 constraints.

1. Confirm the merge:

   gh pr view <n> --json state,mergedAt,mergeCommit
   

   state == "MERGED" is authoritative — that's the standard signal, no need to separately diff branch content against main. Squash merges break git branch -d's normal ancestry check (the branch's commits never become reachable from main), so force-delete below is expected, not a sign something's wrong.

2. Remove the worktree — via ExitWorktree action=remove discard_changes=true if the session is still in it, or git worktree remove <path> from the main repo root for a .worktrees/-created one.

3. Force-delete the local branch and prune stale remote refs:

   git branch -D <branch-name>
   git fetch origin --prune
   

   GitHub auto-deletes the remote branch on merge by default; --prune just clears the now-stale local tracking ref.

Rationalizations — Reality

Excuse	Reality
"quality-check.sh passed, that's enough"	Green tests prove the suite is satisfied, not that the fix behaves correctly against the real scenario. Step 8 requires observed output, every time.
"the diagnosis is obviously right, skip the confirm gate"	Wrong diagnoses are exactly when confidence is highest. One message, cheap insurance.
"I'll clean up this other thing while I'm in here"	Out of scope. Minimal fix only.
"code review can wait until after I've verified it works"	Reordered on purpose — catch cheap issues before spending time on manual verification, not after.
"there's already a bot diagnosis, let me re-derive it anyway to be safe"	Re-verify the cited evidence; don't redo the whole investigation.
"the plan doc is useful context, keep it in the PR"	Once code and tests exist, the plan only drifts — it's not the source of truth. Delete it before Step 9; keep the spec if one exists.
"the user is in a hurry, just open the PR"	Time pressure from the user is not permission to skip Step 8 — it's the reason to say so explicitly and give a real ETA instead.
"I'll just watch the background agent run"	Defeats the point — the whole reason it's backgrounded is so the session isn't held open through the slow suite. Let the notification bring you back.
"the fix is small, docs don't need touching"	Small fixes are exactly what silently invalidates a one-line doc claim (a removed threshold, a renamed formula). Grep the two design docs before opening the PR, every time.
"a unit test on the changed function is enough"	Not for DP/intent/control-mapping changes — a synthetic-input unit test can pass while the new branch is unreachable by any real optimizer-derived scenario. docs/agents/simulator.md requires R == P for exactly this class of change.
"the existing suite still passes, so nothing broke"	Passing unchanged means the new code path may simply be untested, not unbroken — check whether any existing fixture actually reaches the new branch before treating a green suite as coverage.

Red Flags — Stop and Go Back

• About to commit or open the PR without having actually run/observed the fix — only ran automated tests.
• About to skip the Step 3 or Step 7 confirm gate because of time pressure.
• About to open the PR before /code-review CONFIRMED findings are resolved.
• About to re-run the full bess-analyst diagnosis when a verified bot comment already exists.
• About to run the slow suite inline in the main session instead of dispatching the Step 6 background agent.
• About to open the PR without checking whether the fix invalidates a claim in docs/agents/bess-knowledge.md or docs/SOFTWARE_DESIGN.md.
• About to write only a synthetic-input unit test for a DP/intent/control- mapping change instead of a plan-faithfulness (R == P) scenario test.
• About to write a repro test from hand-built data when a user debug log/ bundle is available and from_debug_log.py could build it from real data.

Quick Reference

Step	Skill/Tool	Skippable?
1. Fetch & scope	gh issue view	No
2. Diagnose	bess-analyst (if no bot comment)	Conditional
3. Confirm gate	—	No
4. Worktree	using-git-worktrees	No
5. TDD	test-driven-development	No
6. Quality gate + code review	quality-check.sh + slow suite + code-review (background agent)	No
7. Confirm gate 2	—	No
8. Local run & observe	verify	Never
9. Commit + PR	finishing-a-development-branch	No