Add software-development/development-workflow
This commit is contained in:
@@ -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-<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.
|
||||
Reference in New Issue
Block a user