Turn recurring AI code review comments into automated Go checks
AI coding agents are inconsistent reviewers. An agent can catch a missing resource cleanup in one pass and walk right past the same bug in the next. Every review costs time and tokens, even when the requirement hasn’t changed.
Stanislav Korolev built a Go repository that addresses this directly. It converts selected code review requirements into automated checks: linter configurations, behavioral tests, an architecture test that traces indirect imports, and verification scripts that confirm the checks actually catch known violations. He calls these checks guardrails.
The repository is open and the command output in the article was produced with Go 1.27.1 and golangci-lint 2.13.2, pinned so results are reproducible. Here is how each check category works and what it can and cannot catch.
️ What a guardrail actually is
A guardrail is a condition a code change must meet before it’s accepted. The key property: it must be a command the agent can rerun after making a fix and get a clear pass or fail signal. A naming violation produces a diagnostic with the file, line, and expected spelling. An architecture violation shows the full forbidden dependency chain. A cancellation test shows which goroutine is blocked and why.
Korolev organized the checks into six areas, each requiring a different observation method:
- Formatting and naming: inspect source text with
gofmtandrevive - Complexity and duplication: static analysis flags for human review
- Resource cleanup and error propagation: linters paired with behavioral tests using controlled failures
- Concurrency: synchronization tests, goroutine leak checks, and the race detector
- Architecture: direct import rules plus a test that walks the full import graph
- Test quality: result comparisons, field initialization rules, and mutation testing
1. Formatting and naming
gofmt -w . applies standard formatting automatically. Naming conventions need explicit rules. The initialism ID should be capitalized, so a field declared as UserId breaks the convention. The var-naming rule in revive catches it:
testdata/naming/naming.go:5:2: var-naming: struct field UserId should be UserID (revive)The agent changes the field name and reruns the check. Korolev recommends agreeing on a rule with the team and running it against existing code before enabling it. Enabling a naming rule without that step can turn a small task into an unrelated rename sweep.
Run revive through golangci-lint with a minimal config that enables only the target rule. Use default: none in golangci-lint and enable-default-rules: false in the revive settings so the diagnostic is isolated to the specific check.
⚙️ 2. Complexity thresholds
Korolev used gocognit with a threshold of 3 for the test case. A function with three nested if statements scores 6 (contributions of 1, 2, and 3 for each nesting level) and triggers:
cognitive complexity 6 of func `CanExport` is high (> 3)
Combining the conditions into return active && allowed && ready resolves it. One practical trap: golangci-lint‘s diff filtering can hide a new violation if the diagnostic points to a function declaration the agent didn’t touch. The --whole-files flag restores visibility but also surfaces existing violations elsewhere in the file. Choose the threshold and reporting scope together so relevant findings actually reach the agent.
3. Resource cleanup and error contracts
For an HTTP download function, Korolev defined a four-case contract: close the response body exactly once, preserve any data already read, and return both the read error and the close error when both occur. He paired a bodyclose linter check for the missing close with a behavioral test covering all four combinations of read and close outcomes.
The correct implementation uses a named return and a deferred function:
defer func() {
err = errors.Join(err, response.Body.Close())
}()After the fix, bodyclose is silent and the behavioral test passes all four scenarios. The same pattern applies to context cancellation: context.WithTimeout returns a cancel function that must be deferred, and omitting it causes lostcancel in go vet to fire.
The key principle: for each dependency, define what its failure should mean to the caller, then make the dependency fail in a test and check the required response.
⏱️ 4. Cancellation, deadlocks, and data races
This is where the checks get more sophisticated. Korolev used testing/synctest to test a Forward function that sends on an unbuffered channel. Without a receiver, the sender blocks. The test needs to confirm the sender is blocked before canceling the context, which synctest.Wait() handles without an arbitrary sleep.
If Forward is missing the cancellation case in its select, the test output is:
--- FAIL: TestForwardCancellation (0.00s)
panic: deadlock: all goroutines in bubble are blocked [recovered, repanicked]Adding case <-ctx.Done(): to the select resolves it. The fix is one line. The diagnostic identifies exactly which goroutines are blocked and at which lines.
For goroutine leak detection in ordinary tests, goleak works alongside synctest. Use VerifyNone for sequential tests and VerifyTestMain for parallel ones to avoid false positives from goroutines belonging to other running tests.
For data races, the race detector is the right tool. Run tests with -race -count=1. The detector reports conflicting accesses even when a particular run happens to produce the expected output. A passing run without -race says nothing about whether races exist.
Mutex deadlocks fall outside synctest‘s durable-blocking definition. Use a timeout instead:
go test -count=1 -timeout=2s -run '^TestMutexDeadlock$' ./testdata/deadlockThe stack output when it times out shows the second lock waiting on a mutex the same goroutine already holds.
️ 5. Architecture: enforcing package independence
The reporting service has one architecture rule: packages under internal/reportcalc must not depend on internal/storage or any of its subpackages, including through intermediate packages.
Korolev used two checks. depguard catches direct imports. Its config uses two deny entries to cover both exact matches and subpackages:
deny:
- pkg: example.com/guardrails/internal/storage$
desc: calculations must not depend on storage
- pkg: example.com/guardrails/internal/storage/
desc: calculations must not depend on storageA single prefix ending in storage would also match an unrelated package named storagecache. The two-entry approach avoids that false positive.
But depguard only sees direct imports. If calculation code imports internal/shared which imports internal/storage, the rule passes silently. The architecture test catches this by loading the full package graph with golang.org/x/tools/go/packages and walking imports recursively. A violation produces:
ARCH001: calculation depends on storage: example.com/guardrails/internal/reportcalc -> example.com/guardrails/internal/shared -> example.com/guardrails/internal/storageThe test also fails if either the calculation packages or the storage package can’t be found in the graph. This prevents a package rename from silently leaving the rule checking nothing.
6. Checking whether tests actually catch bugs
An agent writing both the implementation and the test can make the same mistake in both. Two specific failure modes appear in the repository.
Derived expectations: If a test computes its expected value using the same expression as the implementation (want := size < 10), both sides evaluate the same way and the test passes even when the implementation is wrong. Set expectations from the requirement itself: want := true // The requirement allows 10.
Partial comparisons: A test that checks only got.ID == 7 won’t catch a lost Region field. Compare the complete struct. To enforce this at scale, the repository uses exhaustruct_v5, which requires all fields to be listed in struct literals. When a new Currency field is added to View, the linter fires if the implementation or test expectation doesn’t explicitly set it.
Mutation testing with go-mutesting handles boundary coverage. For a function that allows values up to and including 10, three mutations are generated: size < 10, size <= 9, and size <= 11. A test suite covering only Allowed(9) misses all three. Adding cases for 10 and 11, with expectations from the requirement, kills all three mutants.
One important caveat Korolev flags: the built-in runner in the revision of go-mutesting used here can count a build failure or timeout as a detected mutation. His verification scripts use a separate executor that reads go test -json events and only counts a mutant as killed when the target test runs and fails with the expected assertion message. Build failures, timeouts, and unrelated assertions are all classified as invalid results.
How to apply this to your own project
The workflow Korolev describes is a short loop: change the code, run the check, read the diagnostic, fix the cause, rerun the check. Once checks pass, review the change.
- Pick one recurring review comment and write down its acceptance criterion precisely.
- Write a check that observes the relevant failure, not just the symptom.
- Create a faulty implementation that should trigger the check and verify it does.
- Create a case that should pass and verify it does too.
- Give the agent the check command with a clear description of what it should do after a failure.
- Run the same checks in CI on the revision under review.
Before making any check mandatory, validate it on existing code. Many violations on day one means fixing gradually while blocking new ones, not blocking the merge queue immediately.
What still needs human review
Korolev is direct about scope. These checks do not replace review. They focus it. A test that passes doesn’t prove the contract is correct for the application. A select that may send a value after cancellation is technically valid; whether it’s acceptable depends on the caller’s requirements. An architecture rule covers only the packages its configuration selects. A complexity score can drop while execution flow gets harder to follow.
Changes to the checks themselves need scrutiny too. Raising a threshold, adding an exclusion, or splitting a function just to lower its score removes a finding without fixing the underlying problem. Review those changes against the original acceptance criterion, not just the resulting metric.
The recorded verification runs in the repository cover 50 successful steps on Go 1.27.1 and golangci-lint 2.13.2 on macOS arm64. Agent behavior and token savings were not measured.
I’m founder of BizStack at Cagri Sarigoz LLC and a passionate advocate for entrepreneurs.
With over 14 years in tech, marketing, and AI, including my role as Head of SEO at CitizenShipper and co-founder of TaleBot at Intale AI, I’m dedicated to sharing genuine, useful product insights and tips.
At BizStack, I aim to cut through the digital noise to provide clear, actionable advice.
And more than all else, I’m a father to a (always) little girl and a husband.
Contact me at [email protected] for assistance.
