fix(doctor): derive languages from source files, not manifests alone - #356
Open
mtschoen wants to merge 2 commits into
Open
fix(doctor): derive languages from source files, not manifests alone#356mtschoen wants to merge 2 commits into
mtschoen wants to merge 2 commits into
Conversation
doctor consumed discoverProject's manifest-based language list verbatim, so any root package.json made the project look like JavaScript. In a repository whose package.json exists only to pin a CLI tool (the code is C# .csproj and C++ .vcxproj under subdirectories, no JS/TS source at all) that reads as "javascript (mixed)" and the JS-gated planners short-circuit the whole plan to biome, oxlint and knip. scan does not have this problem: it overrides languages with detectSourceLanguages over the files it will actually read, so scan on the same repository runs the C#/C++ engines that doctor claims do not apply. doctor now derives its language view the same way, keeping the manifest languages only when the tree has no source files to speak for itself. primaryLanguage also gains csharp and cpp, which were never added when those languages were: without them a mixed C#/C++ repository falls through to the bare "mixed" label with no headline language. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
aislop skipped this PRThis workspace does not have an active paid scanaislop plan. Choose a plan to resume hosted scans and PR gates: This comment appears once per PR. Further pushes will be skipped silently until the workspace has paid access. |
mtschoen
marked this pull request as ready for review
August 16, 2026 00:00
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b1ca16260
ℹ️ 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".
Two review findings on the previous commit, both rooted in doctor enumerating its own file list instead of scan's. getSourceFilesForRoot answers a narrower question than scan asks: it returns non-test source files and applies neither config.include, config.exclude, nor .aislopignore. An excluded JavaScript subtree therefore still made doctor promise biome and oxlint, and a C# application whose only JavaScript is its tests made scan select the JS tools while doctor advertised only the .NET ones. doctor now loads the config first and reads collectScanFileScope in full mode, the same scope scan derives its languages from, source and test files alike. That enumeration also ran ahead of discoverProject, which owns the reset of the cached gitignore snapshot, so the reset landed too late for it. In a long-lived consumer of the exported doctorCommand (the interactive loop, the MCP server) a source file added since the previous call was classified against the earlier snapshot and its language went missing. The reset now precedes the enumeration. The tree is still walked once for the language view: the scope's file list is passed into discoverProject as its source files. Manifest languages remain the answer when the tree has no source files, and discoverProject still derives them the same way, so dependency-audit selection is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
aislop scan: 100/100 (Healthy) +Clean run — no errors, no warnings, no auto-fixable findings. |
mtschoen
added a commit
to mtschoen/aislop
that referenced
this pull request
Aug 16, 2026
…ured scan scope) into schoen/main
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
aislop doctorplanned engines from the languages a project's manifests name rather than the languages its source files are written in. A repository whose root package.json exists only to pin a CLI tool reported as "javascript (mixed)" and planned biome, oxlint, and knip - with no C# or C++ engine in the plan at all - even thoughaislop scanon the same tree correctly detected csharp and ran clang-tidy, cppcheck, and jb.This completes the direction of #258, which moved scan to source-file-derived languages but left doctor consuming manifest languages verbatim. scan avoids this by overriding the discovered languages with detectSourceLanguages over the files it will actually read; doctor now derives its language view from that same scope: it loads the config first and reads collectScanFileScope in full mode (source and test files alike, with config.include, config.exclude, and .aislopignore applied), passes the scope's files into discoverProject so the tree is not walked twice, and keeps manifest languages only when the tree has no source files yet. The cached gitignore snapshot is reset before that enumeration, so a long-lived consumer of the exported doctorCommand (the interactive loop, the MCP server) does not classify files added since a previous call against a stale snapshot. So the doctor report now matches what a scan will actually do. primaryLanguage also gains the csharp and cpp entries it was never given, which otherwise leave a mixed C#/C++ project labeled just "mixed".
The alternative of fixing detectLanguages centrally (dropping uncorroborated manifest languages) was rejected: discoverProject's languages also drive dependency-audit selection, so a tooling-pin repository would silently lose its npm/pnpm audit, and an existing test asserts package.json alone implies javascript for that purpose.
Regression tests: the tooling-pin package.json at root + C# source in a subdirectory fails before the fix with the exact reported symptom and passes after; a guard test confirms a manifest-only project still reports javascript; review-round tests cover a config-excluded and an .aislopignore-excluded JavaScript subtree, a C# project whose only JavaScript is its tests, and two doctorCommand calls in one process with a source file added between them (each confirmed to fail without its source fix). Full suite 2025 passed / 14 skipped / 0 failed; self-scan 100/100. Smoke-tested against the real-world reproducing repository: doctor now reports "csharp (mixed)" with dotnet format and jb inspectcode planned, and the lockfile-driven npm audit row is unchanged.
🤖 Generated with Claude Code