Repository navigation
ci: go vet/lint steps don't use the per-module loop convention [cli part] #1478
Description
Activity
Reviewed commit:
be11bdcb5. Note:origin/mainmoved to3e9660d06during the review; re-verify against currentmainbefore changing code, since a finding may have been fixed or moved.The 2026-07-28 full review re-derived this independently and reached the same conclusion, so this is confirmation rather than a new issue. Adding the quantification and three details the original write-up does not have, plus a recommendation to raise the severity.
Where (line numbers from
3e9660d06).github/workflows/ci.yml:99(go test -race -short ./...) and:189(go test -tags=integration ./...), both from the repo root.github/workflows/ci.yml:48(golangci-lint, runs in repo root),:54(go vet ./...),:94(go mod download && go mod verify)go.work:3-10;Makefile:65,70.pre-commit-config.yaml:19(go-vethook, root only),:187(go-testhook ispre-push-stage only, so it never runs in CI)- The correct pattern already in the same file:
ci.yml:312(govulncheck per-module loop),:333(gosec per-module loop)
Quantification
Counted with
git ls-tree origin/mainplusgit grep -c '^func Test':Module packages .go files _test.go files func Test*never executedpkg11 61 30 351 providers/aws12 65 31 518 providers/azure14 38 17 516 providers/gcp5 15 8 184 total 42 179 (~30% of the repo's Go source) 86 1,569 Three details not in the original issue
-
Production code in these modules is still compiled.
go list -deps ./...from the root reaches 30providers/*packages and 10pkg/*packages through thereplacedirectives atgo.mod:191-197. So compile errors are caught today. What is never run isgo vet(vet analyses only named packages, not their dependencies), golangci-lint, and every one of the 1,569 test functions. Scoping the fix around this matters: the change is "run the existing tests", not "make these modules build". -
.golangci.yml:134listspkg/ladder/...in a misspell exclusion path. Someone wrote a lint exclusion for a module that lint has never visited. That is direct evidence the gap is invisible to authors, not merely unaddressed. -
The per-module loop pattern already exists in the same file.
ci.yml:308-310documents this exact./...behaviour for govulncheck, andci.yml:333loops all six modules for gosec. So the fix is applying an established, already-reviewed in-repo pattern to three more jobs, not a redesign. That should lower the perceived cost of doing it.
Failure scenario (why this warrants a severity bump)
These are the money paths.
providers/aws/services/{ec2,rds,savingsplans,...}andproviders/azure/services/computeexecute reservation and savings-plan purchases. A regression in reservation-term handling, idempotency-token scoping, or price math lands with a fully green CI, because the 518 AWS-provider and 516 Azure-provider tests that would have caught it are never invoked. This is precisely the "green tests are NOT proof the feature works" failure mode the repo has already been bitten by; here it is structural rather than incidental, since the tests do not merely fail to cover the path, they do not run at all.Suggest raising this from
severity/hightoseverity/criticaland keepingpriority/p1: it is the single largest gap in the project's verification story, and several other findings in this review are only findable by review because CI cannot see them.Fix direction (unchanged from the original, with one addition)
Replicate the
for mod in . pkg providers/aws providers/azure providers/gcp tests/e2eloop fromci.yml:312into theunit-tests,integration-testsandlintjobs. golangci-lint needsworking-directoryper module, because the action does not honourgo.work. Merge the per-module coverage profiles before the Codecov upload so the coverage number stops describing only the root module.- changed the title
[-]ci: go vet/unit-tests/integration-tests/lint only cover root module, silently skip providers/*, pkg, tests/e2e[/-][+]ci: go vet/lint steps don't use the per-module loop convention [cli part][/+]on Sep 27, 2026 This is a cross-component issue from before the CUDly monorepo split into four repos. The
providers/*/pkgmodules it originally referenced now live incloud-commitments-go, which has the genuine live version of this gap;tests/e2enow lives incloud-commitments-platform, with the same live gap. This repo (cloud-commitments-cli) is single-module, so the bug doesn't currently reproduce here; kept open as a smaller consistency/hardening item.- ci: go vet/lint only cover root module, silently skip pkg, providers/*, tests/e2e [go part] cloud-commitments-go#118
- ci: go vet/lint only cover root module, silently skip tests/e2e [platform part] cloud-commitments-platform#346
- ci: go vet/lint steps don't use the per-module loop convention [mcp part] cloud-commitments-mcp#21
Merged and locally verified: PR #2126, merge commit
93b9b6db5773a999cd55a06e2b2458853dea2ff4.The complete merged tree is identical to independently reviewed commit
1321c11f7af6fd02a67e67bf28dab50c69c5256a(both tree IDs4390ac73ec5d30e326fc6a4df8e2bf1471ae948f). Checked out the exact merge commit in the preserved local clone for fresh verification.Fresh macOS verification with Go 1.26.6 / golangci-lint 2.10.1: parsed and executed the actual merged YAML lint/vet blocks; both passed, and stdout/stderr match the pre-change baseline after removing only the new module banners. Native actionlint 1.7.12 passed. Five supplemental shell fixtures passed, including configuration failure preventing lint and lint/vet failure propagation.
This meets the post-split consistency criteria. The CLI remains one module; no new coverage or automatic module discovery is claimed. Pre-merge race-short tests, build and normal hooks also passed; they were not redundantly rerun for this identical tree. Main CI runs
36653580722and36653580751remain under root monitoring; no main-CI result is claimed here.Recommendation: the issue can remain closed. Follow-up audit found no separate actionable defect; no follow-up issues filed. Clone and branch preserved.
Summary
ci.yml's three core code-health checks silently skip every module except the root one, in a repo that has been multi-module (go.work:.,pkg,providers/aws,providers/azure,providers/gcp,tests/e2e) for some time:.github/workflows/ci.ymlaround line 53-54):go vet ./...go test -v -race -short -coverprofile=coverage.out -covermode=atomic ./...go test -v -race -tags=integration -coverprofile=coverage-integration.out ./...golangci-lint run --timeout=10m(no path arg)In a Go workspace, a bare
./...from the repo root only expands to packages in the current module (confirmed locally:go list ./...from repo root returns 38 packages, zero of which are underproviders/).go.workonly affects cross-module dependency resolution, not what./...expands to.The govulncheck and gosec steps already handle this correctly — both loop
for mod in . pkg providers/aws providers/azure providers/gcp tests/e2e; do (cd "$mod" && <tool> ./...); done, with an inline comment explaining exactly why. The same fix was never applied togo vet, unit tests, integration tests, or golangci-lint.Impact
providers/aws,providers/azure,providers/gcp,pkg, andtests/e2eget zerogo vet/lint coverage and their Go tests never run in CI, even though these packages contain the actual cloud-provider purchase logic (the highest money-risk code in the repo).providers/aws/services/ec2, sogo build/vet/testand lint for that package are exercised only by whatever a contributor runs locally, not by CI.providers/aws/services/ec2alone (not./...bare) surfaces ~27 pre-existing findings (misspellings, gocritic rangeValCopy, a gosec G117 field-name match, godot comment-period nits) that have presumably been accumulating silently since this gap opened.Suggested fix
Mirror the govulncheck/gosec per-module loop for the go vet, unit-test, integration-test, and golangci-lint steps. Since this will likely surface the accumulated per-module lint/vet debt (~27+ in
providers/awsalone, azure/gcp not yet surveyed), budget this as a two-part effort: (1) wire up the per-module loop, (2) batch-fix the debt it uncovers rather than suppressing it (per this repo's "no masking CI debt" convention) or gating it behindcontinue-on-error.Follow-up
Filed while adversarially verifying a merged PR; not fixed here to keep that fix scoped. Assessing severity as high (money-risk provider code has zero CI vet/test/lint coverage) but not critical (no active incident; the gap is latent, not currently causing production harm), priority p1 (should be next up, not urgent-drop-everything), effort m (loop change is small; the debt cleanup it uncovers is the real size unknown until surveyed).
Scope after the split
The CUDly monorepo was split into four repos. The
providers/aws,providers/azure,providers/gcp, andpkgmodules named above now live incloud-commitments-go; that repo has the genuine live version of this gap (go vetand lint still bare, still skipping five of its sixgo.workmodules) — see LeanerCloud/cloud-commitments-go#118.cloud-commitments-platforminherited thetests/e2emodule and has the same live gap for it — see LeanerCloud/cloud-commitments-platform#346.cloud-commitments-cliitself is a single-module repo (go.workonly declaresuse .): there is no second module forgo vet/lint to silently skip here today, and the unit-test/integration-test steps already loopfor mod in .. This issue is kept open, scoped down to a consistency/hardening item:go vetand the lint action are the only two CI checks in this repo not using that same loop convention, which would silently regress into this exact bug if a second module is ever added.Sibling issues:
Acceptance criteria (this repo)
go vetand the golangci-lint invocation are rewritten to use the samefor mod in .loop pattern as the unit-test/integration-test/govulncheck/gosec steps, for consistency and to guard against a silent regression if a second module is added later.go vet/lint output is unchanged before and after.