Conversation
Rewrite AGENTS.md from project-specific documentation to comprehensive style guide covering destructuring, variables, control flow, schema definitions, and testing conventions. Simplify CLAUDE.md to reference AGENTS.md instead of duplicating content.
Migrate from Claude Code slash commands to the skills format: - Remove commands/set-custom-rules.md, set-statusline.md, verify-custom-rules.md - Add skills/set-custom-rules and skills/verify-custom-rules with SKILL.md and OpenAI agent configs - Update CONTRIBUTING.md directory structure references - Update README.md to reference skills instead of slash commands - Update generate-changelog.ts CLAUDE_CODE_PATHS from 'commands/' to 'skills/' - Update test expectations for skills path migration
…tput Replace file-based detection (extension-enablement.json + settings.json) with parsing command output. This approach checks for the safety-net source URL and verifies Enabled (User) and Enabled (Workspace) flags, eliminating the need to read local config files directly. Also improve defaultVersionFetcher to return stderr output when a successful command writes no stdout (needed for gemini CLI behavior).
Detect safety-net@cc-marketplace plugin status by parsing Installed plugins:
❯ code-simplifier@claude-plugins-official
Version: 1.0.0
Scope: user
Status: ✔ enabled
❯ frontend-design@claude-plugins-official
Version: unknown
Scope: project
Status: ✘ disabled
❯ safety-net@cc-marketplace
Version: 0.8.2
Scope: user
Status: ✔ enabled
❯ skill-creator@claude-plugins-official
Version: unknown
Scope: user
Status: ✔ enabled
❯ telegram@claude-plugins-official
Version: 0.0.6
Scope: user
Status: ✔ enabled
output instead of reading ~/.claude/settings.json directly. This mirrors
the Gemini CLI detection migration and provides more reliable status
information from the actual plugin registry.
- Detect plugin blocks by matching the safety-net plugin ID
- Parse enabled/disabled status from the plugin block
- Return n/a when plugin list is unavailable or plugin not found
- Return disabled with error when status is unrecognized
Add Codex platform support to the doctor command's hook detection system. - Add 'codex' to HookPlatform type union - Add detectCodex function that checks plugin cache and config.toml - Add CodexConfig interface for parsing codex configuration - Detect plugin_hooks feature flag and safety-net plugin enabled status - Support CODEX_HOME environment variable for custom codex directory - Add comprehensive tests for all Codex detection scenarios
Wrap error messages in the hooks section with red ANSI color codes to make them more visually distinct and noticeable. Add test coverage for TTY color output behavior.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
==========================================
+ Coverage 98.51% 98.53% +0.02%
==========================================
Files 52 52
Lines 6879 6975 +96
==========================================
+ Hits 6777 6873 +96
Misses 102 102 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughAdds Codex, Claude Code, and Gemini CLI detection to the doctor CLI; expands system-info to capture plugin/extension outputs; implements new detectors and formatting changes; migrates command docs into a new skills/ directory and adds skill manifests and docs for custom-rule workflows. ChangesDocumentation & Skills Migration
Doctor CLI — Multi-platform hook detection
Sequence Diagram(s)sequenceDiagram
participant User as User / CLI
participant Doctor as Doctor Command
participant SysInfo as getSystemInfo()
participant Hooks as detectAllHooks()
participant Claude as detectClaudeCode()
participant Gemini as detectGeminiCLI()
participant Codex as detectCodex()
participant Format as formatHooksSection()
participant Output as Output
User->>Doctor: run doctor
Doctor->>SysInfo: fetch versions & plugin/extension outputs
SysInfo-->>Doctor: return SystemInfo (incl. claude/gemini outputs)
Doctor->>Hooks: detectAllHooks(options + plugin outputs)
Hooks->>Claude: detectClaudeCode(claudePluginListOutput)
Claude-->>Hooks: HookStatus
Hooks->>Gemini: detectGeminiCLI(geminiExtensionsListOutput)
Gemini-->>Hooks: HookStatus
Hooks->>Codex: detectCodex(env/config)
Codex-->>Hooks: HookStatus
Hooks-->>Doctor: aggregated HookStatuses
Doctor->>Format: formatHooksSection(hooks, systemInfo)
Format-->>Output: display formatted statuses (errors colored red)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Greptile SummaryThis PR completes multi-platform hook detection in the
Confidence Score: 5/5Safe to merge; all new detection paths degrade gracefully and the core Codex/Claude Code/Gemini detection logic is well tested. The core logic paths are well-tested and CLI-output parsers handle null/empty input without throwing. The two findings are edge cases unlikely to affect production users. src/bin/doctor/hooks.ts — the _parseCodexConfig section-matching logic and the detectClaudeCode silent-null path. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[detectAllHooks] --> CC[detectClaudeCode]
A --> GC[detectGeminiCLI]
A --> CX[detectCodex]
CC --> CC1{pluginListOutput?}
CC1 -- null --> CC2[n/a]
CC1 -- present --> CC3[find plugin block]
CC3 --> CC4{Status line}
CC4 -- enabled --> CC5[configured]
CC4 -- disabled --> CC6[disabled]
GC --> GC1{extensionsListOutput?}
GC1 -- null --> GC2[n/a]
GC1 -- present --> GC3[parse extensions list]
GC3 --> GC4{source found?}
GC4 -- no --> GC5[n/a]
GC4 -- yes --> GC6{effectiveEnabled}
GC6 -- true --> GG7[configured]
GC6 -- false --> GG8[disabled]
CX --> CX1{cache exists?}
CX1 -- no --> CX2[n/a]
CX1 -- yes --> CX3[read config.toml]
CX3 --> CX4{safetyNetEnabled?}
CX4 -- false --> CX5[disabled]
CX4 -- true --> CX6{pluginHooks?}
CX6 -- false --> CX7[disabled+warning]
CX6 -- true --> CX8[configured]
Reviews (2): Last reviewed commit: "fix(doctor): exclude metadata email line..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+52
to
+57
| const CLAUDE_SAFETY_NET_PLUGIN_ID = 'safety-net@cc-marketplace'; | ||
| const GEMINI_EXTENSIONS_LIST_CONFIG_PATH = 'gemini extensions list'; | ||
| const GEMINI_SAFETY_NET_SOURCE = 'https://github.com/kenryu42/gemini-safety-net'; | ||
| const CODEX_PLUGIN_HOOKS_WARNING = | ||
| 'Codex plugin hooks are behind a feature flag. Add `plugin_hooks = true` under [features] in $CODEX_HOME/config.toml.'; | ||
| const CODEX_SAFETY_NET_PLUGIN_ID = 'safety-net@cc-marketplace'; |
There was a problem hiding this comment.
CODEX_SAFETY_NET_PLUGIN_ID and CLAUDE_SAFETY_NET_PLUGIN_ID share the identical value 'safety-net@cc-marketplace'. If the ID ever needs to change for one platform, it would be easy to update only one constant and silently leave the other stale. A single shared constant removes the ambiguity.
Suggested change
| const CLAUDE_SAFETY_NET_PLUGIN_ID = 'safety-net@cc-marketplace'; | |
| const GEMINI_EXTENSIONS_LIST_CONFIG_PATH = 'gemini extensions list'; | |
| const GEMINI_SAFETY_NET_SOURCE = 'https://github.com/kenryu42/gemini-safety-net'; | |
| const CODEX_PLUGIN_HOOKS_WARNING = | |
| 'Codex plugin hooks are behind a feature flag. Add `plugin_hooks = true` under [features] in $CODEX_HOME/config.toml.'; | |
| const CODEX_SAFETY_NET_PLUGIN_ID = 'safety-net@cc-marketplace'; | |
| const SAFETY_NET_PLUGIN_ID = 'safety-net@cc-marketplace'; | |
| const GEMINI_EXTENSIONS_LIST_CONFIG_PATH = 'gemini extensions list'; | |
| const GEMINI_SAFETY_NET_SOURCE = 'https://github.com/kenryu42/gemini-safety-net'; | |
| const CODEX_PLUGIN_HOOKS_WARNING = | |
| 'Codex plugin hooks are behind a feature flag. Add `plugin_hooks = true` under [features] in $CODEX_HOME/config.toml.'; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
32-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove stale Status Line links from Contents.
Line 34 and Line 35 point to headings that no longer exist, so those anchors are broken in rendered docs. Please remove or rename them to match current section headings.
Suggested README TOC fix
- [Status Line Integration](`#status-line-integration`) - - [Setup via Slash Command](`#setup-via-slash-command`) - - [Manual Setup](`#manual-setup`) - [Emoji Mode Indicators](`#emoji-mode-indicators`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 32 - 36, Remove the stale "Status Line Integration" TOC entries: locate the two markdown links "[Status Line Integration](`#status-line-integration`)" and the nested "- [Setup via Slash Command](`#setup-via-slash-command`)" / "- [Manual Setup](`#manual-setup`)" (or any other broken anchors) in the README table of contents and either delete them or update their anchor text to match the current section headings; ensure remaining TOC links match existing headings so no broken anchors remain.
🧹 Nitpick comments (4)
AGENTS.md (1)
13-13: 💤 Low valueConsider adding a code example for the functional array methods guideline.
Every other principle in this section (destructuring, variables, control flow) has a paired before/after code block, but "Prefer functional array methods (flatMap, filter, map) over for loops" on Line 13 has none. A short example would keep the style consistent and help contributors understand the type-guard use case mentioned in the guideline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` at line 13, Add a before/after code example demonstrating converting a for loop that builds a typed array into a composition of flatMap/filter/map, and show a TypeScript type guard (e.g., function isDesired(x): x is DesiredType) used in the filter to preserve downstream type inference; reference the array methods flatMap, filter, and map and the type guard function name (e.g., isDesired) in the example so readers see both the refactor and how the type guard keeps the inferred type.src/bin/doctor/index.ts (1)
29-45: ⚡ Quick winStart the independent doctor probes in parallel.
getSystemInfo(), config loading, environment inspection, activity summary, and the update check do not depend on each other, but this flow waits for them serially. Kicking them off together and awaiting once would trim CLI latency without changing behavior.♻️ One way to restructure it
- const system = await getSystemInfo(); - const hooks = detectAllHooks(cwd, { - claudePluginListOutput: system.claudePluginListOutput, - geminiExtensionsListOutput: system.geminiExtensionsListOutput, - copilotCliVersion: system.copilotCliVersion, - copilotPluginInstalled: system.copilotPluginInstalled, - }); - const configInfo = getConfigInfo(cwd); - const environment = getEnvironmentInfo(); - const activity = getActivitySummary(7); - const update = options.skipUpdateCheck - ? { - currentVersion: getPackageVersion(), - latestVersion: null, - updateAvailable: false, - } - : await checkForUpdates(); + const systemPromise = getSystemInfo(); + const configInfoPromise = Promise.resolve(getConfigInfo(cwd)); + const environmentPromise = Promise.resolve(getEnvironmentInfo()); + const activityPromise = Promise.resolve(getActivitySummary(7)); + const updatePromise = options.skipUpdateCheck + ? Promise.resolve({ + currentVersion: getPackageVersion(), + latestVersion: null, + updateAvailable: false, + }) + : checkForUpdates(); + + const [system, configInfo, environment, activity, update] = await Promise.all([ + systemPromise, + configInfoPromise, + environmentPromise, + activityPromise, + updatePromise, + ]); + + const hooks = detectAllHooks(cwd, { + claudePluginListOutput: system.claudePluginListOutput, + geminiExtensionsListOutput: system.geminiExtensionsListOutput, + copilotCliVersion: system.copilotCliVersion, + copilotPluginInstalled: system.copilotPluginInstalled, + });Based on learnings: ALWAYS USE PARALLEL TOOLS WHEN APPLICABLE.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bin/doctor/index.ts` around lines 29 - 45, Run independent async probes concurrently: start getSystemInfo(), getEnvironmentInfo(), getActivitySummary(7), and the conditional checkForUpdates() (when !options.skipUpdateCheck) without awaiting them immediately, then await their results together (e.g., with Promise.all). Note detectAllHooks(cwd, {...}) requires the system result so call it after awaiting getSystemInfo(); getConfigInfo(cwd) appears synchronous and can be called immediately. Ensure you still call getPackageVersion() when forming the fallback update object if options.skipUpdateCheck is true. This will preserve behavior while reducing CLI latency.tests/bin/doctor/hooks.test.ts (2)
538-575: ⚡ Quick winMake this test assert the commented
config.jsonpath is actually used.With
copilotPluginInstalled: trueon Line 563, this still passes even if the commented file is never parsed. A stronger check is to put a meaningful setting in the commented JSON (disableAllHooksor an inline hook) and assert the resulting status/config path, so the JSONC support added here is the thing under test. As per coding guidelines, "Test actual implementation, do not duplicate logic into tests".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/bin/doctor/hooks.test.ts` around lines 538 - 575, The test currently passes without verifying the commented JSON was parsed; modify the test that calls detectAllHooks so the commented config.json contains a meaningful setting (e.g., "disableAllHooks": true or an inline hook entry) that would change the detected copilot-cli hook result, then assert the returned hook reflects that setting (for example status/method or a configPath indicating config.json was used) — locate the test around detectAllHooks(...) and the copilot variable and replace the passive parse-check with an assertion that the specific setting from the commented JSON affected copilot?.status or copilot?.configPath, ensuring the test fails if the JSONC parsing is not performed.
1256-1431: ⚡ Quick winAdd one Codex case for repo-local vs user-level config precedence.
These tests cover
~/.codex,CODEX_HOME, feature flags, and missing cache, but they never exercise conflicting local and user configs. Since the PR explicitly changes Codex support around configuration resolution, a precedence fixture here would prevent that behavior from regressing silently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/bin/doctor/hooks.test.ts` around lines 1256 - 1431, Add a new test covering repo-local vs user-level Codex config precedence: create both a user codexHome (using _writeCodexConfig or writeFileSync to set plugin_hooks=true and enabled=false) and a project-local config (inside projectDir/.codex or repo-local path used by detectAllHooks) that sets plugin_hooks=true and enabled=true, then call detectAllHooks(projectDir, { homeDir }) (or withEnv to set CODEX_HOME) and assert that the returned codex hook reflects the repo-local config (status 'configured', method 'plugin cache', configPath pointing at the project-local config, and selfTest present) rather than the user-level config; reference existing helpers _writeCodexConfig, writeFileSync, detectAllHooks, withEnv to locate where to add this fixture.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bin/doctor/hooks.ts`:
- Around line 354-375: Current logic incorrectly requires both
extension.enabledUser and extension.enabledWorkspace to be true; instead compute
the effective enabled state using scope precedence (workspace overrides user,
and if neither is set treat as enabled by default). Change the check in hooks.ts
to derive an effectiveEnabled boolean from extension.enabledWorkspace (if
defined) else extension.enabledUser (if defined) else true, then only push an
error or return status 'disabled' when effectiveEnabled is false; keep
references to GEMINI_EXTENSIONS_LIST_CONFIG_PATH, method 'extension list', and
preserve runSelfTest() in the configured return path.
---
Outside diff comments:
In `@README.md`:
- Around line 32-36: Remove the stale "Status Line Integration" TOC entries:
locate the two markdown links "[Status Line
Integration](`#status-line-integration`)" and the nested "- [Setup via Slash
Command](`#setup-via-slash-command`)" / "- [Manual Setup](`#manual-setup`)" (or any
other broken anchors) in the README table of contents and either delete them or
update their anchor text to match the current section headings; ensure remaining
TOC links match existing headings so no broken anchors remain.
---
Nitpick comments:
In `@AGENTS.md`:
- Line 13: Add a before/after code example demonstrating converting a for loop
that builds a typed array into a composition of flatMap/filter/map, and show a
TypeScript type guard (e.g., function isDesired(x): x is DesiredType) used in
the filter to preserve downstream type inference; reference the array methods
flatMap, filter, and map and the type guard function name (e.g., isDesired) in
the example so readers see both the refactor and how the type guard keeps the
inferred type.
In `@src/bin/doctor/index.ts`:
- Around line 29-45: Run independent async probes concurrently: start
getSystemInfo(), getEnvironmentInfo(), getActivitySummary(7), and the
conditional checkForUpdates() (when !options.skipUpdateCheck) without awaiting
them immediately, then await their results together (e.g., with Promise.all).
Note detectAllHooks(cwd, {...}) requires the system result so call it after
awaiting getSystemInfo(); getConfigInfo(cwd) appears synchronous and can be
called immediately. Ensure you still call getPackageVersion() when forming the
fallback update object if options.skipUpdateCheck is true. This will preserve
behavior while reducing CLI latency.
In `@tests/bin/doctor/hooks.test.ts`:
- Around line 538-575: The test currently passes without verifying the commented
JSON was parsed; modify the test that calls detectAllHooks so the commented
config.json contains a meaningful setting (e.g., "disableAllHooks": true or an
inline hook entry) that would change the detected copilot-cli hook result, then
assert the returned hook reflects that setting (for example status/method or a
configPath indicating config.json was used) — locate the test around
detectAllHooks(...) and the copilot variable and replace the passive parse-check
with an assertion that the specific setting from the commented JSON affected
copilot?.status or copilot?.configPath, ensuring the test fails if the JSONC
parsing is not performed.
- Around line 1256-1431: Add a new test covering repo-local vs user-level Codex
config precedence: create both a user codexHome (using _writeCodexConfig or
writeFileSync to set plugin_hooks=true and enabled=false) and a project-local
config (inside projectDir/.codex or repo-local path used by detectAllHooks) that
sets plugin_hooks=true and enabled=true, then call detectAllHooks(projectDir, {
homeDir }) (or withEnv to set CODEX_HOME) and assert that the returned codex
hook reflects the repo-local config (status 'configured', method 'plugin cache',
configPath pointing at the project-local config, and selfTest present) rather
than the user-level config; reference existing helpers _writeCodexConfig,
writeFileSync, detectAllHooks, withEnv to locate where to add this fixture.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed31616a-f359-4472-bf65-8b38a25516d5
⛔ 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 (23)
.github/pull_request_template.mdAGENTS.mdCLAUDE.mdCONTRIBUTING.mdREADME.mdcommands/set-custom-rules.mdcommands/set-statusline.mdcommands/verify-custom-rules.mdscripts/generate-changelog.tsskills/set-custom-rules/SKILL.mdskills/set-custom-rules/agents/openai.yamlskills/verify-custom-rules/SKILL.mdskills/verify-custom-rules/agents/openai.yamlsrc/bin/doctor/format.tssrc/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.tstests/scripts/generate-changelog.test.ts
💤 Files with no reviewable changes (3)
- commands/set-statusline.md
- commands/set-custom-rules.md
- commands/verify-custom-rules.md
…ection Previously required both User and Workspace enabled scopes to be true. Now uses workspace scope when present, falls back to user scope, and defaults to enabled when neither is specified. This handles Gemini CLI versions that don't list enabled scopes in extension output.
…ck detection Use [^\w\s@] instead of \S in plugin line patterns to prevent Publisher/metadata lines containing @ from being merged into the safety-net plugin block.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bin/doctor/hooks.ts`:
- Around line 346-348: The current find over
_parseGeminiExtensionsList(extensionsListOutput) uses
item.source?.includes(GEMINI_SAFETY_NET_SOURCE) which can falsely match repos
with the GEMINI_SAFETY_NET_SOURCE substring; update the predicate to check exact
equality (e.g., item.source === GEMINI_SAFETY_NET_SOURCE) so only the exact
configured source is detected, keeping the rest of the find logic and references
to extension, _parseGeminiExtensionsList, and GEMINI_SAFETY_NET_SOURCE
unchanged.
- Around line 455-462: The current check treats any non-empty pluginCachePath as
"installed" even if it only contains stray files; update the block that reads
readdirSync(pluginCachePath) so it filters entries to real version directories
before deciding "n/a" vs installed: use fs.readdirSync(pluginCachePath) and for
each entry call fs.lstatSync(path.join(pluginCachePath, entry)).isDirectory()
(and optionally filter names with a semver/^\d+\.\d+\.\d+/ pattern) and only
treat the cache as present if at least one such directory exists; keep the
existing return shape ({ platform: 'codex', status: 'n/a', configPath:
pluginCachePath }) when no real version dirs found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5eb816b0-ab91-40e0-bf5c-be0ceb0b57ca
⛔ 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
Comment on lines
+346
to
+348
| const extension = _parseGeminiExtensionsList(extensionsListOutput).find((item) => | ||
| item.source?.includes(GEMINI_SAFETY_NET_SOURCE), | ||
| ); |
There was a problem hiding this comment.
Match the Gemini source exactly instead of by substring.
Line 347 uses includes(...), so a different repo like https://github.com/kenryu42/gemini-safety-net-fork would be treated as the official extension and reported as configured. That can mask a missing or replaced Safety Net install.
💡 Suggested fix
function detectGeminiCLI(extensionsListOutput: string | null | undefined): HookStatus {
if (!extensionsListOutput) {
return { platform: 'gemini-cli', status: 'n/a' };
}
- const extension = _parseGeminiExtensionsList(extensionsListOutput).find((item) =>
- item.source?.includes(GEMINI_SAFETY_NET_SOURCE),
- );
+ const extension = _parseGeminiExtensionsList(extensionsListOutput).find(
+ (item) => item.source === GEMINI_SAFETY_NET_SOURCE,
+ ); return blocks.map((block) => ({
- source: /^\s*Source:\s*(.+)$/m.exec(block)?.[1],
+ source: /^\s*Source:\s*(.+?)(?:\s+\(Type:.*\))?$/m.exec(block)?.[1],
enabledUser: _parseGeminiEnabledValue(block, 'User'),
enabledWorkspace: _parseGeminiEnabledValue(block, 'Workspace'),
}));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bin/doctor/hooks.ts` around lines 346 - 348, The current find over
_parseGeminiExtensionsList(extensionsListOutput) uses
item.source?.includes(GEMINI_SAFETY_NET_SOURCE) which can falsely match repos
with the GEMINI_SAFETY_NET_SOURCE substring; update the predicate to check exact
equality (e.g., item.source === GEMINI_SAFETY_NET_SOURCE) so only the exact
configured source is detected, keeping the rest of the find logic and references
to extension, _parseGeminiExtensionsList, and GEMINI_SAFETY_NET_SOURCE
unchanged.
Comment on lines
+455
to
+462
| if (!existsSync(pluginCachePath)) { | ||
| return { platform: 'codex', status: 'n/a', configPath: pluginCachePath }; | ||
| } | ||
|
|
||
| // Plugin not found | ||
| if (!isInstalled) { | ||
| try { | ||
| if (readdirSync(pluginCachePath).length === 0) { | ||
| return { platform: 'codex', status: 'n/a', configPath: pluginCachePath }; | ||
| } |
There was a problem hiding this comment.
Require a real version directory before treating the Codex plugin cache as installed.
Line 460 only checks that the cache directory is non-empty. A stray file like .DS_Store would make Codex look installed and can produce a false configured result.
💡 Suggested fix
try {
- if (readdirSync(pluginCachePath).length === 0) {
+ const hasVersionDir = readdirSync(pluginCachePath, { withFileTypes: true }).some((entry) =>
+ entry.isDirectory(),
+ );
+ if (!hasVersionDir) {
return { platform: 'codex', status: 'n/a', configPath: pluginCachePath };
}
} catch (e) {
return {
platform: 'codex',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bin/doctor/hooks.ts` around lines 455 - 462, The current check treats any
non-empty pluginCachePath as "installed" even if it only contains stray files;
update the block that reads readdirSync(pluginCachePath) so it filters entries
to real version directories before deciding "n/a" vs installed: use
fs.readdirSync(pluginCachePath) and for each entry call
fs.lstatSync(path.join(pluginCachePath, entry)).isDirectory() (and optionally
filter names with a semver/^\d+\.\d+\.\d+/ pattern) and only treat the cache as
present if at least one such directory exists; keep the existing return shape ({
platform: 'codex', status: 'n/a', configPath: pluginCachePath }) when no real
version dirs found.
Owner
Author
|
@greptileai review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/bin/doctor/hooks.test.ts`:
- Around line 160-166: The test creates temp dirs with a Date.now()-based name
(tmpBase in the "Claude Code: configured when plugin list shows safety-net
enabled" test), which can collide; replace the Date.now() approach by using
fs.mkdtempSync with a 'doctor-hooks-' prefix to create a collision-safe
temporary base directory, then derive homeDir and projectDir from that base;
ensure you import mkdtempSync (or mkdtemp) from fs and keep the existing
mkdirSync calls for homeDir and projectDir creation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 712fd312-6129-47b7-a784-d472d94a4334
📒 Files selected for processing (1)
tests/bin/doctor/hooks.test.ts
Comment on lines
+160
to
166
| test('Claude Code: configured when plugin list shows safety-net enabled', () => { | ||
| const tmpBase = join(tmpdir(), `doctor-hooks-${Date.now()}`); | ||
| const homeDir = join(tmpBase, 'home'); | ||
| const projectDir = join(tmpBase, 'project'); | ||
| mkdirSync(homeDir, { recursive: true }); | ||
| mkdirSync(projectDir, { recursive: true }); | ||
|
|
There was a problem hiding this comment.
Use collision-safe temp directories to prevent flaky test interference.
Date.now()-based directory names can collide across fast/parallel runs, which can cause shared state and cleanup races across tests. Prefer mkdtempSync (or mkdtemp) with a prefix for guaranteed uniqueness.
Suggested change
-import { mkdirSync, rmSync, writeFileSync } from 'node:fs';
+import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
-const tmpBase = join(tmpdir(), `doctor-hooks-${Date.now()}`);
+const tmpBase = mkdtempSync(join(tmpdir(), 'doctor-hooks-'));📝 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
| test('Claude Code: configured when plugin list shows safety-net enabled', () => { | |
| const tmpBase = join(tmpdir(), `doctor-hooks-${Date.now()}`); | |
| const homeDir = join(tmpBase, 'home'); | |
| const projectDir = join(tmpBase, 'project'); | |
| mkdirSync(homeDir, { recursive: true }); | |
| mkdirSync(projectDir, { recursive: true }); | |
| test('Claude Code: configured when plugin list shows safety-net enabled', () => { | |
| const tmpBase = mkdtempSync(join(tmpdir(), 'doctor-hooks-')); | |
| const homeDir = join(tmpBase, 'home'); | |
| const projectDir = join(tmpBase, 'project'); | |
| mkdirSync(homeDir, { recursive: true }); | |
| mkdirSync(projectDir, { recursive: true }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/bin/doctor/hooks.test.ts` around lines 160 - 166, The test creates temp
dirs with a Date.now()-based name (tmpBase in the "Claude Code: configured when
plugin list shows safety-net enabled" test), which can collide; replace the
Date.now() approach by using fs.mkdtempSync with a 'doctor-hooks-' prefix to
create a collision-safe temporary base directory, then derive homeDir and
projectDir from that base; ensure you import mkdtempSync (or mkdtemp) from fs
and keep the existing mkdirSync calls for homeDir and projectDir creation.
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, completing support for all five agent platformscommands/) to the skills format (skills/) for broader agent compatibility (including Codex/OpenAI)Changes
Doctor: Codex Hook Detection
detectCodex()insrc/bin/doctor/hooks.ts— reads$CODEX_HOME/config.tomlto check plugin cache, plugin enablement, andplugin_hooksfeature flag_parseCodexConfig()and_readCodexConfig()helpers for TOML-style config parsing with section header and inline comment support'codex'to theHookPlatformunion insrc/bin/doctor/types.ts'Codex'toPLATFORM_NAMESinsrc/bin/doctor/format.tsDoctor: Claude Code & Gemini CLI Detection Migration
detectClaudeCode()from reading~/.claude/settings.jsonto parsingclaude plugin listoutput — detection now uses theclaudePluginListOutputparameter instead ofhomeDirdetectGeminiCLI()from reading extension JSON files to parsinggemini extensions listoutput — detection now uses thegeminiExtensionsListOutputparameter instead ofhomeDir/cwd_parseGeminiExtensionsList()and_parseGeminiEnabledValue()for structured parsing of the Gemini extensions list formatdetectAllHooks()signature to pass new CLI output fieldsclaudePluginListOutputandgeminiExtensionsListOutputfields toSystemInfoandHookDetectOptionstypesgetSystemInfo()to fetchclaude plugin listandgemini extensions listin parallel with version checksdefaultVersionFetcherto capture and fall back to stderr output (for commands that write version info to stderr)Doctor: Copilot CLI Improvements
detectCopilot→detectCopilotCLIfor claritystripJsonComments()helper and use it when parsing Copilot CLI config files to handle commented JSONSkills Migration
commands/set-custom-rules.md→skills/set-custom-rules/SKILL.mdwith structured skill metadatacommands/verify-custom-rules.md→skills/verify-custom-rules/SKILL.mdwith structured skill metadataagents/openai.yamlfor each skill to support Codex agent invocationcommands/set-statusline.md(statusline setup moved to manual docs)CONTRIBUTING.mddirectory tree to reflectskills/instead ofcommands/scripts/generate-changelog.tsto mapskills/paths toclaudeCodechangelog categoryDocumentation
README.mdwith step-by-step marketplace plugin setupREADME.mdreferences from/set-custom-rulesslash command toset-custom-rulesskillAGENTS.mdwith style guide conventionsTesting
Manual Verification
bunx cc-safety-net doctorwith and without Codex installed to verify correct status reportingdoctorcorrectly detects Claude Code hooks viaclaude plugin listoutputdoctorcorrectly detects Gemini CLI hooks viagemini extensions listoutputset-custom-rulesandverify-custom-rulesskills load correctly in supported agentsRelated Issue
closes #46
PR Checklist
bun run checkpasses (lint, types, dead code, rules, tests)package.jsonSummary by CodeRabbit
New Features
Improvements
Documentation
Tests
Chore