feat(toolchain): add pyright to python ecosystem check-cmd - #859
Conversation
CI gates pyright but the local /toolchain:check batch was ruff-only, so agents could pass locally and still fail CI on type errors. Closes #834 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: 0f69ae76-07e0-4aae-9c4a-11688fc0e8ea
|
Claude finished @kyle-sexton's task in 2m 47s —— View job PR Review — feat(toolchain): add pyright to python ecosystem check-cmd
SummaryThis is a clean, well-scoped change that closes a real CI/local parity gap. The change is minimal and correct. No blocking issues. Two observations below, both minor. What looks goodCore change is correct. Appending Rung-4 semantics preserved. The PR description correctly notes that consumer Version bump is right.
CHANGELOG is accurate and detailed. The entry correctly describes the rung-4 scope, that Observations1.
No change required if the formula is confirmed — just worth a quick 2. The winget entry doesn't add pyright (or ruff), so a Windows user who uses only winget would get uv but still need VerdictApprove with minor follow-up. The functional change is correct and well-documented. The two observations are low-priority and don't block merge — the pip path covers all platforms, and the brew question is a quick lookup. No security, correctness, or repo-agnostic compliance issues. |
|
Claude finished @kyle-sexton's task in 1m 11s —— View job Security Review — PR #859
ScopeFour files changed: FindingsNo security issues found. The surface area here is minimal and all additions are benign: Static Trusted dependency (pyright) Static analysis only — no code execution Plugin sandbox unaffected Version bump and changelog |
|
Post-green review classification (work lane):
No DEFER-FORBIDDEN class applies. No inline review threads to resolve (PR-level comments only). Flipping to ready, handing off to the babysit lane for ongoing watch. |
|
Claude finished @kyle-sexton's task in 1m 9s —— View job Security Review — PR #859
ScopeFour files changed: FindingsNo security issues found. The surface area is minimal and all additions are benign. Each vector reviewed: Command injection — none The
Supply chain — trusted dependency Static analysis only — no code execution Plugin sandbox — unaffected Secrets / credentials — none |
|
Claude finished @kyle-sexton's task in 1m 38s —— View job PR Review — feat(toolchain): add pyright to python ecosystem check-cmd
SummaryClean, well-scoped change that closes a real CI/local parity gap. All four files are correct. No blocking issues found. What looks goodCore change is correct. Appending
Gotcha is genuinely useful and actionable. The basic-mode noise warning points to Version bump is correct. Rung-4 semantics preserved. The PR doesn't touch the resolution ladder; consumer One observation (non-blocking)
VerdictApprove. The functional change is correct, the documentation is accurate, and the codebase conventions are followed. No security, correctness, or repo-agnostic compliance issues. The prior review's two observations (brew formula staleness, winget gap) were correctly classified as non-blocking; nothing new to add. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2ff2c5499
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Claude finished @kyle-sexton's task in 2m 39s —— View job PR Review — feat(toolchain): add pyright to python ecosystem check-cmd
SummaryThis is a clean change that closes a real CI/local parity gap. The merge of What looks goodCore change is correct. Pattern is consistent with
Gotcha is actionable. The basic-mode noise warning points to Version bump is correct. On the Codex finding: pyright-absent ruff-only projectsCodex flagged that a project with ruff configured (so it activates the python ecosystem via rung-4 globs) but without pyright installed will now fail The skip granularity is ecosystem-level, not per-tool.
This is a single-tool check per ecosystem. With This is pre-existing behavior, not a regression unique to this PR. What this PR does correctly: it updates The concrete follow-up (not a blocker here, but the right next action) is a check-skill improvement to make tool-presence per-command rather than per-ecosystem — or at minimum, to parse the compound VerdictApprove. Functional change is correct, follows established |
|
Claude finished @kyle-sexton's task in 1m 36s —— View job Security Review — PR #859
ScopeFour files changed: FindingsNo new security issues found. Each vector examined: Command injection — none. The
Supply chain — trusted dependency.
Plugin sandbox — unaffected. GitHub Actions patterns — not applicable. Secrets / credentials — none. Verdict: no security issues. Safe to merge from a security standpoint. |
Resolves conflicts in plugins/toolchain/.claude-plugin/plugin.json and CHANGELOG.md against #860's fleet-wide 0.5.2 shell-declaration sweep — kept the 0.7.0 lychee-offline entry stacked on top (one past sibling PR #859's 0.6.0 claim on the same plugin). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: 0f69ae76-07e0-4aae-9c4a-11688fc0e8ea
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c444986bde
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Pyright's default typeCheckingMode is `standard`, not `basic`, and the reportMissingTypeStubs/reportUnknown* diagnostics default to `none` in both basic and standard modes (per microsoft/pyright configuration.md). Fix the false "basic mode" / reportUnknown-noise claim in the python ecosystem opt-in text, the python.md gotcha, and the CHANGELOG entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 5s —— View job PR Review — feat(toolchain): add pyright to python ecosystem check-cmd
Latest commit —
|
| Finding | Status |
|---|---|
| P2 — Pyright default mode doc incorrect (basic vs standard) | RESOLVED by bd3ff56 |
| P1 — Pyright-absent project turns pass → fail | Correctly deferred; pre-existing check-skill design limitation (ecosystem-level tool-presence check insufficient for compound check-cmd). Follow-up issue for per-tool presence checking is the right path. |
One minor CHANGELOG note (non-blocking)
The CHANGELOG includes a [0.5.2] entry documenting the shell: bash additions merged in from main (7688ae7). That change was already published on main — it will never ship as a standalone 0.5.2 release; the plugin jumps directly 0.5.1 → 0.6.0. The entry correctly documents the change but could be mildly confusing to a consumer diffing releases. Not a blocker; just noting it exists.
Verdict
Approve. The P2 documentation error is resolved. P1 is correctly classified as a pre-existing design limitation and deferred. Everything else from prior review rounds stands — core change is correct, version bump is appropriate, documentation is accurate. No security or repo-agnostic compliance issues.
|
Claude finished @kyle-sexton's task in 1m 37s —— View job Security Review — PR #859 (commit
|
…n' into feat/834-toolchain-pyright-python
|
Claude finished @kyle-sexton's task in 2m 2s —— View job PR Review — feat(toolchain): add pyright to python ecosystem check-cmd
SummaryClean, well-scoped change that closes a real CI/local parity gap. All prior review findings have been resolved or correctly deferred. One new blocking issue surfaced by this sync pass: a version gap introduced when the branch re-versioned after the dotnet PR (#890) merged ahead of it on What looks goodCore change is correct. Mode documentation is now correct.
Rung-4 semantics preserved. Consumer Blocking findingVersion gap:
The correct next semver after Deferred findings (status unchanged)
VerdictRequest changes. One blocking fix required: rename |
|
Claude finished @kyle-sexton's task in 1m 18s —— View job Security Review — PR #859
ScopeFour files changed: FindingsNo security issues found. Each vector examined: Command injection — none. No
Supply chain — trusted dependency. Static analysis only — no code execution. Plugin sandbox — unaffected. GitHub Actions — not applicable. Secrets / credentials — none. Verdict: no security issues. Safe to merge from a security standpoint. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e6f1ab76f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Resolves conflicts in plugins/toolchain/.claude-plugin/plugin.json and CHANGELOG.md against #890's real merge (dotnet opt-in gate, 0.5.2 -> 0.6.0) — re-derives the version to 0.9.0, one past sibling PR #859's own re-rebased 0.8.0 claim on the same plugin (issue #834, pyright). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: 0f69ae76-07e0-4aae-9c4a-11688fc0e8ea
|
This was generated by AI (decide lane — decision-brief pass; parked PR-thread decision surfaced via periodic scan). Decision brief — how to gate
|
main advanced to 0.6.0 (dotnet feature #890) after this branch had bumped to 0.8.0, leaving a skipped 0.7.0 with no CHANGELOG entry. The pyright addition is a single minor feature over 0.6.0, so 0.7.0 is the correct next version. Retitle the CHANGELOG head entry and the plugin.json version from 0.8.0 to 0.7.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 54s —— View job PR Review — feat(toolchain): add pyright to python ecosystem check-cmd
Latest commit —
|
| Finding | Status |
|---|---|
| Blocking — version skipped 0.7.0 (was 0.8.0) | RESOLVED by 2f0ef49 |
| P2 — Pyright default mode doc incorrect (basic vs standard) | RESOLVED by bd3ff56 |
| P1 — Pyright-absent project turns ruff-only pass → fail | Deferred — open decision brief pending kyle-sexton ruling |
On the open decision brief (P1)
The decision brief (kyle-sexton, 2026-07-21T22:09) is well-constructed and surfaces the right tradeoff. The three options map cleanly onto the current skill design:
Why Option B (current PR state) is a defensible merge position:
The SKILL.md tool-presence check at L110 is ecosystem-level and keyed on a single binary (uv for the python ecosystem). Adding pyright to install-hint (python.yaml L11) is the mechanism the plugin uses to declare "this tool is a required part of this ecosystem's default." The behavior change (ruff-only pass → fail for pyright-absent projects) is the intended signal that pyright is now a first-class requirement of the Python default toolchain — the install-hint drives the remediation.
Why Option A is better long-term but requires new skill work:
SKILL.md L114–116 explicitly documents the "multi-tool check-cmd atomicity" gotcha: when a compound check-cmd bundles multiple tools, the opt-in gate cannot suppress just one sub-tool — "splitting a multi-tool check-cmd into separately gateable ecosystem keys would need a schema change." Option A's presence-gate for pyright is exactly that schema change. It is the right long-term solution but it's out of scope for this PR and requires a separate issue.
What Option C rules out:
The brief correctly notes Option C (advisory / non-failing) defeats the PR's own stated purpose of closing the CI/local parity gap. Not a viable option here.
My read: The PR's current state (Option B) is a valid, documented design choice with a clear install-hint remediation path. The PLUGIN-PHILOSOPHY.md "detection-first or presence-gated" guidance (cited in the brief) is a preference, not a hard prohibition — the tool presence signal (install-hint) is present, and the behavior change is intentional and documented. Option A follow-up via a separate issue for per-tool skip granularity in SKILL.md is the right path.
Verdict
Approve. The one blocking issue (version skipped 0.7.0) is resolved by 2f0ef49. All prior findings are either resolved or correctly classified. The core change is correct, the documentation is accurate, and the version sequence is clean.
The P1 decision brief is an open design question for kyle-sexton to rule on before merge — the current PR state (Option B) is a valid merge position, but if the author prefers presence-gating (Option A) that would require additional skill work and should be a separate PR or a pre-merge amendment. Nothing else blocks.
|
Claude finished @kyle-sexton's task in 1m 20s —— View job Security Review — PR #859 (commit
|
| File | Nature |
|---|---|
plugin.json |
Version bump only (0.5.1 → 0.7.0) |
CHANGELOG.md |
Documentation only |
reference/ecosystems/python.yaml |
Adds uv run pyright to check-cmd, updates opt-in and install-hint |
skills/check/context/python.md |
Documentation: pyright section + standard-mode gotcha |
No GitHub Actions workflows modified.
Findings
No security issues found.
Each vector examined:
Command injection — none.
The check-cmd value at python.yaml L8 is a static string literal:
uv run ruff check . --no-fix && uv run ruff format . --check && uv run pyright
No ${variable} interpolation, no consumer-supplied values embedded in the command, no injection surface.
uv run script shadowing — pre-existing, not widened.
uv run pyright resolves pyright from the project virtualenv before PATH. A malicious pyproject.toml [project.scripts] entry named pyright could theoretically shadow the real binary. However, this attack surface already exists for uv run ruff and uv run pytest, both present before this PR. This change does not widen the existing trust boundary.
Supply chain — trusted dependency.
pyright is Microsoft's official Python type checker, published to PyPI by the microsoft organization. The install-hint at python.yaml L11 references pip install, brew install, and winget install — all standard paths to the official package, mirroring the pre-existing pattern for ruff and uv.
Static analysis only — no code execution.
uv run pyright reads Python source files for type information; it does not execute them. Output is diagnostics to stdout only — no data exfiltration, no privilege escalation, no code execution path.
Plugin sandbox — unaffected.
No changes to ${CLAUDE_PLUGIN_ROOT}, ${CLAUDE_PLUGIN_DATA}, or any path resolution logic.
GitHub Actions — not applicable.
No workflow files modified.
Secrets / credentials — none.
No tokens, secrets, or credentials appear in any changed file. plugin.json and CHANGELOG.md are purely metadata.
Verdict: no security issues. Safe to merge from a security standpoint.
|
Automated babysit-lane classification (safe tier — reply only).
Fixed in Deferred items from prior rounds are unchanged and remain author-owned: the P1 pyright-absent behavior-change ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f0ef49721
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
#867) ## Summary Adds `lychee-offline` to the `toolchain` plugin's bundled `cross-cutting.yaml` ecosystem default, alongside the existing `typos`/`gitleaks`/editorconfig-checker tools — on-disk link/anchor integrity checking with no network dependency. ## Fix - `reference/ecosystems/cross-cutting.yaml`: `check-cmd` gains `lychee --offline --no-progress './**/*.md'` (`--offline` = "Only check local files and block network requests" per lychee's own CLI help — external URLs are skipped, only local file and fragment targets are verified); `opt-in` and `install-hint` documented following the same per-tool-config pattern as the existing tools (optional `lychee.toml` at repo root customizes the ruleset; absent means lychee's own defaults). - `skills/setup/SKILL.md`: cross-cutting inference-candidate line mentions `lychee-offline`, and all four cross-cutting tool names backtick-wrapped consistently. - `skills/lint/evals/evals.json`: eval 3's `expected_output` updated to include `lychee-offline` in the enumerated cross-cutting tool set. - `plugins/toolchain/.claude-plugin/plugin.json` + `CHANGELOG.md`: version bump. **Version note**: sibling PR #859 (issue #834) landed first and took the intervening `0.6.0`/`0.7.0` versions; PR #910 (issue #832) then landed `0.8.0`. Rebased onto current `main` and renumbered to `0.9.0`, sequential CHANGELOG history intact. Collision with #859 is resolved (merged); `do-not-merge` label removed. ## Verification - Confirmed `--offline` and `--no-progress` are real lychee CLI flags, and the winget package id (`lycheeverse.lychee`) / brew formula (`lychee`) are correct, against lychee's own upstream README (`lycheeverse/lychee`, `master` branch, Installation and Commandline usage sections). - Smoke-tested the actual `check-cmd` (`lychee --offline --no-progress './**/*.md'`) against this repo: 0 errors across 2645 links (1079 unique, 1056 excluded). - `evals.json` still parses as valid JSON; structural shape (required `id`/`prompt` fields) unchanged — only `expected_output` prose edited. - `skills/setup/SKILL.md` re-checked against the 500-line skill-quality hard cap. - Rebased onto current `origin/main`; `claude plugin validate --strict`, `node scripts/validate-plugin-contracts.mjs`, and full `bash scripts/run-plugin-tests.sh` all green. Closes #833 ## Related - Epic: #830 - Brief item 3, `docs/topics/lint-static-analysis-gaps/PLAN.md` - Sibling PR on the same plugin: #859 (issue #834, pyright) — merged, serialization resolved --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…opt-in wording gaps (#1025) ## Summary Three mature, well-scoped follow-up findings on the bundled python ecosystem default (`plugins/toolchain/reference/ecosystems/python.yaml` and its owning check-skill context doc), deferred from PR #859's review threads and batched here via the #657 triage sweep. All three are scoped to the python ecosystem surface; no cross-ecosystem mechanism changes. ## Fix **Site 1 (P1) — pyright skip granularity.** `uv run pyright` is the first `check-cmd` tool whose presence is not implied by the ruff-config opt-in gate, and tool presence is evaluated **per ecosystem, not per tool** (per the check skill's tool-presence rule). So a ruff-configured project with pyright absent reports a whole-ecosystem missing-tool `skip` that also drops ruff coverage, rather than skipping pyright alone. Chose the issue's **defaulted decision** — document pyright as a hard prerequisite of the python default once ruff opts the ecosystem in (`skills/check/context/python.md`, Type-check section) — over a per-tool skip-granularity code change. Rationale: per-tool skip is a larger cross-ecosystem mechanism change (every `check-cmd` tool would need independent-skip semantics); the documented prerequisite is the smaller, reversible fix that resolves the reported fail-closed surprise without redesigning the skip model. A maintainer wanting the larger mechanism can veto and redirect. **Site 2 (P2) — detection globs.** Added `pyrightconfig.json` to the python `globs` so a standalone pyright-config-only change is auto-detected as python-affecting by `/toolchain:check` and `/toolchain:lint` local runs (CI already enforces its own check). The `[tool.pyright]` table is already covered by the existing `pyproject.toml` glob. **Site 3 (P2) — opt-in wording.** Reduced the `opt-in` value to its single ruff-config gating condition. The trailing semicolon-separated pyright-mode clause structurally resembled the multi-clause per-tool `opt-in` form (per the ecosystem-commands contract, whose schema defines `opt-in` as either a single whole-`check-cmd` condition **or** a per-tool multi-clause value), risking a misread of python's single-condition gate as multi-condition. **Removed rather than reworded:** `opt-in` is a *condition* field, not a mode note, and the descriptive pyright-mode text already lives verbatim in `skills/check/context/python.md` gotchas (single source of truth — no duplication). The reduced value is now identical to the contract's own clean example (`docs/conventions/ecosystem-commands/examples/python.yaml`), confirming the intended single-condition form. Includes the per-plugin version bump (`0.9.0` → `0.9.1`, patch — scoped bugfix/hardening) and a matching top-inserted `CHANGELOG.md` entry. ## Verification Ran against the changed files in the worktree: ``` check-jsonschema --schemafile docs/conventions/ecosystem-commands/ecosystem.schema.json \ plugins/toolchain/reference/ecosystems/python.yaml -> ok -- validation done check-jsonschema --schemafile https://json.schemastore.org/claude-code-plugin-manifest.json \ plugins/toolchain/.claude-plugin/plugin.json -> ok -- validation done markdownlint-cli2 --config .markdownlint-cli2.jsonc \ plugins/toolchain/skills/check/context/python.md plugins/toolchain/CHANGELOG.md -> Summary: 0 error(s) typos --config _typos.toml <changed files> -> clean ``` (`/toolchain:check` and `/toolchain:lint` are model-driven skills, not runnable scripts; the ecosystem YAML is validated against its published `ecosystem.schema.json` contract as the concrete static gate. No CI lane enforces the ecosystem schema — it is a reference contract — so the schema run above is the authoritative structural check for the YAML edit.) ## Related - Closes #1017 - #859 — originating PR whose review threads deferred these as follow-ups - #657 — triage-sweep source (T8h batch conversion) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Work-class: C3 (bug-fix-shaped) — attended triage 2026-07-23, operator-ratified. 🤖 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Adds
pyrightto the bundledtoolchainplugin's Python ecosystem defaultcheck-cmd, closing the gap where CI gated pyright but the local/toolchain:checkbatch was ruff-only.Fix
plugins/toolchain/reference/ecosystems/python.yaml:check-cmdnow runsuv run ruff check . --no-fix && uv run ruff format . --check && uv run pyright.fix-cmdis unchanged (pyright has no fix mode).opt-inandinstall-hintupdated to mention pyright.plugins/toolchain/skills/check/context/python.md: documents pyright as part ofcheck-cmd(not fix), and adds a gotcha for the default basic-mode noise on untyped projects absent apyrightconfig.json/pyproject.toml [tool.pyright]..claude/ecosystems/python.yamloverride still wins key-by-key and is unaffected.plugins/toolchain0.5.1→0.6.0(minor: new default check behavior), with a CHANGELOG entry.Verification
plugins/toolchain/(checkedgh pr list --json headRefName,files), so no serialization needed against sibling issue feat(toolchain): add lychee-offline to cross-cutting ecosystem default #833.0.5.1is still the currentorigin/mainversion forplugins/toolchain, so the0.6.0bump is not stale.Closes #834
Related
docs/topics/lint-static-analysis-gaps/PLAN.md