fix(ast-grep): address code review issues in error handling and documentation

- Fix silent failures in error handling:
  - Add error logging when JSON parsing fails (cli.ts)
  - Add error logging in binary verification (downloader.ts)
  - Add warning when reading version marker fails (downloader.ts)

- Improve error handling robustness:
  - Use .finally() instead of .then() for timeout cleanup (cli.ts)
  - Include error details in nested catch blocks (cli.ts)
  - Add retry counter to prevent infinite download loops (cli.ts)
  - Return exit code when process exits with no output (cli.ts)
  - Log full error on NAPI load failure (napi.ts)

- Security improvements:
  - Escape single quotes in PowerShell extractZip command
  - Use -LiteralPath for safer path handling

- Better error messages:
  - Add Zod-specific error formatting for validation errors (index.ts)

- Documentation updates:
  - Update ADR to document all 4 tools (search, replace, analyze, transform)
  - Update agent skill frontmatter with correct MCP tool names
  - Add NAPI tool documentation (ast_grep_analyze, ast_grep_transform)

Author: amondnet

agents/ast-grep.md

@@ -40,7 +40,7 @@ description: |
   Pattern debugging - common mistake with Python syntax.
   </commentary>
   </example>
-tools: Bash, Read, Write
+tools: ast_grep_search, ast_grep_replace, ast_grep_analyze, ast_grep_transform
 model: sonnet
 ---
 
@@ -289,6 +289,38 @@ Transform code (dry-run by default):
 }
 ```
 
+### ast_grep_analyze (NAPI - In-Memory)
+
+Analyze code in-memory without file I/O (faster, supports 5 languages: html, javascript, tsx, css, typescript):
+
+```json
+{
+  "code": "console.log('hello'); console.log('world');",
+  "pattern": "console.log($MSG)",
+  "lang": "javascript",
+  "extractMetaVars": true
+}
+```
+
+Returns matches with optional meta-variable extraction. Use for single-file analysis or quick pattern testing.
+
+### ast_grep_transform (NAPI - In-Memory)
+
+Transform code in-memory without modifying files (supports 5 languages: html, javascript, tsx, css, typescript):
+
+```json
+{
+  "code": "console.log('hello');",
+  "pattern": "console.log($MSG)",
+  "rewrite": "logger.info($MSG)",
+  "lang": "javascript"
+}
+```
+
+Returns transformed code without writing to disk. Use for previewing transformations or processing code strings.
+
+**Note:** NAPI tools require `@ast-grep/napi` optional dependency. If not installed, use CLI-based `ast_grep_search` and `ast_grep_replace` instead.
+
 ---
 
 ## WORKFLOW

docs/adr/0001-ast-grep-integration-architecture.md

@@ -27,10 +27,12 @@ We will implement **both MCP Tools and an Agent/Skill** for ast-grep integration
 
 Create a new `ast-grep` provider in `packages/dora/src/providers/ast-grep/` with:
 
-| Tool | Purpose |
-|------|---------|
-| `ast_grep_search` | Pattern-based code search across files |
-| `ast_grep_replace` | AST-aware code transformation (dry-run by default) |
+| Tool | Purpose | Backend |
+|------|---------|---------|
+| `ast_grep_search` | Pattern-based code search across files | CLI (25 languages) |
+| `ast_grep_replace` | AST-aware code transformation (dry-run by default) | CLI (25 languages) |
+| `ast_grep_analyze` | In-memory code analysis with meta-variable extraction | NAPI (5 languages) |
+| `ast_grep_transform` | In-memory code transformation (no file I/O) | NAPI (5 languages) |
 
 **Features:**
 - Both inline patterns (`console.log($MSG)`) and YAML rule files (`--rule file.yaml`)
@@ -69,10 +71,11 @@ agents/
 ### Binary Management
 
 Follow the Dart LSP pattern:
-1. Check system PATH first (`Bun.which('sg')`)
+1. Check system PATH first (`Bun.which('ast-grep')` or `Bun.which('sg')`)
 2. If not found, download to `~/.cache/dora/ast-grep/`
 3. Platform-specific binaries (win-x64, linux-x64, linux-arm64, osx-x64, osx-arm64)
 4. Version tracking via marker file
+5. Binary verification to ensure it's actually ast-grep (checks `--version` output)
 
 ## Consequences
 

packages/dora/src/providers/ast-grep/cli.ts

@@ -44,8 +44,11 @@ export async function isCliAvailable(): Promise<boolean> {
 
 /**
  * Run ast-grep CLI command
+ *
+ * @param options - Command options
+ * @param retried - Internal flag to prevent infinite retry loops
  */
-export async function runSg(options: RunSgOptions): Promise<SgResult> {
+export async function runSg(options: RunSgOptions, retried = false): Promise<SgResult> {
   const cliPath = await getAstGrepPath()
 
   if (!cliPath) {
@@ -117,7 +120,8 @@ export async function runSg(options: RunSgOptions): Promise<SgResult> {
       proc.kill()
       reject(new Error(`Search timeout after ${timeout}ms`))
     }, timeout)
-    proc.exited.then(() => clearTimeout(id))
+    // Use .finally() to ensure cleanup even if proc.exited rejects
+    proc.exited.finally(() => clearTimeout(id))
   })
 
   let stdout: string
@@ -147,20 +151,20 @@ export async function runSg(options: RunSgOptions): Promise<SgResult> {
       || nodeError.message?.includes('ENOENT')
       || nodeError.message?.includes('not found')
     ) {
-      // Binary not found, try to download
-      const downloadedPath = await ensureAstGrepBinary()
-      if (downloadedPath) {
-        resolvedCliPath = downloadedPath
-        return runSg(options) // Retry
-      }
-      else {
-        return {
-          matches: [],
-          totalMatches: 0,
-          truncated: false,
-          error: getInstallInstructions(),
+      // Binary not found, try to download (only once to prevent infinite loops)
+      if (!retried) {
+        const downloadedPath = await ensureAstGrepBinary()
+        if (downloadedPath) {
+          resolvedCliPath = downloadedPath
+          return runSg(options, true) // Retry once
         }
       }
+      return {
+        matches: [],
+        totalMatches: 0,
+        truncated: false,
+        error: getInstallInstructions(),
+      }
     }
 
     return {
@@ -179,7 +183,13 @@ export async function runSg(options: RunSgOptions): Promise<SgResult> {
     if (stderr.trim()) {
       return { matches: [], totalMatches: 0, truncated: false, error: stderr.trim() }
     }
-    return { matches: [], totalMatches: 0, truncated: false }
+    // Non-zero exit with no output - include exit code for diagnosis
+    return {
+      matches: [],
+      totalMatches: 0,
+      truncated: false,
+      error: `ast-grep exited with code ${exitCode} (no output)`,
+    }
   }
 
   // No output
@@ -196,7 +206,7 @@ export async function runSg(options: RunSgOptions): Promise<SgResult> {
   try {
     matches = JSON.parse(outputToProcess) as CliMatch[]
   }
-  catch {
+  catch (parseError) {
     if (outputTruncated) {
       // Try to parse partial JSON
       try {
@@ -209,18 +219,27 @@ export async function runSg(options: RunSgOptions): Promise<SgResult> {
           }
         }
       }
-      catch {
+      catch (recoveryError) {
+        const errorMsg = recoveryError instanceof Error ? recoveryError.message : 'unknown'
         return {
           matches: [],
           totalMatches: 0,
           truncated: true,
           truncatedReason: 'max_output_bytes',
-          error: 'Output too large and could not be parsed',
+          error: `Output too large and could not be parsed: ${errorMsg}`,
         }
       }
     }
     else {
-      return { matches: [], totalMatches: 0, truncated: false }
+      // Non-truncated output but failed to parse - log and return error
+      const errorMsg = parseError instanceof Error ? parseError.message : 'unknown'
+      console.error(`[ast-grep] Failed to parse output: ${errorMsg}`)
+      return {
+        matches: [],
+        totalMatches: 0,
+        truncated: false,
+        error: `Failed to parse ast-grep output: ${errorMsg}`,
+      }
     }
   }
 

packages/dora/src/providers/ast-grep/downloader.ts

@@ -32,7 +32,8 @@ async function verifyAstGrepBinary(binaryPath: string): Promise<boolean> {
     // ast-grep version output contains "ast-grep" or version number like "0.x.x"
     return stdout.includes('ast-grep') || /^\d+\.\d+\.\d+/.test(stdout.trim())
   }
-  catch {
+  catch (e) {
+    console.error(`[ast-grep] Binary verification failed for ${binaryPath}: ${e instanceof Error ? e.message : String(e)}`)
     return false
   }
 }
@@ -77,8 +78,9 @@ export function getCachedBinary(): string | null {
         return null
       }
     }
-    catch {
-      // Can't read version, assume outdated
+    catch (e) {
+      // Log warning but continue - will trigger re-download
+      console.warn(`[ast-grep] Failed to read version marker at ${versionPath}: ${e instanceof Error ? e.message : String(e)}`)
       return null
     }
   }
@@ -90,17 +92,23 @@ export function getCachedBinary(): string | null {
  * Extract zip archive
  */
 async function extractZip(archivePath: string, destDir: string): Promise<void> {
-  const proc
-    = process.platform === 'win32'
-      ? spawn(
-          [
-            'powershell',
-            '-command',
-            `Expand-Archive -Path '${archivePath}' -DestinationPath '${destDir}' -Force`,
-          ],
-          { stdout: 'pipe', stderr: 'pipe' },
-        )
-      : spawn(['unzip', '-o', archivePath, '-d', destDir], { stdout: 'pipe', stderr: 'pipe' })
+  let proc
+  if (process.platform === 'win32') {
+    // Escape single quotes for PowerShell by doubling them
+    const escapedArchive = archivePath.replace(/'/g, "''")
+    const escapedDest = destDir.replace(/'/g, "''")
+    proc = spawn(
+      [
+        'powershell',
+        '-command',
+        `Expand-Archive -LiteralPath '${escapedArchive}' -DestinationPath '${escapedDest}' -Force`,
+      ],
+      { stdout: 'pipe', stderr: 'pipe' },
+    )
+  }
+  else {
+    proc = spawn(['unzip', '-o', archivePath, '-d', destDir], { stdout: 'pipe', stderr: 'pipe' })
+  }
 
   const exitCode = await proc.exited
 

packages/dora/src/providers/ast-grep/index.ts

@@ -4,7 +4,7 @@
  * Provides AST-aware code search and transformation tools
  */
 
-import { z } from 'zod'
+import { z, ZodError } from 'zod'
 import type { Provider, ToolDefinition, ToolResult } from '../provider'
 import type { RegistryConfig } from '../registry'
 import { CLI_LANGUAGES, NAPI_LANGUAGES } from './constants'
@@ -13,6 +13,17 @@ import { isNapiAvailable, getNapiError, analyzeCode, transformCode } from './nap
 import { formatSearchResult, formatReplaceResult, formatAnalyzeResult, formatTransformResult, getEmptyResultHint } from './utils'
 import type { NapiLanguage } from './types'
 
+/**
+ * Format error for tool result, with special handling for Zod validation errors
+ */
+function formatToolError(e: unknown): string {
+  if (e instanceof ZodError) {
+    const issues = e.issues.map(i => `  - ${i.path.join('.')}: ${i.message}`).join('\n')
+    return `Invalid arguments:\n${issues}`
+  }
+  return `Error: ${e instanceof Error ? e.message : String(e)}`
+}
+
 // Tool definitions
 const AST_GREP_TOOLS: ToolDefinition[] = [
   {
@@ -228,7 +239,7 @@ export class AstGrepProvider implements Provider {
     }
     catch (e) {
       return {
-        content: [{ type: 'text', text: `Error: ${e instanceof Error ? e.message : String(e)}` }],
+        content: [{ type: 'text', text: formatToolError(e) }],
         isError: true,
       }
     }
@@ -267,7 +278,7 @@ export class AstGrepProvider implements Provider {
     }
     catch (e) {
       return {
-        content: [{ type: 'text', text: `Error: ${e instanceof Error ? e.message : String(e)}` }],
+        content: [{ type: 'text', text: formatToolError(e) }],
         isError: true,
       }
     }
@@ -312,7 +323,7 @@ export class AstGrepProvider implements Provider {
     }
     catch (e) {
       return {
-        content: [{ type: 'text', text: `Error: ${e instanceof Error ? e.message : String(e)}` }],
+        content: [{ type: 'text', text: formatToolError(e) }],
         isError: true,
       }
     }
@@ -357,7 +368,7 @@ export class AstGrepProvider implements Provider {
     }
     catch (e) {
       return {
-        content: [{ type: 'text', text: `Error: ${e instanceof Error ? e.message : String(e)}` }],
+        content: [{ type: 'text', text: formatToolError(e) }],
         isError: true,
       }
     }

packages/dora/src/providers/ast-grep/napi.ts

@@ -31,6 +31,8 @@ export function isNapiAvailable(): boolean {
   }
   catch (e) {
     napiLoadError = e instanceof Error ? e : new Error(String(e))
+    // Log full error on first load failure for diagnostics
+    console.error('[ast-grep] NAPI module load failed:', e)
     return false
   }
 }