Skip to content

fix(ci): reject untidy files in every tracked Go module - #5778

Merged
devantler merged 3 commits into
mainfrom
codex/ci-tidy-modules-5326-r73
Nov 3, 2026
Merged

devantler merged 3 commits into
mainfrom
codex/ci-tidy-modules-5326-r73

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

Dependency updates can pass Platform CI with inconsistent module files, leaving an unrelated later change to discover the failure.

What

Require consistent dependencies across every tracked Go module before accepting a change, with a clear repair message and no automatic edits. Clean up the existing inconsistencies so the baseline satisfies the new requirement.

Fixes #5326

Part of #5326. Check module boundaries and use read-only tidy diffs before the required Go regression suite. Cover nested modules, sum drift, missing files, workspace redirection and failed discovery with real offline controls. Regenerate existing module drift with Go 1.26.10.
@devantler

devantler commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

At head 9a794cbaf425ae78e133c267e1687162fbfdf521, the real guard passes all nine tracked modules using Go 1.26.10, the version selected by this repository's CI. Existing drift was regenerated with that toolchain: unused sums in the root and admission module, and direct/unused requirements plus sums in the WebApp compiler. Module Go declarations are unchanged, including the intentional vulnerability control.

The offline fixture reproduces the original gap (ordinary Go tests accept an unused requirement) and passes 12 actual guard controls. These cover root/nested dependency drift, a newly indexed module, sum-only drift, a missing sum, inherited workspace/modfile settings, missing/linked manifests, linked directories, partial failed discovery and empty discovery. Module files are compared before and after each guard invocation. The fixture also passes with inherited GONOPROXY=* while using only its local module proxy.

ShellCheck, shfmt, actionlint, the policy routing/result-boundary fixture, and the affected Go packages validate-merge-group-heal, guard-arc-recovery and validate-eks-ci-role-policy pass. The existing required aggregate rejects failed, skipped, cancelled, empty and unknown Go regression results. The standalone complete Go regression command and read-only job permissions are preserved.

Independent static review found no remaining actionable correctness/security finding after the missing-manifest and proxy-isolation controls were added. Hosted CI, a current-head permitted review, protected merge and post-merge proof remain required; these local receipts are not a claim of delivery.

Hosted evidence at prior signed head 70fc3fa806f111131b7f726d23919c396621fedb: all 64 native checks passed. The complete Go regression job 125524140110 ran in 9m48s, including the nine-module tidy guard and 12 real controls; the required aggregate 125545777985 also passed.

Current signed head 387ede9f69c5699a24f64a75d5a9ab835606a67e tightens the four drift-fixture assertions after review: they must now identify an actual go.mod or go.sum diff, so unrelated toolchain/fetch failures cannot satisfy them. All 12 real controls pass locally with exact Go 1.26.10, and the inline finding is resolved. The production guard is unchanged. Current-head hosted CI and fresh substantive review remain pending; the prior head's green run is not substituted for either.

At current head 387ede9f69c5699a24f64a75d5a9ab835606a67e, all 64 native checks have now settled green. Review availability: cr:no-gate@387ede9f69c5699a24f64a75d5a9ab835606a67e. CodeRabbit 6589439537 refused at 21:55:09Z with a nine-minute included-review limit; no current-head CodeRabbit review exists and the live no-gate guard returns ADMIT. The next permitted lane is Codex. No review success or paid overage is inferred from the refusal.

Readiness at exact signed head 387ede9f69c5699a24f64a75d5a9ab835606a67e: all 64 hosted checks are settled green. The current-head complete Go job 125556199524 passed in 10m17s. Codex's substantive receipt 6589543008 names reviewed commit 387ede9f69 and reports no major issues. The prior inline finding is resolved and fresh thread readback is 0/1 unresolved.

As a user I exercised the actual nine-module read-only tidy guard and all 12 real module controls, including exact go.mod/go.sum drift assertions; missing and malformed boundaries fail before module work, and failed checks leave source unchanged. This validates the CI guard behavior, not a production change or a portfolio-wide latency improvement. All readable required gates pass. The configured code-quality rule remains unreadable to the helper and is left to the ordinary protected merge service, without an inferred pass.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor
Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 7754ca5 and 387ede9.

Files ignored due to path filters: 3
  • go.sum is excluded by !**/*.sum
  • scripts/tests/tenant-webapp-admission/go.sum is excluded by !**/*.sum
  • scripts/tests/webapp-compiler/go.sum is excluded by !**/*.sum
Files selected for processing: 7
  • .github/workflows/ci.yaml
  • AGENTS.md
  • docs/validation.md
  • scripts/check-go-modules-tidy.sh
  • scripts/tests/test-go-modules-tidy.sh
  • scripts/tests/webapp-compiler/go.mod
  • scripts/validate-merge-group-heal/go_regressions_test.go
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 7754ca5 and 70fc3fa.

Files ignored due to path filters: 3
  • go.sum is excluded by !**/*.sum
  • scripts/tests/tenant-webapp-admission/go.sum is excluded by !**/*.sum
  • scripts/tests/webapp-compiler/go.sum is excluded by !**/*.sum
Files selected for processing: 7
  • .github/workflows/ci.yaml
  • AGENTS.md
  • docs/validation.md
  • scripts/check-go-modules-tidy.sh
  • scripts/tests/test-go-modules-tidy.sh
  • scripts/tests/webapp-compiler/go.mod
  • scripts/validate-merge-group-heal/go_regressions_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Recent review details
Additional context used
Learnings: 1
Learning: 2026-08-10T13:01:12.782Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 3057
File: .github/workflows/ci.yaml:622-659
Timestamp: 2026-08-10T13:01:12.782Z
Learning: Repository shell tests and scripts must remain compatible with macOS Bash 3.2. Do not use Bash 4+ features such as `mapfile`; use portable constructs, such as a `while IFS= read -r` loop, instead.

Applied to files:

  • scripts/check-go-modules-tidy.sh
Additional comments: 6
scripts/check-go-modules-tidy.sh: 1

1-57: LGTM!

scripts/tests/webapp-compiler/go.mod: 1

6-8: LGTM!

.github/workflows/ci.yaml: 1

83-91: LGTM!

scripts/validate-merge-group-heal/go_regressions_test.go: 1

68-68: LGTM!

Also applies to: 70-71, 76-76, 96-96, 103-106

AGENTS.md: 1

86-86: LGTM!

docs/validation.md: 1

24-40: LGTM!

Walkthrough

The pull request adds a read-only check for every tracked Go module. The guard rejects failed or empty module discovery and invalid module paths, then runs go mod tidy -diff with ambient Go settings disabled. Offline tests cover tidy drift, refusal cases, and unchanged module files. The CI regression job runs the guard and its tests before the full Go test suite. Documentation describes the check and its failure conditions.

Possibly related PRs

Severity of issue fixed: Medium

Merge Risk: Low · previous review 70fc3

The guard’s tests could pass without proving that module drift was detected. Strengthen their assertions; this is a bounded risk to validation confidence.

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

Failed Failed checks: 1 warning · 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check Inconclusive The PR implements the coding requirements in #5326: the repository-local guard checks all tracked modules, rejects discovery and tidy failures, reports the module and go mod tidy, runs before the ex… Provide hosted CI evidence on the pull request that the guard passes for the tracked modules and that the required aggregate propagates a guard failure. Then provide the required real-PR proof for the tidy tree.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: CI rejects untidy files in every tracked Go module.
Description check Passed The description explains why the change is needed and what it does, including checks for all tracked Go modules and read-only failure reporting.
Out of Scope Changes check Passed The workflow, guard script, regression controls, validation documentation, contributor guidance, regression-order test, and webapp-compiler module cleanup all support the requirements in #5326. No u…
Full details: Out of Scope Changes check

Explanation

The workflow, guard script, regression controls, validation documentation, contributor guidance, regression-order test, and webapp-compiler module cleanup all support the requirements in #5326. No unrelated service, credential, cache-writer, or organization-wide workflow change appears in the supplied whole-PR summary.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 3 files. (4 skipped: 4 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Linked Issues check

Explanation

The PR implements the coding requirements in #5326: the repository-local guard checks all tracked modules, rejects discovery and tidy failures, reports the module and go mod tidy, runs before the existing Go regression command, and includes offline controls for nested modules, sum-only drift, missing sums, ambient settings, and unchanged module files. The summary also reports propagation through the required aggregate and preservation of the vulnerability fixture. However, #5326 requires proof on a real PR. The available evidence states that hosted CI and protected-merge proof remain outstanding, so hosted success and the final tidy-tree proof cannot be established.

Resolution

Provide hosted CI evidence on the pull request that the guard passes for the tracked modules and that the required aggregate propagates a guard failure. Then provide the required real-PR proof for the tidy tree.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Comment thread scripts/tests/test-go-modules-tidy.sh Resolved
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor
Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 9 minutes.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T22:00:26.670197Z 387ede9 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 387ede9f69

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@devantler
devantler marked this pull request as ready for review November 2, 2026 22:04
@devantler
devantler added this pull request to the merge queue Nov 2, 2026
@devantler devantler moved this from 🫴 Ready to 🚀 Ready to Merge in 🌊 Project Board Nov 2, 2026
Merged via the queue into main with commit 84977dd Nov 3, 2026
64 checks passed
@devantler
devantler deleted the codex/ci-tidy-modules-5326-r73 branch November 3, 2026 07:49
@github-project-automation github-project-automation Bot moved this from 🚀 Ready to Merge to ✅ Done in 🌊 Project Board Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Fail a pull request whose Go modules are not tidy

1 participant