Skip to content

feat(doctor): toolchain check — missing pip/venv/node named with fix: hints - #104

Merged
Icaro0310 merged 2 commits into
mainfrom
feat/doctor-toolchain
Oct 20, 2026
Merged

Icaro0310 merged 2 commits into
mainfrom
feat/doctor-toolchain

Conversation

@Icaro0310

@Icaro0310 Icaro0310 commented Oct 20, 2026 •

Copy link
Copy Markdown
Owner

What

New read-only doctor check toolchain — detects the missing test/build toolchain per-OS, the failure class behind the 2026-10-02 incident where 16 E2E runs were triaged as code-fail while the environment itself was bare (env-missing vs code-fail must be diagnosable instantly).

Probes (shutil.which + tiny subprocess calls where a binary alone is ambiguous):

  • python3 / python / py
  • pip3 / pip — plus <python> -m pip --version when no script is on PATH (module-only = WARN degraded, not missing)
  • <python> -m venv --help + import ensurepip — the Debian python3-venv split a PATH lookup cannot see; a venv that builds without pip FAILs as env-missing
  • node (with nodejs alias), npm, git

Each missing piece is a named FAIL with a concrete fix: for the detected package manager: apt/dnf/pacman on Linux (via /etc/os-release ID/ID_LIKE), brew on macOS, winget on Windows, generic on unknown distros. Read-only always: 10 s subprocess timeouts, no network, nothing installed or written. No 'not detected' state — a bare system reports FAILs, that loudness is the point.

Changes

  • checks/toolchain.py + registry after deps-internals
  • README check table row, CHANGELOG Added entry
  • conftest.py autouse _no_toolchain_probes stub (same doctrine as _no_scheduler_probes) so run_all tests stay hermetic on any host
  • tests/test_toolchain.py: 23 tests, all probes injected — no real system probing

Verify

  • pytest packages/doctor/tests — 442 passed, 2 skipped
  • ruff check packages/doctor — clean
  • Real run on the dev box correctly FAILs pip (no pip/pip3 on PATH, python3 -m pip absent — Ubuntu split) while python/venv/node/npm/git PASS

Devin Review

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 20, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 5 potential issues.

Devin Review

venv = _venv_finding(probe, pyexe, manager)
if venv is not None:
findings.append(venv)
findings.append(_binary_finding("node", _NODE_NAMES, which, manager))

@devin-ai-integration devin-ai-integration Bot Oct 20, 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.

🟡 Node alias fix leaves scripts broken

When only nodejs exists, _binary_finding recommends installing nodejs again. On systems where that package supplies only nodejs, the node command remains unavailable.

Learn more

An alias-only nodejs executable triggers the warning because build scripts require the node name. The node install command for apt installs the same nodejs package that provided the existing alias. On distributions where that package supplies only nodejs, following the hint cannot change the failed command.

Example: An older Debian machine has /usr/bin/nodejs but no /usr/bin/node. The check warns and suggests sudo apt install nodejs; after running it, scripts invoking node still fail.

Recommended fix: For the alias-only case, provide a remedy that creates or installs an actual node executable, with platform-appropriate guidance distinct from the missing-node package install.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in b0cf8e3 — a nodejs-only hit is now WARN 'only nodejs on PATH ... commands invoking node still fail' with the install hint, since the alias never makes the shell resolve the name test scripts actually run.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in b0cf8e3 — an alias-only hit is now WARN, not PASS: with only nodejs on PATH the check reports "only 'nodejs' on PATH, commands invoking 'node' still fail" and points at the real package, since build/test scripts invoke node and the shell cannot resolve it through the alias.

Comment on lines +203 to +211
resolved = which(name)
if resolved:
alias = "" if name == "python3" else f" (resolved as '{name}')"
return (
Finding(
CHECK_ID, Status.PASS,
f"python on PATH at {resolved}{alias}",
),
resolved,

@devin-ai-integration devin-ai-integration Bot Oct 20, 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.

🟡 Broken Python alias hides working launcher

When python --version raises an OSError, _python_finding stops before trying a working py launcher. The check reports Python unverified and probes pip and venv through the broken alias.

Learn more

The probe wrapper returns None for both timeouts and execution failures such as OSError. An execution failure means the candidate is unusable, but this branch stops the search and passes that candidate to pip and venv instead of trying the next interpreter.

Example: On Windows, which('python') returns a Store alias that cannot execute and which('py') returns a working launcher. The first probe returns None; the check never tries py, and subsequent probes target the unusable alias.

Recommended fix: Preserve the distinction between timeout and execution failure in _probe. Continue to the next candidate after an execution failure; retain an unverified verdict for timeouts without hiding a later working candidate.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in b0cf8e3 — each python candidate must answer --version before it counts: a Store alias/broken shim falls through to the next name (a working py behind a dead python still wins). All-broken FAILs naming the shims; an unanswerable probe stays WARN 'could not be probed' per the unavailable-not-missing doctrine.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in b0cf8e3 — which alone only proves a file exists; each python candidate must now answer --version, so a broken shim/Store alias falls through to the next name (a working py behind a dead python still wins). An unanswerable probe stays WARN 'could not be probed', never FAIL.

Comment on lines +287 to +303
ensurepip = probe([pyexe, "-c", "import ensurepip"])
if ensurepip is None:
return Finding(
CHECK_ID, Status.WARN,
"'import ensurepip' did not answer — pip bootstrap inside "
"fresh venvs unverified",
)
if ensurepip.returncode != 0:
return Finding(
CHECK_ID, Status.FAIL,
"'python -m venv' works but ensurepip is missing — fresh "
"venvs get no pip (Debian splits this into 'python3-venv')",
fix=_fix("venv", manager),
)
return Finding(
CHECK_ID, Status.PASS,
f"'{pyexe} -m venv' works and ensurepip can bootstrap pip",

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.

🟡 Missing bootstrap wheels pass venv check

When ensurepip imports but its pip wheels are absent, _venv_finding reports PASS. Creating a default venv then fails during pip bootstrap, leaving the toolchain incorrectly green.

Learn more

The venv help command checks only that the module's CLI exists. Importing ensurepip does not load or run the pip wheel that venv uses to populate a fresh environment. On a partial installation whose ensurepip wheels are absent, both probes succeed while default venv creation fails during _setup_pip.

Example: A Python installation has the ensurepip module, but its bundled pip wheel or distro wheel directory is missing. python -m venv --help and python -c 'import ensurepip' exit zero. python -m venv new-env fails when bootstrapping pip, despite the PASS finding.

Recommended fix: Verify that pip bootstrap resources exist and can actually bootstrap, ideally by creating and deleting a temporary venv with pip and checking its pip module. Keep the probe isolated from the project and bound its runtime.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in b0cf8e3 — after import ensurepip succeeds the check now lists ensurepip/_bundled for *.whl; an importable ensurepip with no wheel FAILs 'bundles no pip wheel — fresh venvs fail during pip bootstrap' with the python3-venv hint.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in b0cf8e3 — after import ensurepip succeeds the check also lists ensurepip/_bundled for .whl files; a partial install that imports but ships no wheel now FAILs (fresh venvs die in _setup_pip), and an unanswerable wheel probe degrades to WARN. 445 tests green incl. the new partial-install case.

Comment thread packages/doctor/README.md Resolved
@Icaro0310
Icaro0310 force-pushed the feat/doctor-toolchain branch from b0cf8e3 to b0e0100 Compare October 20, 2026 20:01
… bundled wheels

- python candidates must answer --version: a Store-alias/broken shim
  falls through to the next name (a working py behind a dead python
  still wins); all-broken FAILs naming the shims, unanswerable probe
  stays WARN 'could not be probed'
- nodejs-only is WARN, not PASS: commands invoking 'node' still fail
- ensurepip that imports but bundles no pip wheel FAILs — fresh venvs
  die during _setup_pip
- Windows pip fix names the resolved interpreter (python -m ensurepip)
  instead of hardcoding the py launcher
- per-OS toolchain guidance moved to the platform guides; shared table
  stays manager-neutral

Generated with [Devin](https://devin.ai) & QA Engineer Icaro Galvao

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@Icaro0310
Icaro0310 force-pushed the feat/doctor-toolchain branch from b0e0100 to 5a2077d Compare October 20, 2026 20:13
@Icaro0310

Copy link
Copy Markdown
Owner Author

Admin merge — par-por-finding justification for the held gate:

The auto-approve gate holds this PR with 3 unresolved blocking threads. All three were filed at 12:59 against the original head 9d776cb and fixed in b0cf8e3 (14:36); the Devin re-review returned STATUS_CLEAN on both b0e0100 and 5a2077d, and each thread has a reply citing the fix.

  1. Node alias (toolchain.py ~386) — fixed: an alias-only nodejs hit now returns WARN naming the real node command as still failing, pointing at the concrete package; it no longer silently passes while build scripts break.
  2. Broken Python shim (toolchain.py ~234) — fixed: each candidate must answer --version, so a Store-alias/broken shim falls through to the next name (working py behind dead python still wins); unanswerable probe degrades to WARN, never FAIL.
  3. ensurepip bootstrap wheels (toolchain.py ~287) — fixed: after import ensurepip the check lists ensurepip/_bundled for .whl files; a partial install that imports but ships no wheel now FAILs with the venv fix hint.

Gate mechanics: the blocking counter only clears when devin-ai-integration resolves the threads itself; two clean re-review passes did not resolve them (the bot posts STATUS_CLEAN but did not mark resolution). Per gate-closure doctrine (re-push + one clean pass → --admin with per-finding justification), merging administratively. 445 doctor tests green on ubuntu+windows; ruff clean.

@Icaro0310
Icaro0310 merged commit 0defd00 into main Oct 20, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant