Summary
Issue #62304 (filed as sg73a1 in a prior Sergo run) was auto-closed as not_planned. Re-reading the code confirms the bug is still 100 percent present and untested. This is the 11th confirmed reverse-phantom across the Sergo history (an issue auto-expired by the tracker without the underlying code changing).
Location
pkg/linters/internal/resourcetracker/resourcetracker.go, inspectBody (lines 89-112) and inspectNode (lines 125-170).
Evidence
inspectBody creates exactly one map[K]*state entry per acquired resource key (line 142, tracked[acquired.Key] = &state{...}), then walks the entire function body with a single flat ast.Inspect (line 94-96) with no branch scoping. If a resource is acquired once and then an if/else statement has one arm call the cleanup manually and the sibling arm defer the cleanup, BOTH arms write into the SAME state object, since the traversal does not distinguish which branch it is currently inside. The manual arm sets st.hasManual = true; the sibling defer arm sets st.hasDefer = true. The final check at line 102 (st.hasManual and not st.hasDefer) then evaluates to false, so the manual-cleanup violation is silently masked, even though the two branches are mutually exclusive at runtime and the manual arm genuinely never gets a deferred release.
A new feature was added to this file since the bug was first filed: lines 136-143 now report a previously tracked violation before overwriting its state entry, when the SAME key is acquired a second time (see the ReassignReportsPreviousViolation test case in testdata/src/resourcetracker/resourcetracker.go lines 61-67). This fixes a different, narrower scenario (re-acquisition of the same variable), but it does not help the sibling-branch case described above, because in that case there is only ONE acquisition before the if/else, so the report-before-overwrite guard never triggers.
The testdata file has zero coverage for the sibling if/else branch scenario; only straight-line and single-branch cases (ConditionalAcquisition) are covered.
Concrete repro
func Example(mu *sync.Mutex, ok bool) {
mu.Lock()
if ok {
// do work
mu.Unlock() // manual cleanup, should be flagged but is masked
} else {
defer mu.Unlock()
}
}
Impact
This shared framework backs three linters at once: manualmutexunlock (CI-enforced both native and wasm builds, cgo.yml lines 1487 and 1490), fileclosenotdeferred, and contextcancelnotdeferred. A single fix to resourcetracker.go fixes all three simultaneously. No live production true positive was found via grep of Lock()/os.Open/context.With* call sites in pkg/ (all real sites use straight-line defer), so this is currently latent rather than actively causing a missed CI finding, but it remains an untested structural gap in a CI-enforced linter.
Recommendation
Track cleanup state per branch path rather than per resource key alone: for example, clone the tracked state when entering each arm of an if/else (or any construct with mutually exclusive control flow) and merge conservatively afterward, treating a manual-without-defer result in ANY branch as a reportable violation regardless of what a sibling branch does. Add a testdata case with an if/else where one arm calls cleanup manually and the sibling defers, expecting a diagnostic on the manual arm.
Validation checklist
Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 195.7 AIC · ⌖ 7.58 AIC · ⊞ 6.9K · ◷
Summary
Issue #62304 (filed as sg73a1 in a prior Sergo run) was auto-closed as not_planned. Re-reading the code confirms the bug is still 100 percent present and untested. This is the 11th confirmed reverse-phantom across the Sergo history (an issue auto-expired by the tracker without the underlying code changing).
Location
pkg/linters/internal/resourcetracker/resourcetracker.go, inspectBody (lines 89-112) and inspectNode (lines 125-170).
Evidence
inspectBody creates exactly one map[K]*state entry per acquired resource key (line 142, tracked[acquired.Key] = &state{...}), then walks the entire function body with a single flat ast.Inspect (line 94-96) with no branch scoping. If a resource is acquired once and then an if/else statement has one arm call the cleanup manually and the sibling arm defer the cleanup, BOTH arms write into the SAME state object, since the traversal does not distinguish which branch it is currently inside. The manual arm sets st.hasManual = true; the sibling defer arm sets st.hasDefer = true. The final check at line 102 (st.hasManual and not st.hasDefer) then evaluates to false, so the manual-cleanup violation is silently masked, even though the two branches are mutually exclusive at runtime and the manual arm genuinely never gets a deferred release.
A new feature was added to this file since the bug was first filed: lines 136-143 now report a previously tracked violation before overwriting its state entry, when the SAME key is acquired a second time (see the ReassignReportsPreviousViolation test case in testdata/src/resourcetracker/resourcetracker.go lines 61-67). This fixes a different, narrower scenario (re-acquisition of the same variable), but it does not help the sibling-branch case described above, because in that case there is only ONE acquisition before the if/else, so the report-before-overwrite guard never triggers.
The testdata file has zero coverage for the sibling if/else branch scenario; only straight-line and single-branch cases (ConditionalAcquisition) are covered.
Concrete repro
Impact
This shared framework backs three linters at once: manualmutexunlock (CI-enforced both native and wasm builds, cgo.yml lines 1487 and 1490), fileclosenotdeferred, and contextcancelnotdeferred. A single fix to resourcetracker.go fixes all three simultaneously. No live production true positive was found via grep of Lock()/os.Open/context.With* call sites in pkg/ (all real sites use straight-line defer), so this is currently latent rather than actively causing a missed CI finding, but it remains an untested structural gap in a CI-enforced linter.
Recommendation
Track cleanup state per branch path rather than per resource key alone: for example, clone the tracked state when entering each arm of an if/else (or any construct with mutually exclusive control flow) and merge conservatively afterward, treating a manual-without-defer result in ANY branch as a reportable violation regardless of what a sibling branch does. Add a testdata case with an if/else where one arm calls cleanup manually and the sibling defers, expecting a diagnostic on the manual arm.
Validation checklist