| name | bug-fix |
| description | Use this skill when the user wants to fix a bug — a deviation between current behaviour and intended behaviour in code that already exists. Triggers on "fix bug", "fix this bug", "diagnose and fix", "investigate this regression", "this is broken", "post-mortem this", "why is X failing". Do NOT use for new features (use `new-spec`) or refactors that don't fix incorrect behaviour. Inkwell-specific: the regression test BEFORE the fix is mandatory and lands in the same commit (`docs/CONVENTIONS.md §5.7`). |
Skill: bug-fix
Fix a defect in the smallest, most root-causing way. The discipline is
universal: reproduce before fixing, write the failing test first,
identify root vs symptom, minimum diff, commit body documents why.
The inkwell-specific tightening: docs/CONVENTIONS.md §5.7 ("the
institutional memory for the bugs that have already shipped") and
§16 (the common-review-findings ledger) make the regression test
mandatory and same-commit. v0.2.6 → v0.2.7, v0.2.8 → v0.2.9,
v0.3.0 → v0.3.1 each happened because the test that would have caught
the next regression hadn't been written. Every fix adds one.
When to invoke
The user is pointing at a defect — observed behaviour differs from
intended behaviour in code that already exists. Even a one-line fix
benefits from walking this discipline; it forces the question "is
this fixing the cause or hiding it?"
For multi-file changes that go beyond fixing one defect — refactors,
new features triggered by discovering the bug — stop and use
new-spec instead. This skill is for bug fixes, not opportunistic
restructuring.
Procedure
-
Reproduce first. Don't write a fix until you have one of: a
failing test, documented manual reproduction steps that fail
reliably, or a captured error / stack trace / log signature. No
reproduction = no fix; you might be fixing the wrong thing.
For TUI bugs: the visible-delta rule (docs/CONVENTIONS.md §5)
applies. "User sees stale row" needs a teatest reproduction that
captures the frame, not a unit assertion on internal state. The
v0.2.6 → v0.2.7 cycle proves this: dispatch tests were green; the
user-visible glyph was wrong.
-
Write the failing test (red). It should pin the observable
contract being violated, not the current implementation. Push
back on these failure modes:
- Mock-shape assertion. Asserting
stubGraph.Calls == 2 when
the observable contract is the resulting store.Message row or
the rendered viewer string. Test the contract, not the
implementation. (See quality-engineer's "mock-shape
assertions" check.)
- Test passes for the wrong reason. Run the test against the
unfixed code; confirm it fails because of the bug, not
because the setup is wrong.
require.Equal on the wrong field
can pass for the wrong reason — re-read the assertion.
- Test asserts in the wrong layer. A bug surfaced in the TUI
viewer might actually live in
internal/render; pin the test
at the layer where the invariant lives. Avoid e2e tests for
unit-shaped invariants.
-
Identify root cause before writing the fix. Write down a
one-line answer to each:
- Where is the defect actually? In the called function, the
caller, their shared assumption, or upstream of both? A nil
*store.Body that crashes the renderer may originate in
sync.Engine that should never have called render with a nil.
- When did it start?
git log -p -- <path> and git blame
on the affected code. For regression-shaped bugs the commit
that broke it often tells you why. For bugs that go back
before recorded history, the surrounding docs/specs/NN-<title>/spec.md
and any docs/adr/ files surface the original intent.
- Could the same class of bug exist elsewhere? Grep for
similar patterns — same function called from other sites, same
assumption made elsewhere. Common inkwell-shaped repeats:
ETag-handling between sync paths, context.Background() in
request-path goroutines, missing redaction on a new log site,
missing FK cascade on a new table. If yes, decide whether the
fix's scope widens or whether you file follow-ups in the
same-PR "noted for follow-up" list.
-
Minimum fix. Write the smallest change that turns the failing
test green. Refuse to fix adjacent issues in the same PR; note
them for follow-up. Scope discipline is enforced by
adversarial-reviewer — out-of-scope changes are a Blocker until
justified or extracted.
-
Verify root vs symptom. Look at the diff and ask: does this
address what step 3 identified, or does it mask the symptom?
Inkwell-flavoured symptom-only anti-patterns:
- Silently dropping nil from the sync→render handoff instead
of fixing the engine that should never have called with nil.
- Defensive
if err != nil { return } at every call site of a
function whose contract should not return that error.
- Retries around flaky Graph 429s when the right fix is to
respect
Retry-After in internal/graph/batch_retry.go once.
- Disabling a feature flag (
[body_index].enabled = false)
instead of fixing the bug — config knobs are for opt-in scope,
not for hiding broken code.
- Suppressing the redaction test instead of removing the
log site that emits PII (§7 invariant 3).
If the failing test from step 2 still passes under a symptom-only
fix, you wrote the wrong test — go back to step 2 and sharpen it.
-
Regression test stays. The failing test from step 2 lands in
the same commit as the fix (CONVENTIONS §5.7 is explicit). It
joins the relevant package's test file as the institutional memory
for the next time someone repeats this mistake. If the bug
surfaced a deeper invariant, add a bullet to CONVENTIONS §16 so
future PRs grep this surface before re-introducing the pattern.
-
Run the right subset of gates. The full make regress is the
pre-tag gate; for a bug-fix PR the relevant subset is usually:
gofmt -s -d <changed files>
go vet ./...
go test -race ./internal/<pkg>/...
go test -tags=integration ./internal/<pkg>/...
go test -tags=e2e ./internal/ui/...
make regress before pushing if the fix touches more than one
package.
-
Commit body documents the root cause. Conventional commit
subject — fix(spec-NN): <subject> or fix(<pkg>): <subject>
(CONVENTIONS §10). The body explains:
- What was wrong (the observable bug).
- Why it was wrong (the root cause from step 3).
- Why the fix takes the shape it does (and why not the obvious
alternatives).
The diff shows what; the commit body shows why. Future readers
care more about the latter. No Co-Authored-By: Claude trailer
(CONVENTIONS §10).
-
Update the plan file if this fix relates to a shipped spec.
docs/specs/NN-<title>/plan.md is the journal; add an iteration entry
noting "Bug X reported on YYYY-MM-DD; regression test
TestFooHandlesBar added in commit <sha>."
Anti-patterns to refuse
- Fixing forward without a reproduction. The obvious fix is
wrong about a third of the time, and you can't tell which third
until the test fails red first.
- Fixing the bug plus adjacent cleanup in one PR. Each cleanup
is its own PR with its own justification. Bug-fix PRs are for
fixing bugs.
- Adjusting the spec or the test to match the buggy behaviour.
If the spec and the fix disagree, one of them is wrong — surface
that explicitly before continuing, don't paper over it.
- Giving up before exhausting reproduction strategies. Different
OS, different tenant, different account, fresh DB, RSS over time,
network conditions. Document what was tried, on what version,
with what data, before asking the user for more. "Couldn't
reproduce on my machine" is a hypothesis worth testing, not a
closing condition.
- Cherry-picking the fix to
main without rebasing. The feature
branch may be ahead of main in unrelated commits (CONVENTIONS
§16 "Process" bullet). Verify the worktree state first.
- Tagging without checking CI. Local
make regress green is
necessary, not sufficient. After every push, gh run list --limit 5 (CONVENTIONS §10).
