diff options
Diffstat (limited to 'feedback_fix_entire_bug_class.md')
| -rw-r--r-- | feedback_fix_entire_bug_class.md | 18 |
1 files changed, 18 insertions, 0 deletions
diff --git a/feedback_fix_entire_bug_class.md b/feedback_fix_entire_bug_class.md new file mode 100644 index 0000000..d37c759 --- /dev/null +++ b/feedback_fix_entire_bug_class.md @@ -0,0 +1,18 @@ +--- +name: feedback-fix-entire-bug-class +description: "When fixing a bug, fix the whole class it belongs to and add a structural guard that prevents it recurring -- not just the specific instance found" +metadata: + node_type: memory + type: feedback + originSessionId: 2607e018-d4ad-4ec6-bb68-5f1d0f9a3906 +--- + +**Any time you fix a bug, fix the entire class of bug, and add something that prevents it in the future.** Patching the specific instance that got reported is not done. Ask: what other places in the codebase have this same shape of bug, and what mechanism (test, lint rule, structural refactor) makes this class of bug fail loudly next time instead of shipping silently again. + +**Why:** Said explicitly on 2026-08-05, the day after the doot production-wedge incident ([[feedback-verify-before-asserting-root-cause]]), citing `internal/api/context_audit_test.go` as the good example to repeat. That test came out of finding one instance of a bug (a `.Do()` call missing `.Context(ctx)`) in `google_calendar.go`, then not stopping there: auditing every `.Do()` call in the codebase for the same pattern, and writing an AST-based test that fails CI if any future Google-SDK call in `google_*.go` is missing `.Context(...)` anywhere in its chain -- verified by deliberately reintroducing the original bug against a backup and confirming the new test actually catches it. That test makes the whole bug class structurally hard to reintroduce, not just the three call sites found that day. + +**How to apply:** +- After fixing a reported instance, grep/search the codebase (or the relevant package) for the same pattern elsewhere before considering the fix done. +- Then ask what would make this class of bug fail fast next time: a unit/structural test (like the AST-based context-audit test), a lint rule, a type-level change that makes the mistake impossible to express, or a refactor that removes the manual-diligence step entirely (e.g. `internal/api/http.go`'s `BaseClient` bakes ctx into the request object itself via `NewRequestWithContext`, so it structurally can't have this bug -- that's the deeper fix; the Google SDK's fluent-builder pattern couldn't be changed that way, so an audit test was the next-best structural guard). +- Verify the guard actually works by trying to reintroduce the original bug (against a real backup, not a fake "restore") and confirming the guard catches it before calling the work done. +- This applies beyond this one incident -- treat it as a standing bar for any bug fix on this project, not a one-off request. |