Skip to content

ci: gate fixture runner on analyzer binary presence#2

Merged
ThomasPluck merged 4 commits into
masterfrom
fix/rust-analyzer-semgrep-hallucinated-import
Apr 18, 2026
Merged

ci: gate fixture runner on analyzer binary presence#2
ThomasPluck merged 4 commits into
masterfrom
fix/rust-analyzer-semgrep-hallucinated-import

Conversation

@ThomasPluck
Copy link
Copy Markdown
Owner

Summary

CI on master is red on the fixture integration step at commit 2a31911. Two fixtures fail with "expected hallucinated-import diagnostic":

  • rust_analyzer/rust-analyzer binary is not installed in CI (the workflow only installs pyright and semgrep). The runner had no notion of optional analyzer binaries, so the fixture failed unconditionally.
  • semgrep/ — the equivalent cargo test (semgrep_fixture_flags_multiple_rules_when_installed) passes in the same CI run, so semgrep itself works. The shell runner was suppressing daemon stderr (2>/dev/null), which made this undebuggable.

The deeper issue: test/run_fixtures.sh checked an overly broad "≥1 diagnostic per fixture" invariant. Detailed per-analyzer assertions already live in daemon/tests/fixtures.rs; this script is just the end-to-end smoke check.

Changes

  • Map each fixture name to its required external binary; skip cleanly when missing — same pattern daemon/tests/fixtures.rs already uses.
  • Capture daemon stderr to a tempfile and dump it on failure so the next regression doesn't require source-code spelunking.
  • Update docstring + replace the misleading "expected hallucinated-import" message.

Test plan

  • CI green on this branch (rust_analyzer skips, semgrep should now produce diagnostics OR surface stderr if it doesn't)
  • If semgrep still emits 0, the captured stderr will tell us why and a follow-up commit can address it

ThomasPluck and others added 4 commits April 18, 2026 09:07
Two fixtures were failing CI on master:
- rust_analyzer/ — rust-analyzer is not installed in CI (the workflow
  only installs pyright + semgrep). The runner had no notion of
  optional analyzer binaries, so the fixture failed with 'expected
  hallucinated-import diagnostic'.
- semgrep/ — the equivalent cargo integration test
  (semgrep_fixture_flags_multiple_rules_when_installed) passes, so
  semgrep itself works. The shell runner was redirecting all daemon
  stderr to /dev/null, which made the failure undebuggable.

Fix:
- Map each fixture name to its required external binary; skip when
  missing. rust-analyzer / semgrep / pyright / tsc are now treated as
  optional capabilities the way daemon/tests/fixtures.rs already does.
- Capture daemon stderr to a temp file and dump it on failure so the
  next regression doesn't require source-code spelunking.
- Update the docstring + the misleading 'expected hallucinated-import'
  message — the script checks a generic non-green invariant; detailed
  per-analyzer expectations live in daemon/tests/fixtures.rs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI on the previous commit revealed two refinements needed:

1. `command -v rust-analyzer` returns true on a stock rustup install
   because rustup ships a shim at ~/.cargo/bin/rust-analyzer that fails
   unless `rustup component add rust-analyzer` has been run. The
   daemon's own `binary_present()` check (`<bin> --version`) catches
   this; the shell runner now mirrors that.

2. The semgrep fixture is producing 0 findings via the daemon binary
   even though semgrep itself is installed and the equivalent cargo
   integration test passes. Captured stderr just shows
   `semgrep pass complete n=0` with no clue why. Add diagnostic
   logging in semgrep.rs that, on a 0-result run, surfaces the
   parsed `errors` field plus a tail of semgrep's stderr — that
   should pinpoint the misconfig in the next CI run.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root cause for the semgrep fixture failing in CI but passing in cargo
tests:

When the target directory lives inside a git repo, semgrep auto-applies
a "tracked by git only" filter. From semgrep's own vantage point in CI
(target = /repo/test/fixtures/ai-slop/semgrep, daemon CWD = /repo) the
filter sees 0 files and the scan returns 0 findings, even though the
files are tracked at the repo root. Captured semgrep stderr from the
prior debugging commit:

    Ran 15 rules on 0 files: 0 findings.
    Scan was limited to files tracked by git

The cargo integration test passes because `isolate()` copies the
fixture into a tempdir outside any git repo, so the filter never
engages.

`--no-git-ignore` disables both the .gitignore respect AND the
git-tracked-only filter — IVE owns workspace traversal end-to-end, so
we never want semgrep second-guessing which files to scan.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second piece of the semgrep CI fix. With --no-git-ignore alone, semgrep
still consulted its built-in default .semgrepignore, whose patterns
include test/ — and since semgrep walks up from the fixture target to
the IVE git root, both app.py and requirements.txt got filtered as
'test/fixtures/ai-slop/semgrep/...'. Captured stderr from the previous
CI run:

    Ran 15 rules on 0 files: 0 findings.
    Files matching .semgrepignore patterns: 2

The only override mechanism semgrep CLI accepts is a .semgrepignore
file at the project (git) root, which it then uses *instead of* the
defaults. Empty contents = scan everything semgrep would otherwise
ignore.

This is the right behaviour for IVE specifically: scanner::walk_workspace
already filters node_modules / target / .git / .ive, and our
test/fixtures/ai-slop/ tree exists precisely to be scanned by the
semgrep analyzer in fixture tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ThomasPluck ThomasPluck marked this pull request as ready for review April 18, 2026 08:35
@ThomasPluck
Copy link
Copy Markdown
Owner Author

Status

The original CI failure on master (the daemon (Rust)fixture integration step) is fixed by this PR. The daemon (Rust) job is fully green:

  • cargo fmt / cargo test --release --all
  • fixture integration (rust_analyzer skips cleanly, semgrep produces 4 diagnostics, all others pass)

Two follow-up items surfaced

1. extension + webview (TS) job is broken (pre-existing, unrelated)

This job has needs: daemon so it never actually ran on master while daemon was red. With daemon now green it runs for the first time — and all 26 Playwright tests fail with net::ERR_CONNECTION_REFUSED at http://127.0.0.1:4173/. The vite preview webServer block in webview/playwright.config.ts is configured but the server never seems to start (no "Local:"/"Network:" output in the log). Likely needs separate investigation. Worth a dedicated PR.

2. Cargo test flake on pyright_fixture_flags_type_error_when_pyright_is_installed

Failed once on this PR (run attempt 1) with 0 pyright diagnostics, passed on rerun. Multiple integration tests invoke pyright in parallel; pyright-via-pip uses nodeenv which downloads node on first invocation, so the first concurrent caller wins and the others can race. Cheap fix: set IVE_SKIP_PYRIGHT=1 for the integration tests that don't specifically test pyright (they currently inherit the env). Out of scope for this PR.

3. Design question: how should IVE handle user workspaces with their own tests/ dir?

The .semgrepignore at the IVE root unblocks our self-tests, but for any user repo IVE scans, semgrep's built-in default .semgrepignore (which excludes test/, tests/, vendor/, node_modules/, etc.) will silently filter those files. IVE's scanner::walk_workspace already handles workspace traversal — semgrep second-guessing it produces silent false-negatives. Worth a daemon-level fix: either pass each file to semgrep explicitly, or write a temporary .semgrepignore at the project root for the duration of a scan.

@ThomasPluck ThomasPluck merged commit 69dd0f2 into master Apr 18, 2026
2 of 4 checks passed
@ThomasPluck ThomasPluck deleted the fix/rust-analyzer-semgrep-hallucinated-import branch April 18, 2026 13:04
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.

1 participant