feat: add copilot cli plugin detection to doctor command#40
Merged
kenryu42 merged 4 commits intoMar 24, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR adds Copilot plugin detection by running Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Doctor Runner
participant Sys as System Info
participant Copilot as Copilot CLI
participant Hooks as Hook Detection
Runner->>Sys: getSystemInfo()
Sys->>Copilot: copilot plugin list
Copilot-->>Sys: plugin list output
Sys->>Sys: hasCopilotSafetyNetPlugin(output)
Sys-->>Runner: { copilotPluginInstalled: boolean, ... }
Runner->>Hooks: detectAllHooks({ copilotPluginInstalled, ... })
alt copilotPluginInstalled === true
Hooks->>Hooks: mark Copilot configured via plugin
Hooks-->>Runner: { status: "configured", method: "plugin list", configPath: "copilot-plugin" | path, configPaths: undefined | [...] }
else
Hooks->>Hooks: inspect local hook config files
Hooks-->>Runner: { status: "...", method: "hook config", configPath: "path", configPaths: [...] }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
==========================================
- Coverage 99.53% 99.51% -0.02%
==========================================
Files 50 50
Lines 5365 5379 +14
==========================================
+ Hits 5340 5353 +13
- Misses 25 26 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile SummaryThis PR adds a second Copilot CLI detection path to the
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant D as runDoctor (index.ts)
participant SI as getSystemInfo (system-info.ts)
participant CF as copilot binary
participant AH as detectAllHooks (hooks.ts)
D->>SI: getSystemInfo()
par Parallel fetches
SI->>CF: copilot --binary-version
SI->>CF: copilot --version
SI->>CF: copilot plugin list
end
CF-->>SI: pluginListRaw (or null)
SI->>SI: hasCopilotSafetyNetPlugin(pluginListRaw)
SI-->>D: SystemInfo { copilotPluginInstalled: boolean, copilotCliVersion, ... }
D->>AH: detectAllHooks(cwd, { copilotCliVersion, copilotPluginInstalled })
AH->>AH: _checkCopilotEnabled() → check hook config files
alt disableAllHooks found
AH-->>D: { status: 'disabled', method: 'hook config', configPath }
else plugin installed OR hook configs found
Note over AH: viaPlugin = copilotPluginInstalled === true
AH->>AH: runSelfTest()
AH-->>D: { status: 'configured', method: 'plugin list' | 'hook config', configPath, configPaths }
else neither
AH-->>D: { status: 'n/a' }
end
Reviews (3): Last reviewed commit: "fix(doctor): set configPath to 'copilot-..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
tests/bin/doctor/system-info.test.ts (1)
160-165: Add explicit return type annotations to new test fetchers.Lines 160, 173, 186, and 199 introduce async functions without explicit return types. Please annotate them to satisfy the repo’s TypeScript typing rule.
✍️ Example pattern
- const fetcher = async (args: string[]) => { + const fetcher: (args: string[]) => Promise<string | null> = async (args: string[]) => { if (args[0] === 'copilot' && args[1] === 'plugin') { return 'Installed plugins:\n • copilot-safety-net (v1.0.0)'; } return null; };As per coding guidelines: All functions require type annotations in TypeScript.
Also applies to: 173-178, 186-191, 199-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/bin/doctor/system-info.test.ts` around lines 160 - 165, The new async test helper functions (e.g., the const fetcher declared in this diff) lack explicit TypeScript return annotations; update each async fetcher introduced at the referenced locations to include an explicit return type of Promise<string | null> so the compiler rule is satisfied (i.e., change the implicit-typed async arrow functions to have an explicit Promise<string | null> return annotation for each fetcher).src/bin/doctor/system-info.ts (1)
97-103: Static analysis ReDoS warning is a false positive.The static analyzer flagged the
RegExpconstructor as a potential ReDoS risk. However,COPILOT_PLUGIN_IDis a compile-time constant (not user input), and the resulting pattern(^|[^a-z0-9-])copilot-safety-net([^a-z0-9-]|$)contains no dangerous quantifiers or nested alternations—it's linear and safe.Minor consideration: You could make the regex a module-level constant (computed once) instead of creating it on each call, though the performance difference is negligible for this use case.
♻️ Optional: Move regex to module-level constant
const COPILOT_PLUGIN_ID = 'copilot-safety-net'; +const COPILOT_PLUGIN_PATTERN = new RegExp( + `(^|[^a-z0-9-])${COPILOT_PLUGIN_ID}([^a-z0-9-]|$)`, + 'm', +); function hasCopilotSafetyNetPlugin(output: string | null): boolean { if (!output) return false; - - const pluginPattern = new RegExp(`(^|[^a-z0-9-])${COPILOT_PLUGIN_ID}([^a-z0-9-]|$)`, 'm'); - - return pluginPattern.test(output); + return COPILOT_PLUGIN_PATTERN.test(output); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bin/doctor/system-info.ts` around lines 97 - 103, The RegExp construction in hasCopilotSafetyNetPlugin is flagged as ReDoS but COPILOT_PLUGIN_ID is a compile-time constant and the pattern is safe; update the code to avoid rebuilding the regex on every call by hoisting the pattern to a module-level constant (e.g. define a PLUGIN_PATTERN or COPILOT_PLUGIN_REGEX using COPILOT_PLUGIN_ID) and then have hasCopilotSafetyNetPlugin(output) simply test against that constant; keep the same pattern semantics ((^|[^a-z0-9-])...([^a-z0-9-]|$)) and return pluginRegex.test(output) as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 21: Update the AGENTS.md description for the "bun run check" command to
include the missing "ast-grep scan" step; locate the sentence that currently
lists the pipeline ("biome check → typecheck → knip → bun test") and change it
to explicitly enumerate "biome check → typecheck → knip → ast-grep scan → bun
test" so contributors know the full validation pipeline when running `bun run
check`.
In `@CLAUDE.md`:
- Line 22: Update the CLAUDE.md sentence that describes what `bun run check`
runs so it includes the missing `ast-grep scan`; specifically edit the line
containing "Always use `bun run check` to verify changes." (the paragraph
listing checks for `bun run check`) to reflect the full sequence: biome check →
typecheck → knip → ast-grep scan → bun test, ensuring the documented order
matches the actual script.
In `@tests/helpers.ts`:
- Around line 72-75: The mock that handles multi-word commands in
tests/helpers.ts is too permissive (it matches any "copilot plugin ..."); update
the condition in that block (the args array check) to match exactly the "copilot
plugin list" invocation (e.g., check args[0] === 'copilot' && args[1] ===
'plugin' && args[2] === 'list' or compare args.join(' ') === 'copilot plugin
list') so only the precise command returns the mocked "Installed plugins"
string.
---
Nitpick comments:
In `@src/bin/doctor/system-info.ts`:
- Around line 97-103: The RegExp construction in hasCopilotSafetyNetPlugin is
flagged as ReDoS but COPILOT_PLUGIN_ID is a compile-time constant and the
pattern is safe; update the code to avoid rebuilding the regex on every call by
hoisting the pattern to a module-level constant (e.g. define a PLUGIN_PATTERN or
COPILOT_PLUGIN_REGEX using COPILOT_PLUGIN_ID) and then have
hasCopilotSafetyNetPlugin(output) simply test against that constant; keep the
same pattern semantics ((^|[^a-z0-9-])...([^a-z0-9-]|$)) and return
pluginRegex.test(output) as before.
In `@tests/bin/doctor/system-info.test.ts`:
- Around line 160-165: The new async test helper functions (e.g., the const
fetcher declared in this diff) lack explicit TypeScript return annotations;
update each async fetcher introduced at the referenced locations to include an
explicit return type of Promise<string | null> so the compiler rule is satisfied
(i.e., change the implicit-typed async arrow functions to have an explicit
Promise<string | null> return annotation for each fetcher).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14aca37e-3d39-434b-b394-341164de212b
⛔ Files ignored due to path filters (3)
dist/bin/cc-safety-net.jsis excluded by!**/dist/**dist/bin/doctor/hooks.d.tsis excluded by!**/dist/**dist/bin/doctor/types.d.tsis excluded by!**/dist/**
📒 Files selected for processing (10)
AGENTS.mdCLAUDE.mdsrc/bin/doctor/hooks.tssrc/bin/doctor/index.tssrc/bin/doctor/system-info.tssrc/bin/doctor/types.tstests/bin/doctor/format.test.tstests/bin/doctor/hooks.test.tstests/bin/doctor/system-info.test.tstests/helpers.ts
| | Doctor | `bun src/bin/cc-safety-net.ts doctor` | | ||
|
|
||
| **`bun run check`** runs: biome check → typecheck → knip → ast-grep scan → bun test | ||
| **Always use `bun run check` to verify changes.** This runs typecheck, knip, biome lint, and tests together. Do not run these separately. |
There was a problem hiding this comment.
Include AST scan in the bun run check description.
Line 21 explicitly lists what runs, but it omits ast-grep scan, which can mislead contributors about the full validation pipeline.
📝 Suggested doc tweak
-**Always use `bun run check` to verify changes.** This runs typecheck, knip, biome lint, and tests together. Do not run these separately.
+**Always use `bun run check` to verify changes.** This runs biome check, typecheck, knip, ast-grep scan, and tests together. Do not run these separately.Based on learnings: Run bun run check before committing which executes: biome check → typecheck → knip → ast-grep scan → bun test.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **Always use `bun run check` to verify changes.** This runs typecheck, knip, biome lint, and tests together. Do not run these separately. | |
| **Always use `bun run check` to verify changes.** This runs biome check, typecheck, knip, ast-grep scan, and tests together. Do not run these separately. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` at line 21, Update the AGENTS.md description for the "bun run
check" command to include the missing "ast-grep scan" step; locate the sentence
that currently lists the pipeline ("biome check → typecheck → knip → bun test")
and change it to explicitly enumerate "biome check → typecheck → knip → ast-grep
scan → bun test" so contributors know the full validation pipeline when running
`bun run check`.
| - **Build**: `bun run build` | ||
| - **Doctor**: `bun src/bin/cc-safety-net.ts doctor` (diagnostics) | ||
|
|
||
| **Always use `bun run check` to verify changes.** This runs typecheck, knip, biome lint, and tests together. Do not run these separately. |
There was a problem hiding this comment.
Keep bun run check contents accurate here as well.
Line 22 omits ast-grep scan from the listed checks, so the instruction is incomplete.
📝 Suggested doc tweak
-**Always use `bun run check` to verify changes.** This runs typecheck, knip, biome lint, and tests together. Do not run these separately.
+**Always use `bun run check` to verify changes.** This runs biome check, typecheck, knip, ast-grep scan, and tests together. Do not run these separately.Based on learnings: Run bun run check before committing which executes: biome check → typecheck → knip → ast-grep scan → bun test.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **Always use `bun run check` to verify changes.** This runs typecheck, knip, biome lint, and tests together. Do not run these separately. | |
| **Always use `bun run check` to verify changes.** This runs biome check, typecheck, knip, ast-grep scan, and tests together. Do not run these separately. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` at line 22, Update the CLAUDE.md sentence that describes what `bun
run check` runs so it includes the missing `ast-grep scan`; specifically edit
the line containing "Always use `bun run check` to verify changes." (the
paragraph listing checks for `bun run check`) to reflect the full sequence:
biome check → typecheck → knip → ast-grep scan → bun test, ensuring the
documented order matches the actual script.
Comment on lines
+72
to
+75
| // Handle multi-word commands like `copilot plugin list` | ||
| if (args[0] === 'copilot' && args[1] === 'plugin') { | ||
| return 'Installed plugins:\n • copilot-safety-net (v1.0.0)'; | ||
| } |
There was a problem hiding this comment.
Tighten the mock to copilot plugin list specifically.
Lines 73-74 currently match any copilot plugin ... command. That can let incorrect command shapes pass tests unintentionally.
✅ Suggested fix
- if (args[0] === 'copilot' && args[1] === 'plugin') {
+ if (args[0] === 'copilot' && args[1] === 'plugin' && args[2] === 'list') {
return 'Installed plugins:\n • copilot-safety-net (v1.0.0)';
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/helpers.ts` around lines 72 - 75, The mock that handles multi-word
commands in tests/helpers.ts is too permissive (it matches any "copilot plugin
..."); update the condition in that block (the args array check) to match
exactly the "copilot plugin list" invocation (e.g., check args[0] === 'copilot'
&& args[1] === 'plugin' && args[2] === 'list' or compare args.join(' ') ===
'copilot plugin list') so only the precise command returns the mocked "Installed
plugins" string.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bin/doctor/hooks.ts`:
- Around line 688-697: The branch that returns the Copilot config object uses
primaryConfigPath from hooksCheck.activeConfigPaths and therefore yields
undefined when copilotPluginInstalled is true but no hook files exist; update
the return in that if-block (the code referencing
options.copilotPluginInstalled, viaPlugin, primaryConfigPath, and
hooksCheck.activeConfigPaths) to set configPath to a plugin-source fallback when
viaPlugin is true and hooksCheck.activeConfigPaths is empty (for example derive
a value from options (e.g. options.copilotPluginSource) or use a sentinel like
'copilot-plugin' as the configPath), and ensure configPaths remains undefined or
empty as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e4c354b-3887-40cd-b3d8-f62a7eba8596
⛔ Files ignored due to path filters (1)
dist/bin/cc-safety-net.jsis excluded by!**/dist/**
📒 Files selected for processing (2)
src/bin/doctor/hooks.tstests/bin/doctor/hooks.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/bin/doctor/hooks.test.ts
Owner
Author
|
@greptileai pls review |
Owner
Author
|
@coderabbitai pls review |
|
✅ Actions performedReview triggered.
|
Owner
Author
|
@greptileai pls review |
kenryu42
deleted the
feat/add-copilot-cli-plugin-detection-to-doctor-command
branch
7 tasks
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
1 participant
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.
Summary
doctorcommand viacopilot plugin listcopilot-safety-netis installed as a Copilot CLI plugin, not just via hook config filesAGENTS.mdcheck command description and addbun run checkdescription toCLAUDE.mdChanges
src/bin/doctor/system-info.ts: Addedcopilot plugin listto the parallel version fetch calls; addedhasCopilotSafetyNetPlugin()helper to parse plugin list output and detect thecopilot-safety-netplugin ID; addedcopilotPluginInstalledboolean toSystemInforeturnsrc/bin/doctor/types.ts: AddedcopilotPluginInstalled: booleanfield toSystemInfointerface andcopilotPluginInstalled?: booleantoHookDetectOptionssrc/bin/doctor/hooks.ts: UpdateddetectCopilotCliHook()to acceptcopilotPluginInstalledoption; when plugin is installed, report status as'configured'with method'plugin list'even without hook config filessrc/bin/doctor/index.ts: PasscopilotPluginInstalledfrom system info todetectAllHooks()tests/bin/doctor/system-info.test.ts: Added tests forcopilotPluginInstalleddetection — true when plugin is listed, false for partial matches, false when unavailabletests/bin/doctor/hooks.test.ts: Added tests for Copilot CLI plugin-based detection — configured via plugin list, override bydisableAllHooks, and global hook configtests/bin/doctor/format.test.ts: AddedcopilotPluginInstalled: trueto test fixturetests/helpers.ts: UpdatedmockVersionFetcherto handlecopilot plugin listargs and return mock plugin list outputAGENTS.md: Simplifiedbun run checkdescriptionCLAUDE.md: Addedbun run checkdescription for consistencyTesting
PR Checklist
bun run checkpasses (lint, types, dead code, rules, tests)package.jsonSummary by CodeRabbit
New Features
Documentation
Tests