Repository navigation
staticcheck flags a possible typed-nil interface guard in writ — locally, but not in CI #669
Description
./build/star lint go ./... reports four staticcheck SA4023 findings that CI does not:
cmd/writ/writ/deploy/deploy.go:389:31 SA4023: this comparison is always true
cmd/writ/writ/readback/readback.go:295:12 SA4023(related): the lhs is the 1st return value of this call
cmd/writ/writ/readback/readback.go:296:5 SA4023: this comparison is never true
Both sites guard the same call:
origin := graph.Origin()
if origin == nil || origin.Tool() != "writ" {
return
}if origin := graph.Origin(); origin != nil && origin.Scope() != "" {Why this may be real
op.Origin is an interface (pkg/op/origin.go:16) and Graph.Origin() returns the interface field directly (pkg/op/graph.go:699). SA4023 fires on an impossible interface-vs-nil comparison — the classic typed-nil trap. If a nil *OriginBase is ever boxed into that field, origin != nil evaluates true and the next line calls origin.Tool() on a nil pointer.
The three assignment sites are graph.go:153, :208, and :478. Whether any can box a nil concrete pointer is the thing to check.
If it can, both guards are ineffective and the panic is one nil origin away. If it cannot, the comparisons are dead code that should say so.
Why it matters beyond the two sites
CI does not report these. A run on #668 reported exactly four issues, all of them mine in mem, and none of these. So the local gate and the CI gate disagree about what staticcheck finds.
That divergence is worth understanding on its own: either CI's lint is weaker than a developer's — in which case a class of defect reaches develop unflagged — or the local toolchain (Go 1.27) infers something CI's older toolchain does not, in which case these may be false positives that will start failing CI on the next toolchain bump.
Either way, "green in CI" and "green locally" currently mean different things, and neither is a superset of the other.
Found how
While fixing dead code that CI's lint caught on #668 and my local make check could not — make check's golangci-lint stage fails on Go 1.27's own standard library, so I had been treating lint as locally unavailable. ./build/star lint go ./... is the gate CI actually runs and it works fine locally. Discovering that is what surfaced this.
Activity
David-Noble-at-work commentedon Aug 25, 2026
Correction: not a nil-panic risk, and not safe to simply delete
I filed this as a possible typed-nil interface trap. That was wrong, and I reasoned from a pointer receiver I had not checked.
OriginBase.Toolhas a value receiver:An
OriginBasevalue boxed into theOrigininterface is never nil, and its methods work fine on a zero value. So there is no panic risk at either guard — staticcheck is right that the comparisons are impossible, but they are dead guards, not a hazard.But deleting them is not obviously safe either
The three sites that populate
Graph.originare not uniform:pkg/op/graph.go:145var graphOrigin OriginBase— a valuepkg/op/graph.go:808Origin OriginBase— a valuepkg/op/graph.go:1083Origin Origin— an interfaceThe third can hold a nil interface. If any path assigns it nil — a decoded document with no origin, say — then
origin == nilis meaningful and the guard is load-bearing, and staticcheck's "impossible" is drawn from the other two.So the fix is not "remove the dead check." It is to determine whether that third path can produce a nil interface, and then either normalize it to a zero
OriginBase(making the guards genuinely dead and removable) or keep the guards and silence the check at the site with a reason.Why this is not being fixed opportunistically
make lintnow runs the same tool CI runs (#670), so this finding is red locally. Deleting a nil-guard to turn a gate green — without establishing that nil cannot arrive — is exactly the failure mode that made the gate worth unifying. The red stays until the third path is understood.Note the two guards are also asymmetric in consequence:
readback.go:295callsorigin.Tool()after the check, whiledeploy.go:389callsorigin.Scope(). Both are value-receiver methods today, so both are safe today — that safety is a property ofOriginBase's receiver choice, not of the guards.David-Noble-at-work commentedon Aug 26, 2026
Fixed.
make lintreports 0 issues; test counts unchanged at 34 / 69 / 201.The open question is answered: the panic could not happen
This issue asked whether a nil concrete pointer can be boxed into the
Origininterface — "if it can, bothguards are ineffective and the panic is one nil origin away."
It cannot.
graphMetadata.originis typedOriginBase(pkg/op/graph.go:1207) — the concrete valuetype, not the
Origininterface and not a pointer. All three assignment sites store a value:graph.go:145declaresvar graphOrigin OriginBaseand type-asserts into it, so a nil or non-OriginBasespec value yields the zero origin
graph.go:208passesmetadata.origin, alreadyOriginBasegraph.go:478, the deserialization path, passesp.Origininto that sameOriginBasefieldNo
*OriginBaseis boxed anywhere inpkg/orcmd/, and everyOriginmethod has a value receiver, soNewOriginBasereturns a value in the first place.Graph.Origin()therefore never returns nil.staticcheck was right on all four findings, and they were not false positives. The guards were dead code.
Fixed
cmd/internal/cli/store.goif origin := graph.Origin(); origin != nil { … }→ unconditional; the block always rancmd/writ/writ/deploy/deploy.goorigin != nil &&;origin.Scope() != ""was always the real testcmd/writ/writ/readback/readback.gopkg/op/graph.goOrigin()— never nil, absent origin reports the zero value, callers testTool/Scoperather than the interfaceThe doc comment is what makes it stick: without it the next reader sees an interface return and re-adds a guard.
The divergence half moves to #670
This issue also asked why CI did not report these. Measured 2026-08-25 — it is version skew, not tool choice:
ci.yaml:32)ci.yaml:73)CI already runs both tools —
./build/star lint go ./...(ci.yaml:89) andmake lint-all(
ci.yaml:95). Localmake checkruns onlylint, i.e.golangci-lint run.So the gaps run in both directions:
were real defects, CI missed genuine dead code and will begin reporting it on the next toolchain bump.
make checkis weaker on GOOS — it runslint(host only); CI runslint-allacross linux, darwin,and windows.
That is #670's thesis exactly. Closing here; the divergence is tracked there.
chore: sandbox vocabulary, lint-gate parity, and the access-directive…
Providers claim what they guarantee, and a type-checked pass holds th…