Skip to content

fix(doctor): reject failed and malformed local Go runtimes - #50

Merged
johnny4young merged 5 commits into
mainfrom
codex/doctor-runtime-validation
Oct 8, 2026
Merged

johnny4young merged 5 commits into
mainfrom
codex/doctor-runtime-validation

Conversation

@johnny4young

@johnny4young johnny4young commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Problem

gos doctor treated locating go on PATH as evidence of a working runtime. go version failures were masked by || true, so the command could return a healthy report and exit 0 even when Go exits 7. A harmless executable fixture reproduces the false positive on main 05d9e25361e1d7e328f0fc057b7d79b442ee2304.

Change

  • Preserve the local version probe's status and mark any nonzero result as a go problem, with the original exit code and actionable PATH/runtime repair guidance
  • Require recognizable version/platform output and reject empty, malformed, multiline or control-bearing responses
  • Preserve stable, beta, rc, development and vendor-suffixed versions, experiment metadata ( X: and -X:), and Windows CRLF
  • Keep GOTOOLCHAIN=local; never request an SDK download or automatically reinstall during diagnosis
  • Update README and changelog; add a standalone portable fixture suite with both human and JSON assertions

The parser is a runtime diagnostic, not proof of binary authenticity. gos verify remains the archive-backed integrity command.

Duplicate-PR audit

Inspected every open patch before implementing: #41 dependency bump, #44 project resolver, #45 concurrency, #46 suite observations, #47 partial/skip classification, #48 user-command execution, and #49 sudo-output replay. None changes this check. This fix is independent on main. Existing work and user changes were preserved.

The CI proposal to remove the later gos verify --json walk is deliberately not included: it exercises the installed CLI/JSON integration separately from action verify:true. Preserving both contracts is more valuable than an unproven saving. No matrix, checksum/feed policy, integrity check, timeout, threshold, release or deployment is changed.

Verification

  • Red first: a valid-looking go version response with exit 7 fails the new test against unchanged main because doctor incorrectly returns 0
  • Final focused regression passes in both text and JSON: exits 7/126/127, malformed/empty/multiline/control output, local-only probe argument/environment, valid version variants, and CRLF
  • Full local scripts/validate-local.bash passes all 32 Linux Bash suites, command-surface sync, workflow YAML/invariants, Bash syntax, ShellCheck 0.11.0, Zsh syntax, CLI smoke and whitespace checks
  • Fixtures use temporary HOME/install/cache/PATH and harmless Go executables; no real Go runtime, SDK download, or user's toolchain mutation
  • shfmt, Fish, PowerShell and native macOS Bash 3.2 were unavailable locally; hosted CI below supplies those applicable checks

Review notes

Compatibility review checked official Go sources: cmd/go/internal/version/version.go prints runtime.Version plus GOOS/GOARCH; toolchain names permit vendor suffixes; linker metadata permits experiment markers with either separators. These cases are explicit regressions. Existing Windows fixtures establish the CRLF boundary.

Hosted qualification

CI run 37551330622 completed successfully on exact source head 5e72248ef64549919b8f4005305c2bb14149d580: all 10 workflow jobs and all 11 checks (including GitGuardian) are green.

  • The new tests/doctor-runtime.bash regression ran and passed on Linux, macOS Bash 3.2.57 (and the regular macOS pass), and Windows Git Bash. It was not skipped on any host.
  • Linux: 32 suites passed, 0 OS-skipped. macOS: 30 passed, 2 OS-skipped (homebrew-tap and packaging, covered on Linux). Windows: 13 passed, 19 existing OS-skipped suites. These are the existing platform rules, not 32 executed suites on each platform.
  • Windows PowerShell parsing, PSScriptAnalyzer, installer/lifecycle fixtures, and Action PowerShell/cmd launcher checks passed. Linux Fish completion checks passed. Fish behavior is intentionally not exercised on macOS/Windows; Windows also skips Zsh behavior and filesystem-specific read-only/symlink assertions.
  • The shallow-checkout changelog guard reports no reachable tag; this is existing unrelated conditional coverage. No skip policy was weakened.
  • shfmt, ShellCheck, actionlint, workflow invariants and all three installed-Action integration jobs passed. Both distinct integrity verification calls remain intact.

Qualified for review; merge remains the maintainer's decision. No merge or release was performed.

Main was refreshed before publication and remains 05d9e25361e1d7e328f0fc057b7d79b442ee2304. The doctor parser uses a local C locale for deterministic character classes. The final source tree passed hosted shfmt. Changelog additions may need a small conflict resolution after other pending PRs merge; retain all entries.

claude added 4 commits October 8, 2026 05:41
- move the check into a helper that scopes LC_ALL=C to the regex alone
- reject C1 controls and non-ASCII bytes instead of echoing them
- accept the minor-less go1 release output
- cover more platforms, devel and distro formats, and malformed bytes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExmnfUngrDYjAheGvYwMnn
Git Bash's C locale classifies bytes above 0x7e as printable, so the
pattern alone accepted a 0xff byte there. Check for printable ASCII
byte-wise with tr before matching.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ExmnfUngrDYjAheGvYwMnn
@johnny4young
johnny4young merged commit fd93760 into main Oct 8, 2026
11 checks passed
@johnny4young
johnny4young deleted the codex/doctor-runtime-validation branch October 8, 2026 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants