From 747379077ca2175ac78cdee197e2e8b138f67c49 Mon Sep 17 00:00:00 2001 From: hsliu_ustc Date: Thu, 15 Oct 2026 16:51:42 +0800 Subject: [PATCH] Add the fix-issue skill: one issue end to end The #71 loop (facts, worktree, red test, gates, PR, independent review, merge on green) worked but lived only in this session. Write it down as a skill beside review-pr, including the environment gotchas a fresh worktree hits (harness node_modules, HTTPS push timeouts, screenshot attachments, patches topics), and point at it from the iteration loop. --- .claude/skills/fix-issue/SKILL.md | 64 +++++++++++++++++++++++++++++++ AGENTS.md | 1 + 2 files changed, 65 insertions(+) create mode 100644 .claude/skills/fix-issue/SKILL.md diff --git a/.claude/skills/fix-issue/SKILL.md b/.claude/skills/fix-issue/SKILL.md new file mode 100644 index 00000000..76e3b5a2 --- /dev/null +++ b/.claude/skills/fix-issue/SKILL.md @@ -0,0 +1,64 @@ +--- +name: fix-issue +description: Fix one FlyBot issue end to end — establish the facts, worktree, fail-without-change test, gates, PR, independent review loop, merge on green. The Verifier stage is the review-pr skill. +--- + +# FlyBot fix-issue + +One issue per branch, one worktree per issue. The loop: establish the facts → test red → fix → gates → PR → independent review → merge on green. Magpie-*style* habits for FlyBot; do not copy another product's docs. This skill grants **no merge authority by itself**: the maintainer merges, or explicitly delegates for one instance. + +## 0. Establish the work + +1. `git fetch origin`, then read the **current** rules from `origin/main`: [AGENTS.md](../../../AGENTS.md), [docs/code-standards.md](../../../docs/code-standards.md), [LESSONS.md](../../../LESSONS.md), [CONTRIBUTING.md](../../../CONTRIBUTING.md) — not stale local copies. +2. Turn the report into observable facts before touching code. For a screenshot: crop the regions and read the exact strings. If no issue exists yet, file it first (see *Filing* below) and reference the umbrella it belongs to (#9 fences/display names, #5 social). +3. Confirm scope: one issue, or one tightly coupled pair (code-standards, Tiny loops). Reproduce or locate the defect in code; cite `file:line` in the PR. + +## 1. Worktree and environment + +- `git worktree add /tmp/flybot- -b fix-- origin/main`. +- `npm ci` in `core/` (build tool only). Unit tests load `@deepseek-ai/*` from `DSH_HARNESS` or `core/dev/harness/node_modules`; a fresh worktree has neither, and the resolver's git fallback can land on a repo-root harness checkout that misses packages — `at-menu`/`time-context` failures mean this, not your diff. Symlink an installed `dev/harness/node_modules` or set `DSH_HARNESS`. +- `git push` over HTTPS may time out while `gh api` works; push via the SSH URL (`git push git@github.com:ThinkFlowLab/FlyBot.git `). + +## 2. Implement (fail without change) + +1. Write the regression test **first** and run it red on the unbroken tip. Keep both halves of the evidence — red at base, green at the tip — a reviewer re-verifies this mechanically. +2. Fix the class, not the sample (code-standards). Respect the trust boundaries (#9): peer-controlled strings never become trusted or model-visible text; display names come from local records, never from message text. +3. Keep the host untouched when the defect is display-only — a client-only change skips the smoke gate. +4. Tests are flat `node:test` blocks like the neighbors; assertion helpers are local to each test file. User-facing strings are English with a `ZH` entry in `core/src/client/i18n.js` (the i18n test scans for missing keys). + +## 3. Gates (every one, at the tip you will link) + +```sh +npm run build # core/client.js from src/client +npm test # unit tests; fails if client.js is stale +node scripts/build-client.mjs --check +npm run lint # oxlint, then arch-docs +node scripts/ds-bot-patches.mjs generate && node scripts/ds-bot-patches.mjs check +``` + +- An owned file ([docs/subsystems/](../../../docs/subsystems/README.md)) changed → update that subsystem doc in the same PR; `Docs-not-needed:` only when the doc truly says nothing about the change. +- A `core/` file changed for the first time must join a topic in `patches/ds-bot/topics.json` before `generate`. +- Compare lint warnings against base: new warnings are findings. +- `index.js`/`src/host/` changes → run the smoke test as well. + +## 4. PR + +- Subject: imperative; the reason in the body; `Fixes #N` on its own line. +- A Verification section per Verifier honesty: tip SHA, each gate with **real numbers**, Not run with reasons. +- A reviewer note for environment traps (e.g. the worktree harness symlink above). + +## 5. Independent review loop + +1. A **separate agent with fresh context** reviews the PR tip following [review-pr](../review-pr/SKILL.md): it re-runs the gates itself and re-verifies red-at-base mechanically (worktree at base + only the test files copied in). The implementer's claims are narrative, not evidence. +2. Findings → fix the class, add the test, push. **A tip advance voids the previous PASS** — the reviewer re-reviews the delta and re-runs the gates at the new tip. +3. A non-blocking note whose root cause lies outside the diff becomes a follow-up issue immediately (security ones into the #9 queue), not a comment lost in the PR. + +## 6. Merge and cleanup + +- Merge only when all hold **on the same tip**: review PASS (and implementer↔reviewer agreement, or a maintainer waiver of one side), required checks green, maintainer's go. Tip moved → start over from the review. +- Then: fetch and fast-forward main, delete the branch locally and remotely, remove the worktree, and carry follow-up issues (not TODOs). + +## Filing (before the fix) + +- `gh` cannot attach images to issues and gists reject binaries: push the screenshot to the `issue-assets` branch and embed the `raw.githubusercontent.com/ThinkFlowLab/FlyBot//` URL in the issue body. +- Issue shape (matches recent issues): Environment / Steps to reproduce / Actual / Expected / Where it sits (`file:line` pointers) / Suggested direction (for discussion, not prescriptive). diff --git a/AGENTS.md b/AGENTS.md index 183b4137..d1644f8d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,7 @@ FlyBot agent work uses a small, visible loop. Adapt the *habits* below; do not c 5. **LESSONS before code.** Skim LESSONS and code-standards before editing. When a class of bug repeats, date a lesson (`Seen N×`); after **≥3 days** promote the durable rule into `docs/code-standards.md` / CONTRIBUTING. 6. **Tiny loops.** Fail-without-change tests; fix the class; list what you ran and what you **not run** at the tip SHA. Merge only a reviewed tip (see code-standards). 7. **Review skill.** Use [`.claude/skills/review-pr/SKILL.md`](.claude/skills/review-pr/SKILL.md) for the Verifier report shape (PASS / FAIL / INC). +8. **Fix loop skill.** [`.claude/skills/fix-issue/SKILL.md`](.claude/skills/fix-issue/SKILL.md) walks one issue end to end: facts → worktree → red test → gates → PR → independent review (review-pr) → merge on green, with the repo’s environment gotchas (worktree harness, screenshots, patches topics). ## What this project is