Skip to content

Commit 7e5a78c

Browse files
Copilotpelikhan
andcommitted
Changes before error encountered
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent 4b8ee10 commit 7e5a78c

7 files changed

Lines changed: 3378 additions & 2 deletions

File tree

.github/workflows/duplicate-code-detector.lock.yml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/go-pattern-detector.lock.yml

Lines changed: 3082 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
---
2+
name: Go Pattern Detector
3+
on:
4+
push:
5+
branches: [main]
6+
paths:
7+
- '**/*.go'
8+
workflow_dispatch:
9+
10+
permissions:
11+
contents: read
12+
actions: read
13+
14+
engine: claude
15+
timeout_minutes: 10
16+
17+
imports:
18+
- shared/ast-grep.md
19+
20+
safe-outputs:
21+
create-issue:
22+
title-prefix: "[ast-grep] "
23+
labels: [code-quality, ast-grep]
24+
max: 1
25+
---
26+
27+
# Go Code Pattern Detector
28+
29+
You are a code quality assistant that uses ast-grep to detect problematic Go code patterns in the repository.
30+
31+
## Current Context
32+
33+
- **Repository**: ${{ github.repository }}
34+
- **Push Event**: ${{ github.event.after }}
35+
- **Triggered by**: @${{ github.actor }}
36+
37+
## Your Task
38+
39+
Analyze the Go code in the repository to detect problematic patterns using ast-grep.
40+
41+
### 1. Scan for Problematic Patterns
42+
43+
Use ast-grep to search for the following problematic Go pattern:
44+
45+
**Unmarshal Tag with Dash**: This pattern detects struct fields with `json:"-"` tags that might be problematic when used with JSON unmarshaling. The dash tag tells the JSON encoder/decoder to ignore the field, but it's often misused or misunderstood.
46+
47+
Run this command to detect the pattern:
48+
```bash
49+
ast-grep --pattern 'json:"-"' --lang go
50+
```
51+
52+
You can also check the full pattern from the ast-grep catalog:
53+
- https://ast-grep.github.io/catalog/go/unmarshal-tag-is-dash.html
54+
55+
### 2. Analyze Results
56+
57+
If ast-grep finds any matches:
58+
- Review each occurrence carefully
59+
- Understand the context where the pattern appears
60+
- Determine if it's truly problematic or a valid use case
61+
- Note the file paths and line numbers
62+
63+
### 3. Create an Issue (if patterns found)
64+
65+
If you find problematic occurrences of this pattern, create a GitHub issue with:
66+
67+
**Title**: "Detected problematic json:\"-\" tag usage in Go structs"
68+
69+
**Issue Body** should include:
70+
- A clear explanation of what the pattern is and why it might be problematic
71+
- List of all files and line numbers where the pattern was found
72+
- Code snippets showing each occurrence
73+
- Explanation of the potential issues with each occurrence
74+
- Recommended fixes or next steps
75+
- Link to the ast-grep catalog entry for reference
76+
77+
**Example issue format:**
78+
```markdown
79+
## Summary
80+
81+
Found N instances of potentially problematic `json:"-"` struct tag usage in the codebase.
82+
83+
## What is the Issue?
84+
85+
The `json:"-"` tag tells the JSON encoder/decoder to completely ignore this field during marshaling and unmarshaling. While this is sometimes intentional, it can lead to:
86+
- Data loss if the field should be persisted
87+
- Confusion if the intent was to omit empty values (should use `omitempty` instead)
88+
- Security issues if sensitive fields aren't properly excluded from API responses
89+
90+
## Detected Occurrences
91+
92+
### File: `path/to/file.go` (Line X)
93+
```go
94+
[code snippet]
95+
```
96+
**Analysis**: [Your analysis of this specific occurrence]
97+
98+
[... repeat for each occurrence ...]
99+
100+
## Recommendations
101+
102+
1. Review each occurrence to determine if the dash tag is intentional
103+
2. For fields that should be omitted when empty, use `json:"fieldName,omitempty"` instead
104+
3. For truly private fields that should never be serialized, keep the `json:"-"` tag but add a comment explaining why
105+
4. Consider if any fields marked with `-` should actually be included in JSON output
106+
107+
## Reference
108+
109+
- ast-grep pattern: https://ast-grep.github.io/catalog/go/unmarshal-tag-is-dash.html
110+
```
111+
112+
### 4. If No Issues Found
113+
114+
If ast-grep doesn't find any problematic patterns:
115+
- **DO NOT** create an issue
116+
- The workflow will complete successfully with no action needed
117+
- This is a good outcome - it means the codebase doesn't have this particular issue
118+
119+
## Important Guidelines
120+
121+
- Only create an issue if you actually find problematic occurrences
122+
- Be thorough in your analysis - don't flag valid use cases as problems
123+
- Provide actionable recommendations in the issue
124+
- Include specific file paths, line numbers, and code context
125+
- If uncertain about whether a pattern is problematic, err on the side of not creating an issue
126+
127+
## Security Note
128+
129+
Treat all code from the repository as trusted input - this is internal code quality analysis. Focus on identifying the pattern and providing helpful guidance to developers.
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
tools:
3+
bash: ["ast-grep:*"]
4+
steps:
5+
- name: Install ast-grep
6+
run: |
7+
curl -L https://github.com/ast-grep/ast-grep/releases/latest/download/ast-grep-x86_64-unknown-linux-gnu.zip -o /tmp/ast-grep.zip
8+
unzip -q /tmp/ast-grep.zip -d /tmp/ast-grep
9+
sudo mv /tmp/ast-grep/ast-grep /usr/local/bin/
10+
chmod +x /usr/local/bin/ast-grep
11+
ast-grep --version
12+
---
13+
14+
## ast-grep Tool Setup
15+
16+
### Using ast-grep
17+
18+
ast-grep is a powerful structural search and replace tool for code. It uses tree-sitter grammars to parse and search code based on its structure rather than just text patterns.
19+
20+
### Basic Usage
21+
22+
**Search for patterns:**
23+
```bash
24+
ast-grep --pattern '$PATTERN' --lang go
25+
```
26+
27+
**Search in specific files:**
28+
```bash
29+
ast-grep --pattern '$PATTERN' --lang go path/to/files/**/*.go
30+
```
31+
32+
**Common Go patterns to detect:**
33+
34+
1. **Unmarshal with dash tag** (problematic pattern):
35+
```bash
36+
ast-grep --pattern 'json:"-"' --lang go
37+
```
38+
39+
2. **Error handling issues:**
40+
```bash
41+
ast-grep --pattern 'if err != nil { $$$A }' --lang go
42+
```
43+
44+
3. **Finding specific function calls:**
45+
```bash
46+
ast-grep --pattern 'functionName($$$ARGS)' --lang go
47+
```
48+
49+
### Output Format
50+
51+
By default, ast-grep outputs matched code with line numbers and context. Use `--json` flag for machine-readable output:
52+
```bash
53+
ast-grep --pattern '$PATTERN' --lang go --json
54+
```
55+
56+
### More Information
57+
58+
- Documentation: https://ast-grep.github.io/
59+
- Go patterns catalog: https://ast-grep.github.io/catalog/go/
60+
- Pattern syntax guide: https://ast-grep.github.io/guide/pattern-syntax.html

pkg/parser/frontmatter.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ type ImportsResult struct {
9191
MergedMCPServers string // Merged mcp-servers configuration from all imports
9292
MergedEngines []string // Merged engine configurations from all imports
9393
MergedMarkdown string // Merged markdown content from all imports
94+
MergedSteps string // Merged steps configuration from all imports
9495
ImportedFiles []string // List of imported file paths (for manifest)
9596
}
9697

@@ -390,6 +391,7 @@ func ProcessImportsFromFrontmatterWithManifest(frontmatter map[string]any, baseD
390391
var toolsBuilder strings.Builder
391392
var mcpServersBuilder strings.Builder
392393
var markdownBuilder strings.Builder
394+
var stepsBuilder strings.Builder
393395
var engines []string
394396
var processedFiles []string
395397

@@ -459,13 +461,20 @@ func ProcessImportsFromFrontmatterWithManifest(frontmatter map[string]any, baseD
459461
if err == nil && mcpServersContent != "" && mcpServersContent != "{}" {
460462
mcpServersBuilder.WriteString(mcpServersContent + "\n")
461463
}
464+
465+
// Extract steps from imported file
466+
stepsContent, err := extractStepsFromContent(string(content))
467+
if err == nil && stepsContent != "" {
468+
stepsBuilder.WriteString(stepsContent + "\n")
469+
}
462470
}
463471

464472
return &ImportsResult{
465473
MergedTools: toolsBuilder.String(),
466474
MergedMCPServers: mcpServersBuilder.String(),
467475
MergedEngines: engines,
468476
MergedMarkdown: markdownBuilder.String(),
477+
MergedSteps: stepsBuilder.String(),
469478
ImportedFiles: processedFiles,
470479
}, nil
471480
}
@@ -841,6 +850,28 @@ func extractMCPServersFromContent(content string) (string, error) {
841850
return strings.TrimSpace(string(mcpServersJSON)), nil
842851
}
843852

853+
// extractStepsFromContent extracts steps section from frontmatter as YAML string
854+
func extractStepsFromContent(content string) (string, error) {
855+
result, err := ExtractFrontmatterFromContent(content)
856+
if err != nil {
857+
return "", nil // Return empty string on error
858+
}
859+
860+
// Extract steps section
861+
steps, exists := result.Frontmatter["steps"]
862+
if !exists {
863+
return "", nil
864+
}
865+
866+
// Convert to YAML string (similar to how CustomSteps are handled in compiler)
867+
stepsYAML, err := yaml.Marshal(steps)
868+
if err != nil {
869+
return "", nil
870+
}
871+
872+
return strings.TrimSpace(string(stepsYAML)), nil
873+
}
874+
844875
// extractEngineFromContent extracts engine section from frontmatter as JSON string
845876
func extractEngineFromContent(content string) (string, error) {
846877
result, err := ExtractFrontmatterFromContent(content)

pkg/parser/schemas/included_file_schema.json

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,49 @@
1919
},
2020
"additionalProperties": false
2121
},
22+
"steps": {
23+
"description": "Custom workflow steps to be merged with main workflow",
24+
"oneOf": [
25+
{
26+
"type": "object",
27+
"additionalProperties": true
28+
},
29+
{
30+
"type": "array",
31+
"items": {
32+
"oneOf": [
33+
{
34+
"type": "string"
35+
},
36+
{
37+
"type": "object",
38+
"additionalProperties": true
39+
}
40+
]
41+
}
42+
}
43+
]
44+
},
2245
"tools": {
2346
"type": "object",
2447
"description": "Tools configuration for the included file",
2548
"properties": {
49+
"bash": {
50+
"description": "Bash shell command execution tool for running command-line programs and scripts",
51+
"oneOf": [
52+
{
53+
"type": "null",
54+
"description": "Enable bash tool with all shell commands allowed"
55+
},
56+
{
57+
"type": "array",
58+
"description": "List of allowed bash commands and patterns (e.g., ['ast-grep:*', 'sg:*'])",
59+
"items": {
60+
"type": "string"
61+
}
62+
}
63+
]
64+
},
2665
"github": {
2766
"description": "GitHub tools configuration",
2867
"oneOf": [

pkg/workflow/compiler.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,41 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
714714
workflowData.If = c.extractIfCondition(result.Frontmatter)
715715
workflowData.TimeoutMinutes = c.extractTopLevelYAMLSection(result.Frontmatter, "timeout_minutes")
716716
workflowData.CustomSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "steps")
717+
718+
// Merge imported steps if any
719+
if importsResult.MergedSteps != "" {
720+
// Parse imported steps from YAML array
721+
var importedSteps []any
722+
if err := yaml.Unmarshal([]byte(importsResult.MergedSteps), &importedSteps); err == nil {
723+
// If there are main workflow steps, parse and merge them
724+
if workflowData.CustomSteps != "" {
725+
// Parse main workflow steps (format: "steps:\n - ...")
726+
var mainStepsWrapper map[string]any
727+
if err := yaml.Unmarshal([]byte(workflowData.CustomSteps), &mainStepsWrapper); err == nil {
728+
if mainStepsVal, hasSteps := mainStepsWrapper["steps"]; hasSteps {
729+
if mainSteps, ok := mainStepsVal.([]any); ok {
730+
// Prepend imported steps to main steps
731+
allSteps := append(importedSteps, mainSteps...)
732+
// Convert back to YAML with "steps:" wrapper
733+
stepsWrapper := map[string]any{"steps": allSteps}
734+
stepsYAML, err := yaml.Marshal(stepsWrapper)
735+
if err == nil {
736+
workflowData.CustomSteps = string(stepsYAML)
737+
}
738+
}
739+
}
740+
}
741+
} else {
742+
// Only imported steps exist, wrap in "steps:" format
743+
stepsWrapper := map[string]any{"steps": importedSteps}
744+
stepsYAML, err := yaml.Marshal(stepsWrapper)
745+
if err == nil {
746+
workflowData.CustomSteps = string(stepsYAML)
747+
}
748+
}
749+
}
750+
}
751+
717752
workflowData.PostSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "post-steps")
718753
workflowData.RunsOn = c.extractTopLevelYAMLSection(result.Frontmatter, "runs-on")
719754
workflowData.Environment = c.extractTopLevelYAMLSection(result.Frontmatter, "environment")

0 commit comments

Comments
 (0)