feat(toolchain): add lychee-offline to cross-cutting ecosystem default - #867
Conversation
kyle-sexton
added
do-not-merge
|
Claude Code is working… I'll analyze this and get back to you. |
|
Claude finished @kyle-sexton's task in 1m 36s —— View job Security Review — PR #867
|
|
Claude finished @kyle-sexton's task in 1m 58s —— View job PR #867 Review — feat(toolchain): add lychee-offline to cross-cutting ecosystem default
Overall the change is clean, well-scoped, and follows the existing cross-cutting pattern faithfully. No blocking issues. Three findings below, all minor. Finding 1 — CHANGELOG.md: missing version-gap note (minor)The changelog jumps from Suggestion: add a brief inline note in the ## [0.7.0]
Finding 2 — SKILL.md: backtick inconsistency on the inference-candidate line (nit)- **cross-cutting** — repo-root config for `typos`/`gitleaks`/editorconfig-checker/`lychee-offline` present (lint-only).
Finding 3 —
|
| Area | Assessment |
|---|---|
--offline robustness |
CLI flag takes precedence over lychee.toml; a consumer config cannot silently re-enable network access. Confirmed. |
| Single-quoted glob | Shell does not expand ./**/*.md; lychee handles it internally. Correct quoting for the skill's execution context. |
install-hint entries |
lycheeverse.lychee (winget) / lychee (brew) match lychee's upstream README. Consistent with existing tool format. |
Version skip 0.5.1 → 0.7.0 |
Intentional to avoid collision with #859. Documented in PR body. Not a defect. |
Eval 3 expected_output |
Minimal, accurate update — adds lychee-offline to the enumerated tool set; id/prompt unchanged. |
opt-in / install-hint pattern |
Matches exactly the pattern used by typos/gitleaks/editorconfig-checker. |
| Plugin isolation / path traversal | No ../ references; no plugin-form violations. |
| CLAUDE.md design rules | Repo-agnostic, no hardcoded paths, configurable without editing the plugin, no PII. |
|
Claude finished @kyle-sexton's task in 1m 34s —— View job Security Review — PR #867
No security issues found. Reviewed across the standard threat categories: Command injection / shell injection ( The added line is: lychee --offline --no-progress './**/*.md'All arguments are static — no user-controlled input is interpolated. The glob is single-quoted, so the shell does not expand it; lychee receives the literal string Network egress / data exfiltration
Supply-chain / auto-install The Plugin isolation / path traversal No Secrets / PII None introduced. GitHub Actions / workflow changes No workflow files were modified.
Both are prose/metadata — no code execution path. The Version skip ( Not a security issue. The PR notes this is intentional to avoid a collision with sibling PR #859 claiming |
|
Claude finished @kyle-sexton's task in 2m 0s —— View job PR #867 Review — feat(toolchain): add lychee-offline to cross-cutting ecosystem default
The change set is focused and correctly executed. Five files touched, all for the right reasons. No correctness defects found. Security was already independently confirmed (no injection surface;
|
|
Claude finished @kyle-sexton's task in 1m 52s —— View job PR #867 Review — feat(toolchain): add lychee-offline to cross-cutting ecosystem default
Post-merge-sync confirmation: the No correctness or security issues found. The change set is focused, follows the existing cross-cutting pattern faithfully, and is CLAUDE.md-compliant. Three prior findings remain unaddressed (all carried over from earlier review passes); none are blocking. Finding 1 — CHANGELOG.md: version gap has no in-file explanation (minor)The changelog jumps ## [0.7.0]
...
## [0.5.2]The PR body explains this (sibling PR #859 reserves ## [0.7.0]
Finding 2 — SKILL.md: backtick inconsistency on the inference-candidate line (nit)- **cross-cutting** — repo-root config for `typos`/`gitleaks`/editorconfig-checker/`lychee-offline` present (lint-only).
Finding 3 —
|
| Area | Assessment |
|---|---|
--offline robustness |
CLI flag takes precedence over lychee.toml; consumer config cannot silently re-enable network access. Confirmed. |
| Single-quoted glob | Shell does not expand ./**/*.md; lychee handles globbing. Correct for the skill's execution context. |
install-hint entries |
lycheeverse.lychee (winget) / lychee (brew) match lychee upstream. Consistent format with existing tools. |
opt-in field |
lychee.toml appended following the same per-tool-config pattern. |
Version skip 0.5.2 → 0.7.0 |
Intentional to avoid collision with #859. Marked do-not-merge. Not a defect. |
Eval 3 expected_output |
Accurate minimal update; id/prompt/expectations unchanged. |
| CLAUDE.md design rules | Repo-agnostic, configurable without editing, plugin-form-safe, no PII, explicitly versioned. |
| Merge-sync impact | No conflicts; 9533a14 (standards sync from main) does not touch any of the 5 PR files. |
The only pending gate is the version-collision serialization with #859, already tracked and correctly reflected in the do-not-merge label. Once that resolves, this is ready to merge.
|
Claude finished @kyle-sexton's task in 1m 22s —— View job Security Review — PR #867
No security issues found. Command injection —
|
|
Claude finished @kyle-sexton's task in 3m 47s —— View job PR #867 Review — feat(toolchain): add lychee-offline to cross-cutting ecosystem default
Post-sync state ( Two carried findings and one new finding. Finding 1 — NEW: PR body is now stale relative to the code (minor, needs update before merge)The PR description's version note reads:
But the code now has
A reviewer reading only the PR body will see a mismatch: the stated version ( Finding 2 — CARRIED: CHANGELOG gap now spans 0.6.0 → 0.9.0 with no inline explanation (minor)The changelog in this PR reads: ## [0.9.0]
...
## [0.6.0]The gap now skips both ## [0.9.0]
Finding 3 — CARRIED: SKILL.md backtick inconsistency (nit)Line 84 still reads: - **cross-cutting** — repo-root config for `typos`/`gitleaks`/editorconfig-checker/`lychee-offline` present (lint-only).
What looks correct
The pending gate remains the #859 serialization. Once that resolves and the PR body is updated to reflect current version history (Finding 1), this is ready to merge. Findings 2 and 3 are minor polish items that can be addressed here or in a follow-up. |
|
Claude finished @kyle-sexton's task in 1m 53s —— View job Security Review — PR #867
No security issues found. Reviewed across all applicable threat categories: Command injection —
|
|
Claude finished @kyle-sexton's task in 1m 21s —— View job PR #867 Review — feat(toolchain): add lychee-offline to cross-cutting ecosystem default
Sync commit Finding resolved — SKILL.md backtick inconsistency (was Finding 3)Line 84 now reads: - **cross-cutting** — repo-root config for `typos`/`gitleaks`/`editorconfig-checker`/`lychee-offline` present (lint-only).All four tool names are consistently backtick-wrapped. Resolved. Remaining open item — CHANGELOG version gap has no inline explanation (minor)The changelog jumps ## [0.9.0]
...
## [0.6.0]The PR body explains the serialization story (0.6.0 landed via #890, #859 claims 0.8.0, this PR steps to 0.9.0), but that context disappears post-merge. A one-line callout in the What looks correct
The only pending gate remains the #859 serialization. The CHANGELOG gap note is minor polish that can be addressed here or in a follow-up. Once #859 resolves, this is ready to merge. |
|
Claude finished @kyle-sexton's task in 1m 16s —— View job Security Review — PR #867
No security issues found. This review pass covers the synchronize commit Reviewed across all applicable threat categories against the 5 changed files: Command injection —
|
Also re-verified the failing |
On-disk link/anchor integrity check with no network dependency — --offline skips external URLs and only verifies local file/fragment targets, matching the other rung-4 cross-cutting tools' opt-in pattern. Per Brief item 3, docs/topics/lint-static-analysis-gaps/PLAN.md. Closes #833. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: 0f69ae76-07e0-4aae-9c4a-11688fc0e8ea
The automated review check on the prior push failed with an infra
error ("no execution file was produced") rather than a real review
verdict; gh run rerun could not retry it directly. Empty commit to
get a fresh review pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: 0f69ae76-07e0-4aae-9c4a-11688fc0e8ea
Wrap gitleaks and editorconfig-checker in backticks to match typos and lychee-offline in the cross-cutting inference-candidate line.
kyle-sexton
removed
the
do-not-merge
9543e71 to
84a9aaf
Compare
|
Claude finished @kyle-sexton's task in 2m 1s —— View job PR #867 Review — feat(toolchain): add lychee-offline to cross-cutting ecosystem default
All prior findings resolved. Commit Prior findings — current status
What looks correct at HEAD
No issues found. The change is focused, well-scoped, and follows the existing cross-cutting pattern faithfully. Ready to merge once the |
|
Claude finished @kyle-sexton's task in 1m 39s —— View job Claude finished @kyle-sexton's task —— View job Security Review — PR #867
No security issues found. This pass covers HEAD Command injection —
|
…ding markdownlint MD022/MD032: a heading and the list above it need a blank line between them. Lost during the 0.9.0 rebase conflict resolution. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 48s —— View job PR #867 Review — feat(toolchain): add lychee-offline to cross-cutting ecosystem default
All prior findings resolved. Commit Prior findings — current status
What the latest commit does- precompute convention — bash-only pipelines must not fall through to a
- PowerShell host.
+
## [0.5.1]Restores a blank line between the prior section's list and the What looks correct at HEAD
No issues found. The change is focused, well-scoped, and correctly executed. Ready to merge once the |
|
Claude finished @kyle-sexton's task in 2m 8s —— View job Security Review — PR #867
No security issues found. Reviewed across all applicable threat categories against the 5 files changed by the 4 PR-specific commits ( Command injection —
|
|
Claude finished @kyle-sexton's task in 1m 19s —— View job PR #867 Review — feat(toolchain): add lychee-offline to cross-cutting ecosystem default
All prior findings resolved. No issues found. Ready to merge. Prior findings — current status
What looks correct
|
|
Claude finished @kyle-sexton's task in 1m 40s —— View job Security Review — PR #867
No security issues found. Reviewed across all applicable threat categories against the 5 files changed by this PR's 4 commits ( Command injection —
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32d0ddcfc3
ℹ️ 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".
#926) (#1020) ## Summary `docs/conventions/ecosystem-commands/ecosystem.schema.json` defines a structured `gates` array (`name`/`cmd`/`trigger-globs`/`remediation`) that any ecosystem config — bundled default or consumer-tracked — can declare, and both `plugins/toolchain/reference/ecosystems/go.yaml` (`go-mod-tidy-drift`) and `docs/conventions/ecosystem-commands/examples/dotnet.yaml` (`nuget-lockfile-drift`) ship one. The array was resolved as part of each ecosystem's command surface (`check/SKILL.md` §1.5) but **never invoked by any workflow step** — so every declared gate was inert. The only gate-execution mechanism `check/SKILL.md` documented was a prose, consumer-CLAUDE.md-only step whose closing sentence ("this plugin ships none of its own") was itself stale once `go.yaml` shipped a bundled gate. ## Fix Scoped to `plugins/toolchain/skills/check/SKILL.md`, precise and additive (no restructuring of unrelated parts): - **New structured gate-execution step** in §2 "Run checks", replacing the stale prose step at the old `:121`. After an affected ecosystem's build → test → lint, iterate its resolved `gates` array (§1.5 ladder — bundled default or consumer file). Per gate: - **Fire condition** — `trigger-globs` present → run only when ≥1 changed file matches, against the **full** changed-files set (a gate's trigger files need not classify into the ecosystem's own `globs`); omitted → run whenever the ecosystem runs; present-but-no-match (e.g. `check all` on a clean tree) → does not fire. - **Independent of the build/test/lint short-circuit** — a fired gate runs even when an earlier phase failed and stopped (a lockfile / `go mod tidy` gate is meaningful regardless of build success). - **Tool presence** — missing tool → `skip` with the ecosystem's `install-hint`, never `FAIL`. - **Outcome** — `pass`/`FAIL` by name; on `FAIL` surface `gate.remediation`; a fired-and-failed gate counts toward the run's FAIL verdict. - **Overall verdict** now covers gate-only failures — a failed fired gate flips Overall to `FAIL` and is counted even when every ecosystem's build/test/lint cell passed (e.g. `Overall: FAIL (0 of 2 ecosystems failed, 1 gate failed)`). - **Stale sentence removed** — the "this plugin ships none of its own" framing is replaced with an accurate ladder pointer (bundled default e.g. `go.yaml`'s `go-mod-tidy-drift`; consumer-declared in `.claude/ecosystems/<ecosystem>.yaml`). - **Cross-reference reconciled** at the old `:119` ("CI-parity gates … run independent of `check-cmd`") and the report scaffold (`:140`). **`lint/SKILL.md` deliberately unchanged** (the one maintainer-vetoable fork, per the planning comment): every schema-described gate and both shipped examples are CI-parity checks beyond build/test/lint (lockfile drift, module drift, generated-artifact freshness) — none is a lint/format op. They belong with the heavier `check` step and `/verification:confirm` (the sanctioned pre-commit gate); adding them to `/toolchain:lint` would break its "fast" contract. Veto before merge if a maintainer wants gates on the lint path. **Version bump (rule 6d):** fresh `origin/main` had `plugins/toolchain` at `0.9.0` (claimed by the lychee PR #867); this is a new executable behavior, so bumped to `0.10.0` with a matching `## [0.10.0]` → `### Added` CHANGELOG entry. (The planning comment's `0.8.0 → 0.9.0` predated #867 landing on main.) ## Verification No skill-behavior test harness exists (`find plugins/toolchain -iname '*test*'` and a repo-level search for `toolchain:check`/`gates` tests both empty), so verification is static gate + hand-trace: - **`/skill-quality:check` on the edited `check` skill:** PASS — 0 errors, markdownlint clean, 165/500 lines, all base-ref trigger phrases preserved (frontmatter `description` untouched). The lone WARN (description lacks `Use when:` phrasing) is pre-existing and out of scope. - **Hand-trace — bundled default (`go.yaml`, a real rung-4 default that runs for this repo):** a change set including `go.sum` (or any `*.go`) makes `go` affected; §1.5 resolves `go.yaml`; the new §2 step reaches its `gates` and matches `go-mod-tidy-drift`'s `trigger-globs: ["go.mod","go.sum","*.go"]` against the full changed-files set → gate fires → runs `go mod tidy -diff` from `$REPO_ROOT`. Drift → `FAIL` + remediation + Overall FAIL; clean → `pass`; missing Go toolchain → `skip` with `install-hint`. - **Hand-trace — consumer-declared shape (`examples/dotnet.yaml`, an example fixture under `docs/`, not a bundled default):** for a consumer whose tracked `.claude/ecosystems/dotnet.yaml` carries the `nuget-lockfile-drift` gate, a changed `*.csproj` matches `trigger-globs: ["*.csproj","Directory.Packages.props","packages.lock.json"]` → gate fires → runs `dotnet restore --locked-mode`. Same pass/FAIL/skip semantics. Both gate entries are invoked by the new step given a matching changed-files set; neither required a per-ecosystem change. Closes #926 ## Related - PR #910 (Go ecosystem) — **merged** `2026-07-22`; added `go.yaml`'s `go-mod-tidy-drift` gate whose Codex review surfaced this wiring gap. This change is the cross-cutting follow-up that makes that gate (and every other declared gate) actually run — additive, no `go.yaml` change. - Planning-subagent comment on #926 (`2026-07-22T03:10:48Z`) — the primary implementation plan followed here; the only deviation is the version target (`0.10.0`, re-derived from fresh `origin/main` per rule 6d, since `0.9.0` was taken by #867 after the plan was written). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019xSPACJvbsqGsjYCP91DoX Work-class: C3 (bug-fix-shaped) — attended triage 2026-07-23, operator-ratified. 🤖 --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 participant
Summary
Adds
lychee-offlineto thetoolchainplugin's bundledcross-cutting.yamlecosystemdefault, alongside the existing
typos/gitleaks/editorconfig-checker tools — on-disklink/anchor integrity checking with no network dependency.
Fix
reference/ecosystems/cross-cutting.yaml:check-cmdgainslychee --offline --no-progress './**/*.md'(--offline= "Only check local files andblock network requests" per lychee's own CLI help — external URLs are skipped, only local
file and fragment targets are verified);
opt-inandinstall-hintdocumented followingthe same per-tool-config pattern as the existing tools (optional
lychee.tomlat reporoot customizes the ruleset; absent means lychee's own defaults).
skills/setup/SKILL.md: cross-cutting inference-candidate line mentionslychee-offline,and all four cross-cutting tool names backtick-wrapped consistently.
skills/lint/evals/evals.json: eval 3'sexpected_outputupdated to includelychee-offlinein 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.0versions; PR #910 (issue #832) then landed0.8.0. Rebased onto currentmainand renumbered to0.9.0, sequential CHANGELOG history intact. Collision with #859is resolved (merged);
do-not-mergelabel removed.Verification
--offlineand--no-progressare real lychee CLI flags, and the wingetpackage id (
lycheeverse.lychee) / brew formula (lychee) are correct, against lychee'sown upstream README (
lycheeverse/lychee,masterbranch, Installation and Commandlineusage sections).
check-cmd(lychee --offline --no-progress './**/*.md') againstthis repo: 0 errors across 2645 links (1079 unique, 1056 excluded).
evals.jsonstill parses as valid JSON; structural shape (requiredid/promptfields)unchanged — only
expected_outputprose edited.skills/setup/SKILL.mdre-checked against the 500-line skill-quality hard cap.origin/main;claude plugin validate --strict,node scripts/validate-plugin-contracts.mjs, and fullbash scripts/run-plugin-tests.shall green.
Closes #833
Related
docs/topics/lint-static-analysis-gaps/PLAN.md