apply: header may void Go //go:build / // +build constraints (verify + fix) #6

New Issue

Closed

opened 2026-06-04 16:06:53 +00:00 by KofTwentyTwo · 2 comments

KofTwentyTwo commented 2026-06-04 16:06:53 +00:00 (Migrated from github.com)

Copy link

Copy source

Summary

When license-tool inserts a header into a Go file that begins with a build-constraint block (//go:build ... and/or legacy // +build ...), the header is spliced in at byte 0 — before the constraint lines. Go's build-constraint rules (go/build) require the constraint to appear in the file's leading comment group, before the package clause, preceded only by blank lines and // line comments and followed by a blank line. Inserting our /* ... */ block comment ahead of the constraint very likely demotes the constraint to an ordinary comment, silently voiding the build tag. A file that was meant to compile only on linux (or only under a custom tag) would then compile unconditionally — a correctness/portability regression introduced by a tool whose entire job is to leave source semantics intact.

This is the direct analogue of the shebang bug fixed in PR #5 / v0.2.1, which made a leading #! preserved universally in both boundary implementations. Build constraints are another "must-be-first" construct that the Go file type currently does not preserve.

This issue asks the implementer to FIRST verify the breakage with a test, then fix it if confirmed.

Type / Severity

• Type: Bug (source-semantics corruption on insert)
• Severity: High — silently changes which platforms/configurations a Go file compiles for. No error, no diff warning; the file still parses and builds, just with the wrong constraint set.

Reproduction

Verification-first. The build constraint must sit in the leading comment group, before package, with a blank line after it.

Create a sample file buildtag.go:

//go:build linux

package foo

(Also worth covering the legacy form and the combined form:)

//go:build linux
// +build linux

package foo

Run the tool's apply path against it (adjust the binary/subcommand to match the CLI; the relevant logic is render.Insert / render.Splice driven by the applier):

license-tool apply --holder "Acme" --license MIT buildtag.go
cat buildtag.go

Observed (current behavior): the header lands at byte 0, ahead of the constraint:

/*
SPDX-License-Tool: license-tool:managed
SPDX-FileCopyrightText: 2026 Acme
SPDX-License-Identifier: MIT
*/

//go:build linux

package foo

Now the //go:build linux line is no longer in the file's leading comment group preceded only by blank lines and line comments — a block comment precedes it — so Go's constraint parser does not treat it as a constraint.

Confirm the constraint was voided (any one of these is sufficient acceptance evidence):

• go/build: ctxt.MatchFile(dir, "buildtag.go") returns true for a non-linux GOOS after the header is inserted, where it returned false before. (go/build.Context.MatchFile reads the file and evaluates its build constraints.)
• go vet / build: with a tag the file should be excluded from, the file is now (wrongly) included; or go list -f '{{.GoFiles}}' under a non-matching GOOS/tag now lists the file when it should not.
• A direct unit assertion using go/build/constraint: parse the leading comment group and assert the constraint is no longer recognized after insertion.

If — and only if — verification shows the constraint is voided, apply the fix below. (It is theoretically possible Go tolerates a leading block comment before the constraint group; the PR must record the verification result either way.)

Root cause

The Go file type does not list build constraints as a preserve-first construct, so the computed insertion boundary stops at byte 0 (after an optional BOM), which is ahead of any leading //go:build / // +build lines.

• internal/filetype/filetype.go — the Go builtin entry:

  {
      Name:          "Go",
      Extensions:    []string{".go"},
      CommentStyle:  block("/*", "*/"),
      PreserveFirst: []model.PreserveRule{after(model.PreserveBOM), before(model.PreservePackageDecl)},
  },
  

  The only AFTER rule is PreserveBOM; PreservePackageDecl is a before rule (header goes before package, boundary does not advance past it). There is no rule covering the build-constraint comment group, so the boundary is byte 0 (post-BOM) — i.e. before the constraint.

• internal/render/render.go — preserveBoundary(content, ft) (the insert path used by Insert/Splice → insertAt). It consumes BOM, then the universal #! shebang, then iterates ft.PreserveFirst AFTER rules (PreserveShebang, PreserveXMLDecl, PreservePHPOpen, PreserveCodingPragma). There is no case for a Go build-constraint group, so for a .go file it returns the post-BOM offset (0) and insertAt splices the header there — ahead of //go:build.

• internal/detect/detect.go — PreserveBoundary(content, ft) (the detect path). Same gap: it consumes BOM, universal #!, then only the gated allowXML/allowPHP/allowPragma lines. A leading //go:build line is not a #! and is not consumed, so the boundary stops before it. Worse, because //go:build and // +build are // line comments, leadingCommentRegion → lineCommentRegion will treat them as the start of the leading comment run; a managed header inserted below them (after the fix) plus these constraint lines could be folded into one detected region unless the boundary correctly steps past the constraint group first. This is why both boundaries must change in lockstep, exactly as the shebang fix did.

Why the shebang precedent is the right template: PR #5 / v0.2.1 made a leading #! preserved universally in both render.preserveBoundary and detect.PreserveBoundary (consumed right after the BOM, independent of the per-type rule list, guarded so it only fires when the line literally starts with #! at the very top of the file). Go build constraints are the same shape of problem — a construct that MUST lead the file — but they are Go-specific, so unlike the universal shebang they should be gated on the file type (or on a new PreserveKind) rather than applied to all languages.

Proposed fix

Preserve the leading Go build-constraint block so the header lands after it (and after the blank line that must follow it, keeping the constraint group's required trailing blank line intact).

Define the constraint block precisely: starting at the current boundary (after BOM and any universal #!), the block is the contiguous run of lines that are either //go:build ... or // +build ... constraint lines, plus any blank lines interleaved/immediately following, up to and including the blank line that terminates the leading comment group. Per Go's rules the constraint group is the leading // comments followed by a blank line before package. The header must be inserted after that blank line.

Concretely (mirroring the universal-shebang implementation):

1. New preserve kind + file type rule. Add a PreserveGoBuildConstraint (name to match the existing PreserveKind enum in internal/model) and add it as an after(...) rule to the Go entry in internal/filetype/filetype.go:

   PreserveFirst: []model.PreserveRule{after(model.PreserveBOM), after(model.PreserveGoBuildConstraint), before(model.PreservePackageDecl)},
   

   Gate it on the file type (Go only) rather than universally — a leading //go:build-looking line is only build-constraint-significant in Go, and we must not over-consume //-comment headers in C-family or JS files.

2. render path — internal/render/render.go preserveBoundary. After consuming BOM and the universal #!, when the type lists the new rule, advance the boundary over the contiguous constraint block: consume each leading line that is //go:build or // +build (trim leading whitespace; be \r\n/\n-agnostic exactly like advanceLineIf), then consume the single terminating blank line so the inserted header sits after the constraint group's required blank separator. Reuse the existing line-walking helpers (advanceLineIf, indexByteFrom) and the EOL-agnostic trimming already used for #!.

3. detect path — internal/detect/detect.go PreserveBoundary. Make the identical change in the line-consuming loop: when ft lists the new rule, treat a leading //go:build / // +build line as consumable (and consume the terminating blank line) so detection's boundary matches render's and leadingCommentRegion begins after the constraint group. The two boundaries must agree byte-for-byte (the package doc invariant in render.go calls this out: producer and splicer must agree on the header region's shape). Keep the existing var preserveBoundaryFn = PreserveBoundary seam working.

4. Keep BOM-first and the universal shebang ordering. Order must be: BOM → universal #! → Go build-constraint block → (package decl is before, never consumed). Do not gate the existing universal #! behind the file type; leave it as-is.

5. Note on cgo /* ... */ import "C". This is not a concern for this fix: the cgo preamble comment is attached to the import "C" statement which comes after the package clause, so it is below the insertion boundary and is never at byte 0. No special handling needed; the header still goes before package.

6. 100% coverage gate must stay green. The repo enforces 100% coverage via .testcoverage.yml (CI gate). Every new branch — Go file with //go:build only, with // +build only, with both, with a preceding BOM, with CRLF endings, with a #! line then a constraint (rare but possible for go run scripts), and a Go file with no constraint (must be unchanged) — needs test coverage. Add tests in both internal/render and internal/detect (and internal/filetype if the table change needs a lookup assertion). Do not introduce unreachable branches; if the new PreserveKind switch arm in either boundary has paths that can't be hit, they will fail the gate.

Tests & acceptance criteria

• Verification test (write first): a test that inserts a header into //go:build linux\n\npackage foo\n and asserts, via go/build Context.MatchFile (or go/build/constraint parsing of the leading comment group), that the constraint is still honored afterward. This test should FAIL on main (demonstrating the bug) and PASS after the fix.
• Render boundary tests (internal/render): for each input shape (constraint-only, legacy // +build only, both forms, BOM + constraint, CRLF, #! + constraint, no constraint), assert the header is inserted after the constraint block and its trailing blank line, and that bytes before the insertion point are byte-identical to the original (BOM, constraint lines, EOLs, blank line preserved).
• Detect boundary tests (internal/detect): assert PreserveBoundary returns the offset after the constraint block; assert a managed header placed after the constraint round-trips through Detect (span starts after the constraints, not absorbing them) and that re-applying is idempotent (action == "none").
• Negative tests: a Go file with no constraint is untouched (boundary stays at byte 0/post-BOM, header before package). A C/JS file with a leading // foo line is NOT treated as a Go build constraint (rule is Go-gated) — guard against regressing the universal //-comment handling.
• Round-trip / idempotency: applying twice yields no second edit; relicensing replaces only the header span and leaves the constraint block intact.
• Coverage: go test ./... passes and the .testcoverage.yml 100% gate stays green (no uncovered new branches).

References

• internal/filetype/filetype.go — Go builtin entry (PreserveFirst currently after(PreserveBOM), before(PreservePackageDecl)); block/after/before helpers; model.PreserveKind rule set.
• internal/render/render.go — preserveBoundary (insert boundary), insertAt, Insert, Splice, advanceLineIf, indexByteFrom; the universal #! block ("A leading "#!" is a kernel-level shebang ... preserved universally"); the package-doc SAFETY INVARIANTS (header placed AFTER preserve-first / BEFORE package; bytes outside the header region never altered; producer and splicer must agree byte-for-byte).
• internal/detect/detect.go — PreserveBoundary (detect boundary, same universal #! block), preserveBoundaryFn seam, leadingCommentRegion / lineCommentRegion (would otherwise fold leading //go:build lines into the detected header run).
• Precedent: PR #5 / v0.2.1 — universal shebang preservation in both boundary implementations. Use it as the structural template, but gate the Go build-constraint rule on the file type rather than applying it universally.
• Go reference: go/build build constraints — the constraint must be in the leading comment group before package, preceded only by blank lines and // line comments and followed by a blank line (https://pkg.go.dev/go/build#hdr-Build_Constraints, https://pkg.go.dev/cmd/go#hdr-Build_constraints).

🤖 Generated with Claude Code

## Summary When `license-tool` inserts a header into a Go file that begins with a build-constraint block (`//go:build ...` and/or legacy `// +build ...`), the header is spliced in at byte 0 — **before** the constraint lines. Go's build-constraint rules (`go/build`) require the constraint to appear in the file's leading comment group, before the `package` clause, preceded only by blank lines and `//` line comments and **followed by a blank line**. Inserting our `/* ... */` block comment ahead of the constraint very likely demotes the constraint to an ordinary comment, silently **voiding the build tag**. A file that was meant to compile only on `linux` (or only under a custom tag) would then compile unconditionally — a correctness/portability regression introduced by a tool whose entire job is to leave source semantics intact. This is the direct analogue of the shebang bug fixed in PR #5 / v0.2.1, which made a leading `#!` preserved **universally** in both boundary implementations. Build constraints are another "must-be-first" construct that the Go file type currently does not preserve. **This issue asks the implementer to FIRST verify the breakage with a test, then fix it if confirmed.** ## Type / Severity - **Type:** Bug (source-semantics corruption on insert) - **Severity:** High — silently changes which platforms/configurations a Go file compiles for. No error, no diff warning; the file still parses and builds, just with the wrong constraint set. ## Reproduction Verification-first. The build constraint must sit in the leading comment group, before `package`, with a blank line after it. Create a sample file `buildtag.go`: ```go //go:build linux package foo ``` (Also worth covering the legacy form and the combined form:) ```go //go:build linux // +build linux package foo ``` Run the tool's apply path against it (adjust the binary/subcommand to match the CLI; the relevant logic is `render.Insert` / `render.Splice` driven by the applier): ```sh license-tool apply --holder "Acme" --license MIT buildtag.go cat buildtag.go ``` Observed (current behavior): the header lands at byte 0, ahead of the constraint: ```go /* SPDX-License-Tool: license-tool:managed SPDX-FileCopyrightText: 2026 Acme SPDX-License-Identifier: MIT */ //go:build linux package foo ``` Now the `//go:build linux` line is no longer in the file's leading comment group preceded only by blank lines and line comments — a block comment precedes it — so Go's constraint parser does not treat it as a constraint. **Confirm the constraint was voided** (any one of these is sufficient acceptance evidence): - `go/build`: `ctxt.MatchFile(dir, "buildtag.go")` returns `true` for a non-`linux` `GOOS` after the header is inserted, where it returned `false` before. (`go/build.Context.MatchFile` reads the file and evaluates its build constraints.) - `go vet` / build: with a tag the file should be excluded from, the file is now (wrongly) included; or `go list -f '{{.GoFiles}}'` under a non-matching `GOOS`/tag now lists the file when it should not. - A direct unit assertion using `go/build/constraint`: parse the leading comment group and assert the constraint is no longer recognized after insertion. If — and only if — verification shows the constraint is voided, apply the fix below. (It is theoretically possible Go tolerates a leading block comment before the constraint group; the PR must record the verification result either way.) ## Root cause The Go file type does not list build constraints as a preserve-first construct, so the computed insertion boundary stops at byte 0 (after an optional BOM), which is ahead of any leading `//go:build` / `// +build` lines. - `internal/filetype/filetype.go` — the `Go` builtin entry: ```go { Name: "Go", Extensions: []string{".go"}, CommentStyle: block("/*", "*/"), PreserveFirst: []model.PreserveRule{after(model.PreserveBOM), before(model.PreservePackageDecl)}, }, ``` The only AFTER rule is `PreserveBOM`; `PreservePackageDecl` is a `before` rule (header goes *before* `package`, boundary does not advance past it). There is no rule covering the build-constraint comment group, so the boundary is byte 0 (post-BOM) — i.e. before the constraint. - `internal/render/render.go` — `preserveBoundary(content, ft)` (the insert path used by `Insert`/`Splice` → `insertAt`). It consumes BOM, then the universal `#!` shebang, then iterates `ft.PreserveFirst` AFTER rules (`PreserveShebang`, `PreserveXMLDecl`, `PreservePHPOpen`, `PreserveCodingPragma`). There is no case for a Go build-constraint group, so for a `.go` file it returns the post-BOM offset (0) and `insertAt` splices the header there — ahead of `//go:build`. - `internal/detect/detect.go` — `PreserveBoundary(content, ft)` (the detect path). Same gap: it consumes BOM, universal `#!`, then only the gated `allowXML`/`allowPHP`/`allowPragma` lines. A leading `//go:build` line is not a `#!` and is not consumed, so the boundary stops before it. Worse, because `//go:build` and `// +build` are `//` line comments, `leadingCommentRegion` → `lineCommentRegion` will treat them as the start of the leading comment run; a managed header inserted *below* them (after the fix) plus these constraint lines could be folded into one detected region unless the boundary correctly steps past the constraint group first. This is why both boundaries must change in lockstep, exactly as the shebang fix did. Why the shebang precedent is the right template: PR #5 / v0.2.1 made a leading `#!` preserved **universally** in both `render.preserveBoundary` and `detect.PreserveBoundary` (consumed right after the BOM, independent of the per-type rule list, guarded so it only fires when the line literally starts with `#!` at the very top of the file). Go build constraints are the same shape of problem — a construct that MUST lead the file — but they are Go-specific, so unlike the universal shebang they should be gated on the file type (or on a new `PreserveKind`) rather than applied to all languages. ## Proposed fix Preserve the leading Go build-constraint block so the header lands **after** it (and after the blank line that must follow it, keeping the constraint group's required trailing blank line intact). Define the constraint block precisely: starting at the current boundary (after BOM and any universal `#!`), the block is the contiguous run of lines that are either `//go:build ...` or `// +build ...` constraint lines, **plus** any blank lines interleaved/immediately following, up to and including the blank line that terminates the leading comment group. Per Go's rules the constraint group is the leading `//` comments followed by a blank line before `package`. The header must be inserted after that blank line. Concretely (mirroring the universal-shebang implementation): 1. **New preserve kind + file type rule.** Add a `PreserveGoBuildConstraint` (name to match the existing `PreserveKind` enum in `internal/model`) and add it as an `after(...)` rule to the `Go` entry in `internal/filetype/filetype.go`: ```go PreserveFirst: []model.PreserveRule{after(model.PreserveBOM), after(model.PreserveGoBuildConstraint), before(model.PreservePackageDecl)}, ``` Gate it on the file type (Go only) rather than universally — a leading `//go:build`-looking line is only build-constraint-significant in Go, and we must not over-consume `//`-comment headers in C-family or JS files. 2. **render path — `internal/render/render.go` `preserveBoundary`.** After consuming BOM and the universal `#!`, when the type lists the new rule, advance the boundary over the contiguous constraint block: consume each leading line that is `//go:build` or `// +build` (trim leading whitespace; be `\r\n`/`\n`-agnostic exactly like `advanceLineIf`), then consume the single terminating blank line so the inserted header sits after the constraint group's required blank separator. Reuse the existing line-walking helpers (`advanceLineIf`, `indexByteFrom`) and the EOL-agnostic trimming already used for `#!`. 3. **detect path — `internal/detect/detect.go` `PreserveBoundary`.** Make the identical change in the line-consuming loop: when `ft` lists the new rule, treat a leading `//go:build` / `// +build` line as consumable (and consume the terminating blank line) so detection's boundary matches render's and `leadingCommentRegion` begins **after** the constraint group. The two boundaries must agree byte-for-byte (the package doc invariant in `render.go` calls this out: producer and splicer must agree on the header region's shape). Keep the existing `var preserveBoundaryFn = PreserveBoundary` seam working. 4. **Keep BOM-first and the universal shebang ordering.** Order must be: BOM → universal `#!` → Go build-constraint block → (package decl is `before`, never consumed). Do not gate the existing universal `#!` behind the file type; leave it as-is. 5. **Note on cgo `/* ... */ import "C"`.** This is **not** a concern for this fix: the cgo preamble comment is attached to the `import "C"` statement which comes *after* the `package` clause, so it is below the insertion boundary and is never at byte 0. No special handling needed; the header still goes before `package`. 6. **100% coverage gate must stay green.** The repo enforces 100% coverage via `.testcoverage.yml` (CI gate). Every new branch — Go file with `//go:build` only, with `// +build` only, with both, with a preceding BOM, with CRLF endings, with a `#!` line then a constraint (rare but possible for `go run` scripts), and a Go file with *no* constraint (must be unchanged) — needs test coverage. Add tests in both `internal/render` and `internal/detect` (and `internal/filetype` if the table change needs a lookup assertion). Do not introduce unreachable branches; if the new `PreserveKind` switch arm in either boundary has paths that can't be hit, they will fail the gate. ## Tests & acceptance criteria - **Verification test (write first):** a test that inserts a header into `//go:build linux\n\npackage foo\n` and asserts, via `go/build` `Context.MatchFile` (or `go/build/constraint` parsing of the leading comment group), that the constraint is **still honored** afterward. This test should FAIL on `main` (demonstrating the bug) and PASS after the fix. - **Render boundary tests** (`internal/render`): for each input shape (constraint-only, legacy `// +build` only, both forms, BOM + constraint, CRLF, `#!` + constraint, no constraint), assert the header is inserted after the constraint block and its trailing blank line, and that bytes before the insertion point are byte-identical to the original (BOM, constraint lines, EOLs, blank line preserved). - **Detect boundary tests** (`internal/detect`): assert `PreserveBoundary` returns the offset after the constraint block; assert a managed header placed after the constraint round-trips through `Detect` (span starts after the constraints, not absorbing them) and that re-applying is idempotent (`action == "none"`). - **Negative tests:** a Go file with no constraint is untouched (boundary stays at byte 0/post-BOM, header before `package`). A C/JS file with a leading `// foo` line is NOT treated as a Go build constraint (rule is Go-gated) — guard against regressing the universal `//`-comment handling. - **Round-trip / idempotency:** applying twice yields no second edit; relicensing replaces only the header span and leaves the constraint block intact. - **Coverage:** `go test ./...` passes and the `.testcoverage.yml` 100% gate stays green (no uncovered new branches). ## References - `internal/filetype/filetype.go` — `Go` builtin entry (`PreserveFirst` currently `after(PreserveBOM), before(PreservePackageDecl)`); `block`/`after`/`before` helpers; `model.PreserveKind` rule set. - `internal/render/render.go` — `preserveBoundary` (insert boundary), `insertAt`, `Insert`, `Splice`, `advanceLineIf`, `indexByteFrom`; the universal `#!` block ("`A leading "#!" is a kernel-level shebang ... preserved universally`"); the package-doc SAFETY INVARIANTS (header placed AFTER preserve-first / BEFORE package; bytes outside the header region never altered; producer and splicer must agree byte-for-byte). - `internal/detect/detect.go` — `PreserveBoundary` (detect boundary, same universal `#!` block), `preserveBoundaryFn` seam, `leadingCommentRegion` / `lineCommentRegion` (would otherwise fold leading `//go:build` lines into the detected header run). - **Precedent:** PR #5 / v0.2.1 — universal shebang preservation in both boundary implementations. Use it as the structural template, but gate the Go build-constraint rule on the file type rather than applying it universally. - Go reference: `go/build` build constraints — the constraint must be in the leading comment group before `package`, preceded only by blank lines and `//` line comments and followed by a blank line (https://pkg.go.dev/go/build#hdr-Build_Constraints, https://pkg.go.dev/cmd/go#hdr-Build_constraints). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

KofTwentyTwo commented 2026-06-04 16:33:01 +00:00 (Migrated from github.com)

Copy link

Copy source

Review (maintainer)

Status: confirmed. The header is spliced at byte 0, ahead of a leading //go:build / // +build constraint.

Severity: high — silent, no error, file still compiles, but the build constraint is voided (platform/portability behavior changes quietly).

Verified root cause:

• internal/render/render.go preserveBoundary — the Go file-type rules are after(PreserveBOM), before(PreservePackageDecl). After the BOM step and the universal #! check (which does not match //go:build), the switch has no case for a Go build constraint, so pos stays at 0 and insertAt splices the header before //go:build.
• internal/detect/detect.go PreserveBoundary has the identical gap (only shebang/xml/php/pragma are consumed).
• internal/filetype/filetype.go Go entry — no build-constraint rule.

Language-spec check: correct. A //go:build line must sit in the leading comment group before package, preceded only by blank lines and // line comments and followed by a blank line. A /* */ block comment before it demotes it to an ordinary comment and silently disables the tag. (cgo's /* ... */ import "C" is after package, below the boundary, so unaffected.)

Suggested fix (mirror the v0.2.1 universal-shebang precedent, but gate on the Go type — do not make it universal, or it would over-consume leading //-comment headers in C-family/JS):

1. Add a PreserveGoBuildConstraint kind to the model PreserveKind enum (append at the end; only the first member uses = iota).
2. In both boundary impls, when the type lists it, consume the contiguous leading //go:build and // +build lines plus a single following blank line if present (tolerate its absence — a missing blank line is an already-broken constraint, don't assume it).
3. Add the rule to the Go filetype entry (after BOM/shebang, before package).
4. Keep the 100% coverage gate: add branch tests for build-tag preservation in render + detect + applier idempotency; avoid unreachable switch arms.

Corrections to the issue:

• The detect-path explanation cites lineCommentRegion "folding" the constraint lines, but Go uses block("/*","*/"), so detection routes through blockCommentRegion. The lockstep conclusion (both boundaries must change) still holds — the real detect-side reason is that a correctly-placed post-constraint /* managed header */ is only re-found if PreserveBoundary first steps past //go:build; otherwise blockCommentRegion sees a non-/* line, returns not-found, and idempotency/round-trip breaks.
• "Consume the single terminating blank line" should be "consume it if present."

Open questions: none blocking. (Optional: also handle a //go:build that appears after a legit leading doc comment? Rare; current scope—leading constraint group—is sufficient.)

Overlaps: same "preserve-first / must-be-first" class as #7, #8, #24.

Automated maintainer review.

## Review (maintainer) **Status**: confirmed. The header is spliced at byte 0, ahead of a leading `//go:build` / `// +build` constraint. **Severity**: high — silent, no error, file still compiles, but the build constraint is voided (platform/portability behavior changes quietly). **Verified root cause**: - `internal/render/render.go` `preserveBoundary` — the Go file-type rules are `after(PreserveBOM), before(PreservePackageDecl)`. After the BOM step and the universal `#!` check (which does not match `//go:build`), the `switch` has no case for a Go build constraint, so `pos` stays at 0 and `insertAt` splices the header before `//go:build`. - `internal/detect/detect.go` `PreserveBoundary` has the identical gap (only shebang/xml/php/pragma are consumed). - `internal/filetype/filetype.go` Go entry — no build-constraint rule. **Language-spec check**: correct. A `//go:build` line must sit in the leading comment group before `package`, preceded only by blank lines and `//` line comments and followed by a blank line. A `/* */` block comment before it demotes it to an ordinary comment and silently disables the tag. (cgo's `/* ... */ import "C"` is after `package`, below the boundary, so unaffected.) **Suggested fix** (mirror the v0.2.1 universal-shebang precedent, but gate on the Go type — do not make it universal, or it would over-consume leading `//`-comment headers in C-family/JS): 1. Add a `PreserveGoBuildConstraint` kind to the `model` `PreserveKind` enum (append at the end; only the first member uses `= iota`). 2. In both boundary impls, when the type lists it, consume the contiguous leading `//go:build` and `// +build` lines plus a single following blank line **if present** (tolerate its absence — a missing blank line is an already-broken constraint, don't assume it). 3. Add the rule to the Go filetype entry (after BOM/shebang, before package). 4. Keep the 100% coverage gate: add branch tests for build-tag preservation in render + detect + applier idempotency; avoid unreachable switch arms. **Corrections to the issue**: - The detect-path explanation cites `lineCommentRegion` "folding" the constraint lines, but Go uses `block("/*","*/")`, so detection routes through `blockCommentRegion`. The lockstep conclusion (both boundaries must change) still holds — the real detect-side reason is that a correctly-placed post-constraint `/* managed header */` is only re-found if `PreserveBoundary` first steps past `//go:build`; otherwise `blockCommentRegion` sees a non-`/*` line, returns not-found, and idempotency/round-trip breaks. - "Consume the single terminating blank line" should be "consume it if present." **Open questions**: none blocking. (Optional: also handle a `//go:build` that appears after a legit leading doc comment? Rare; current scope—leading constraint group—is sufficient.) **Overlaps**: same "preserve-first / must-be-first" class as #7, #8, #24. _Automated maintainer review._

KofTwentyTwo commented 2026-06-04 20:27:56 +00:00 (Migrated from github.com)

Copy link

Copy source

Resolved on develop in commit 742c124e6ec2afa9312075db3112bc2def3c91ff.

Validation:

• CI: success, https://github.com/KofTwentyTwo/license-tool/actions/runs/26977377097
• CodeQL: success, https://github.com/KofTwentyTwo/license-tool/actions/runs/26977376421
• Coverage gate: 100% (2087/2087)

Closing as fixed.

Resolved on `develop` in commit `742c124e6ec2afa9312075db3112bc2def3c91ff`. Validation: - CI: success, https://github.com/KofTwentyTwo/license-tool/actions/runs/26977377097 - CodeQL: success, https://github.com/KofTwentyTwo/license-tool/actions/runs/26977376421 - Coverage gate: 100% (2087/2087) Closing as fixed.

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

develop

ci/native-homebrew-main-20261104

ci/native-homebrew-completion-20261104

dependabot/go_modules/github.com/stretchr/testify-1.12.1

dependabot/github_actions/actions/setup-go-7

dependabot/go_modules/golang.org/x/term-0.45.0

dependabot/github_actions/goreleaser/goreleaser-action-7.2.3

dependabot/github_actions/actions/checkout-7

dependabot/go_modules/github.com/charmbracelet/bubbletea-1.3.10

dependabot/go_modules/github.com/charmbracelet/bubbles-1.0.0

main

v0.4.0

v0.3.0

v0.2.1

v0.2.0

v0.1.1

v0.1.0

Labels

Clear labels

bug

Something isn't working

dependencies

Pull requests that update a dependency file

documentation

Improvements or additions to documentation

duplicate

This issue or pull request already exists

enhancement

New feature or request

github_actions

Pull requests that update GitHub Actions code

go

Pull requests that update go code

good first issue

Good for newcomers

help wanted

Extra attention is needed

invalid

This doesn't seem right

question

Further information is requested

wontfix

This will not be worked on

No labels bug

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

forge-recovery (Forge Recovery) james.maes (James Maes)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/license-tool#6