You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
ci: add golangci-lint + actionlint as PR gates (#370)
* ci: add golangci-lint + actionlint as PR gates
CI currently builds and tests but no static lint runs as a merge gate.
This PR adds two minimal lint workflows:
1. .golangci.yml + .github/workflows/go-lint.yml — golangci-lint v2
on Go-source PRs. Initial set: errcheck, govet, ineffassign,
staticcheck, unused. Runs cleanly on the current codebase.
2. .github/workflows/lint.yml — actionlint on workflow YAML PRs.
Catches GitHub Actions specific issues plus shellcheck-detectable
bash issues in run: blocks.
Deferred to follow-up PRs (so this one merges cleanly):
- gosec: 8 findings on existing code need owner triage before fixing
or suppressing (most look like false positives in build-time
generators / scripts). See PR description for the list.
- revive (exported godoc): backfill is single-purpose work better
reviewed on its own.
- depguard for DDD layers (domain MUST NOT import application/...):
rule design deserves owner review before going live.
Workflow details:
- paths filter so docs-only PRs don't trigger unrelated lint runs
- All actions SHA-pinned (golangci-lint-action v9.2.0,
reviewdog/action-actionlint v1.72.0, actions/checkout v6.0.2,
actions/setup-go v6.4.0)
- permissions: minimal (contents/pull-requests read only)
Verified locally:
golangci-lint run ./... # 0 issues
actionlint .github/workflows/{lint,go-lint}.yml # 0 findings
go build ./... # clean
* chore: update Copilot pattern log and coding rules
- Record 3 new pattern(s) from PR #370 (ci-path-filters, dependency-pinning, ci-permissions)
- No promotions this cycle (new categories below 2-PR threshold)
Co-authored-by: Kota Kanbe <kotakanbe@users.noreply.github.com>
* chore: update Copilot pattern log and coding rules
- Promote defensive-coding (PRs #345, #366) to rule: use dedicated
predicates and full renderers for sentinel/composed values
- Remove 4 promoted entries from pending_patterns
Co-authored-by: Kota Kanbe <kotakanbe@users.noreply.github.com>
---------
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Copy file name to clipboardExpand all lines: .claude/rules/copilot-learned-coding.md
+17-21Lines changed: 17 additions & 21 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -50,6 +50,7 @@ Rules extracted from recurring Copilot review patterns on coding-standards topic
50
50
-**Respect Context Lifecycle in Concurrent and Delegated I/O**: (1) When a function issues I/O (HTTP, database, external API) on behalf of a caller that provides a `context.Context`, thread that context through every intermediate function in the call chain — never substitute `context.Background()` at an internal callsite. `context.Background()` ignores the caller's cancellation and deadline signals, causing orphaned requests that persist after cancellation or exceed timeout budgets. (2) When dispatching goroutines under bounded concurrency, acquire the semaphore before launching the goroutine (not inside it) and `select` on `ctx.Done()` alongside the semaphore send to stop dispatch on cancellation — this avoids spawning parked goroutines that outlive the context.
51
51
-**Guard Nil Structs Consistently Across Output Formats**: When a struct field may be nil (e.g., `ReleaseInfo`), apply the nil guard in every output renderer that accesses it (text, CSV, JSON). If one renderer has the guard and another does not, the unguarded path will panic on nil input.
52
52
-**Gate Fallback Logic on Error, Not Result Nilness**: When deciding whether to trigger fallback or retry logic, check the error value — not whether the result is nil. A nil result with nil error is a valid success case (e.g., zero matches found), and treating it as a failure triggers unnecessary retries or incorrect fallback paths.
53
+
-**Use Dedicated Predicates and Full Renderers for Sentinel/Composed Values**: When a domain type uses sentinel values (e.g., NOASSERTION), use dedicated predicate methods (e.g., `IsUsableSPDX()`) in all guard checks — not ad-hoc field comparisons that miss sentinel states. Canonicalize sentinels before rendering compound expressions (parser leaf fields may preserve non-canonical casing). When a type has a renderer composing sub-components (e.g., `ExprLicense.String()` includes Identifier + OrLater + WITH exception), use the renderer — not a single field — for the full representation. When recording provenance in multi-branch resolution functions, set evidence fields to the input that actually produced the match, not a prior failed branch's input.
53
54
-**Minimize Allocations in Hot Paths**: In batch-processing or frequently-called functions, avoid unnecessary O(n) allocations when only a subset of data is needed. Cache results of expensive parsing calls when the same value is checked multiple times in a loop iteration, and iterate to a known cutoff point rather than materializing the full collection (e.g., iterate runes up to a count rather than converting the entire string to `[]rune`).
54
55
-**Use Structured Parsers for Structured Identifier Properties**: When checking properties of structured identifiers (PURLs, URIs, import paths), use the appropriate parser rather than naive string operations (`strings.Contains`, `strings.Split`). For example, `strings.Contains(purl, "@")` misclassifies npm scoped packages like `pkg:npm/@scope/name` as versioned because `@` appears in the namespace. Use `packageurl.FromString(p).Version != ""` or an equivalent parser-based check.
55
56
-**Use `u.Hostname()` for Port-Safe Host Comparison**: When comparing URL hostnames, use `u.Hostname()` instead of `u.Host`. The `Host` field includes the port component (e.g., `github.com:443`), so direct string comparison against a bare hostname fails silently, misclassifying URLs and triggering unnecessary fallback processing. Similarly, when parsing multi-entry `go-import`/`go-source` meta tags, select the entry whose import prefix most specifically matches the requested path per the Go module spec — blindly taking the first match can resolve to the wrong repository on monorepo vanity pages.
@@ -98,32 +99,16 @@ Schema (YAML-in-Markdown):
98
99
99
100
```yaml
100
101
pending_patterns:
101
-
- category: "defensive-coding"
102
-
summary: "When a multi-branch resolution function (e.g., name-first then URL fallback) records evidence in a Raw/provenance field, set Raw to the input that actually produced the match — not the input from a prior branch that failed. Misattributed Raw values lose traceability for debugging and audit"
103
-
pr: 345
104
-
file: "internal/infrastructure/maven/license.go"
105
-
date: "2026-04-29"
106
-
- category: "defensive-coding"
107
-
summary: "When a domain type uses a sentinel value that is 'recognized but not usable' (e.g., NOASSERTION), use the dedicated predicate method (e.g., IsUsableSPDX()) in all guard/gate checks rather than ad-hoc field checks (Expression != \"\", IsZero() || IsNonStandard()). Ad-hoc guards miss sentinel states and cause inconsistent behavior (propagating NOASSERTION as a real license, blocking enrichment, or corrupting leaf counts)"
summary: "Canonicalize sentinel values (e.g., NOASSERTION) when rendering compound SPDX expressions — the parser's leaf Raw field preserves original casing, so without explicit rewriting before String(), rendering produces non-canonical output like 'MIT OR noassertion' instead of 'MIT OR NOASSERTION'"
summary: "Use bytes.Fields tokenization instead of fixed-separator prefix checks when matching directives — tabs and multiple spaces are valid separators"
summary: "When a structured type has a renderer that composes all sub-components (e.g., ExprLicense.String() includes Identifier + OrLater '+' + WITH exception), use the renderer instead of a single field (e.g., leaf.Identifier) when the full representation is needed — partial extraction silently drops components the renderer handles"
summary: "Use **/*.ext (not **.ext) in GitHub Actions path filters — ** must be a standalone path segment (followed by /) to reliably match across directory boundaries; **.ext is non-standard and may behave like *.ext in some glob implementations"
109
+
pr: 370
110
+
file: ".github/workflows/go-lint.yml"
111
+
date: "2026-05-02"
127
112
- category: "comment-doc-drift"
128
113
summary: "Doc comments must match implementation boundary conditions — RetryConfig said retryDecider controls 429 retries (it doesn't); rateLimitBackoff comment said 'negative' for a zero-inclusive guard (should say 'non-positive')"
summary: "Pin CI tool binary versions explicitly (e.g., golangci-lint version: '2.2.1') — not just the action SHA. version: latest makes CI non-deterministic; new releases can introduce checks or behavior changes between runs on the same commit"
129
+
pr: 370
130
+
file: ".github/workflows/go-lint.yml"
131
+
date: "2026-05-02"
142
132
- category: "testing"
143
133
summary: "When generating time-based test fixtures with coarse-grained formatters (e.g., http.TimeFormat at 1-second granularity), truncate to the format boundary and add enough offset (e.g., 2s) so the formatted value is deterministically in the expected range — sub-granularity offsets (50ms) can collapse to the current or past second"
summary: "Verify that GitHub Actions reporter actions (e.g., reviewdog with github-pr-check) have the permissions they need — github-pr-check reporter requires checks: write to publish annotations; missing permissions cause silent failures or degraded reporting"
149
+
pr: 370
150
+
file: ".github/workflows/lint.yml"
151
+
date: "2026-05-02"
157
152
- category: "defensive-coding"
158
153
summary: "Use time.NewTimer + Stop/drain instead of time.After in select with ctx.Done() to prevent timer accumulation during long cancellable waits"
159
154
pr: 359
@@ -167,6 +162,7 @@ pending_patterns:
167
162
```
168
163
169
164
<!-- Promotion history (kept for audit trail):
165
+
# defensive-coding: promoted to copilot-learned-coding.instructions.md (PRs #345, #366 — use dedicated predicates and full renderers for sentinel/composed values: IsUsableSPDX() not ad-hoc checks; canonicalize sentinel casing; use type renderer not single field; attribute provenance to correct resolution branch)
170
166
# defensive-coding: promoted to copilot-learned-coding.instructions.md (PRs #345, #366 — match write guard quantifiers to write semantics: an "any" flag guarding an "all/only" write misrepresents mixed inputs; verify replacement is a net improvement)
171
167
# testing (PR #366): already covered by "Scope Test Assertions to Specific Output Regions" in testing-performance + "Use Spec-Compliant Parsers for Standardized Formats" — use encoding/csv to parse CSV output and assert exact cells by header name instead of fragile strings.Contains on boolean patterns that match the wrong column
172
168
# defensive-coding (PR #338): stale pending entry removed — already promoted as "Sanitize Dynamic Content in GitHub Actions Workflow Commands" rule
Copy file name to clipboardExpand all lines: .github/instructions/copilot-learned-coding.instructions.md
+17-21Lines changed: 17 additions & 21 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -48,6 +48,7 @@ Rules extracted from recurring Copilot review patterns on coding-standards topic
48
48
-**Respect Context Lifecycle in Concurrent and Delegated I/O**: (1) When a function issues I/O (HTTP, database, external API) on behalf of a caller that provides a `context.Context`, thread that context through every intermediate function in the call chain — never substitute `context.Background()` at an internal callsite. `context.Background()` ignores the caller's cancellation and deadline signals, causing orphaned requests that persist after cancellation or exceed timeout budgets. (2) When dispatching goroutines under bounded concurrency, acquire the semaphore before launching the goroutine (not inside it) and `select` on `ctx.Done()` alongside the semaphore send to stop dispatch on cancellation — this avoids spawning parked goroutines that outlive the context.
49
49
-**Guard Nil Structs Consistently Across Output Formats**: When a struct field may be nil (e.g., `ReleaseInfo`), apply the nil guard in every output renderer that accesses it (text, CSV, JSON). If one renderer has the guard and another does not, the unguarded path will panic on nil input.
50
50
-**Gate Fallback Logic on Error, Not Result Nilness**: When deciding whether to trigger fallback or retry logic, check the error value — not whether the result is nil. A nil result with nil error is a valid success case (e.g., zero matches found), and treating it as a failure triggers unnecessary retries or incorrect fallback paths.
51
+
-**Use Dedicated Predicates and Full Renderers for Sentinel/Composed Values**: When a domain type uses sentinel values (e.g., NOASSERTION), use dedicated predicate methods (e.g., `IsUsableSPDX()`) in all guard checks — not ad-hoc field comparisons that miss sentinel states. Canonicalize sentinels before rendering compound expressions (parser leaf fields may preserve non-canonical casing). When a type has a renderer composing sub-components (e.g., `ExprLicense.String()` includes Identifier + OrLater + WITH exception), use the renderer — not a single field — for the full representation. When recording provenance in multi-branch resolution functions, set evidence fields to the input that actually produced the match, not a prior failed branch's input.
51
52
-**Minimize Allocations in Hot Paths**: In batch-processing or frequently-called functions, avoid unnecessary O(n) allocations when only a subset of data is needed. Cache results of expensive parsing calls when the same value is checked multiple times in a loop iteration, and iterate to a known cutoff point rather than materializing the full collection (e.g., iterate runes up to a count rather than converting the entire string to `[]rune`).
52
53
-**Use Structured Parsers for Structured Identifier Properties**: When checking properties of structured identifiers (PURLs, URIs, import paths), use the appropriate parser rather than naive string operations (`strings.Contains`, `strings.Split`). For example, `strings.Contains(purl, "@")` misclassifies npm scoped packages like `pkg:npm/@scope/name` as versioned because `@` appears in the namespace. Use `packageurl.FromString(p).Version != ""` or an equivalent parser-based check.
53
54
-**Use `u.Hostname()` for Port-Safe Host Comparison**: When comparing URL hostnames, use `u.Hostname()` instead of `u.Host`. The `Host` field includes the port component (e.g., `github.com:443`), so direct string comparison against a bare hostname fails silently, misclassifying URLs and triggering unnecessary fallback processing. Similarly, when parsing multi-entry `go-import`/`go-source` meta tags, select the entry whose import prefix most specifically matches the requested path per the Go module spec — blindly taking the first match can resolve to the wrong repository on monorepo vanity pages.
@@ -96,32 +97,16 @@ Schema (YAML-in-Markdown):
96
97
97
98
```yaml
98
99
pending_patterns:
99
-
- category: "defensive-coding"
100
-
summary: "When a multi-branch resolution function (e.g., name-first then URL fallback) records evidence in a Raw/provenance field, set Raw to the input that actually produced the match — not the input from a prior branch that failed. Misattributed Raw values lose traceability for debugging and audit"
101
-
pr: 345
102
-
file: "internal/infrastructure/maven/license.go"
103
-
date: "2026-04-29"
104
-
- category: "defensive-coding"
105
-
summary: "When a domain type uses a sentinel value that is 'recognized but not usable' (e.g., NOASSERTION), use the dedicated predicate method (e.g., IsUsableSPDX()) in all guard/gate checks rather than ad-hoc field checks (Expression != \"\", IsZero() || IsNonStandard()). Ad-hoc guards miss sentinel states and cause inconsistent behavior (propagating NOASSERTION as a real license, blocking enrichment, or corrupting leaf counts)"
summary: "Canonicalize sentinel values (e.g., NOASSERTION) when rendering compound SPDX expressions — the parser's leaf Raw field preserves original casing, so without explicit rewriting before String(), rendering produces non-canonical output like 'MIT OR noassertion' instead of 'MIT OR NOASSERTION'"
summary: "Use bytes.Fields tokenization instead of fixed-separator prefix checks when matching directives — tabs and multiple spaces are valid separators"
summary: "When a structured type has a renderer that composes all sub-components (e.g., ExprLicense.String() includes Identifier + OrLater '+' + WITH exception), use the renderer instead of a single field (e.g., leaf.Identifier) when the full representation is needed — partial extraction silently drops components the renderer handles"
summary: "Use **/*.ext (not **.ext) in GitHub Actions path filters — ** must be a standalone path segment (followed by /) to reliably match across directory boundaries; **.ext is non-standard and may behave like *.ext in some glob implementations"
107
+
pr: 370
108
+
file: ".github/workflows/go-lint.yml"
109
+
date: "2026-05-02"
125
110
- category: "comment-doc-drift"
126
111
summary: "Doc comments must match implementation boundary conditions — RetryConfig said retryDecider controls 429 retries (it doesn't); rateLimitBackoff comment said 'negative' for a zero-inclusive guard (should say 'non-positive')"
summary: "Pin CI tool binary versions explicitly (e.g., golangci-lint version: '2.2.1') — not just the action SHA. version: latest makes CI non-deterministic; new releases can introduce checks or behavior changes between runs on the same commit"
127
+
pr: 370
128
+
file: ".github/workflows/go-lint.yml"
129
+
date: "2026-05-02"
140
130
- category: "testing"
141
131
summary: "When generating time-based test fixtures with coarse-grained formatters (e.g., http.TimeFormat at 1-second granularity), truncate to the format boundary and add enough offset (e.g., 2s) so the formatted value is deterministically in the expected range — sub-granularity offsets (50ms) can collapse to the current or past second"
summary: "Verify that GitHub Actions reporter actions (e.g., reviewdog with github-pr-check) have the permissions they need — github-pr-check reporter requires checks: write to publish annotations; missing permissions cause silent failures or degraded reporting"
147
+
pr: 370
148
+
file: ".github/workflows/lint.yml"
149
+
date: "2026-05-02"
155
150
- category: "defensive-coding"
156
151
summary: "Use time.NewTimer + Stop/drain instead of time.After in select with ctx.Done() to prevent timer accumulation during long cancellable waits"
157
152
pr: 359
@@ -165,6 +160,7 @@ pending_patterns:
165
160
```
166
161
167
162
<!-- Promotion history (kept for audit trail):
163
+
# defensive-coding: promoted to copilot-learned-coding.instructions.md (PRs #345, #366 — use dedicated predicates and full renderers for sentinel/composed values: IsUsableSPDX() not ad-hoc checks; canonicalize sentinel casing; use type renderer not single field; attribute provenance to correct resolution branch)
168
164
# defensive-coding: promoted to copilot-learned-coding.instructions.md (PRs #345, #366 — match write guard quantifiers to write semantics: an "any" flag guarding an "all/only" write misrepresents mixed inputs; verify replacement is a net improvement)
169
165
# testing (PR #366): already covered by "Scope Test Assertions to Specific Output Regions" in testing-performance + "Use Spec-Compliant Parsers for Standardized Formats" — use encoding/csv to parse CSV output and assert exact cells by header name instead of fragile strings.Contains on boolean patterns that match the wrong column
170
166
# defensive-coding (PR #338): stale pending entry removed — already promoted as "Sanitize Dynamic Content in GitHub Actions Workflow Commands" rule
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?v=4&size=40){kind=avatar}
0 commit comments