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.

CI mode (GitHub Actions)

issue-fix.yml runs this skill on claude-code-action instead of duplicating its instructions. The numbered Process below applies verbatim except where an interactive-session mechanism has a pipeline equivalent, per this table. User-level plugins (superpowers:*, code-review) are not installed on CI runners — only repo-level .claude/skills/ and .claude/agents/ exist there.

Step	CI mode
2. Diagnose	Stage 2 comment absent → STOP. Post "No deep analysis found. Run @claude-bot analyze first" and exit — never self-diagnose in CI; the analyze/fix split is the human gate.
3. Confirm gate	The owner's @claude-bot fix comment is the go-ahead. Still perform the workaround check and scope assessment — put them in a ## Scope assessment section of the PR body instead of chat. Escalation path (can't confidently pass the workaround check) still applies: dispatch a fresh general-purpose Agent to critique the design before implementing.
4. Worktree	Skip — the CI checkout is already isolated. Create the branch directly (naming per Step 1).
5. TDD	The substance applies verbatim (RED test first, required test shape); there is just no superpowers:test-driven-development skill to invoke — follow this section's own rules.
6. Quality gate + code review	Run inline, no background agent (CI is one throwaway session — the cost-discipline reason to background doesn't exist). The code-review plugin is unavailable; the Stage-4 @claude-bot PR review covers it. Checks 1–3 (fast suite, slow suite, required-test-shape) still apply.
7. Confirm gate 2	Replaced by the draft PR itself — the owner reviews the draft before anything merges.
8. Local run & observe	Structurally unavailable in CI — this is the documented reason the local flow exists. Skip, and say so in the PR body's test plan so the reviewer knows verification is still owed.
9. Commit + draft PR	Applies verbatim, including the CHANGELOG.md ## [Unreleased] entry and the documentation check. Add the ## Scope assessment section (Step 3 above). The workflow file owns CI-only mechanics: issue comment with the PR link, has-fix-pr label.
10. Hard constraints	Apply verbatim.

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, proposed fix, AND its scope assessment per docs/agents/rules.md's Debugging Protocol step 8. That includes the workaround check: state explicitly that the diff adds nothing — no parameter, flag, default-fallback, second construction site, extra trigger or branch — whose only job is to route around an ordering/timing/dependency problem instead of fixing it. If you can't state that confidently, dispatch a fresh Plan/general-purpose agent to critique the design before presenting anything. Then the scope category: does the fix stay within the target method's existing contract (local), does it need a different/new owner (structural), or does it have multiple plausible owners worth a second opinion? State which, explicitly — don't let the user infer it from the diff description. A structural assessment with no stated reason for the chosen owner is not ready to present. Wait for explicit go-ahead before touching code. One message — cheap insurance against building an entire implementation on a wrong diagnosis or a wrong placement.

4. Worktree + branch

git fetch origin main first — using-git-worktrees' git fallback branches from the current local HEAD, not origin/main, so a stale local checkout silently cuts the branch behind main (missed release cuts, changelog rewrites, other merged fixes), surfacing later as an avoidable merge conflict. Then invoke superpowers:using-git-worktrees, basing the new branch on origin/main.

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.

If the fix changes optimizer economics or behavior, the fixture from from_debug_log.py --issue <N> also needs its expected_results/ expected_behavior set from the fixed code's output (docs/agents/testing.md → Test Data) — this wires it into test_scenarios.py::test_all_scenarios, the codebase's existing auto-discovered, always-run regression harness for every *.json fixture in core/bess/tests/unit/data/. Do this instead of writing a standalone test file that re-derives _scenario_inputs and hand-asserts cost numbers — that duplicates a mechanism the codebase already has and runs on every test invocation. Verify the pin actually discriminates (temporarily feed it the pre-fix/buggy input, confirm it fails) before trusting it. Reserve a standalone test file for what that harness genuinely can't express: a private method's internal formula, or plan-faithfulness (R == P) — test_all_scenarios never runs the inverter simulator.

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 — one line, per docs/agents/workflow.md's CHANGELOG Format (bold lead-in, issue/PR link, ~25-word cap; no root cause or file/function names — that's the PR description's job, not the changelog's). Match existing entries' format only, never their length — several past entries are multi-sentence root-cause essays; do not use those as a length precedent. 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