Skip to content

fix: apply 13 lint-monster correctness/style findings (2026-06-29) #42165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Jun 29, 2026
Merged

fix: apply 13 lint-monster correctness/style findings (2026-06-29) #42165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
408849e
Initial plan
Copilot Jun 29, 2026
bddcf97
fix: apply 13 lint-monster correctness/style findings
Copilot Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions .github/extensions/agentic-workflows-dashboard/extension.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,19 @@ let workspacePath = process.cwd();

function execp(bin, args, cwd) {
return new Promise((resolve, reject) => {
execFile(bin, args, {
cwd,
env: { ...process.env, NO_COLOR: "1", GH_NO_UPDATE_NOTIFIER: "1" },
maxBuffer: 10 * 1024 * 1024,
}, (err, stdout, stderr) => {
if (err) reject(Object.assign(err, { stderr: stderr ?? "" }));
else resolve(stdout);
});
execFile(
bin,
args,
{
cwd,
env: { ...process.env, NO_COLOR: "1", GH_NO_UPDATE_NOTIFIER: "1" },
maxBuffer: 10 * 1024 * 1024,
},
(err, stdout, stderr) => {
if (err) reject(Object.assign(err, { stderr: stderr ?? "" }));
else resolve(stdout);
}
);
});
}

Expand Down Expand Up @@ -136,10 +141,7 @@ async function startServer() {

try {
if (pathname === "/" || pathname === "/index.html") {
const [html, css] = await Promise.all([
readFile(join(__dirname, "web", "index.html"), "utf8"),
readFile(join(__dirname, "web", "styles.css"), "utf8"),
]);
const [html, css] = await Promise.all([readFile(join(__dirname, "web", "index.html"), "utf8"), readFile(join(__dirname, "web", "styles.css"), "utf8")]);

@github-actions github-actions Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/zoom-out] Readability regression: Promise.all was collapsed to a single long line, contradicting the expansion applied to execFile and handler in the same PR.

The original multi-line form was clearer and consistent with this PR's other formatting improvements. The new single-line form is harder to scan at a glance.

💡 Suggested formatting 
const [html, css] = await Promise.all([
  readFile(join(__dirname, "web", "index.html"), "utf8"),
  readFile(join(__dirname, "web", "styles.css"), "utf8"),
]);

This mirrors the before-state and keeps each readFile call on its own line, which is easier to read and diff.

@copilot please address this.

res.setHeader("Content-Type", "text/html; charset=utf-8");
Comment on lines 142 to 145
res.end(html.replace("/*__APP_CSS__*/", css));
} else if (pathname === "/app.js") {
Expand Down Expand Up @@ -268,7 +270,10 @@ It never calls Go code directly — all data is fetched by running CLI subcomman
name: "refresh",
description: "Clear the data cache so the next listDefinitions/listRuns fetches fresh data from the CLI.",
inputSchema: { type: "object", additionalProperties: false },
handler: () => { cache.clear(); return { ok: true }; },
handler: () => {
cache.clear();
return { ok: true };
},
},
],
open: async ctx => {
Expand Down
16 changes: 4 additions & 12 deletions .github/extensions/agentic-workflows-dashboard/web/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,14 @@ <h2 class="Box-title">Workflow definitions <span class="Counter" x-text="definit
<div>
<div class="text-bold" x-text="definition.workflow"></div>
<div class="color-fg-muted text-small mt-1">Engine: <span x-text="definition.engine_id ?? '—'"></span></div>
<div class="color-fg-muted text-small" x-show="definition.labels && definition.labels.length">
Labels: <span x-text="(definition.labels ?? []).join(', ')"></span>
</div>
<div class="color-fg-muted text-small" x-show="definition.time_remaining">
Time remaining: <span x-text="definition.time_remaining"></span>
</div>
<div class="color-fg-muted text-small" x-show="definition.labels && definition.labels.length">Labels: <span x-text="(definition.labels ?? []).join(', ')"></span></div>
<div class="color-fg-muted text-small" x-show="definition.time_remaining">Time remaining: <span x-text="definition.time_remaining"></span></div>
</div>
<span :class="definitionStatusClass(definition)" x-text="definitionStatusLabel(definition)"></span>
</div>
</div>
</template>
<div class="Box-row color-fg-muted text-center py-3" x-show="!loadingDefinitions && definitionsPaged.totalItems === 0">
No workflow definitions found. Run <code>make build</code> then open this canvas again.
</div>
<div class="Box-row color-fg-muted text-center py-3" x-show="!loadingDefinitions && definitionsPaged.totalItems === 0">No workflow definitions found. Run <code>make build</code> then open this canvas again.</div>
</div>
<div class="Box-footer d-flex flex-items-center flex-justify-between" x-show="!loadingDefinitions && definitionsPaged.totalPages > 1">
<button class="btn btn-sm" :disabled="!definitionsPaged.hasPreviousPage" @click="loadDefinitionPage(definitionsPaged.page - 1)">Previous</button>
Expand Down Expand Up @@ -106,9 +100,7 @@ <h2 class="Box-title">Workflow runs <span class="Counter" x-text="runsPaged.tota
</div>
</div>
</template>
<div class="Box-row color-fg-muted text-center py-3" x-show="!loadingRuns && runsPaged.totalItems === 0">
No runs found. Run <code>gh aw logs</code> to check availability.
</div>
<div class="Box-row color-fg-muted text-center py-3" x-show="!loadingRuns && runsPaged.totalItems === 0">No runs found. Run <code>gh aw logs</code> to check availability.</div>
</div>
<div class="Box-footer d-flex flex-items-center flex-justify-between" x-show="!loadingRuns && runsPaged.totalPages > 1">
<button class="btn btn-sm" :disabled="!runsPaged.hasPreviousPage" @click="loadRunPage(runsPaged.page - 1)">Previous</button>
Expand Down
2 changes: 1 addition & 1 deletion pkg/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (c CommandPrefix) String() string {

// IsValid returns true if the command prefix is non-empty
func (c CommandPrefix) IsValid() bool {
return len(c) > 0
return c != ""
}

// WorkflowID represents a workflow identifier (basename without .md extension).
Expand Down
4 changes: 2 additions & 2 deletions pkg/constants/job_constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (j JobName) String() string {

// IsValid returns true if the job name is non-empty
func (j JobName) IsValid() bool {
return len(j) > 0
return j != ""
}

// StepID represents a GitHub Actions step identifier.
Expand All @@ -37,7 +37,7 @@ func (s StepID) String() string {

// IsValid returns true if the step ID is non-empty
func (s StepID) IsValid() bool {
return len(s) > 0
return s != ""
}

// MCPServerID represents a built-in MCP server identifier.
Expand Down
2 changes: 1 addition & 1 deletion pkg/constants/url_constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (d DocURL) String() string {

// IsValid returns true if the documentation URL is non-empty
func (d DocURL) IsValid() bool {
return len(d) > 0
return d != ""
}

// DefaultMCPRegistryURL is the default MCP registry URL.
Expand Down
2 changes: 1 addition & 1 deletion pkg/constants/version_constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (v Version) String() string {

// IsValid returns true if the version is non-empty
func (v Version) IsValid() bool {
return len(v) > 0
return v != ""
}

// ModelName represents an AI model name identifier.
Expand Down
4 changes: 2 additions & 2 deletions pkg/parser/frontmatter_content.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func extractFrontmatterMetadata(frontmatterYAML string, frontmatterStart int) ([
continue
}

if len(line) > 0 && line[0] != ' ' && line[0] != '\t' {
if line != "" && line[0] != ' ' && line[0] != '\t' {
colonIdx := strings.IndexByte(trimmed, ':')
if colonIdx > 0 {
key := strings.TrimSpace(trimmed[:colonIdx])
Expand Down Expand Up @@ -368,7 +368,7 @@ func generateDefaultWorkflowName(filePath string) string {
// Capitalize first letter of each word
words := strings.Fields(baseName)
for i, word := range words {
if len(word) > 0 {
if word != "" {
words[i] = strings.ToUpper(word[:1]) + word[1:]
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/parser/import_error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func findImportItemLocation(yamlContent string, importPath string) (line int, co
// If we're in the imports section and find a line with our import path
if inImportsSection {
// Check if this line exits the imports section (new top-level key)
if len(line) > 0 && line[0] != ' ' && line[0] != '-' && line[0] != '\t' {
if line != "" && line[0] != ' ' && line[0] != '-' && line[0] != '\t' {
break
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/parser/json_path_locator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ func TestExtractJSONPathFromValidationError(t *testing.T) {
// Compile schema and validate
compiler := jsonschema.NewCompiler()
var schemaDoc any
json.Unmarshal([]byte(schemaJSON), &schemaDoc)
if err := json.Unmarshal([]byte(schemaJSON), &schemaDoc); err != nil {
t.Fatalf("json.Unmarshal error: %v", err)
}
Comment on lines +141 to +143

schemaURL := "http://example.com/schema.json"
compiler.AddResource(schemaURL, schemaDoc)
Expand Down
2 changes: 1 addition & 1 deletion pkg/parser/remote_fetch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ func checkRemoteSymlink(client *api.RESTClient, owner, repo, dirPath, ref string

// If the response is an array, this is a directory listing — not a symlink
trimmed := strings.TrimSpace(string(raw))
if len(trimmed) > 0 && trimmed[0] == '[' {
if trimmed != "" && trimmed[0] == '[' {
remoteLog.Printf("Path component %s is a directory (not a symlink)", dirPath)
return "", false, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/parser/schema_errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func cleanOneOfMessage(message string) string {
return message
}

schemaErrorsLog.Printf("Simplifying oneOf error message (%d lines)", len(strings.Split(message, "\n")))
schemaErrorsLog.Printf("Simplifying oneOf error message (%d lines)", strings.Count(message, "\n")+1)

@github-actions github-actions Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/zoom-out] The allocation optimization is nullified by the very next statement.

The PR description says this change avoids allocating a []string just to count lines, but strings.Split(message, "\n") is called unconditionally on line 95 to populate lines. The strings.Count call saves nothing in practice because the []string allocation still happens immediately after.

💡 When this optimization would matter

This approach is a genuine win when the count is used in a conditional path that doesn't always reach the Split call:

// Worth optimizing: count guards the Split
if strings.Count(message, "\n")+1 > threshold {
    lines := strings.Split(message, "\n")
    // ...
}

Here, however, lines is always assigned right after the log line, so the split always runs. The change is still valid idiomatic Go — but the performance rationale in the PR description is misleading.

@copilot please address this.

lines := strings.Split(message, "\n")
var meaningful []string

Expand Down
2 changes: 1 addition & 1 deletion pkg/stringutil/sanitize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func SanitizeIdentifierName(name string, extraAllowed func(rune) bool) string {
}, name)

// Ensure it doesn't start with a number
if len(result) > 0 && result[0] >= '0' && result[0] <= '9' {
if result != "" && result[0] >= '0' && result[0] <= '9' {
result = "_" + result
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/stringutil/stringutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func NormalizeWhitespace(content string) string {
// Join back and ensure exactly one trailing newline if content is not empty
normalized := strings.Join(lines, "\n")
normalized = strings.TrimRight(normalized, "\n")
if len(normalized) > 0 {
if normalized != "" {
normalized += "\n"
}

Expand Down
Loading