How to turn recurring code review comments into automated checks, verify that they catch bugs, and decide what still needs human review.
An AI coding agent can catch a bug in one review and miss it in the next. Adding instructions or another reviewer still leaves someone to read the code and decide whether it meets the requirement. Each review takes time and tokens, even when the same requirement has been checked before.
I built a Go repository that turns selected code review requirements into automated checks. It combines linter configurations with behavioral tests, an architecture test that follows indirect imports, and scripts that validate the checks against deliberately broken code.
I call these checks guardrails: conditions a code change must meet before we accept it. For example, a goroutine that must stop after cancellation gets a test for that behavior. A rule that report calculations must be independent of storage gets a check of the package dependency graph. The agent receives a diagnostic it can act on, then runs the same check after the fix.
A passing test may miss a bug, and a failing command may indicate a build error or timeout. I validated the checks against both faulty implementations and failures of the tools themselves.
The sections below explain how to construct these checks, establish what they detect, and identify the decisions that still need review. The examples use a model reporting service to make each requirement and failure reproducible.
This article assumes you know Go tests, contexts, channels, and CI. The repository contains the source code, configurations, and reproduction commands. The command output shown here was obtained with Go 1.27.1 and golangci-lint 2.13.2. These versions are pinned so you can reproduce the results.
Each requirement needs a check that can observe the relevant failure. Naming rules can inspect source code. Cancellation tests must execute the goroutine and wait for it to finish. A restriction on indirect dependencies requires walking the package import graph. I combined these checks around the requirements of the reporting service:
|
Area |
Requirement or review concern |
Check |
|---|---|---|
|
Formatting and names |
Follow agreed formatting and naming conventions |
|
|
Complexity and duplication |
Flag code that exceeds agreed limits or repeats existing logic |
Static analysis, followed by review |
|
Errors and resources |
Close resources and return the errors required by the contract |
Linters and tests with controlled failures |
|
Concurrency |
Finish work after cancellation; detect leaks, deadlocks, and data races in exercised scenarios |
Synchronization tests, leak checks, and the race detector |
|
Architecture |
Keep calculations independent of storage, including through intermediate packages |
Import rules and a test that traverses dependencies |
|
Test quality |
Check the required results and detect selected behavior changes |
Result comparisons, field initialization rules, and mutation testing |
Verification scripts create faulty variants in temporary copies, run the relevant commands, and inspect their diagnostics. Separate cases check that build errors and timeouts are rejected as evidence of detecting a violation.
These scripts validate the guardrails. The ordinary tests and linters remain the commands used to accept a code change. Keeping those purposes separate matters: a verification script can pass because it successfully caused a test to fail on broken code.
Run the commands from the root of the example repository. Select Go 1.27.1 for the terminal session first. In fish:
set -gx GOTOOLCHAIN go1.27.1
go version
golangci-lint version
The first version check should report Go 1.27.1. The second should report golangci-lint 2.13.2 built with Go 1.27.1. The README covers tool and dependency installation.
Set GOTOOLCHAIN again in each new session. The go 1.27.0 line in go.mod sets a minimum version; it does not pin the toolchain to 1.27.1.
Formatting and naming establish the baseline for the rest of the checks. Once the team agrees on a convention, a tool can apply or check it on every change. gofmt applies standard Go formatting to files in the current directory and its subdirectories:
gofmt -w .
Naming conventions need separate rules. For example, the initialism ID should be capitalized. This declaration breaks that convention:
type request struct {
UserId int
}
The var-naming rule in revive reports the problem and the expected spelling:
testdata/naming/naming.go:5:2: var-naming: struct field UserId should be UserID (revive)
The agent can change UserId to UserID and run the check again. Choosing a name that captures the domain, such as account or customer, still requires a developer's judgment.
Agree on a rule with the team and try it on the existing code before enabling it. Otherwise, a small task can accumulate unrelated renames.
To check formatting without changing files, use gofmt -l .. It lists files that need formatting, but still exits successfully. Configure CI to fail when that list is nonempty. The gofmt documentation describes the flags.
I run revive through golangci-lint, which runs multiple analyzers. The linter catalog lists the available checks. To verify the naming rule in isolation, this configuration enables it without unrelated rules. Save it as naming.yml:
version: "2"
linters:
default: none
enable:
- revive
settings:
revive:
enable-default-rules: false
rules:
- name: var-naming
This disables two sets of defaults. default: none disables golangci-lint's default linters; enable-default-rules: false disables revive's default rules. The resulting diagnostic can then be checked against the specific naming violation. The configuration documentation explains the structure.
Run this from the example module directory:
golangci-lint run --config naming.yml ./testdata/naming
The path is explicit because ./... skips testdata. The command reports the diagnostic above, including the file, location, and linter name.
Working code can still become harder to maintain after an agent's change. Nested conditions add paths to follow, and duplicated logic creates multiple places to update. Static analysis can flag these changes for review, but the rule must express what the team wants to control.
A complexity threshold sets an upper limit for a function. Preventing complexity from increasing requires a comparison with the previous version. These are different acceptance criteria: with a threshold of 30, a score rising from 20 to 25 still passes.
For the repository's complexity check, I used gocognit and a function whose nested conditions have a predictable score. CanExport allows an export when all three conditions hold:
func CanExport(active, allowed, ready bool) bool {
if active {
if allowed {
if ready {
return true
}
}
}
return false
}
gocognit assigns a cognitive complexity score to control flow. Here, the three nested if statements contribute 1, 2, and 3, for a total of 6. Each level of nesting increases the contribution. The gocognit rules explain the calculation.
I set the threshold to 3 so this implementation triggers a known diagnostic:
cognitive complexity 6 of func `CanExport` is high (> 3)
This threshold is part of the test case. Choose a threshold for a project after examining its existing code and agreeing on the limit with the team.
Combining the conditions preserves the behavior:
func CanExport(active, allowed, ready bool) bool {
return active && allowed && ready
}
The diagnostic identifies the function for the agent to change. Review must still assess the resulting code: splitting a function into many helpers can lower its score while making the execution flow harder to follow.
An existing project may have many violations when a rule is first enabled. Filtering findings to changed lines helps focus on a patch, but the location of a complexity diagnostic can make that filter hide a new violation.
The diagnostic may point to the function declaration. If the agent changes only the body, the declaration is outside the diff. The linter can detect the violation, and the filter can remove it from the reported results.
With revision filtering, --whole-files reports findings for the entire changed file. That includes the unchanged declaration, but also brings back existing violations elsewhere in the file. The CLI documentation explains these filters.
If the requirement is to prevent any increase in complexity, compare the function's score before and after the change, using a branch such as main as the baseline. The threshold check shown here does not implement that comparison. Choose the acceptance criterion and reporting scope together so that a relevant finding reaches the agent and reviewer.
Complexity is one source of maintenance work. Other analyzers identify code that needs a different review decision:
|
Tool |
What it finds |
|---|---|
|
|
Functions that exceed configured length limits |
|
|
Similar code fragments that may be duplicates |
|
|
Unused declarations in the analyzed packages |
A duplication finding is a prompt to decide whether the fragments should share an implementation. Introducing an interface or wrapper still needs a design justification. Analyzers also have technical limits: unused, for example, does not flag an exported declaration solely because nothing in the project calls it.
A resource check needs to cover what happens when an operation fails. For Download, I defined the contract after a successful HTTP request: close the response body exactly once, preserve any data already read, and return both the read and close errors when both occur.
I paired a linter check for the missing close with a behavioral test of that contract. The test controls read and close failures independently, including the case where reading returns data before failing.
This implementation reads the response body but leaves it open:
func Download(url string) ([]byte, error) {
response, err := http.Get(url)
if err != nil {
return nil, err
}
return io.ReadAll(response.Body)
}
bodyclose detects the missing close in this implementation. Closing the body through defer handles cleanup when reading succeeds or fails. The deferred function also needs to preserve any close error required by the contract:
func Download(url string) (data []byte, err error) {
response, err := http.Get(url)
if err != nil {
return nil, err
}
defer func() {
err = errors.Join(err, response.Body.Close())
}()
return io.ReadAll(response.Body)
}
The return statement assigns the results of io.ReadAll to the named return variables, data and err. The deferred function then closes the body and uses errors.Join to combine any close error with the read error. The caller receives the result after that deferred function finishes.
When both operations succeed, the returned error is nil. When either fails, the caller can inspect the returned error with errors.Is. The function may return data together with an error, so receiving data alone does not establish success.
I used TestDownloadReadAndClose to check all four combinations of read and close outcomes:
|
Read outcome |
Close outcome |
Expected error |
|---|---|---|
|
Success |
Success |
|
|
Failure after returning data |
Success |
Contains the read error |
|
Success |
Failure |
Contains the close error |
|
Failure after returning data |
Failure |
Contains both errors |
Every case also checks that Close is called exactly once and that the returned data matches what the body supplied. Checking partial data matters: a change that discards those bytes would violate the contract even if it closes the body and returns the expected error.
The test supplies an http.RoundTripper that returns a controlled response without making a network request. Its body counts calls to Close and can return a configured close error. For read failures, the reader supplies data first, then returns the configured error. This makes each failure combination reproducible without depending on network behavior.
Because Download calls http.Get, the test temporarily replaces http.DefaultClient. The subtests run sequentially and restore the original client after each case. Run the test with:
go test -race -count=1 -run '^TestDownloadReadAndClose$' ./internal/download
After the fix, the bodyclose finding disappears and the function passes the repository's baseline linters. The behavioral test checks the contract in the four scenarios above. bodyclose recognizes known code patterns, and passing these checks does not prove cleanup on every possible execution path.
The scope here is response body cleanup and error propagation. Timeouts, context propagation, and HTTP status handling need separate requirements and checks.
A context with a timeout also needs cleanup. context.WithTimeout returns a cancel function; deferring it releases the associated resources when the function returns, without waiting for the timeout:
ctx, cancel := context.WithTimeout(parent, timeout)
defer cancel()
Replacing cancel with _ causes the lostcancel check in go vet to report the missing cancellation call.
The errcheck linter helps find ignored errors. If code checks an error and then returns success, nilerr detects some of those cases.
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. That contract determines whether the function should propagate an error, preserve partial results, or return another agreed outcome.
For concurrent code, I built separate checks for completion after cancellation, blocked goroutines, and conflicting access to shared data. Each check needs a scenario that reaches the failure and a diagnostic that identifies it. A timeout alone cannot distinguish a deadlock from slow execution.
The cancellation test starts with an observable contract: while no receiver is available, the sender waits; after cancellation, it finishes and signals completion.
A goroutine sends a result on an unbuffered channel. If the receiver stops reading, the send stays blocked. Handling context cancellation gives the goroutine a way to stop waiting.
Forward sends a value to out and closes done when it finishes. The caller can wait on done for completion:
func Forward(ctx context.Context, out chan<- int, value int) <-chan struct{} {
done := make(chan struct{})
go func() {
defer close(done)
select {
case out <- value:
case <-ctx.Done():
}
}()
return done
}
If a receiver is ready, the goroutine can send the value and return. If the send is blocked, cancellation allows it to return. Both paths close done.
The caller must cancel the operation when it no longer needs the result and wait for completion with <-done. Forward leaves out open because the caller owns that channel.
If the send and cancellation are ready at the same time, select may choose the send. This implementation does not guarantee that no value is sent after cancellation.
I used testing/synctest to establish when the sender is blocked before canceling it. This synchronizes the test with the goroutine without choosing an arbitrary sleep duration:
func TestForwardCancellation(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
ctx, cancel := context.WithCancel(t.Context())
defer cancel()
out := make(chan int) // There is deliberately no receiver.
done := Forward(ctx, out, 42)
synctest.Wait()
select {
case <-done:
t.Fatal("sender completed before cancellation without a receiver")
default:
}
cancel()
<-done
})
}
After synctest.Wait(), the test checks that done is still open. Without that assertion, a function that returns immediately without sending anything could pass.
The test then cancels the context and waits for done to close. If the sender stays blocked, synctest reports a deadlock within the group. In this scenario, that failure detects a goroutine that did not finish after cancellation.
A separate TestForwardDelivery checks delivery of the value 42 and completion after the send. Together, the tests cover successful delivery and completion after cancellation.
synctest.Test runs the test and the goroutines it creates in an isolated group called a bubble. synctest.Wait() waits until the other goroutines have either finished or become durably blocked. It cannot tell a waiting sender from one that has already finished, so the test checks done separately.
The channels are created inside the bubble. Waiting on an external channel may depend on events outside it and does not count as durable blocking. With no receiver in this example, the sender cannot proceed until the test cancels the context.
If the sender ignores cancellation or fails to close done, the test cannot finish waiting on <-done. In this isolated scenario, synctest reports a deadlock because all goroutines in the bubble are blocked.
To check the test itself, I included five faulty implementations in the verification script: immediate completion without a send, a missing cancellation case, a missing close of done, cancellation checked only before a blocking send, and an unconditional send. The script verifies the expected failure for each variant. A separate change sends 43 instead of 42 to confirm that the delivery test checks the value too.
This test establishes the response to explicit cancellation. Whether the caller cancels when the receiver exits early needs a separate test.
Automatic cancellation of t.Context() can hide a bug here. That context is canceled after the test function returns. If the code under test fails to cancel and the test returns without waiting for completion, the test's own cancellation may allow it to pass. Waiting on <-done inside the test body exposes the deadlock.
Goroutines can keep running or waiting after an operation finishes. goleak helps detect these leaks in tests outside synctest.
First, run the scenario and wait for the work to finish, for example by waiting for done to close. Then let goleak check for remaining goroutines. Its retries and short waits do not replace waiting for completion in the test itself.
Choose the check based on how tests run:
VerifyNone checks for goroutines left after an individual test.VerifyTestMain checks after all tests in the package finish. Otherwise, a goroutine belonging to another running test may look like a leak.The cancellation test uses synctest to detect the blocked sender. I used goleak in the ordinary delivery test and included a deliberately leaking sender to verify that the leak check detects it.
Suppose two goroutines exchange data over unbuffered channels. Both send first and plan to receive afterward. Reproduce that ordering inside synctest:
func TestDeadlock(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
left := make(chan struct{})
right := make(chan struct{})
go func() {
left <- struct{}{}
<-right
}()
right <- struct{}{}
<-left
})
}
The new goroutine blocks while sending to left; the test goroutine blocks while sending to right. Each needs a receiver, but neither reaches its receive.
Run the example from the module directory:
go test -count=1 -run '^TestDeadlock$' ./testdata/deadlock
synctest fails the test with deadlock: all goroutines in bubble are blocked. The stack shows both blocked sends. For this check, create the channels and goroutines inside the bubble, as the example does.
Mutex waits need a different approach. Here, a goroutine tries to lock a mutex it already holds:
func TestMutexDeadlock(t *testing.T) {
var mu sync.Mutex
mu.Lock()
defer mu.Unlock()
mu.Lock() // Waits for the mutex that is already locked.
}
The second Lock prevents the function from returning, so the deferred Unlock never runs. Waiting on sync.Mutex does not count as durable blocking for synctest. Run this test with a timeout:
go test -count=1 -timeout=2s -run '^TestMutexDeadlock$' ./testdata/deadlock
When it times out, the stack shows the second Lock. A timeout alone only tells us that the test did not finish in time. The code and stack establish the cause. To test a deadlock caused by inconsistent ordering across multiple locks, reproduce that specific ordering.
External I/O and system calls also fall outside durable blocking. The testing/synctest documentation describes these limits.
Two goroutines increment a shared counter. A WaitGroup lets the test wait for them, but does not synchronize their accesses to the counter:
func TestRace(t *testing.T) {
var value int
var wg sync.WaitGroup
for range 2 {
wg.Go(func() { value++ })
}
wg.Wait()
t.Log(value)
}
value++ reads, increments, and writes the value. The goroutines access the same variable without synchronizing those operations, causing a data race. The code is still incorrect if a particular run prints 2.
Run the example with the race detector:
go test -race -count=1 ./testdata/race
The detector reports WARNING: DATA RACE and points to the conflicting accesses at value++. The test fails even if it printed the expected counter value. Protect the increment with a mutex or use an atomic operation.
-race enables the detector. -count=1 reruns tests without using cached results. To test your project's packages, use ./... in place of ./testdata/race. The wildcard skips testdata, which is why this example uses an explicit path.
The race detector observes test execution. If the problematic combination of accesses does not occur during that run, the race may go undetected. A passing run does not guarantee that the code has no races, and it says nothing about deadlocks. The race detector documentation explains its use.
The verification script keeps the leak, channel deadlock, and data race cases separate and checks their respective diagnostics. It also runs a mutex timeout case and confirms that the validator rejects it as evidence of a synctest deadlock. This prevents a failed command from being reported as a successful detection of the wrong failure.
For the reporting service, I encoded an architecture rule: packages under internal/reportcalc must remain independent of internal/storage and its subpackages. The restriction covers both direct imports and dependencies through intermediate packages.
I used depguard for direct imports and built an architecture test for the full import chain. Both checks use package boundaries so that the rule covers storage subpackages without rejecting unrelated packages with similar names.
depguard checks imports in selected files. I configured it to report a violation when calculation code imports storage.
This golangci-lint 2.13.2 configuration covers the calculation package and its subpackages, excluding test files:
version: "2"
linters:
default: none
enable:
- depguard
settings:
depguard:
rules:
calculation:
files:
- '**/internal/reportcalc/*.go'
- '**/internal/reportcalc/**/*.go'
- '!$test'
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 storage
files selects the source files to check. The first two patterns cover the calculation directory and its descendants; !$test excludes tests. Remove that exclusion if the architecture rule also applies to test files.
deny uses full import paths. example.com/guardrails is the example module's name. The two entries distinguish these cases, shown relative to that module path:
|
Import |
Result |
|---|---|
|
|
Forbidden: |
|
|
Forbidden: the trailing |
|
|
Allowed by this rule: it is a separate package |
A single prefix ending in storage would also match storagecache. An exact match alone would miss storage/reader. The repository checks all three cases. The depguard documentation explains matching.
An agent might reuse internal/shared, which imports storage itself:
internal/reportcalc -> internal/shared -> internal/storage
The architecture rule covers the entire import chain, including dependencies through shared packages.
The configured depguard rule accepts this change because calculation code does not import storage directly. But the dependency exists through shared.
The architecture test loads the packages, confirms that the required packages exist, and walks their imports. If calculation code can reach storage, it fails and reports the dependency path:
ARCH001: calculation depends on storage: example.com/guardrails/internal/reportcalc -> example.com/guardrails/internal/shared -> example.com/guardrails/internal/storage
The agent can see which package introduced the forbidden dependency. Fixing it may require moving a shared calculation or changing the dependency direction while preserving program behavior.
I also made configuration failures visible. After a package rename, an old path could stop matching anything and leave the rule checking no code. The test fails if either side of the rule is missing or the package graph cannot be loaded. A successful result therefore requires the intended packages to be present and checked.
Its scope is the import graph for the selected build. If platforms or build tags change the files included, check those configurations separately. Accessing storage over the network can happen without a Go package dependency, so this test will not catch it.
The test loads packages with golang.org/x/tools/go/packages. NeedDeps supplies dependencies for traversal. I also requested type information and checked loading errors so an incomplete or invalid graph cannot produce a passing result.
In this code, calculationRoot is example.com/guardrails/internal/reportcalc, and storageRoot is example.com/guardrails/internal/storage. Tests: false excludes test files, matching the depguard configuration above. The helpers and their tests are in architecture_test.go.
func TestArchitecture(t *testing.T) {
pkgs, err := packages.Load(&packages.Config{
Mode: packages.NeedName | packages.NeedImports | packages.NeedDeps | packages.NeedTypes,
Tests: false,
}, "./internal/...")
if err != nil {
t.Fatal(err)
}
if len(pkgs) == 0 || packages.PrintErrors(pkgs) > 0 {
t.Fatal("cannot load the production package graph")
}
targetFound := false
for _, pkg := range pkgs {
if pkg.PkgPath == storageRoot {
targetFound = true
}
}
if !targetFound {
t.Fatal("architecture target package missing; review storageRoot")
}
checked := 0
for _, pkg := range pkgs {
if !inside(pkg.PkgPath, calculationRoot) {
continue
}
checked++
if chain := forbiddenPath(pkg, storageRoot, make(map[string]bool)); chain != nil {
t.Errorf("ARCH001: calculation depends on storage: %s", strings.Join(chain, " -> "))
}
}
if checked == 0 {
t.Fatal("no calculation packages checked; review the architecture rule")
}
}
inside selects a package and its subpackages, respecting the / separator. A rule for storage therefore does not include storagecache.
forbiddenPath walks imports and returns a chain to the forbidden package. It skips packages already visited and sorts imports before traversal. When several forbidden paths exist, the diagnostic consistently reports the same one.
I validated these safeguards with separate cases for a missing storage package, an empty set of calculation packages, and a graph that fails to load. The package boundary cases check a direct dependency, a dependency on storage/reader, and the allowed neighboring package storagecache. These cases test both missed violations and incorrect rejections.
Build tags are easy to lose. go test -tags does not automatically pass its tags to the nested packages.Load call. The test can run with tags while loading a graph without them. Set them through BuildFlags or a shared GOFLAGS environment variable; the repository checks the latter. Test files also need explicit inclusion if you want to inspect them. See the go/packages configuration.
The agent's tests become part of the acceptance criteria, so they need scrutiny too. I included faulty implementations that expose two ways a test can pass incorrectly: deriving its expectation from the same faulty expression and comparing only part of the result. Mutation testing then checks whether selected changes in behavior cause the tests to fail.
When an agent writes both the implementation and the test, it can make the same mistake twice. Suppose the requirement allows values up to and including 10, but the function rejects 10:
func Allowed(size int) bool {
return size < 10
}
The test calculates its expectation using the same expression:
size := 10
want := size < 10
got := Allowed(size)
if got != want {
t.Fatalf("Allowed(%d) = %v, want %v", size, got, want)
}
Both want and got are false. The test passes even though the function violates the requirement. Set the expectation from that requirement:
want := true // The requirement allows 10.
got := Allowed(10)
if got != want {
t.Fatalf("Allowed(10) = %v, want %v", got, want)
}
Now the test catches the bug: it expects true and gets false. Calculating an expectation is also valid when the calculation comes from a requirement, an agreed example, or an independent property, rather than repeating the algorithm under test.
ToView should copy an identifier and a region from a record into a report view. A test that checks only got.ID == 7 will miss a lost region.
type View struct {
ID int
Region string
}
Specify the complete expected result:
input := Record{ID: 7, Region: "north"}
want := View{ID: 7, Region: "north"}
if got := ToView(input); got != want {
t.Fatalf("ToView() = %+v, want %+v", got, want)
}
The test now compares both fields and detects an empty region in place of "north". The != operator works for this struct. For other data, choose a comparison that matches its meaning.
A later schema change can create another gap. Suppose the report gains a currency field:
type View struct {
ID int
Region string
Currency string
}
If neither ToView nor the expected result changes, Currency has its zero value, an empty string, on both sides. The test passes again.
exhaustruct requires fields to be listed in literals such as View{...}, including the implementation's result and the test's expected value. I added a case where the new Currency field leaves the equality test passing but triggers the linter. This checks whether a change to the result type forces the implementation and test expectations to be revisited.
The linter cannot choose the correct value. Explicitly writing Currency: "" in both places removes the finding. A test needs a concrete expected currency derived from the requirement to check that it is copied. If an empty string is valid, it is not a bug.
Comparison semantics matter. Pointer equality compares pointers, not the contents of the objects. Using == on time.Time compares its internal representation as well as the time instant. Structs containing slices or maps cannot be compared with == at all. Ignoring fields or element order must also follow the requirements.
In golangci-lint 2.13, the earlier v4 exhaustruct analyzer is deprecated, so the repository uses exhaustruct_v5:
version: "2"
linters:
default: none
enable:
- exhaustruct_v5
settings:
exhaustruct_v5:
explicit-mode: true
enforce-patterns:
- '^example\.com/guardrails/internal/report\.View$'
The regular expression selects View by its full package path and type name. Without explicit-mode: true, the analyzer would check struct literals by default, subject to its exclusions. A wrong path may select no type at all. Configuration syntax validation cannot catch that, so test the rule against a known missing field.
This check covers literals of the selected type, View{...}. It does not guarantee that fields are populated through var, new, later assignments, copying, or type conversions. Suppression directives and exclusions can also weaken the check; this example uses neither.
These settings apply to v5, whose behavior is documented in its README. After upgrading the analyzer, recheck which types and initialization forms it covers. verify.py shows output limits and other execution details.
Correct expectations still need the right test cases. Return to the function that accepts values up to and including 10, now implemented correctly:
func Allowed(size int) bool {
return size <= 10
}
Suppose the test checks only Allowed(9) and expects true. Replacing <= with < leaves that test passing, even though the function now rejects 10.
Mutation testing automates this experiment: a tool changes the code and runs the tests. Each changed version is a mutant. A mutant is killed when tests detect the behavior change; it survives when they pass.
I ran go-mutesting against Allowed with two test suites: one covering only 9, and another covering the boundary values as well. It produced three mutations, each checked separately by the repository's mutation executor:
|
Mutated condition |
Case that detects it |
Expected and mutated result |
|---|---|---|
|
|
|
Expected |
|
|
|
Expected |
|
|
|
Expected |
All three conditions return true for 9, so the initial test missed every mutation. Adding cases for 10 and 11, with expectations taken from the requirement, killed all three. The experiment identified the missing boundary cases.
This result establishes that the added boundary cases detect these three changes. The mutation set covers one function and does not establish complete test coverage for a service.
The runner's interpretation of a failure affects the result. In the revision used here, go-mutesting's built-in runner can count a build failure or timeout as PASS, meaning that a mutation was detected.
I implemented a separate executor that reads go test -json events. It counts a mutant as killed only when the target test runs and fails with the expected assertion message. A successful run of that test means the mutant survived. A build failure, timeout, panic, missing target test, or unrelated assertion is an invalid result.
The executor saves the mutated source, classification, and command output for each run, and restores the original file afterward. I checked the classification with deliberate build failures, a timeout, an unrelated assertion, and a run without the target test. All four must be rejected as invalid evidence.
A surviving mutation may preserve the program's behavior, so it does not always call for a new test. Conversely, a test with the wrong expectation can protect a faulty implementation. Mutation testing cannot establish that requirements or expected results are correct.
Each mutant requires a test run. Start with a small piece of code, measure the time, and inspect the results. A large mutation set may be too expensive to run after every edit.
The examples use the avito-tech/go-mutesting fork. The repository README pins the revision and provides commands. The built-in runner tests the package containing the changed file. If consumer tests in other packages cover the behavior, arrange to run them separately. In this example, the function and its tests share a package.
Mutation operators also vary by tool. A tool may not support adding a struct field or removing the assignment you need to check. The repository therefore checks the lost field from the earlier example with a separate code change.
Practical Mutation Testing at Scale: A View from Google (2021) describes a review workflow that targets changed code, filters mutations likely to be irrelevant, and limits their number per line and per review. The paper is not specific to Go or coding agents. It explains why mutation selection matters as the workload grows.
The checks can now define a concrete task for a coding agent. For Forward, the task names the required behavior and the commands that establish whether a change meets it.
I reproduced the failure, applied the fix, and reran the checks manually against the repository. The output below comes from those command runs; the agent instruction is a proposed way to use the same checks.
Remove cancellation handling from Forward:
select {
case out <- value:
}
The rest of the function, including defer close(done), stays the same. Delivery still works when a receiver is available. Without one, the goroutine keeps waiting even after cancellation.
The project already has TestForwardCancellation. Give the agent this task:
Fix
Forwardininternal/report/report.go: when there is no receiver, context cancellation must stop the sender. Preserve successful delivery and closure ofdone. Run the cancellation test first. After the fix, rerun it and run the module tests with the race detector. If you cannot run a check, report why. Justify any changes to the tests or the contract separately.
Test changes need their own justification because an agent can remove a failure without fixing its cause. Adding t.Skip, for example, hides the problem while leaving the goroutine blocked.
Rewriting the test without synctest can also remove its ability to detect the violation. Simply removing <-done from the shown test is not enough to hide it: synctest still reports the blocked goroutine. Review test changes against the original acceptance criterion.
From the module directory, with dependencies installed, run:
go test -count=1 -timeout=10s -run '^TestForwardCancellation$' ./internal/report
For the version without cancellation handling, the command exits with code 1. The output begins:
--- FAIL: TestForwardCancellation (0.00s)
panic: deadlock: all goroutines in bubble are blocked [recovered, repanicked]
The test canceled the context and is waiting for the sender to finish. The sender is still blocked on the channel send, so done never closes. synctest reports the deadlock.
This stack excerpt is from the same run, with machine paths and addresses replaced by markers:
goroutine 7 [chan receive (durable), synctest bubble 1]:
example.com/guardrails/internal/report.TestForwardCancellation.func1(<address>)
<experiment>/example/internal/report/report_test.go:33 +<address>
goroutine 8 [chan send (durable), synctest bubble 1]:
example.com/guardrails/internal/report.Forward.func1()
<experiment>/example/internal/report/report.go:26 +<address>
The test goroutine is waiting to receive from done; the sender is blocked at out <- value. While the send is blocked, defer close(done) cannot run. The stack identifies both sides of the wait.
Add the cancellation case to select:
select {
case out <- value:
+case <-ctx.Done():
}
This restores the implementation from section 4. Run the same command again:
go test -count=1 -timeout=10s -run '^TestForwardCancellation$' ./internal/report
After the fix, it exits with code 0. One run produced:
ok example.com/guardrails/internal/report 0.258s
Execution time depends on the machine.
Next, check that the fix preserved the other behavior. Run the whole module's tests, including successful delivery:
go test -race -count=1 -timeout=30s ./...
That run passed too. In the proposed workflow, CI repeats this command on the revision under review using the same Go version. The cancellation test is part of the full suite.
The repository's verification scripts check both the code rules and the interpretation of their results. The same validation process can be applied before making a new check mandatory in a project: validate the configuration, test a known violation, and inspect findings on the existing code.
Start with configuration validation. For the complexity example:
golangci-lint config verify --config complexity.yml
The command validates settings against a schema and catches typos such as min-complexty, which an ordinary run may ignore. Valid settings can still target the wrong code. Run the rule against code that should pass and a known violation that should produce the expected diagnostic.
Check the message as well as the exit code. A tool that failed to start can also return a nonzero status.
I implemented this validation in verify.py and verify_mutation.py. They run faulty variants in temporary copies and inspect structured output. The Go test validator checks that the target package and test ran, then checks the expected diagnostic. The linter validator checks the analyzer name, source position, and diagnostic text.
The saved verification.json report records 50 successful verification steps on Go 1.27.1 and golangci-lint 2.13.2, running on macOS arm64. Those steps include configuration validation, baseline checks, known violations, and cases that must be rejected as invalid evidence. The separate mutation-verification.json report records the weak and boundary test suites and the four invalid-result controls.
For these scripts, detecting an expected test failure is a successful verification step. Acceptance of a code change still requires the ordinary tests and linters to pass.
Pay particular attention to settings that select files, packages, or types. A valid but incorrect path can leave the intended code unchecked. The storageRoot path and the View regular expression above are examples. The README covers offline configuration validation and reproduction commands.
Run the rule on the existing code. Fix configuration errors, agree on exceptions, and review the reported violations before making it block changes.
If there are many violations, first collect a report, then prevent new ones while fixing the old ones gradually. Remember that filtering by changed lines can hide new findings, as the complexity example showed.
Give the agent the check command. Its diagnostics should identify the cause:
The workflow is a short loop: change the code, run the check, inspect the diagnostic, fix the cause, and rerun the same check. Once the checks pass, review the change.
Run quick checks after edits and schedule slower tests and mutation runs at selected stages. Checks used to accept a change must also run in CI on the revision being reviewed.
If a tool cannot run, fix the execution problem and rerun it. A tooling failure tells you nothing about whether the code is correct.
The feedback loop distinguishes rule violations, passing checks, and check execution failures.
The repository combines executable requirements with checks of their scope and diagnostics. It covers behavior such as cancellation and resource cleanup, architecture rules that follow indirect dependencies, and tests of whether the guardrails detect known violations.
Review still needs to establish whether the contract is right for the application. That includes deciding whether a send is allowed when cancellation is also ready and whether report calculations should depend on storage.
Check the scope too. The cancellation test for Forward does not cover a new goroutine in another function. Linters and architecture tests cover only the code their configuration selects.
Changes to the checks deserve the same attention as changes to the code. Raising a threshold, adding an exclusion, deleting an assertion, or splitting a function just to lower its score can remove a finding while leaving the original problem.
The recorded runs establish that the selected checks detect the intended violations and that the validators reject the tested forms of invalid evidence. Agent behavior and token savings were not measured.
To apply this approach, choose a recurring review requirement and write down its acceptance criterion. Keep a known violation alongside the check, verify its diagnostic, and give the agent a command it can rerun after a fix. Review changes to that check against the same requirement.