ReviewOS

stacks/buddy

Add external-tool analyzers (ESLint/pickier, actionlint, ShellCheck, Hadolint, markdownlint)

#1392
Closed chris opened this 8 days ago ·
8 days ago

Follow-up to #1378, which landed the analyzer engine plus two native analyzers (secret scan, workflow audit).

The remaining first-tier analyzers from #1378's table are each now a matter of writing one Analyzer against the interface in src/analysis/types.ts:

  • The repo's own JS/TS linter (detect pickier / ESLint / Biome / Oxlint and its config)
  • actionlint — .github/workflows/*, complements the six native workflow rules
  • ShellCheck — *.sh
  • Hadolint — Dockerfile*, pairs with our Docker update support
  • markdownlint — *.md
  • YAML / JSON syntax validation — cheap, native, no external tool

Each needs: filePatterns, an available() probe (use commandExists from src/analysis/engine.ts), and a run() that maps the tool's output onto ReviewFinding. Findings must anchor to a real line — reviewDiff validates analyzer findings against the diff's line map and drops unanchorable ones.

External tools should be mocked in CI rather than installed; see test/analysis-engine.test.ts for the pattern.

Config surface already exists in shape: analysis.tools keyed by analyzer name (see runAnalyzers's enabled option).

8 days ago

Landed in be91aea.

Six analyzers: native YAML/JSON syntax validation, plus actionlint, ShellCheck, Hadolint, markdownlint and the repository's own JS/TS linter (pickier/ESLint/Biome/oxlint), detected by config file or package script — several run with no config at all, and config-only detection missed this very repository.

Hadolint's info/style tier and markdownlint's formatting conventions are filtered out: a review opening with twelve notes about list indentation is one nobody reads to the end of.

Three correctness details. Tool arguments never go through a shell and the environment is narrowed to PATH/HOME/LANG — the file paths come from a PR's diff, so third-party linters must not inherit the run's credentials. Biome reports byte spans, so the file is read to convert one, and an out-of-range offset drops the finding rather than anchoring it to unrelated code. And commandExists now passes PATH identically to runTool, so the probe and the invocation cannot disagree.

Config surface is analysis.enabled / analysis.tools, with a misspelled analyzer name treated as an error.

Sign in to comment on this issue.