Files

265 lines
12 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
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-<slug>.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 <file>` 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 '<review prompt>' \\
--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 '<review prompt>' \\
--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 <all changed files>
# Import verification (for Python)
PYTHONPATH=backend python3 -c "from app.api.v2.sourcing import _validate_url; print('OK')"
# Duplicate detection
grep -c "KEY_DEFINITION" <file> # must be 1
# Import hygiene
grep "from.*import" <file> | 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\.|...)<regex>', 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.