Replace non-idiomatic len(string) == 0 checks flagged by lenstringzero

[Copilot]

This PR addresses the lint-monster finding that identified 56 non-idiomatic string emptiness checks using len(s) == 0. It updates those call sites to idiomatic Go string comparisons to satisfy the custom lenstringzero rule.

• Scope of remediation

  • Replaced all linter-reported len(<string>) == 0 occurrences with <string> == "".
  • Applied across affected areas in pkg/cli, pkg/console, pkg/parser, pkg/stringutil, pkg/gitutil, and additional reported pkg/workflow call sites.

• Behavior-preserving code changes

  • Kept existing control flow and semantics unchanged; only the string-empty predicate form was updated.
  • Includes equivalent updates where the string expression is nested (e.g., len(strings.TrimSpace(...)) == 0).

• Representative update

  // before
  if len(trimmedLine) == 0 || (trimmedLine[0] != '@' && trimmedLine[0] != '{') { ... }
  
  // after
  if trimmedLine == "" || (trimmedLine[0] != '@' && trimmedLine[0] != '{') { ... }

[Copilot]

Pull request overview

This PR updates string-emptiness checks across the codebase to satisfy the custom lenstringzero linter by replacing len(s) == 0 (and equivalent) with idiomatic s == "", aiming to be behavior-preserving while removing lint noise.

Changes:

• Replaced len(<string>) == 0 checks with <string> == "" in workflow/compiler/runtime helpers and YAML parsing/validation logic.
• Applied the same idiomatic string-empty predicate updates across CLI commands, codemods, console input validation, and parser utilities.
• Updated nested expressions like len(strings.TrimSpace(...)) == 0 to strings.TrimSpace(...) == "" to maintain semantics while satisfying the linter.

Show a summary per file

File	Description
pkg/workflow/xml_comments.go	Uses idiomatic empty-string checks when validating code block markers.
pkg/workflow/template_injection_validation.go	Uses trimmed == "" for blank-line handling during YAML scanning.
pkg/workflow/runtime_validation.go	Uses strings.TrimSpace(line) == "" for block-scalar line emptiness checks.
pkg/workflow/redact_secrets.go	Uses rest == "" guard before indexing during secret reference scanning.
pkg/workflow/engine_helpers.go	Uses value == "" for YAML string formatting early-return.
pkg/workflow/compiler_safe_outputs_job.go	Uses scriptName == "" for handler-name fallback logic.
pkg/workflow/codex_logs.go	Uses part == "" guard before indexing in JSON fallback parsing.
pkg/workflow/cache.go	Uses id == "" in cache ID validation guard.
pkg/stringutil/fuzzy_match.go	Uses a == "" / b == "" for early exits in Levenshtein distance.
pkg/parser/remote_fetch.go	Uses lines[0] == "" when validating git ls-remote output parsing.
pkg/parser/import_directive.go	Uses trimmedLine == "" in fast-path directive detection.
pkg/parser/github_urls.go	Uses s == "" guard in GitHub identifier validation helper.
pkg/gitutil/gitutil.go	Uses s == "" guard in hex-string validation.
pkg/console/input.go	Uses s == "" in secret input validation.
pkg/cli/yaml_frontmatter_utils.go	Uses idiomatic empty-string checks in frontmatter/YAML block utilities.
pkg/cli/validators.go	Uses trimmed == "" for workflow intent validation.
pkg/cli/trial_repository.go	Uses strings.TrimSpace(...) == "" for “no changes” detection.
pkg/cli/trial_helpers.go	Uses strings.TrimSpace(...) == "" for “no changes” detection.
pkg/cli/run_workflow_execution.go	Uses parts[0] == "" for input key validation.
pkg/cli/run_push.go	Uses strings.TrimSpace(...) == "" for “no staged changes” detection.
pkg/cli/remove_command.go	Uses idiomatic empty-string checks in include parsing.
pkg/cli/project_command.go	Uses s == "" guard in string capitalization helper.
pkg/cli/mcp_tools_readonly.go	Uses outputStr == "" when deciding whether missing output implies execution failure.
pkg/cli/codemod_user_rate_limit.go	Uses trimmed == "" in codemod line scanning.
pkg/cli/codemod_steps_run_secrets_env.go	Uses idiomatic empty-string checks throughout YAML line scanning/rewrite logic.
pkg/cli/codemod_playwright_domains.go	Uses trimmed == "" while removing nested YAML properties.
pkg/cli/codemod_mount_as_clis.go	Uses trimmed == "" in codemod line scanning.
pkg/cli/codemod_mcp_network.go	Uses idiomatic empty-string checks throughout YAML line scanning/updates.
pkg/cli/codemod_github_repos.go	Uses trimmed == "" in codemod line scanning.
pkg/cli/codemod_github_app.go	Uses trimmed == "" in codemod line scanning.
pkg/cli/codemod_github_app_client_id.go	Uses trimmed == "" in codemod line scanning.
pkg/cli/codemod_engine_steps.go	Uses idiomatic empty-string checks throughout YAML line scanning/migration logic.
pkg/cli/codemod_engine_env_secrets.go	Uses trimmed == "" while removing unsafe/empty YAML env entries.
pkg/cli/codemod_discussion_flag.go	Uses trimmedLine == "" when removing nested YAML content.
pkg/cli/codemod_checkout_persist_credentials_false.go	Uses idiomatic empty-string checks throughout YAML section scanning/rewrite logic.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 35/35 changed files
• Comments generated: 0

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

🧪 Test Quality Sentinel completed test quality analysis.

No test files were added or modified in PR #38015. The PR only replaces non-idiomatic len(string) == 0 checks with string == "" across 35 production Go files. Test Quality Sentinel skipped.

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #38015 does not have the implementation label (has_implementation_label=false) and has 55 new lines in business logic directories, which is at or below the 100-line threshold (requires_adr_by_default_volume=false).

[github-actions]

Skills-Based Review 🧠

Applied /zoom-out and /improve-codebase-architecture — approving; one minor consistency note on fuzzy_match.go.

📋 Key Themes & Highlights

Key Themes

• Mechanically correct: All 55 len(s) == 0 → s == "" substitutions are semantically equivalent. No logic, control-flow, or abstraction changes.
• Security-sensitive paths verified: pkg/workflow/redact_secrets.go and pkg/workflow/template_injection_validation.go are transformed correctly — the empty guards still precede any index access.
• One minor inconsistency: pkg/stringutil/fuzzy_match.go changes aLen == 0 (an int variable) to a == "". Since aLen is an int, lenstringzero should not flag it — this looks like a linter false-positive or an over-eager manual extension of the pattern. The code is still correct, but aLen/bLen are now computed eagerly and then bypassed in the early exits. See inline comment for two clean alternatives.

Positive Highlights

• ✅ Complete coverage — all 56 reported sites addressed
• ✅ len(slice) == 0 patterns (e.g. len(lines) == 0) are correctly left unchanged
• ✅ Nested expression forms like strings.TrimSpace(string(output)) == "" are handled correctly
• ✅ No imports added or removed — confirms purely idiomatic change

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 253.7 AIC · ⌖ 13.4 AIC

[github-actions]

[/zoom-out] aLen is an int variable — lenstringzero targets len(stringExpr) == 0 patterns, not intVar == 0. This change looks like a linter false-positive or a proactive consistency edit.

The semantics are still correct, but the code now has an asymmetry: aLen and bLen are eagerly computed two lines above (and are needed for the algorithm on lines 82–116), yet the early-exit guards bypass them entirely.

💡 Suggested alternatives

Option A — revert the early exits to use the precomputed variables (if the linter does not actually flag aLen == 0):

aLen := len(a)
bLen := len(b)
// Early exit for empty strings
if aLen == 0 {
    return bLen
}
if bLen == 0 {
    return aLen
}

Option B — move the length declarations after the early exits, making the data-flow obvious:

// Early exit for empty strings
if a == "" {
    return len(b)
}
if b == "" {
    return len(a)
}
aLen := len(a)
bLen := len(b)
// ... rest of algorithm

Either form removes the "declared-but-not-used-here" puzzle for future readers.

[github-actions]

Clean mechanical refactor — no blocking issues.

Review notes

• All len(s) == 0 → s == "" replacements are semantically identical for Go string types.
• aLen/bLen in fuzzy_match.go remain live — used throughout the Levenshtein DP loop; the changed early-exit guards are correct.
• Short-circuit evaluation is preserved everywhere compound conditions are changed (e.g. remove_command.go, redact_secrets.go).
• len(lines) == 0 in remote_fetch.go was always unreachable after strings.Split — pre-existing dead guard, not introduced here.
• outputStr in mcp_tools_readonly.go is intentionally checked untrimmed (same semantics as the original len(outputStr) == 0).

🔎 Code quality review by PR Code Quality Reviewer · ⌖ 13.3 AIC