diff --git a/skills/software-development/development-workflow/SKILL.md b/skills/software-development/development-workflow/SKILL.md new file mode 100644 index 0000000..ac959d8 --- /dev/null +++ b/skills/software-development/development-workflow/SKILL.md @@ -0,0 +1,264 @@ +--- +name: development-workflow +description: "Development workflow methodology: planning, spiking, implementation plans, TDD, code editing patterns, pre-commit review, and subagent-driven execution." +version: 2.0.0 +author: Hermes Agent +license: MIT +platforms: [linux, macos, windows] +metadata: + hermes: + tags: [planning, TDD, code-review, implementation, workflow, development, spike, subagent] + related_skills: [debugging, github] +--- + +# Development Workflow + +End-to-end development methodology: from idea to verified commit. Seven interlocking practices. + +## Section 1: Plan Mode + +When the user wants a plan instead of execution: +- Do NOT implement code, edit project files, or run mutating commands +- Deliverable: a markdown plan saved under `.hermes/plans/YYYY-MM-DD_HHMMSS-.md` +- Include: goal, context, approach, step-by-step plan, files to change, tests/validation, risks + +See `references/plan-mode.md` for full details. + +## Section 2: Spikes (Throwaway Experiments) + +Validate feasibility before committing to a build. Spikes are disposable. + +### Core Loop +``` +decompose → research → build → verdict +``` + +1. **Decompose** — Break idea into 2-5 independent feasibility questions (Given/When/Then) +2. **Research** — Brief each spike, surface approaches, pick one +3. **Build** — One directory per spike (`spikes/NNN-name/`), bias toward runnable output +4. **Verdict** — VALIDATED / PARTIAL / INVALIDATED with evidence + +Key rules: +- Order by risk (kill the idea fast if the hard part doesn't work) +- Depth over speed — never declare "it works" after one happy-path run +- Hardcode everything — it's a spike, not production + +See `references/spike.md` for full methodology including comparison spikes and frontier mode. + +## Section 3: Writing Implementation Plans + +Write plans assuming the implementer has zero codebase context. Bite-sized tasks. DRY. YAGNI. TDD. + +### Task Granularity +Each task = 2-5 minutes of focused work. One action per step. + +### Plan Document Structure +```markdown +# [Feature Name] Implementation Plan + +> **For Hermes:** Use subagent-driven-development skill to execute this plan. + +**Goal:** [One sentence] +**Architecture:** [2-3 sentences] +**Tech Stack:** [Key technologies] + +### Task N: [Descriptive Name] +**Objective:** [One sentence] +**Files:** Create/Modify/Test paths +**Step 1:** Write failing test [code] +**Step 2:** Run test, verify FAIL +**Step 3:** Write minimal implementation [code] +**Step 4:** Run test, verify PASS +**Step 5:** Commit +``` + +Key principles: +- Exact file paths (not "the config file" but `src/config/settings.py`) +- Complete code (copy-pasteable, not "add validation") +- Exact commands with expected output +- Verification steps that prove the task works + +See `references/writing-plans.md` for full plan-writing process and common mistakes. + +## Section 4: Test-Driven Development (TDD) + +### Iron Law +``` +NO PRODUCTION CODE WITHOUT A FAILING TEST FIRST +``` + +### Red-Green-Refactor Cycle +1. **RED** — Write one minimal failing test. Run it. Watch it fail. +2. **GREEN** — Write simplest code to pass. Cheating is OK (hardcode, copy-paste). +3. **REFACTOR** — Remove duplication, improve names. Keep tests green. + +Key rules: +- One behavior per test; name describes behavior not implementation +- Real code, not mocks (unless truly unavoidable) +- If test passes immediately, you're testing existing behavior — fix the test +- If 3+ TDD cycles fail to make progress, question the design + +See `references/test-driven-development.md` for the full TDD methodology, rationalizations table, and anti-patterns. + +## Section 5: Code Editing Patterns + +### Critical Pitfalls with Hermes patch Tool + +1. **Redaction of sensitive values** — `API_KEY = some_var` may get mangled to `API_KEY=***`. Use `data['key']` patterns or `write_file` instead. +2. **Duplicate line insertion** — patch sometimes inserts `new_string` twice. Always read back to verify. +3. **Non-unique old_string** — Always include 2-3 lines of context to make matches unique. +4. **Stale file views** — Re-read the region before patching if the file was modified earlier. +5. **NEVER use sed/regex bulk replace on Python code** — regex can't distinguish variable references from comments/strings/literals. Use line-range surgical replacements or Python `ast` module. + +### Python Unicode Docstring Pitfall + +Python 3.11+ **rejects** certain Unicode characters inside docstrings as a `SyntaxError`, even though they look like valid text. Common offenders: + +| Character | Name | Unicode | Error | +|-----------|------|---------|-------| +| `—` | Em dash | U+2014 | `SyntaxError: invalid character '—' (U+2014)` | +| `→` | Right arrow | U+2192 | `SyntaxError: invalid character '→' (U+2192)` | +| `–` | En dash | U+2013 | Same class of error | + +**Fix**: Replace with ASCII equivalents — `--` for em-dash, `->` for arrow, `-` for en-dash. +**Prevention**: Run `python3 -m py_compile ` after any docstring edit (even if the file previously compiled — these chars can be pre-existing and only surface when `py_compile` runs explicitly). +**Detection**: `grep -Pn '[\\x{2013}\\x{2014}\\x{2192}]' *.py` to find all instances before they cause failures. + +### Safe Patterns +- Batch edits top-to-bottom (line numbers shift after each patch) +- Verify after every non-trivial patch (read back + syntax check) +- For large refactors: read entire file → make surgical replacements → write_file → py_compile + +See `references/code-editing.md` for full patterns and workarounds. + +## Section 6: Pre-Commit Code Review + +Automated verification pipeline before code lands. No agent should verify its own work. + +### Pipeline +1. **Get the diff** — `git diff --cached` (or `git diff HEAD~1 HEAD`) +2. **Static security scan** — grep for hardcoded secrets, shell injection, eval/exec, pickle, SQL injection +3. **Baseline tests + linting** — capture failure count before changes (stash → run → pop) +4. **Self-review pass** — NOT a checkbox scan. Systematic cross-file review: + automated consistency scans, security gap hunting, dead code detection. + See `references/self-review-pass.md` for the full methodology. + Key rule: find issues BEFORE the user finds them — a shallow pass that + misses bugs and requires the user to ask "你自己再审核一遍" is a failure. +5. **Independent reviewer subagent** — dispatch via `delegate_task` with diff only +6. **Evaluate** — all passed → commit; any failure → auto-fix loop (max 2 cycles) +7. **Commit** with `[verified]` prefix + +See `references/requesting-code-review.md` for the full pipeline including auto-fix patterns. +See `references/ssrf-python-fastapi.md` for the reusable SSRF URL validation pattern (three-layer defense: scheme + internal IP block + host whitelist). +See `references/fastapi-route-ordering.md` for the FastAPI route registration order pitfall — literal routes must be placed before parameterized catch-all routes. + +### Cross-Repo Batch Review with Claude Code + +For auditing multiple repos at once (security sweep, post-release review), +use `claude -p` in non-interactive mode with parallel background processes. +See `references/claude-code-batch-review.md` for the full recipe: +`--max-turns 12`, `--dangerously-skip-permissions`, focused file lists, +and parallel `terminal(background=true)` execution. + +### Multi-Round Code Review Pipeline + +For reviewing implementation code across multiple rounds with dual AI reviewers +(Claude Code + GLM-5.1), following the pattern: implement → review → fix → re-review → final score. +See `references/multi-round-code-review.md` for the full pipeline, reviewer selection +matrix, and Claude Code command template. + +### Three-Round Review Pattern (R1→R2→R3→R4 optional) + +When implementing new features (server + desktop together), use this proven pattern: + +``` +R1: GLM-5.1 (via delegate_task) → find bugs + security issues + → fix all 🔴, defer 🟡🟢 +R2: Claude (via terminal background claude -p) → verify R1 findings, check for missed issues + → fix all 🔴, apply quality improvements +R3: Build verification → run typecheck + build, fix any JSX/TS regression +R4: (optional) Claude Code parallel sweep — two repos, two bg agents + → final verification, catch R1/R2 regression-introduced bugs +``` + +**Critical R3 step — Desktop Build Verification:** +After R2 fixes are merged, the delegate_task subagents may introduce TS/JSX regressions. +Always run typecheck before build: +```bash +npx tsc --noEmit -p tsconfig.web.json --composite false +``` +If errors: `git stash` → verify original tag has same errors (pre-existing) vs new regression +→ fix both pre-existing and new errors before build. Common pre-existing bugs in +tag-checkout code: `<>` fragments missing ``, `"json" in {}` needing `typeof` guard, +unused imports (TS6133). **tsconfig.web.json errors DO block the build pipeline** +(unlike tsconfig.node.json config-level warnings). + +**When to add R4:** After feature is "done" and all prior rounds fixed, run +Claude Code (`terminal(background=true, notify_on_complete=true)`) in parallel +for both server + desktop repos. This catches subtle regressions that R1 fixes +sometimes introduce (wrong import sources, stale closures in React hooks, +defensive-but-broken error handling). + +**R4 invocation template:** +```bash +# Launch two parallel Claude Code reviews: +# (1) Server repo review +terminal(command="cd ~/repo && claude -p '' \\ + --output-format text --max-turns 12 --dangerously-skip-permissions 2>&1", + background=true, notify_on_complete=true, timeout=600) +# (2) Desktop repo review (launch simultaneously) +terminal(command="cd ~/desktop && claude -p '' \\ + --output-format text --max-turns 10 --dangerously-skip-permissions 2>&1", + background=true, notify_on_complete=true, timeout=600) +``` + +**Critical lesson from R1→R2:** R1 fixes can introduce new 🔴 bugs: +- Adding validation code but **forgetting required imports** (e.g., `HTTPException`/`status` not imported after adding error handling) +- Moving imports between sources but landing them in the **wrong module** (e.g., `ChevronDown` from `"react"` instead of `"lucide-react"`) +- Adding defensive `finally { if (state === X) setState(Y) }` but reading a **stale closure** — the fix is `finally { setState(null) }` without reading old state +- → R2 must explicitly re-check every R1-modified line, not just the original code + +**R3 checklist (manual):** +```bash +# Compile check +python3 -m py_compile +# Import verification (for Python) +PYTHONPATH=backend python3 -c "from app.api.v2.sourcing import _validate_url; print('OK')" +# Duplicate detection +grep -c "KEY_DEFINITION" # must be 1 +# Import hygiene +grep "from.*import" | sort | uniq -c | grep -v "^ *1 " +# Cross-file consistency: no feature in wrong file +grep -c "a2aInbox" Mail.tsx # should be >0 +grep -c "a2aInbox" Tools.tsx # should be 0 +# SSRF: validate URL guard regex (see references/ssrf-python-fastapi.md) +python3 -c " +import re +p = re.compile(r'^(https?://)?(127\.|...)', re.IGNORECASE) +assert p.match('http://127.0.0.1:8080') +assert not p.match('http://detail.1688.com/offer/123.html') +print('SSRF regex OK') +" +``` + +## Section 7: Subagent-Driven Development + +Execute plans by dispatching fresh subagents per task with two-stage review. + +### Process +1. Read plan once, extract all tasks, create todo list +2. Per task: + - **Dispatch implementer** — full context in `delegate_task`, never make subagent read the plan file + - **Spec compliance review** — does implementation match original spec? + - **Code quality review** — style, error handling, coverage, security + - Fix issues → re-review → mark complete +3. Final integration review across all tasks +4. Full test suite + commit + +### Red Flags +- Spec compliance MUST pass before code quality review (wrong order otherwise) +- Never dispatch multiple subagents for tasks that touch the same files +- Never let implementer self-review replace actual review +- Fresh subagent per task prevents context pollution + +See `references/subagent-driven-development.md` for the full process, including context budget discipline and gates taxonomy.