Skip to content

fix: correct file detection and skill check in code-review#104

Merged
dean0x merged 1 commit intomainfrom
fix/code-review-file-detection
Mar 9, 2026
Merged

fix: correct file detection and skill check in code-review#104
dean0x merged 1 commit intomainfrom
fix/code-review-file-detection

Conversation

@dean0x
Copy link
Owner

@dean0x dean0x commented Mar 9, 2026

Summary

  • Disambiguate TypeScript vs React file extension conditions in Phase 1 tables — .ts/.tsx was too similar to .tsx/.jsx, causing the orchestrating LLM to conflate them and miss plain .ts files
  • Switch skill availability check from Glob to Read — Glob doesn't expand ~ to the home directory, causing all optional language skills to incorrectly report as "not installed"

Root Cause

Two independent bugs combined:

  1. Extension conflation: The condition table listed .ts/.tsx (TypeScript) adjacent to .tsx/.jsx (React). The orchestrating agent misread the TypeScript condition as .tsx/.jsx, skipping TypeScript review when only plain .ts files changed.
  2. Glob tilde bug: Glob("~/.claude/skills/{focus}/SKILL.md") returns no results because Glob doesn't expand ~. The Read tool does expand ~, so switching to Read fixes skill detection on all platforms.

Test plan

  • Run /code-review on a branch with only .ts file changes — TypeScript review should trigger
  • Run /code-review on a branch with .tsx changes — both TypeScript and React reviews should trigger
  • Verify optional language skills (go, python, java, rust) are detected when installed

- Disambiguate TypeScript vs React file conditions to prevent LLM
  conflation (.ts/.tsx was being misread as .tsx/.jsx)
- Switch skill availability check from Glob to Read (Glob doesn't
  expand ~ but Read does, causing false "not installed" for all
  optional language skills)
@dean0x dean0x merged commit 7994485 into main Mar 9, 2026
4 checks passed
@dean0x dean0x deleted the fix/code-review-file-detection branch March 9, 2026 16:11
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