Cover wasm-only files in custom lint CI

[Copilot]

The custom analyzer CI pass only ran under the default Linux build, so js || wasm files were never loaded by go/packages. That left wasm-only production code outside all custom analyzers and allowed drift, including a real sprintfint violation in pkg/console/console_wasm.go.

• Close the concrete wasm drift

  • Align pkg/console/console_wasm.go with the default implementation by replacing the un-migrated integer formatting path with strconv.Itoa.

  lineNumWidth := len(strconv.Itoa(maxLineNum))

• Make golint-custom usable for alternate target analysis

  • Keep the linter runner itself host-built even when GOOS/GOARCH are set for the analysis pass.
  • Add a LINTER_PACKAGES override so CI can scope analysis to wasm-capable packages instead of forcing all packages through a wasm typecheck.

• Add a wasm-targeted custom-lint CI pass

  • Extend cgo.yml with a second custom-linter invocation under GOOS=js GOARCH=wasm.
  • Scope that pass to the packages that actually contain *_wasm.go production files: pkg/console, pkg/parser, pkg/styles, pkg/tty, and pkg/workflow.

• Resolve newly exposed wasm-only findings

  • Update pkg/workflow/github_cli_wasm.go to use context-aware command construction in the wasm stubs, so the new pass starts green instead of introducing a new failing blind spot.

[Copilot]

Pull request overview

This PR extends the project’s custom analyzer CI coverage to include GOOS=js GOARCH=wasm so *_wasm.go production code is loaded by go/packages and checked by the custom analyzers, closing a real wasm-only drift in pkg/console.

Changes:

• Fix wasm-only integer formatting in pkg/console/console_wasm.go by switching to strconv.Itoa.
• Enhance make golint-custom to (a) keep the linter runner host-built even under a wasm analysis environment and (b) allow scoping analysis via LINTER_PACKAGES.
• Add a wasm-targeted custom-lint CI step and adjust wasm stubs to construct commands via exec.CommandContext.

Show a summary per file

File	Description
pkg/workflow/github_cli_wasm.go	Uses a context-aware exec.CommandContext helper in wasm stubs to satisfy custom analyzer expectations under wasm typechecking.
pkg/console/console_wasm.go	Aligns wasm error formatting logic with the non-wasm implementation by avoiding fmt.Sprintf for integer-to-string conversion.
Makefile	Adds LINTER_PACKAGES and ensures the linter runner is built for the host even when GOOS/GOARCH are set for analysis.
.github/workflows/cgo.yml	Adds a second custom-linter invocation targeted at wasm packages under GOOS=js GOARCH=wasm.

Review details

Tip

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

• Files reviewed: 4/5 changed files
• Comments generated: 1
• Review effort level: Low

[github-actions]

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

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

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

No ADR enforcement needed: PR #42650 does not have the 'implementation' label and has only 12 new lines of code in business logic directories (threshold: 100).

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. Test Quality Sentinel skipped.

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs and /codebase-design — approving with minor suggestions.

📋 Key Themes & Highlights

Key Themes

• Flag duplication: Both CI steps carry identical LINTER_FLAGS strings; adding a new analyzer flag to one won't propagate to the other automatically.
• Hardcoded package list: The wasm package scope is an explicit allowlist — new packages with *_wasm.go files could silently fall outside coverage.
• Nil-context guard: The safety check in ghUnavailableCommand is correct but undocumented; a one-line comment would clarify intent.

Positive Highlights

• ✅ Root cause addressed properly — the sprintfint violation in console_wasm.go is the concrete symptom, and the new CI pass is the structural fix that prevents recurrence.
• ✅ env -u GOOS -u GOARCH go build is the right technique to keep the linter runner host-native without touching the analysis environment.
• ✅ Clean DRY improvement in github_cli_wasm.go — three identical stubs collapsed into a single ghUnavailableCommand helper with proper context threading.
• ✅ strconv.Itoa fix aligns the wasm implementation with the non-wasm console.go, which already used this form.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 49.7 AIC · ⌖ 7.61 AIC · ⊞ 6.6K
_{Comment /matt to run again}

[github-actions]

[/codebase-design] The full LINTER_FLAGS string is copy-pasted from the step above — if a new analyzer flag is added to the main step, the wasm step silently misses it.

💡 Suggestion: extract a Makefile variable

Define a shared default in the Makefile so the flag set is the single source of truth:

# Default flags shared by all golint-custom passes
LINTER_FLAGS ?= -errstringmatch -panicinlibrarycode ... -test=false

Then the CI steps simply omit LINTER_FLAGS and both inherit the same defaults automatically:

- name: Run custom linters
  run: make golint-custom
- name: Run custom linters (wasm)
  run: GOOS=js GOARCH=wasm make golint-custom LINTER_PACKAGES="./pkg/console ..."

@copilot please address this.

[github-actions]

[/diagnosing-bugs] The wasm package list (./pkg/console ./pkg/parser ./pkg/styles ./pkg/tty ./pkg/workflow) is hardcoded. Any future package that gains a *_wasm.go production file won't be covered automatically, recreating the same blind spot this PR fixes.

💡 Suggestion: auto-discover packages with wasm files

Consider deriving the list from the repository instead:

WASM_PACKAGES ?= $(shell grep -rl '(go/redacted):build js' --include='*.go' $(shell go list -f '{{.Dir}}' ./...) | xargs -I{} dirname {} | sort -u | sed 's|.*|\./&|')

Or document the manual curation in a comment adjacent to the step so reviewers know to update it when adding new wasm files.

@copilot please address this.

[github-actions]

[/diagnosing-bugs] The nil-context guard is good defensive programming, but none of the current callers can pass nil (they either have a real ctx or use context.Background()). Adding a brief comment would explain the intent for future readers and keep it from looking like dead code.

💡 Suggested comment

func ghUnavailableCommand(ctx context.Context) *exec.Cmd {
    // Guard against nil context: exec.CommandContext panics on nil.
    if ctx == nil {
        ctx = context.Background()
    }
    return exec.CommandContext(ctx, "echo", "gh CLI not available in Wasm")
}

@copilot please address this.

[github-actions]

Review: Cover wasm-only files in custom lint CI

Non-blocking observations. The Go fixes (strconv.Itoa in console_wasm.go, exec.CommandContext refactor in github_cli_wasm.go) are correct, and the Makefile env -u GOOS -u GOARCH trick cleanly solves the cross-compile problem. Three maintenance concerns worth addressing before this pattern gets copied further:

🔍 Findings summary

1. LINTER_FLAGS duplicated verbatim (.github/workflows/cgo.yml line 1211) — 15 flags copy-pasted from the step above. Analyzer additions/removals on the default step silently skip the wasm pass. Extract to a shared Makefile variable or workflow env var.

2. LINTER_PACKAGES hardcoded list (.github/workflows/cgo.yml line 1211) — New packages with *_wasm.go files won't be covered automatically. Add a comment or derive the list dynamically.

3. exec.CommandContext changes observable error type (pkg/workflow/github_cli_wasm.go line 50) — If context is already cancelled when a stub cmd.Start() is called, callers now get context.Canceled instead of the previous platform error. The "never called at runtime" guarantee is a soft contract; a brief comment on the known behavioral difference would help future readers.

🔎 Code quality review by PR Code Quality Reviewer · 80.1 AIC · ⌖ 7.57 AIC · ⊞ 1.6K
_{Comment /review to run again}

[github-actions]

Duplicated LINTER_FLAGS will silently diverge from the default step over time.

💡 Suggested fix

The exact same 15-analyzer flags string is copy-pasted into both CI steps. Whenever an analyzer is added to or removed from the default step, the wasm step silently uses a stale set — meaning wasm-only code could evade a newly added rule, or the step could fail spuriously when an old flag is removed.

Extract the shared flags to a workflow-level env: variable or a Makefile variable so both invocations stay in sync automatically:

env:
  CUSTOM_LINTER_FLAGS: >-
    -errstringmatch -panicinlibrarycode -manualmutexunlock ...

jobs:
  ...
    - name: Run custom linters
      run: make golint-custom LINTER_FLAGS="$CUSTOM_LINTER_FLAGS"

    - name: Run custom linters (wasm)
      run: GOOS=js GOARCH=wasm make golint-custom LINTER_FLAGS="$CUSTOM_LINTER_FLAGS" LINTER_PACKAGES="..."

Or add a DEFAULT_LINTER_FLAGS variable to the Makefile and reference it from both steps. Either approach prevents silent coverage gaps when the analyzer set evolves.

[github-actions]

Using exec.CommandContext changes the observable error type when context is already cancelled.

💡 Details

Previously all three stubs used exec.Command, which returns an *exec.Cmd that fails at Start()/Run() with a platform-not-supported error regardless of context state. After this change, if the caller's context is already cancelled or deadline-exceeded when cmd.Start() is called, the error is context.Canceled / context.DeadlineExceeded instead.

Any caller that inspects the error message or type (e.g., looking for "not available in Wasm") will receive a different error class. The file comment states these stubs are never called at runtime due to WithSkipValidation(true), but that is a runtime contract that tests or future code might not uphold. If using exec.CommandContext is intentional to satisfy the linter, consider adding a comment explaining the known behavioral difference, or wrapping the result to normalize the error.

[github-actions]

Hardcoded LINTER_PACKAGES list will silently miss future wasm-only packages.

💡 Details

The five packages are manually enumerated. Any new package that gains a *_wasm.go file outside this list will never be covered by the wasm lint pass, and there is no automated guard to catch the omission.

At minimum, add a comment that developers must keep this list in sync with:

find pkg -name '*_wasm.go' | xargs -I{} dirname {} | sort -u

A stronger fix would derive the list dynamically in the Makefile by filtering packages that contain wasm build-tag files, so new packages are covered automatically.