Skip to content

Commit 37f73d1

Browse files
chore: update Copilot pattern log and coding rules
- Record 2 new pattern(s) from PR #372 (defensive-coding, comment-doc-drift) - Promote comment-doc-drift category to coding rule: "Attribute Control Claims to the Correct Mechanism" (PRs #359, #372) Co-authored-by: Kota Kanbe <kotakanbe@users.noreply.github.com>
1 parent 0a7adfb commit 37f73d1

2 files changed

Lines changed: 14 additions & 10 deletions

File tree

.claude/rules/copilot-learned-coding.md

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ Rules extracted from recurring Copilot review patterns on coding-standards topic
2525
- **CI Job Gating — Key on Outputs, Not Job Result**: When a CI job intentionally exits non-zero for a primary use case (e.g., policy violations), downstream jobs must not gate on `needs.<job>.result == 'success'`. Use explicit output variables (exit codes, flags) to control downstream behavior, so jobs run in the scenarios they are designed for.
2626
- **Remove Dead Configuration Inputs**: When a configuration surface (CLI flag, workflow input, env var) is no longer honored by the implementation (e.g., hardcoded internally), remove it entirely rather than leaving a misleading interface. A visible input that silently does nothing is worse than no input at all.
2727
- **CI Permissions Documentation — Verify Inheritance**: When documenting GitHub Actions job permissions, verify each job's actual `permissions:` block in the workflow file. Jobs without an explicit `permissions:` key inherit the workflow-level permissions — do not describe them as having "no permissions" or "no extra permissions". State what each job actually has, including inherited defaults.
28+
- **Attribute Control Claims to the Correct Mechanism**: When a comment asserts that a specific configuration, permission, or flag is "required for" or "controls" a behavior, verify the claim against the actual code path. Do not attribute behavior to a nearby-but-uninvolved mechanism (e.g., claiming `permissions:` block is "required for" `gh` commands when a PAT env var provides the effective auth; claiming a config field controls retries for an API where a different mechanism governs the retry decision). Misattributed control claims cause maintainers to debug or tune the wrong component.
2829
- **Lazy I/O During Format Detection**: When probing a file's format, prefer path-based checks first, then read only a small prefix for content-based heuristics. Read the full file only after confirming the format to avoid wasted I/O on non-matching files (e.g., reading an entire docker-compose.yml just to check if it's a GitHub Actions workflow).
2930
- **Deterministic Output from Non-Deterministic Sources**: When building ordered output from non-deterministic sources (Go map iteration, goroutine-collected results, API directory listings), sort the data before further processing. This applies to rendered text, BFS seed queues, and any "first-seen wins" algorithm where input order determines provenance.
3031
- **Post-Filter Fuzzy Search Results**: When using search APIs that perform fuzzy or word-level matching (e.g., GitHub issue search), add a post-filter to verify exact matches before acting on results. Fuzzy matches can cause false-positive deduplication or incorrect state transitions.
@@ -119,16 +120,16 @@ pending_patterns:
119120
pr: 140
120121
file: "internal/infrastructure/depparser/detect.go"
121122
date: "2026-04-05"
123+
- category: "defensive-coding"
124+
summary: "Use quoted heredocs (<<'DELIM') for literal text bodies in CI workflows to prevent accidental parameter expansion and command substitution; inject dynamic values (markers, variables) via append after the heredoc rather than embedding them in an expansion-enabled body"
125+
pr: 372
126+
file: ".github/workflows/copilot-clean-label.yml"
127+
date: "2026-05-02"
122128
- category: "defensive-coding"
123129
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"
124130
pr: 366
125131
file: "internal/infrastructure/clearlydefined/client.go"
126132
date: "2026-04-30"
127-
- category: "comment-doc-drift"
128-
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')"
129-
pr: 359
130-
file: "internal/infrastructure/httpclient/client.go"
131-
date: "2026-04-29"
132133
- category: "defensive-coding"
133134
summary: "Guard time.Duration arithmetic against integer overflow — use strconv.ParseInt, reject values exceeding math.MaxInt64/time.Second, and do not clamp to an arbitrary policy constant (let the caller's configured cap decide)"
134135
pr: 359
@@ -167,6 +168,7 @@ pending_patterns:
167168
```
168169
169170
<!-- Promotion history (kept for audit trail):
171+
# comment-doc-drift: promoted to copilot-learned-coding.instructions.md (PRs #359, #372 — attribute control claims to the correct mechanism: do not claim a permission/config/flag controls a behavior when a different mechanism is the effective authority)
170172
# 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)
171173
# 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
172174
# defensive-coding (PR #338): stale pending entry removed — already promoted as "Sanitize Dynamic Content in GitHub Actions Workflow Commands" rule

.github/instructions/copilot-learned-coding.instructions.md

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ Rules extracted from recurring Copilot review patterns on coding-standards topic
2323
- **CI Job Gating — Key on Outputs, Not Job Result**: When a CI job intentionally exits non-zero for a primary use case (e.g., policy violations), downstream jobs must not gate on `needs.<job>.result == 'success'`. Use explicit output variables (exit codes, flags) to control downstream behavior, so jobs run in the scenarios they are designed for.
2424
- **Remove Dead Configuration Inputs**: When a configuration surface (CLI flag, workflow input, env var) is no longer honored by the implementation (e.g., hardcoded internally), remove it entirely rather than leaving a misleading interface. A visible input that silently does nothing is worse than no input at all.
2525
- **CI Permissions Documentation — Verify Inheritance**: When documenting GitHub Actions job permissions, verify each job's actual `permissions:` block in the workflow file. Jobs without an explicit `permissions:` key inherit the workflow-level permissions — do not describe them as having "no permissions" or "no extra permissions". State what each job actually has, including inherited defaults.
26+
- **Attribute Control Claims to the Correct Mechanism**: When a comment asserts that a specific configuration, permission, or flag is "required for" or "controls" a behavior, verify the claim against the actual code path. Do not attribute behavior to a nearby-but-uninvolved mechanism (e.g., claiming `permissions:` block is "required for" `gh` commands when a PAT env var provides the effective auth; claiming a config field controls retries for an API where a different mechanism governs the retry decision). Misattributed control claims cause maintainers to debug or tune the wrong component.
2627
- **Lazy I/O During Format Detection**: When probing a file's format, prefer path-based checks first, then read only a small prefix for content-based heuristics. Read the full file only after confirming the format to avoid wasted I/O on non-matching files (e.g., reading an entire docker-compose.yml just to check if it's a GitHub Actions workflow).
2728
- **Deterministic Output from Non-Deterministic Sources**: When building ordered output from non-deterministic sources (Go map iteration, goroutine-collected results, API directory listings), sort the data before further processing. This applies to rendered text, BFS seed queues, and any "first-seen wins" algorithm where input order determines provenance.
2829
- **Post-Filter Fuzzy Search Results**: When using search APIs that perform fuzzy or word-level matching (e.g., GitHub issue search), add a post-filter to verify exact matches before acting on results. Fuzzy matches can cause false-positive deduplication or incorrect state transitions.
@@ -117,16 +118,16 @@ pending_patterns:
117118
pr: 140
118119
file: "internal/infrastructure/depparser/detect.go"
119120
date: "2026-04-05"
121+
- category: "defensive-coding"
122+
summary: "Use quoted heredocs (<<'DELIM') for literal text bodies in CI workflows to prevent accidental parameter expansion and command substitution; inject dynamic values (markers, variables) via append after the heredoc rather than embedding them in an expansion-enabled body"
123+
pr: 372
124+
file: ".github/workflows/copilot-clean-label.yml"
125+
date: "2026-05-02"
120126
- category: "defensive-coding"
121127
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"
122128
pr: 366
123129
file: "internal/infrastructure/clearlydefined/client.go"
124130
date: "2026-04-30"
125-
- category: "comment-doc-drift"
126-
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')"
127-
pr: 359
128-
file: "internal/infrastructure/httpclient/client.go"
129-
date: "2026-04-29"
130131
- category: "defensive-coding"
131132
summary: "Guard time.Duration arithmetic against integer overflow — use strconv.ParseInt, reject values exceeding math.MaxInt64/time.Second, and do not clamp to an arbitrary policy constant (let the caller's configured cap decide)"
132133
pr: 359
@@ -165,6 +166,7 @@ pending_patterns:
165166
```
166167
167168
<!-- Promotion history (kept for audit trail):
169+
# comment-doc-drift: promoted to copilot-learned-coding.instructions.md (PRs #359, #372 — attribute control claims to the correct mechanism: do not claim a permission/config/flag controls a behavior when a different mechanism is the effective authority)
168170
# 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)
169171
# 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
170172
# defensive-coding (PR #338): stale pending entry removed — already promoted as "Sanitize Dynamic Content in GitHub Actions Workflow Commands" rule

0 commit comments

Comments
 (0)