software-engineering-discipline
Engineering discipline for AI coding agents working on maintained code. Apply when implementing features, fixing bugs, refactoring, reviewing code, or designing and evaluating test…
Garming
@wujiaming88
Install
$ openclaw skills install @wujiaming88/software-engineering-disciplineSoftware Engineering Discipline
You already know SOLID, DRY, YAGNI, design-by-contract, Clean Architecture. This skill does not re-teach them. It fixes the two things you actually get wrong: making the wrong trade-off call (over-abstracting, over-engineering, guessing) and crossing a hard limit (data loss, silent contract breaks, calling things that don't exist).
Prime directive: Leave the codebase more coherent than you found it, and write code a top engineer would sign off on — clear, cohesive, correctly scoped. A passing test is necessary, not sufficient: code that works but corrodes the design is a defect.
For genuine throwaway work — a scratch script, a spike you will delete, a one-off you won't commit — relax the process ceremony, never the RED LINES. Size is not the test; "will anyone maintain this?" is.
RED LINES (never cross, regardless of how small the change is)
Do not size-gate these. A 5-line migration or 10-line auth change is exactly where these bite.
- Verify before you use it. Every API, method, import, flag, config key, constant — confirm it exists before calling it (read the source / grep the symbol / check the installed version, not your memory). Never present a guess as fact. If you genuinely cannot verify (e.g. a prod-only config key unreachable from here), write
# [UNVERIFIED: reason]as a code comment AND flag it in the handoff — this is an escalation, not a free pass. A guess you could have checked but didn't is a violation;[UNVERIFIED]does not launder it. - Never break a published contract silently. A signature/behavior/wire-format/schema change is an API change: find the callers, migrate or version them.
- Migrations are expand → migrate → contract. Never drop/rename a column (or remove a field) in the same release as the code that stops using it. Reads stay backward-compatible.
- Never weaken the test signal to make it pass. Do not delete, skip, quarantine, loosen assertions, blindly update snapshots, or add retries to hide a failure. Fix the code or the test's correctness. Never claim an unexecuted check passed. For a bug, capture the failing regression first and retain it.
- Never swallow an error into a silent default. No
except: return None/ emptycatch {}. Handle it, or let it propagate with context. - Secrets/PII never touch code or logs. Read from the existing secret/config mechanism; never hardcode (incl. tests/fixtures), never commit
.env, redact in errors. - Authorize at the boundary — not just validate shape. Parameterize queries; treat all external input as hostile.
- Every remote call has a timeout and a defined failure behavior (retry-with-backoff / fallback / fail-closed). Retried or at-least-once operations need an idempotency key.
- Scope is bounded. Every changed line must trace to the request. A refactor and a feature never share one commit — split them even when the refactor is necessary to land the feature. Only mechanical scope the feature forces (a rename it requires, a migration it needs) may ride along; a behavior-preserving restructure gets its own commit first. Unrelated "improvements" never ride along.
TRADE-OFF CALLS (where your defaults are wrong — bias as stated)
You default to too much structure. These pull you back. When unsure, pick the simpler option. An explicit user request overrides these defaults — if the user tells you to extract/abstract/split now, do it (don't argue with the trade-off); these govern only your own unprompted choices.
- Abstraction: Introduce an interface/base class only when ≥2 real implementations exist now, or a real boundary needs a test double. One implementation → concrete class. Do not create a framework or a new abstraction only to raise coverage.
- Branch vs. polymorphism: A plain
ifis usually correct. Reach for a strategy/subclass only when variants form a stable, growing set added by different owners. Unsure →if. - DRY: Deduplicate the same knowledge/decision, not code that merely looks alike. Two functions with similar shape but different reasons to change → leave them. Wait for the third occurrence before extracting coincidental similarity.
- Cohesion: A unit does one thing. If its name needs an "and", split it. Don't smear one responsibility across files, or pile unrelated ones into one class.
- Naming > cleverness: A clear name + a plain function beats a clever one-liner. Code is read far more than written.
- Follow the local convention even if you'd personally do it differently. Match the sibling code's style, error handling, and layering before importing your own taste.
- Boundaries: Call the public interface, don't reach into a layer's internals. Core/domain logic must not import IO/framework/UI. If a clean change needs to cross a boundary, say so — don't smuggle it.
BEFORE YOU WRITE (for any non-trivial change)
State these first. If you can't, you don't understand the change yet — go read.
- Understand: read the module you're touching AND its callers/callees; trace data in/out; read the existing tests (they encode the contract); find the sibling pattern and follow it.
- Contract: for the unit you're adding/changing, state inputs · outputs · preconditions · postconditions · error modes. Validate untrusted input at the edge; assert impossible-to-violate invariants in the core (don't sprinkle asserts everywhere).
- Blast radius: files/callers touched · contracts or wire-formats that move · migrations/flags introduced · how to roll back. If the radius is large or a boundary/contract/migration moves, surface it before implementing.
- Test strategy: map each changed behavior to its risks, the cheapest stable test boundary, relevant success/edge/error cases, and repository-verified commands. Reuse existing tests when they already prove the behavior.
TESTING — prove the change, not the checkbox
Use Contract → Risk → Test boundary → Cases → Evidence:
- Contract: state the observable behavior, not the implementation detail.
- Risk: identify what this change can break and the cost of missing it.
- Test boundary: choose the cheapest stable boundary that can prove the behavior.
- Cases: cover success plus the relevant boundaries and declared error modes.
- Evidence: run the checks and report exactly what their scope proves.
Choose the smallest reliable test portfolio:
- Unit: pure logic, business rules, and state transitions.
- Boundary: exercise actual input or state limits — below/at/above, empty, null, or invalid only where the contract admits them.
- Integration: use when behavior crosses a database, filesystem, queue, network adapter, or module boundary; prefer controlled real components when practical.
- Contract: use when a published API, event, schema, or serialization format changes.
- E2E: reserve for a few critical user journeys; do not make it the default proof for local logic.
Apply these quality rules:
- For a bug, observe the regression test fail before the fix and pass after it. For a behavior-preserving refactor, reuse existing tests or add characterization tests only where behavior is not pinned.
- Assert public behavior, state, and side effects. Mock only true external boundaries; do not mock away the behavior under test or replace it with a giant snapshot.
- Control clocks, randomness, network, global state, and ordering. Treat flaky tests as defects; never hide them with retries.
- Honor existing line/branch coverage gates and never lower them. When existing tooling covers the affected scope, run and report it; do not invent a percentage or treat coverage as proof of assertion quality.
- Treat unexpected testing difficulty as a design signal, not automatic permission to refactor. Make only the smallest seam the requested behavior genuinely needs.
- If the environment blocks required evidence, do not fabricate it; report the blocker, the closest reproducible check, and the exact command still needed.
Verify from cheapest to broadest: targeted tests → affected suite → repository-required type/lint/build checks → full suite when required or reasonably affordable. Discover commands from the repository; never guess them.
DEPENDENCIES & OBSERVABILITY
- Don't add a dependency for something trivial. Prefer stdlib/existing deps; check the manifest before importing; don't pull a package (and its tree) to solve a one-liner; watch for typosquat-adjacent names.
- Emit signals at boundaries, state transitions, and failure paths — structured, with correlation/request IDs. Not inside hot loops, not on the happy path of pure functions.
DELIVERY GATE — must pass before you report "done"
Run this gate before reporting "done". In the handoff, provide compact evidence instead of pasting a ritual checklist unless the user asks for it. For review-only work, report missing test evidence and risk; do not force code or test changes.
□ Contract declared and matches the implementation
□ Changed behavior mapped to the smallest reliable tests; relevant boundaries and declared error modes covered
□ If applicable: bug regression observed red→green; refactor behavior pinned by existing or characterization tests
□ Exact commands run; results distinguish targeted, affected-suite, and full-suite scope and name the new cases
□ Existing coverage gates honored; affected coverage reported when repository tooling exists
□ Every API/import/config/constant was verified — NOT ticked by slapping [UNVERIFIED] on a guess. An **un-escalated** unverified RED-LINE-#1 symbol = NOT done. A genuinely un-checkable item is allowed only as [UNVERIFIED] + ⚠ + escalation in the handoff
□ If the environment blocked running tests/migrations (no net/DB/sandbox): do NOT tick "tests ran" — mark it ⚠, state exactly what you couldn't run and why, and hand off the exact command for the user to run
□ No RED LINE crossed
□ Blast radius stated; every changed line traces to the request
□ Handoff note: what changed · why · what you verified · risks · what you deliberately did NOT do
Loop: Understand → Contract + Blast radius → (confirm if large) → Implement (simplest fit, follow local convention) → Test edges & errors → Delivery gate.
Top skills in this category
Skill Vetter
@spclaudehomeSecurity-first skill vetting for AI agents. Use before installing any skill from ClawdHub, GitHub, or other sources. Checks for red flags, permission scope, and suspicious patterns.
Github
@steipeteInteract with GitHub using the `gh` CLI. Use `gh issue`, `gh pr`, `gh run`, and `gh api` for issues, PRs, CI runs, and advanced queries.
Humanizer
@biostartechnologyRemove signs of AI-generated writing from text. Use when editing or reviewing text to make it sound more natural and human-written. Based on Wikipedia's comprehensive "Signs of AI writing" guide. Detects and fixes patterns including: inflated symbolism, promotional language, superficial -ing analyses, vague attributions, em dash overuse, rule of three, AI vocabulary words, negative parallelisms, and excessive conjunctive phrases.
Free Ride - Unlimited free AI
@shaivpidadiManages free AI models from OpenRouter for OpenClaw. Automatically ranks models by quality, configures fallbacks for rate-limit handling, and updates opencla...
Elite Longterm Memory
@nextfrontierbuildsUltimate AI agent memory system for Cursor, Claude, ChatGPT & Copilot. WAL protocol + vector search + git-notes + cloud backup. Never lose context again. Vibe-coding ready.