ci: add gofmt enforcement and mint embed sync checks by jhutar · Pull Request #1714 · fullsend-ai/fullsend

jhutar · 2026-05-31T05:48:12Z

Summary

Run gofmt across all Go source files, fixing 18 formatting violations (including the switch role{ regression from feat(appsetup): add retro agent app configuration #828)
Add gofmt pre-commit hook that fails on unformatted Go files
Add lint-mint-embed-sync pre-commit hook that detects drift between internal/mint/ source files and their .embed copies in internal/dispatch/gcf/mintsrc/

Closes #960

Test plan

gofmt -l . returns empty after formatting
pre-commit run go-fmt --all-files passes clean
pre-commit run go-fmt fails when a gofmt violation is introduced
pre-commit run lint-mint-embed-sync --all-files passes clean
pre-commit run lint-mint-embed-sync fails when embed copy diverges from source
go vet ./... passes

🤖 Generated with Claude Code

fullsend-ai-review · 2026-05-31T05:56:00Z

Review

Findings

Medium

[protected-path] .pre-commit-config.yaml — This PR modifies a protected infrastructure file. The change is well-motivated (linked to Add gofmt check to pre-commit hooks or CI lint target #960, adds gofmt enforcement and mint embed sync hooks) and the rationale is clear, but human approval is always required for protected-path changes regardless of context.

Low

[scope-creep] hack/lint-mint-embed-sync — The lint-mint-embed-sync hook is not authorized by issue Add gofmt check to pre-commit hooks or CI lint target #960, which specifically requests gofmt enforcement and fixing the switch role{ violation. The mint embed sync hook enforces a separate CLAUDE.md requirement. The PR title is transparent about including both concerns, and the addition is well-motivated, but it would be cleaner to track this as a separate issue.
[edge-case] hack/lint-mint-embed-sync — The script uses cmp -s which suppresses all output including stderr. If a source or embed file is missing entirely (e.g., internal/mint/go.sum), the user sees a generic "DESYNC" message rather than a "file not found" error. The exit code is still non-zero (safe behavior), but the error message is imprecise. Consider adding explicit existence checks or not suppressing stderr.

Info

[import-organization] internal/cli/postreview_test.go:4 — gofmt sorted stdlib imports (io, testing) after third-party imports (testify/assert, testify/require) because they were in a single ungrouped import block with no blank-line separator. This is correct gofmt behavior (alphabetical within a block), but not idiomatic Go style (stdlib and third-party are conventionally separated). A follow-up with goimports would restore conventional grouping across the codebase.

Previous run

Review

Findings

Medium

[protected-path] .pre-commit-config.yaml — This PR modifies a protected infrastructure file. The change is well-motivated (linked to Add gofmt check to pre-commit hooks or CI lint target #960, adds gofmt enforcement and mint embed sync hooks) and the rationale is clear, but human approval is always required for protected-path changes regardless of context.

Low

[scope-creep] hack/lint-mint-embed-sync — The lint-mint-embed-sync hook is not authorized by issue Add gofmt check to pre-commit hooks or CI lint target #960, which specifically requests gofmt enforcement and fixing the switch role{ violation. The mint embed sync hook enforces a separate CLAUDE.md requirement. The PR title is transparent about including both concerns, and the addition is well-motivated, but it would be cleaner to track this as a separate issue.
[edge-case] hack/lint-mint-embed-sync — The script suppresses stderr from diff -q, so if a source or embed file is missing entirely, the user sees a generic "DESYNC" message rather than a "file not found" error. The exit code is still non-zero (safe behavior), but the error message is imprecise. Consider adding explicit existence checks or not suppressing stderr.

Previous run (2)

Review

Findings

Medium

[protected-path] .pre-commit-config.yaml — This PR modifies a protected governance file. The change is well-justified (closes Add gofmt check to pre-commit hooks or CI lint target #960, adds gofmt and mint-embed-sync enforcement), but human approval is always required for protected-path changes regardless of context.

Info

[style] internal/cli/postreview_test.go:7 — gofmt sorted stdlib imports (io, testing) into the same block as third-party imports (testify/assert, testify/require) because they were in a single ungrouped import block. This is valid gofmt output but not idiomatic Go style (stdlib and third-party are conventionally separated by a blank line). A follow-up with goimports would restore conventional grouping across the codebase.

github-actions · 2026-06-01T09:58:30Z

Site preview

Preview: https://bc23bcd2-site.fullsend-ai.workers.dev

Commit: 2ab81fec29a25ade46d8cbcdce7b751623f3fe85

ifireball · 2026-06-02T14:47:37Z

+
+rc=0
+for f in main.go go.mod go.sum; do
+    if ! diff -q "$SRC/$f" "$DST/$f.embed" >/dev/null 2>&1; then


can use cmp here which is a little faster.

Hello! Thank you, doing that.

ifireball · 2026-06-02T14:52:42Z

+DST="$REPO_ROOT/internal/dispatch/gcf/mintsrc"
+
+rc=0
+for f in main.go go.mod go.sum; do


I thing its dangarous to assume here that no additional files would be added to the mint codebase, we already have PRs in flight that add files

Hello Barak. Does it mean we should require anything in internal/mint/ that is not "*_test.go" needs to be mirrored to internal/dispatch/gcf/mintsrc/?

Hello Barak. Does it mean we should require anything in internal/mint/ that is not "*_test.go" needs to be mirrored to internal/dispatch/gcf/mintsrc/?

probably, yes.

rh-hemartin

I agree with @ifireball comments, but I'm approving as my concerns were covered and he still have @ifireball request for changes.

Fix formatting violations across 18 files, including the switch role{} regression on types.go flagged in fullsend-ai#960. Sync mint embed copy. Closes fullsend-ai#960 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Generated-by: Claude rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Jan Hutar <jhutar@redhat.com>

Enforce gofmt formatting via pre-commit so violations fail before commit. Placed before go-vet in the local hooks section. Ref fullsend-ai#960 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Generated-by: Claude rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Jan Hutar <jhutar@redhat.com>

Add lint-mint-embed-sync hook that verifies main.go, go.mod, and go.sum in internal/mint/ match their .embed copies in internal/dispatch/gcf/mintsrc/. Runs only when either directory is touched. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Generated-by: Claude rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Jan Hutar <jhutar@redhat.com>

Let pre-commit pass staged Go files to gofmt instead of scanning the entire tree. Uses gofmt -w to auto-fix formatting in place. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Generated-by: Claude rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED

cmp -s is faster for byte-level equality checks where we don't need the actual diff output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Generated-by: Claude rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED

jhutar force-pushed the 1-gofmt branch from 13072f9 to 4349def Compare May 31, 2026 05:51

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label May 31, 2026

rh-hemartin requested changes Jun 1, 2026

View reviewed changes

Comment thread .pre-commit-config.yaml Outdated

Comment thread .pre-commit-config.yaml

rh-hemartin assigned jhutar Jun 1, 2026

github-actions Bot deployed to site-preview June 1, 2026 09:58 View deployment

jhutar force-pushed the 1-gofmt branch from 4349def to 2ab81fe Compare June 2, 2026 07:22

fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 2, 2026

github-actions Bot deployed to site-preview June 2, 2026 14:48 View deployment

ifireball requested changes Jun 2, 2026

View reviewed changes

rh-hemartin approved these changes Jun 3, 2026

View reviewed changes

jhutar added 5 commits June 3, 2026 08:52

jhutar force-pushed the 1-gofmt branch from 6827140 to fd2eba2 Compare June 3, 2026 06:52

fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 3, 2026

Conversation

jhutar commented May 31, 2026

Summary

Test plan

Uh oh!

fullsend-ai-review Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Review

Findings

Medium

Info

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

ifireball Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

jhutar Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

ifireball Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

jhutar Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

ifireball Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

rh-hemartin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fullsend-ai-review Bot commented May 31, 2026 •

edited

Loading

github-actions Bot commented Jun 1, 2026 •

edited

Loading