Repository navigation
Harden fetch-script portability and structured lint reporting - #71932
Conversation
…nostics Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot resolve the merge conflicts on this branch. |
|
@copilot resolve the merge conflicts on this branch. |
There was a problem hiding this comment.
🔵 Needs a closer look
Default-enabled linting and strict/dry-run reporting have unresolved correctness and MCP integration issues.
6 open issues
Handle malformed actionlint JSON on successful exit · New Report unavailable scanner and lockfile failures · New Preserve Docker failure context in compile diagnostics · New Avoid duplicate ShellCheck stderr diagnostics · New Exclude advisory findings from mixed ShellCheck errors · New Resolve inherited shell before Bash compatibility checks · New
What changed in this PR
This PR improves Bash 3.2 portability and machine-readable lint reporting in gh-aw’s compilation pipeline.
Changes:
- Uses balanced fetch-script case patterns, adds compatibility linting and syntax tests, and adds macOS CI coverage.
- Preserves external scanner findings, severity, rule IDs, and script locations in compile JSON.
- Documents reproducible compiler comparisons and regenerates three affected workflow locks.
| File | Description |
|---|---|
pkg/workflow/lint/rule_contract_test.go |
Adds the case-pattern rule contract. |
pkg/workflow/lint/lint.go |
Registers the compatibility rule by default. |
pkg/workflow/lint/lint_test.go |
Updates rule-registration expectations. |
pkg/workflow/lint/bash_case_pattern.go |
Implements Bash case-pattern compatibility checking. |
pkg/workflow/lint/bash_case_pattern_test.go |
Covers patterns, comments, expressions, and shell selection. |
pkg/workflow/checkout_step_generator.go |
Generates balanced fetch-script case patterns. |
pkg/workflow/checkout_manager_test.go |
Adds syntax checks independent of ShellCheck availability. |
pkg/cli/validation_issue.go |
Adds rule IDs, columns, and script-location metadata. |
pkg/cli/shellcheck.go |
Reports structured findings with step and snippet locations. |
pkg/cli/compile_validation.go |
Preserves extended metadata during sanitization. |
pkg/cli/compile_scanner_reporting.go |
Converts scanner results into batch diagnostics. |
pkg/cli/compile_scanner_reporting_test.go |
Tests reporting modes, failures, severity, and locations. |
pkg/cli/compile_scanner_differential.go |
Retains script metadata during dry-run filtering. |
pkg/cli/compile_pipeline.go |
Integrates scanner reporting into batch compilation. |
pkg/cli/compile_external_tools.go |
Forwards scanner results to the compile reporter. |
pkg/cli/compile_external_tools_test.go |
Updates batch-diagnostic expectations. |
pkg/cli/compile_development.go |
Adjusts warning accounting and scanner diagnostics. |
pkg/cli/actionlint.go |
Captures findings and parsing failures for reporting. |
docs/src/content/docs/reference/workflow-linters.md |
Documents compatibility linting, JSON diagnostics, and comparison procedures. |
.github/workflows/smoke-update-cross-repo-pr.lock.yml |
Regenerates balanced fetch patterns. |
.github/workflows/smoke-claude.lock.yml |
Regenerates the balanced fetch pattern. |
.github/workflows/daily-safeoutputs-git-simulator.lock.yml |
Regenerates balanced fetch patterns. |
.github/workflows/cgo.yml |
Adds macOS system-Bash syntax checking. |
Review effort: Max
| err = handleActionlintExecutionError(runResult.err, strict, lockFiles, totalErrors, parseErr, runResult.stdout) | ||
| if shouldParseOutput && parseErr == nil && totalErrors > 0 { | ||
| return withActionlintFindings(err, runResult.stdout) | ||
| return reportActionlintResult(ctx, err, runResult.stdout, parseErr, shouldParseOutput && totalErrors > 0) |
There was a problem hiding this comment.
Strict mode now returns actionlint JSON parse errors even when the process exits 0; regression coverage exercises malformed successful output. Fixed in 9012715.
| err := runShellcheckOnLockFilesAndResources(ctx, lockFiles, resources, verbose, strict) | ||
| if err != nil { | ||
| reportCompileScannerResult(ctx, "shellcheck", err) | ||
| } |
There was a problem hiding this comment.
Compile reporting now captures missing ShellCheck/Docker and lock extraction failures while preserving non-strict return behavior; reporting regressions cover both paths. Fixed in 9012715.
| diagnostics, failure := collectScannerDiagnostics(err) | ||
| if len(diagnostics) == 0 { | ||
| failure = err | ||
| } |
There was a problem hiding this comment.
Wrapped operational errors now retain the Docker failure context when combined with findings; the mixed-result regression verifies the reported message. Fixed in 9012715.
| result := ValidationResult{ | ||
| Scope: "batch", Workflow: tool, Valid: gateErr == nil, | ||
| Errors: scannerValidationIssues(tool, gateErr), Warnings: []ValidationIssue{}, |
There was a problem hiding this comment.
MCP stderr fallback now runs only when structured ShellCheck findings are absent, avoiding duplicates while retaining fallback for operational-only results. Added regression coverage. Fixed in 9012715.
| shell, shellKnown := step.Fields["shell"].(string) | ||
| if !shellKnown && step.Fields["shell"] != nil { | ||
| continue | ||
| } | ||
| if shell != "" && shell != "bash" && !strings.HasPrefix(shell, "bash ") { |
There was a problem hiding this comment.
The Bash-case rule now resolves step, job, then workflow defaults for both Actions and authored AW inputs; inherited non-Bash shells have regression coverage. Fixed in 9012715.
…eam-lint-regression # Conflicts: # pkg/workflow/lint/rule_contract_test.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Resolved the conflicts and merged
Resolved the conflicts and merged |
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: 0c1e282
|
|
@copilot merge main, fix conflicts, recompile, fix https://github.com/github/gh-aw/actions/runs/40226608770/job/121275600737 |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
| for _, diagnostic := range diagnostics { | ||
| finding := diagnostic.Finding | ||
| issues = append(issues, ValidationIssue{ | ||
| Type: tool + "_finding", Message: finding.Message, |
There was a problem hiding this comment.
Scanner findings now use a data-only prefix with normalized, control-character-safe, bounded text; regression coverage was added. Fixed in 0a85bcc.
|
@copilot run pr-finisher skill |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
| if err := json.Unmarshal([]byte(outputStr), &results); err != nil { | ||
| return outputStr | ||
| } | ||
| for _, result := range results { |
There was a problem hiding this comment.
MCP ShellCheck stderr diagnostics are now neutralized and bounded in both development and standard output paths, with regression coverage. Fixed in 0a85bcc.
| Line: e.Line, | ||
| File: e.File, | ||
| Severity: e.Severity, | ||
| e.Message = stringutil.SanitizeErrorMessage(e.Message) |
There was a problem hiding this comment.
The JSON-boundary sanitizer now also covers Script.JobID, Script.StepID, and Script.Source, with regression coverage. Fixed in 0a85bcc.
|
@copilot fix security bot comments |
…gression' into copilot/close-downstream-lint-regression # Conflicts: # pkg/cli/compile_scanner_reporting_test.go # pkg/cli/shellcheck.go # pkg/workflow/lint/bash_case_pattern.go # pkg/workflow/lint/bash_case_pattern_test.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…eam-lint-regression Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| results = append(results, ValidationResult{Scope: "batch", Workflow: "shellcheck", Valid: false, Warnings: []ValidationIssue{}}) | ||
| index = len(results) - 1 | ||
| } else if index >= 0 && index < len(results) { | ||
| for _, issue := range append(results[index].Errors, results[index].Warnings...) { |
There was a problem hiding this comment.
The shared sanitizer now emits single-line, bounded diagnostics and neutralizes controls/markdown-like markers for both ShellCheck paths. Covered by regressions. Fixed in af50e96.
| if index == -1 { | ||
| results = append(results, ValidationResult{Scope: "batch", Workflow: "shellcheck", Valid: false, Warnings: []ValidationIssue{}}) | ||
| index = len(results) - 1 | ||
| } else if index >= 0 && index < len(results) { |
There was a problem hiding this comment.
The shared sanitizer now redacts common secret assignments and secret-like identifiers before adding ShellCheck messages to JSON; both MCP paths have regression coverage. Fixed in af50e96.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| File: info.LockFile, | ||
| } | ||
| location := &ValidationScriptLocation{ | ||
| JobID: info.JobID, StepID: info.ID, StepName: info.Name, StepIndex: info.Index, |
There was a problem hiding this comment.
Fixed in b675bb1 by omitting all five free-text script identity fields from JSON. Raw identities remain internal so differential matching is unchanged; numeric locations remain available. Regression coverage verifies omission from both errors and warnings, and local validation passes.
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: af50e96
|
|
@copilot merge main, fix ci failures |
…eam-lint-regression Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
@copilot fix ci failure |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Ran |
The CGO failures are fixed in the already-published 251be7d: valid unavailable-tool fixture, PATH-independent Docker stub, and non-strict policy tested through the public runner. Verified the three failing tests and |
|
🎉 This pull request is included in a new release. Release: |


Downstream comparisons exposed Bash 3.2-incompatible fetch scripts and optional linter findings missing from successful compile JSON. These changes address both gaps while preserving default exit behavior and legitimate credential handoffs.
casepatterns without changing intentional glob matching. Add the native Actions/AWbash-case-patternrule, independent syntax checks, and macOS system-Bash CI coverage. Regenerate three affected locks.Verification boundary: All 348 local workflows compile without lock drift. The 243 downstream comparisons and local Bash 3.2 execution remain unverified because required downloads were inaccessible.