fix(hooks): remove mandatory param block consuming stdin when dot-sourced#88
fix(hooks): remove mandatory param block consuming stdin when dot-sourced#88
Conversation
Summary by CodeRabbit
WalkthroughDot-sources the orchestrator and replaces stdin-driven orchestration with parameterized Handle-* entry points; adds 20+ handlers, environment dev-path fallback, extensive per‑phase timing and diagnostic logs, new pre/post-tool and session optimization hooks, token accounting/caching flows, and local-Git vs MCP decisioning. (34 words) Changes
Sequence Diagram(s)sequenceDiagram
participant D as dispatcher.ps1
participant O as token-optimizer-orchestrator.ps1
rect rgb(240,248,255)
Note over D,O: Session init
D->>O: Handle-SessionStartInit(InputJson)
O-->>D: {status, timing}
end
rect rgb(245,255,240)
Note over D,O: Pre-tool flow
D->>O: Handle-SmartRead(InputJson)
O-->>D: {result, duration}
alt SmartRead short-circuit
D->>D: skip tool execution
else Fallback
D->>O: Handle-ContextGuard(InputJson)
O-->>D: {guardStatus}
D->>O: Handle-PreToolUseOptimization(InputJson)
O-->>D: {optimizedContext}
end
end
rect rgb(255,248,240)
Note over D,O: Tool execution & post-processing
D->>D: execute tool (Git/MCP checks)
D->>O: Handle-OptimizeToolOutput(InputJson)
O-->>D: {optimizedOutput, tokens}
D->>O: Handle-LogOperation(InputJson)
O-->>D: {logStatus}
end
rect rgb(248,240,255)
Note over D,O: End-of-turn/session compacting
D->>O: Handle-UserPromptOptimization(InputJson)
O-->>D: {promptOptimized}
D->>O: Handle-PreCompactOptimization(InputJson)
O-->>D: {preCompactResult}
D->>O: Handle-SessionReport()
O-->>D: {report}
D->>O: Handle-OptimizeSession()
O-->>D: {sessionOptimized}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (12)
Comment |
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
1 similar comment
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
Performance Benchmark Results |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
hooks/dispatcher.ps1 (3)
11-13: Don’t force dev env var in production shells. Gate it.Unconditionally setting TOKEN_OPTIMIZER_DEV_PATH leaks into child processes. Set only if not already defined.
-# PERFORMANCE FIX: Use local dev path instead of npx to avoid 60-second downloads -$env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" +# PERFORMANCE FIX: Prefer local dev path if not already set +if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { + $env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" +}
75-86: SmartRead tool-name mismatch.Dispatcher matches Read and two mcp__filesystem__* tools, but Handle-SmartRead currently intercepts only "Read". Extend the handler to cover the others (see orchestrator comment with diff).
Confirm that mcp__filesystem__read_file and mcp__filesystem__read_text_file should be intercepted by SmartRead. If yes, apply the orchestrator diff in the related comment.
49-51: Set stdin encoding defensively to UTF‑8.Prevents mojibake with non‑ASCII payloads while still using ReadToEnd().
- $input_json = [Console]::In.ReadToEnd() + try { [Console]::InputEncoding = [System.Text.Encoding]::UTF8 } catch {} + $input_json = [Console]::In.ReadToEnd()hooks/handlers/token-optimizer-orchestrator.ps1 (7)
16-18: Don’t override TOKEN_OPTIMIZER_DEV_PATH globally. Gate it.-# PERFORMANCE FIX: Use local dev path instead of npx to avoid 60-second downloads -$env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" +# PERFORMANCE FIX: Prefer local dev path if not already set +if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { + $env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" +}
24-33: Harden Write-Log: avoid $Action dependency and ensure log directory exists.-function Write-Log { - param([string]$Message, [string]$Level = "INFO") +function Write-Log { + param([string]$Message, [string]$Level = "INFO", [string]$Tag = "orchestrator") $timestamp = Get-Date -Format "yyyy-MM-dd HH:mm:ss" - $logEntry = "[$timestamp] [$Level] [$Action] $Message" + $logEntry = "[$timestamp] [$Level] [$Tag] $Message" try { + $dir = Split-Path -Parent $LOG_FILE + if (-not (Test-Path $dir)) { New-Item -ItemType Directory -Force -Path $dir | Out-Null } $logEntry | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 -ErrorAction SilentlyContinue } catch { # Silently fail } }
49-70: Ensure session directory exists before writing.function Initialize-Session { # Create new session if needed if (-not (Test-Path $SESSION_FILE)) { + $sessionDir = Split-Path -Parent $SESSION_FILE + if (-not (Test-Path $sessionDir)) { New-Item -ItemType Directory -Force -Path $sessionDir | Out-Null }
133-141: Create operations directory before writing CSV.- # Create file with header if it doesn't exist - if (-not (Test-Path $csvFile)) { + # Ensure directory exists and create file with header if missing + $csvDir = Split-Path -Parent $csvFile + if (-not (Test-Path $csvDir)) { New-Item -ItemType Directory -Force -Path $csvDir | Out-Null } + if (-not (Test-Path $csvFile)) { "timestamp,toolName,tokens,metadata" | Out-File $csvFile -Encoding UTF8 }
1456-1549: Extend SmartRead to cover mcp filesystem tools and gate verbose logging.- # Only intercept Read operations - if ($toolName -ne "Read") { + # Intercept Read and filesystem read tools + if ($toolName -notin @("Read","mcp__filesystem__read_file","mcp__filesystem__read_text_file")) { return } @@ - $filePath = $data.tool_input.file_path + $filePath = $data.tool_input.file_path + if (-not $filePath) { $filePath = $data.tool_input.path } @@ - # PHASE 1 FIX: Add debug logging and null-safe metadata parsing - Write-Log "smart_read raw result: $($resultJson | Out-String)" "DEBUG" + # PHASE 1 FIX: Add debug logging (guarded) and null-safe metadata parsing + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1") { + Write-Log "smart_read raw result: $($resultJson | Out-String)" "DEBUG" + }
1393-1396: Null-safe compressedTokens logging.- Write-Log "After optimize_text: $($optimizeData.metadata.compressedTokens) tokens" "DEBUG" + $compressedTokens = if ($optimizeData.metadata -and $optimizeData.metadata.compressedTokens) { $optimizeData.metadata.compressedTokens } else { "unknown" } + Write-Log "After optimize_text: $compressedTokens tokens" "DEBUG"
1550-1609: Direct-run path is currently non-functional. Either remove it or wire args/stdin.If you still want a CLI mode without a param block, minimally parse $args and pass stdin:
if ($MyInvocation.InvocationName -ne '.') { try { - Write-Log "Phase: $Phase, Action: $Action" "INFO" + $Action = if ($args.Count -gt 0) { $args[0] } else { $env:ACTION } + $Phase = if ($args.Count -gt 1) { $args[1] } else { $env:PHASE } + $stdin = [Console]::In.ReadToEnd() + Write-Log "Phase: $Phase, Action: $Action" "INFO" @@ - "smart-read" { - Handle-SmartRead + "smart-read" { + Handle-SmartRead -InputJson $stdin } "context-guard" { - Handle-ContextGuard + Handle-ContextGuard -InputJson $stdin } @@ "log-operation" { - Handle-LogOperation + Handle-LogOperation -InputJson $stdin } @@ "optimize-tool-output" { - Handle-OptimizeToolOutput + Handle-OptimizeToolOutput -InputJson $stdin } "precompact-optimize" { - Handle-PreCompactOptimization + Handle-PreCompactOptimization -InputJson $stdin } "user-prompt-optimize" { - Handle-UserPromptOptimization + Handle-UserPromptOptimization -InputJson $stdin } "session-start-init" { Handle-SessionStartInit } "pretooluse-optimize" { - Handle-PreToolUseOptimization + Handle-PreToolUseOptimization -InputJson $stdin }If CLI mode isn’t required, consider deleting the whole block to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hooks/dispatcher.ps1(9 hunks)hooks/handlers/token-optimizer-orchestrator.ps1(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test (Node 18)
- GitHub Check: Test (Node 20)
🔇 Additional comments (1)
hooks/handlers/token-optimizer-orchestrator.ps1 (1)
5-9: Good fix: removing the param block avoids stdin being consumed on dot‑source.If you still need a direct-run mode, see the later comment to wire stdin/args without re‑adding a top‑level param block.
| # DEBUG: Log raw JSON to see what we're receiving | ||
| "[DEBUG] [$timestamp] RAW JSON: $($input_json.Substring(0, [Math]::Min(200, $input_json.Length)))" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 | ||
|
|
There was a problem hiding this comment.
Avoid logging raw JSON; risk of PII/secrets leakage.
Limit/guard with a runtime debug flag and use Write-Log consistently.
- "[DEBUG] [$timestamp] RAW JSON: $($input_json.Substring(0, [Math]::Min(200, $input_json.Length)))" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8
+ if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1") {
+ Write-Log "RAW JSON (first 200 chars): $($input_json.Substring(0, [Math]::Min(200, $input_json.Length)))" "DEBUG"
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In hooks/dispatcher.ps1 around lines 52 to 54, the script currently logs raw
JSON which risks leaking PII/secrets; replace this direct Out-File debug line
with a guarded debug path that checks a runtime debug flag (e.g., $Env:DEBUG or
a script-level $DebugMode) before writing, and use the existing Write-Log
function for consistency; when enabled, mask or truncate sensitive fields (or
only log a safe substring/summary) instead of dumping full raw JSON, and ensure
any logging uses UTF8 and the same log file handling as Write-Log.
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
1 similar comment
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
Performance Benchmark Results |
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
1 similar comment
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
Performance Benchmark Results |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hooks/dispatcher.ps1 (1)
20-24: Fix logging API mismatch: support log level parameter.Write-Log only accepts Message but most call sites pass a level. Add an optional Level parameter and include it in the format.
Apply:
-function Write-Log { - param([string]$Message) - $timestamp = Get-Date -Format "yyyy-MM-dd HH:mm:ss" - "[$timestamp] [$Phase] $Message" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 -} +function Write-Log { + param( + [string]$Message, + [ValidateSet('DEBUG','INFO','WARN','ERROR')][string]$Level = 'INFO' + ) + $timestamp = Get-Date -Format "yyyy-MM-dd HH:mm:ss" + "[$timestamp] [$Level] [$Phase] $Message" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 +}hooks/handlers/token-optimizer-orchestrator.ps1 (1)
26-35: Stop referencing $Action in logs; add Level and optional context.-function Write-Log { - param([string]$Message, [string]$Level = "INFO") +function Write-Log { + param( + [string]$Message, + [ValidateSet('DEBUG','INFO','WARN','ERROR')][string]$Level = "INFO", + [string]$Context = "" + ) $timestamp = Get-Date -Format "yyyy-MM-dd HH:mm:ss" - $logEntry = "[$timestamp] [$Level] [$Action] $Message" + $ctx = if ($Context) { " [$Context]" } elseif ($script:Phase) { " [$script:Phase]" } else { "" } + $logEntry = "[$timestamp] [$Level]$ctx $Message" try { $logEntry | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 -ErrorAction SilentlyContinue } catch { # Silently fail } }
♻️ Duplicate comments (4)
hooks/dispatcher.ps1 (3)
54-56: Avoid logging raw JSON; guard and truncate.Guard behind a runtime flag and use Write-Log; truncate to reduce PII risk.
- "[DEBUG] [$timestamp] RAW JSON: $($input_json.Substring(0, [Math]::Min(200, $input_json.Length)))" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1" -and $input_json) { + $snippet = $input_json.Substring(0, [Math]::Min(200, $input_json.Length)) + Write-Log "RAW JSON (first 200 chars): $snippet" "DEBUG" + }
112-131: Tighten Git enforcer: disallow remote ops; add URL/remote guard.Remove remote commands from allowed set and ensure no remote indicators slip through.
- $localGitOps = @( + $localGitOps = @( "git\s+status", "git\s+branch", "git\s+checkout", "git\s+worktree", "git\s+add", "git\s+commit", "git\s+diff", "git\s+log", "git\s+stash", - "git\s+pull", - "git\s+push", - "git\s+fetch", - "git\s+remote", "git\s+rev-parse", "git\s+config", "git\s+merge", "git\s+reset", "git\s+clean" ) @@ - if ($isLocalOp) { - Write-Log "[ALLOW] Local git operation: $($data.tool_input.command)" - exit 0 - } + if ($isLocalOp -and $data.tool_input.command -notmatch '(?i)(https?://|ssh://|git@|--upload-pack|--receive-pack|\borigin\b|\bpush\b|\bpull\b|\bfetch\b|\bremote\b)') { + Write-Log "[ALLOW] Local git operation: $($data.tool_input.command)" "INFO" + exit 0 + } @@ - Block-Tool -Reason "Use GitHub MCP (mcp__github__*) for GitHub operations (pr, issue, etc.)" + Block-Tool -Reason "Remote Git requires MCP (use mcp__github__*)."Also applies to: 141-148
160-165: Short‑circuit on PreToolUse cache hit.Capture return and exit 2 on cache hit.
- $startTime = Get-Date - Handle-PreToolUseOptimization -InputJson $input_json + $startTime = Get-Date + $preResult = Handle-PreToolUseOptimization -InputJson $input_json $duration = ((Get-Date) - $startTime).TotalMilliseconds Write-Log "[PERF] pretooluse-optimize took $([math]::Round($duration, 2))ms" "DEBUG" + if ($preResult -eq 2) { exit 2 }hooks/handlers/token-optimizer-orchestrator.ps1 (1)
1161-1173: Cache hit must block tool execution and return code 2.Emit standard block response and return 2 so dispatcher can exit 2.
- Write-Log "Cache HIT for $toolName - avoiding redundant tool call" "INFO" - # Output cached result so tool execution can be skipped - $result = @{ - cached = $true - toolName = $toolName - cachedOutput = $cachedData.content[0].text - } | ConvertTo-Json -Depth 10 -Compress - Write-Output $result - return + Write-Log "Cache HIT for $toolName - avoiding redundant tool call" "INFO" + $blockResponse = @{ + continue = $false + stopReason = "cache hit" + hookSpecificOutput = @{ + hookEventName = "PreToolUse" + cached = $true + toolName = $toolName + cachedOutput = $cachedData.content[0].text + } + } | ConvertTo-Json -Depth 10 -Compress + Write-Output $blockResponse + [Console]::Out.Flush(); [Console]::Error.Flush() + return 2
🧹 Nitpick comments (4)
hooks/dispatcher.ps1 (2)
11-19: Make paths portable and configurable.Prefer $PSScriptRoot and env fallback over hard‑coded user paths.
-# PERFORMANCE FIX: Prefer local dev path if not already set -if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { - $env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" -} +if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { + $env:TOKEN_OPTIMIZER_DEV_PATH = (Split-Path -Parent (Split-Path -Parent $PSScriptRoot)) +} +$HANDLERS_DIR = Join-Path $env:TOKEN_OPTIMIZER_DEV_PATH 'hooks\handlers' +$LOG_FILE = Join-Path $env:TOKEN_OPTIMIZER_DEV_PATH 'hooks\logs\dispatcher.log' +$ORCHESTRATOR = Join-Path $HANDLERS_DIR 'token-optimizer-orchestrator.ps1' -. "$HANDLERS_DIR\token-optimizer-orchestrator.ps1" +. $ORCHESTRATOR
46-49: Unify logging via Write-Log and guard noisy DEBUG lines.Replace direct Out-File debug lines with Write-Log behind TOKEN_OPTIMIZER_DEBUG.
- $timestamp = Get-Date -Format "yyyy-MM-dd HH:mm:ss" - "[DEBUG] [$timestamp] [$Phase] DISPATCHER INVOKED" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1") { Write-Log "DISPATCHER INVOKED" "DEBUG" } @@ - "[DEBUG] [$timestamp] TOOL NAME: $toolName" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1") { Write-Log "TOOL NAME: $toolName" "DEBUG" }Also applies to: 66-67
hooks/handlers/token-optimizer-orchestrator.ps1 (2)
10-20: Resolve hard‑coded paths; derive from dev root.Make helpers/log/data paths relative to $env:TOKEN_OPTIMIZER_DEV_PATH (or script root).
-$HELPERS_DIR = "C:\Users\cheat\.claude-global\hooks\helpers" -$INVOKE_MCP = "$HELPERS_DIR\invoke-mcp.ps1" -$LOG_FILE = "C:\Users\cheat\.claude-global\hooks\logs\token-optimizer-orchestrator.log" -$SESSION_FILE = "C:\Users\cheat\.claude-global\hooks\data\current-session.txt" -$OPERATIONS_DIR = "C:\Users\cheat\.claude-global\hooks\data" +if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { + $env:TOKEN_OPTIMIZER_DEV_PATH = (Split-Path -Parent (Split-Path -Parent $PSScriptRoot)) +} +$ROOT = Join-Path $env:TOKEN_OPTIMIZER_DEV_PATH 'hooks' +$HELPERS_DIR = Join-Path $ROOT 'helpers' +$INVOKE_MCP = Join-Path $HELPERS_DIR 'invoke-mcp.ps1' +$LOG_FILE = Join-Path $ROOT 'logs\token-optimizer-orchestrator.log' +$OPERATIONS_DIR= Join-Path $ROOT 'data' +$SESSION_FILE = Join-Path $OPERATIONS_DIR 'current-session.txt'
1556-1615: Direct-execution block uses $Phase/$Action no longer supplied. Consider disabling or parsing args.Either remove this block to avoid confusion in dot‑sourced mode or reintroduce non‑mandatory params and parse $Args safely. Avoid any construct that reads stdin implicitly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hooks/dispatcher.ps1(9 hunks)hooks/handlers/token-optimizer-orchestrator.ps1(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Integration Tests
- GitHub Check: Test (Node 20)
- GitHub Check: Test (Node 18)
🔇 Additional comments (3)
hooks/handlers/token-optimizer-orchestrator.ps1 (3)
5-9: Good fix: removing the Mandatory param block avoids stdin consumption when dot‑sourced.
441-467: Verify against MCP tool registry—code shows inconsistent naming conventions across tool invocations.The review comment is valid. Analysis of the file confirms three distinct naming patterns in use:
- Namespaced style (lines 169–328):
mcp__token-optimizer__optimize_session,mcp__token-optimizer__cache_warmup, etc.- Simple underscores (lines 441, 449, 463):
cache_warmup,get_cache_stats,health_monitor- Hyphens (lines 991, 1038, 1081, 1119):
smart-summarization,pattern-recognition,predictive-analytics,intelligent-assistantThe three tools at lines 441–467 (cache_warmup, get_cache_stats, health_monitor) use a simpler naming pattern than the namespaced mcp__token-optimizer__* tools elsewhere, which raises a legitimate consistency concern.
Verify these tool IDs against your actual MCP service tool registry to ensure all invocations will resolve correctly at runtime.
1249-1318: Based on the evidence gathered from multiple successful searches, I've confirmed a genuine schema inconsistency in the codebase. However, I cannot complete the full verification due to file access constraints in the current environment.Schema field name inconsistency:
tool_resultvstool_responseThe evidence shows conflicting field names in the same data structure:
- Line 1247 (Handle-OptimizeToolOutput): reads
$data.tool_result- Line 116 (Handle-LogOperation): reads
$data.tool_response.file.content- read-cache-interceptor.ps1 (lines 230, 233): reads
$data.tool_result.contentSince both
Handle-OptimizeToolOutputandHandle-LogOperationreceive identical$input_jsonfrom dispatcher.ps1, only one field name can be correct. The consensus (3:1 ratio) suggests.tool_resultis correct and line 116 may be incorrect, but the full function context could not be verified to confirm.
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
1 similar comment
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
Performance Benchmark Results |
ooples
force-pushed
the
fix/hook-stdin-param-block
branch
from
f831cdf to
65e5c82
Compare
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
1 similar comment
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
Performance Benchmark Results |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
hooks/dispatcher.ps1 (3)
49-51: Guard diagnostics; don’t dump raw JSON. Use Write-Log.Replace Out-File debug lines with a TOKEN_OPTIMIZER_DEBUG gate and Write-Log; truncate content. This avoids PII leaks and keeps logging consistent.
- "[DEBUG] [$timestamp] [$Phase] DISPATCHER INVOKED" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1") { Write-Log "DISPATCHER INVOKED" "DEBUG" } - "[DEBUG] [$timestamp] RAW JSON: $($input_json.Substring(0, [Math]::Min(200, $input_json.Length)))" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1" -and $input_json) { + $snippet = $input_json.Substring(0, [Math]::Min(200, $input_json.Length)) + Write-Log "RAW JSON (first 200 chars): $snippet" "DEBUG" + } - "[DEBUG] [$timestamp] TOOL NAME: $toolName" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1") { Write-Log "TOOL NAME: $toolName" "DEBUG" }Also applies to: 58-60, 70-70
113-152: Git enforcer still permits remote ops; tighten allowlist and add safety check.Remove pull/push/fetch/remote from local ops and ensure the “allow” branch does not include any remote indicators.
$localGitOps = @( "git\s+status", "git\s+branch", "git\s+checkout", "git\s+worktree", "git\s+add", "git\s+commit", "git\s+diff", "git\s+log", "git\s+stash", - "git\s+pull", - "git\s+push", - "git\s+fetch", - "git\s+remote", "git\s+rev-parse", "git\s+config", "git\s+merge", "git\s+reset", "git\s+clean" ) @@ - if ($isLocalOp) { + if ($isLocalOp -and + $data.tool_input.command -notmatch '(?i)\b(fetch|pull|push|remote)\b' -and + $data.tool_input.command -notmatch '(?i)(https?://|ssh://|git@|\sorigin\b|\supstream\b)') { Write-Log "[ALLOW] Local git operation: $($data.tool_input.command)" exit 0 }
164-169: PreToolUse cache-hit can’t short‑circuit; capture return and exit 2.Check the handler’s return code and exit 2 on hit so the tool doesn’t run.
$startTime = Get-Date - Handle-PreToolUseOptimization -InputJson $input_json + $preResult = Handle-PreToolUseOptimization -InputJson $input_json $duration = ((Get-Date) - $startTime).TotalMilliseconds Write-Log "[PERF] pretooluse-optimize took $([math]::Round($duration, 2))ms" "DEBUG" + if ($preResult -eq 2) { exit 2 }hooks/handlers/token-optimizer-orchestrator.ps1 (2)
1155-1164: Cache hit must block and return 2; make returns explicit and define $cacheKey once.Emit standard block response and return 2 on hit; return 0 otherwise; define $cacheKey before the try.
- Write-Log "PreToolUse optimization for: $toolName" "DEBUG" + Write-Log "PreToolUse optimization for: $toolName" "DEBUG" + # Define cache key once (avoid undefined on error paths) + $cacheKey = "$toolName-$($toolArgs | ConvertTo-Json -Compress -Depth 5)" # Step 1: Check predictive cache for this tool call - try { - $cacheKey = "$toolName-$($toolArgs | ConvertTo-Json -Compress -Depth 5)" + try { $getCachedArgs = @{ key = $cacheKey } @@ - # Output cached result so tool execution can be skipped - $result = @{ - cached = $true - toolName = $toolName - cachedOutput = $cachedData.content[0].text - } | ConvertTo-Json -Depth 10 -Compress - Write-Output $result - return + # Block tool execution with standard response; dispatcher will exit 2 + $blockResponse = @{ + continue = $false + stopReason = "cache hit" + hookSpecificOutput = @{ + hookEventName = "PreToolUse" + cached = $true + toolName = $toolName + cachedOutput = $cachedData.content[0].text + } + } | ConvertTo-Json -Depth 10 -Compress + Write-Output $blockResponse + [Console]::Out.Flush(); [Console]::Error.Flush() + return 2 } } } catch { # Cache miss is normal, continue Write-Log "Cache miss for $toolName" "DEBUG" } @@ - } catch { - Write-Log "PreToolUse optimization failed: $($_.Exception.Message)" "ERROR" - } + return 0 + + } catch { + Write-Log "PreToolUse optimization failed: $($_.Exception.Message)" "ERROR" + return 0 + }Also applies to: 1169-1177, 1235-1238
1514-1514: Don’t log entire smart_read payload; gate and truncate.Guard behind TOKEN_OPTIMIZER_DEBUG and limit snippet length.
- Write-Log "smart_read raw result: $($resultJson | Out-String)" "DEBUG" + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1" -and $resultJson) { + $raw = [string]$resultJson + $snippet = $raw.Substring(0, [Math]::Min(500, $raw.Length)) + Write-Log "smart_read result (first 500 chars): $snippet" "DEBUG" + }
🧹 Nitpick comments (3)
hooks/dispatcher.ps1 (1)
11-14: De-hardcode paths; prefer env-based base dir.Recommend using a base env var (e.g., TOKEN_OPTIMIZER_HOME or CLAUDE_GLOBAL_HOME) with sensible defaults to avoid user-specific absolute paths.
-# PERFORMANCE FIX: Prefer local dev path if not already set -if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { - $env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" -} +# PERFORMANCE FIX: Prefer local dev path if not already set +if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { + $env:TOKEN_OPTIMIZER_DEV_PATH = Join-Path $env:USERPROFILE 'source\repos\token-optimizer-mcp' +} +# TODO (optional): derive HANDLERS_DIR/LOG_FILE from $env:CLAUDE_GLOBAL_HOME or $env:TOKEN_OPTIMIZER_HOME with $env:USERPROFILE fallback.hooks/handlers/token-optimizer-orchestrator.ps1 (2)
1561-1620: Direct-run block uses $Phase/$Action without params; prefer a small runner script.Since the param block was removed to avoid stdin binding when dot-sourced, direct execution won’t receive $Phase/$Action. Move CLI usage into a separate runner (keeps this file pure functions).
Example runner (new file):
# hooks/handlers/token-optimizer-run.ps1 param([string]$Phase, [string]$Action, [string]$InputJson = $null) . "$PSScriptRoot\token-optimizer-orchestrator.ps1" Write-Log "Phase: $Phase, Action: $Action" "INFO" switch ($Action) { "smart-read" { Handle-SmartRead -InputJson $InputJson } "context-guard" { Handle-ContextGuard -InputJson $InputJson } "periodic-optimize" { Handle-PeriodicOptimize } "cache-warmup" { Handle-CacheWarmup } "session-report" { Handle-SessionReport } "log-operation" { Handle-LogOperation -InputJson $InputJson } "optimize-session" { Handle-OptimizeSession } "optimize-tool-output" { Handle-OptimizeToolOutput -InputJson $InputJson } "precompact-optimize" { Handle-PreCompactOptimization -InputJson $InputJson } "user-prompt-optimize" { Handle-UserPromptOptimization -InputJson $InputJson } "session-start-init" { Handle-SessionStartInit } "pretooluse-optimize" { Handle-PreToolUseOptimization -InputJson $InputJson } default { Write-Log "Unknown action: $Action" "WARN" } } exit 0
16-20: De-hardcode paths; centralize base dir.Use a single base dir env var with $USERPROFILE fallback to avoid user-specific paths.
-# PERFORMANCE FIX: Prefer local dev path if not already set -if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { - $env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" -} +# PERFORMANCE FIX: Prefer local dev path if not already set +if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { + $env:TOKEN_OPTIMIZER_DEV_PATH = Join-Path $env:USERPROFILE 'source\repos\token-optimizer-mcp' +} +# Optional: set TOKEN_OPTIMIZER_HOME / CLAUDE_GLOBAL_HOME and derive: +# $BASE = $env:TOKEN_OPTIMIZER_HOME ?? Join-Path $env:USERPROFILE '.claude-global' +# $HELPERS_DIR, $LOG_FILE, $SESSION_FILE, $OPERATIONS_DIR from $BASE
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hooks/dispatcher.ps1(9 hunks)hooks/handlers/token-optimizer-orchestrator.ps1(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test (Node 20)
- GitHub Check: Test (Node 18)
| # Use smart-summarization tool | ||
| try { | ||
| $summarizeArgs = @{ | ||
| operation = "summarize" | ||
| query = $Text | ||
| data = @{ | ||
| context = $Context | ||
| maxLength = 200 | ||
| } | ||
| } | ||
| $summarizeJson = $summarizeArgs | ConvertTo-Json -Compress -Depth 5 | ||
| $summarizeResult = & "$HELPERS_DIR\invoke-mcp.ps1" -Tool "smart-summarization" -ArgumentsJson $summarizeJson | ||
|
|
||
| if ($summarizeResult) { | ||
| $summarizeData = $summarizeResult | ConvertFrom-Json | ||
| if ($summarizeData -and $summarizeData.content) { | ||
| $summarized = $summarizeData.content[0].text | ||
| Write-Log "Summarized from $($Text.Length) to $($summarized.Length) chars" "INFO" | ||
| return $summarized | ||
| } | ||
| } | ||
| } catch { |
There was a problem hiding this comment.
🧩 Analysis chain
Verify MCP tool names (hyphen vs underscore).
Tools like smart-summarization, pattern-recognition, predictive-analytics use hyphens, while others use underscores (smart_read, count_tokens). Confirm actual tool IDs to avoid call failures.
Also applies to: 1032-1055, 1077-1095, 1287-1294
🏁 Script executed:
#!/bin/bash
# List all invoked tool names and spot inconsistencies
rg -nP --type=powershell -o '\-Tool\s+"([^"]+)"' -C1 | sed -E 's/.*-Tool\s+"([^"]+)".*/\1/' | sort | uniq -c | sort -nr
# If you have an enumerate endpoint, also grep for it and compare.Length of output: 34
🏁 Script executed:
#!/bin/bash
# Search for all -Tool parameter invocations in PowerShell files
fd -e ps1 -x rg -n '\-Tool\s+"([^"]+)"' '{}' | grep -oP '(?<=-Tool\s+")[^"]+' | sort | uniq -c | sort -nrLength of output: 46
🏁 Script executed:
#!/bin/bash
# Also directly check the specific file mentioned
cat hooks/handlers/token-optimizer-orchestrator.ps1 | grep -n '\-Tool' | head -20Length of output: 2271
🏁 Script executed:
#!/bin/bash
# Extract tool names from the flagged lines (985-1006, 1032-1055, 1077-1095, 1287-1294)
sed -n '985,1006p;1032,1055p;1077,1095p;1287,1294p' hooks/handlers/token-optimizer-orchestrator.ps1 | grep -oP '(?<=-Tool\s+")[^"]+'Length of output: 46
🏁 Script executed:
#!/bin/bash
# Extract tool names from flagged lines (simplified approach without lookbehind)
echo "=== Lines 985-1006 ===" && sed -n '985,1006p' hooks/handlers/token-optimizer-orchestrator.ps1 | grep '\-Tool'
echo "=== Lines 1032-1055 ===" && sed -n '1032,1055p' hooks/handlers/token-optimizer-orchestrator.ps1 | grep '\-Tool'
echo "=== Lines 1077-1095 ===" && sed -n '1077,1095p' hooks/handlers/token-optimizer-orchestrator.ps1 | grep '\-Tool'
echo "=== Lines 1287-1294 ===" && sed -n '1287,1294p' hooks/handlers/token-optimizer-orchestrator.ps1 | grep '\-Tool'Length of output: 559
🏁 Script executed:
#!/bin/bash
# Extract unique tool names from entire file (simpler)
grep '\-Tool "' hooks/handlers/token-optimizer-orchestrator.ps1 | sed 's/.*-Tool "//' | sed 's/".*//' | sort | uniqLength of output: 543
Verify and correct MCP tool name inconsistencies.
The codebase mixes naming conventions: lines 985, 1032, and 1077 use hyphens (smart-summarization, pattern-recognition, predictive-analytics), while the rest of the file predominantly uses underscores (count_tokens, predictive_cache, optimize_text, etc.). This inconsistency risks runtime failures if tool IDs don't match the MCP registry exactly. Confirm the authoritative tool names and update all invocations to use consistent naming.
Lines affected: 985–1006, 1032–1055, 1077–1095 (and verify remaining ~30 tool invocations throughout the file).
ooples
force-pushed
the
fix/hook-stdin-param-block
branch
from
65e5c82 to
96416ad
Compare
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
1 similar comment
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
Performance Benchmark Results |
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)
hooks/handlers/token-optimizer-orchestrator.ps1 (1)
200-219: ContextGuard should emit a block JSON when returning 2.Returning 2 without a standard block payload can leave the dispatcher without context on why it was blocked. Mirror the SmartRead pattern.
- if ($result) { + if ($result) { Write-Log "Emergency optimization completed" "INFO" $session.lastOptimized = $session.totalOperations $session | ConvertTo-Json | Out-File $SESSION_FILE -Encoding UTF8 } else { - # BLOCK operation if optimization failed - return 2 to signal blocking - return 2 + # BLOCK with standard response so dispatcher exits with 2 + $blockResponse = @{ + continue = $false + stopReason = "context guard block" + hookSpecificOutput = @{ + hookEventName = "PreToolUse" + reason = "optimize_session failed at FORCE_THRESHOLD" + sessionId = $session.sessionId + usagePercent = [Math]::Round($percentage * 100, 1) + } + } | ConvertTo-Json -Depth 10 -Compress + Write-Output $blockResponse + [Console]::Out.Flush(); [Console]::Error.Flush() + return 2 }
♻️ Duplicate comments (4)
hooks/handlers/token-optimizer-orchestrator.ps1 (4)
1235-1238: Return explicit status codes from PreToolUse handler (success=0, failure≠0).Make result handling deterministic for the dispatcher.
} catch { Write-Log "PreToolUse optimization failed: $($_.Exception.Message)" "ERROR" + return 0 } + return 0
970-1016: Verify MCP tool IDs (hyphen vs underscore); normalize names.You call smart-summarization, pattern-recognition, predictive-analytics, intelligent-assistant (hyphens) while most other tools use underscores. Mismatches will 404 at runtime.
#!/bin/bash # List unique tool IDs invoked across the repo and spot inconsistencies. rg -nP --type=powershell '\-Tool\s+"([^"]+)"' -o | sed -E 's/.*-Tool\s+"([^"]+)".*/\1/' | sort | uniq -c | sort -nrIf hyphenated names are aliases, consider centralizing conversions inside invoke-mcp.ps1 to avoid call-site drift.
Also applies to: 1018-1062, 1064-1106, 1107-1140
1169-1178: Cache hit must block downstream tool and return status 2 (standard block JSON).Right now you emit a custom shape and just return, so the dispatcher may continue. Emit the standardized block payload and return 2, same as SmartRead.
- # Output cached result so tool execution can be skipped - $result = @{ - cached = $true - toolName = $toolName - cachedOutput = $cachedData.content[0].text - } | ConvertTo-Json -Depth 10 -Compress - Write-Output $result - return + # Return standard block response so dispatcher can exit 2 + $blockResponse = @{ + continue = $false + stopReason = "cache hit" + hookSpecificOutput = @{ + hookEventName = "PreToolUse" + cached = $true + toolName = $toolName + cachedOutput = $cachedData.content[0].text + } + } | ConvertTo-Json -Depth 10 -Compress + Write-Output $blockResponse + [Console]::Out.Flush(); [Console]::Error.Flush() + return 2
1514-1520: Don’t log full smart_read payloads; guard and truncate.Full dumps can leak PII and large contents to logs. Log minimal, truncated snippets behind an explicit debug env flag.
- Write-Log "smart_read raw result: $($resultJson | Out-String)" "DEBUG" + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1" -and $resultJson) { + $raw = [string]$resultJson + $snippet = $raw.Substring(0, [Math]::Min(500, $raw.Length)) + Write-Log "smart_read result (first 500 chars): $snippet" "DEBUG" + }
🧹 Nitpick comments (3)
hooks/handlers/token-optimizer-orchestrator.ps1 (3)
16-20: Env var introduced but unused; adopt it to derive all paths.Currently TOKEN_OPTIMIZER_DEV_PATH is set but not used. Use a single $BASE_DIR and Join-Path for portability and easier overrides.
Apply this minimal change here:
-# PERFORMANCE FIX: Prefer local dev path if not already set -if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { - $env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" -} +$BASE_DIR = if ($env:TOKEN_OPTIMIZER_DEV_PATH) { $env:TOKEN_OPTIMIZER_DEV_PATH } else { (Split-Path -Parent $PSScriptRoot) }Then update the path declarations (outside this hunk) to use $BASE_DIR:
# Replace these absolute paths: # $HELPERS_DIR = "C:\Users\cheat\.claude-global\hooks\helpers" # $LOG_FILE = "C:\Users\cheat\.claude-global\hooks\logs\token-optimizer-orchestrator.log" # $SESSION_FILE = "C:\Users\cheat\.claude-global\hooks\data\current-session.txt" # $OPERATIONS_DIR = "C:\Users\cheat\.claude-global\hooks\data" $HELPERS_DIR = Join-Path $BASE_DIR 'helpers' $LOG_FILE = Join-Path $BASE_DIR 'logs\token-optimizer-orchestrator.log' $SESSION_FILE = Join-Path $BASE_DIR 'data\current-session.txt' $OPERATIONS_DIR= Join-Path $BASE_DIR 'data'
93-155: Ensure operations directory exists; avoid leaking full file paths in CSV.Create the directory before writes and consider logging only file names, not full paths, to reduce PII exposure.
# Create CSV file path + if (-not (Test-Path $OPERATIONS_DIR)) { + New-Item -ItemType Directory -Path $OPERATIONS_DIR -Force | Out-Null + } $csvFile = "$OPERATIONS_DIR\operations-$($session.sessionId).csv" @@ - $filePath = $data.tool_input.file_path + $filePath = $data.tool_input.file_path @@ - $csvLine = "$timestamp,$toolName,$tokens,`"$metadata`"" + $csvLine = "$timestamp,$toolName,$tokens,`"$metadata`""Optional: replace metadata with [System.IO.Path]::GetFileName($filePath).
Also applies to: 110-148
1561-1620: Direct-run block references $Phase/$Action without parsing; either parse $args or drop the block.Without a param block, $Phase/$Action will be $null when run directly. If direct execution is needed, parse $args; otherwise, remove to avoid confusion.
-if ($MyInvocation.InvocationName -ne '.') { +if ($MyInvocation.InvocationName -ne '.') { try { - Write-Log "Phase: $Phase, Action: $Action" "INFO" + if (-not $Action -and $args.Count -ge 1) { $Action = $args[0] } + Write-Log "Phase: $Phase, Action: $Action" "INFO"Or delete this whole block and rely solely on the dispatcher.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hooks/dispatcher.ps1(9 hunks)hooks/handlers/token-optimizer-orchestrator.ps1(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/dispatcher.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test (Node 20)
- GitHub Check: Test (Node 18)
🔇 Additional comments (2)
hooks/handlers/token-optimizer-orchestrator.ps1 (2)
5-8: Good fix: removed param block to stop stdin consumption when dot-sourced.This directly addresses the root cause; dot‑sourcing no longer slurps JSON from stdin. Nicely done.
1240-1327: PostToolUse: $optimizedText is computed but unused—confirm design intent.The dispatcher does not capture or consume output from PostToolUse handlers (dispatcher.ps1:182). All PostToolUse operations are side-effect only: Handle-OptimizeToolOutput updates session tokens, Handle-LogOperation logs operations, and the phase exits with no output processing (line 200). Within Handle-OptimizeToolOutput, $optimizedText is computed (line 1308) but never returned, emitted, or cached—it appears to be dead code.
Confirm whether:
- $optimizedText should be stored in session cache and retrieved later, or
- This computation is unnecessary and should be removed for clarity.
…rced critical bug fix: hooks were failing to receive json input from claude code root cause: - token-optimizer-orchestrator.ps1 had param block with mandatory=$true parameters - when dot-sourced by dispatcher.ps1, powershell consumed stdin for parameter binding - this prevented dispatcher from reading json payload via [console]::in.readtoend() solution: - removed entire param() block from orchestrator - not needed since it's dot-sourced - dispatcher already has its own param block for -phase parameter - orchestrator functions receive data via function parameters, not script params impact: - fixes smart_read, optimize-tool-output, and all other hook actions - restores normal hook functionality across all phases testing: - verified hooks receive json correctly - confirmed no parameter binding interference - tested pretooluse and posttooluse phases 🤖 generated with [claude code](https://claude.com/claude-code) co-authored-by: claude <noreply@anthropic.com>
changes made per coderabbit nitpick suggestions: 1. dispatcher.ps1:49-51 - set stdin encoding to utf-8 before reading 2. dispatcher.ps1:20-24 - fix write-log function to accept level parameter 3. orchestrator.ps1:26-35 - fix write-log to not reference undefined $action variable all changes are defensive improvements to prevent edge cases: - utf-8 encoding prevents mojibake with non-ascii payloads - write-log api now matches all call sites correctly - removed reference to undefined variable 🤖 generated with [claude code](https://claude.com/claude-code) co-authored-by: claude <noreply@anthropic.com>
ooples
force-pushed
the
fix/hook-stdin-param-block
branch
from
96416ad to
c8d2061
Compare
Performance Benchmark Results |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/handlers/token-optimizer-orchestrator.ps1 (1)
118-139: Operation logging may expose file paths/PII; guard and minimize.Default to coarse metadata; only log file names or hashed paths under debug flag.
- if ($toolName -eq "Read") { + if ($toolName -eq "Read") { $filePath = $data.tool_input.file_path @@ - $metadata = "filePath=$filePath" + $fileName = if ($filePath) { Split-Path -Leaf $filePath } else { "" } + $metadata = "fileName=$fileName" } elseif ($toolName -in @("Write", "Edit")) { $filePath = $data.tool_input.file_path @@ - $metadata = "filePath=$filePath" + $fileName = if ($filePath) { Split-Path -Leaf $filePath } else { "" } + $metadata = "fileName=$fileName" } else { # For other tools, log basic info $metadata = "toolName=$toolName" } @@ - $csvLine | Out-File $csvFile -Append -Encoding UTF8 - Write-Log "Logged operation: $toolName ($tokens tokens)" "DEBUG" + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1") { + $csvLine | Out-File $csvFile -Append -Encoding UTF8 + Write-Log "Logged operation: $toolName ($tokens tokens, $metadata)" "DEBUG" + } else { + "$timestamp,$toolName,$tokens,""redacted""" | Out-File $csvFile -Append -Encoding UTF8 + Write-Log "Logged operation: $toolName ($tokens tokens)" "DEBUG" + }Also applies to: 141-149
♻️ Duplicate comments (8)
hooks/handlers/token-optimizer-orchestrator.ps1 (5)
970-1016: Hyphen vs underscore tool IDs (“smart-summarization”, etc.). Verify actual registry names.#!/bin/bash # Highlight hyphenated tool names; confirm they exist in your MCP registry. rg -nP --type=powershell '\-Tool\s+"[^"]*-[^"]*"' hooks/handlers/token-optimizer-orchestrator.ps1 -n -C2
1018-1062: Same as above for pattern‑recognition, predictive‑analytics, intelligent‑assistant.Also applies to: 1064-1105, 1107-1139
1166-1177: Cache hit must block tool execution and return status 2; also return explicit status codes.Emit standard block response and return 2 on hit; return 0 on pass‑through; non‑zero on error.
- if ($cachedResult) { + if ($cachedResult) { $cachedData = $cachedResult | ConvertFrom-Json if ($cachedData -and $cachedData.content) { Write-Log "Cache HIT for $toolName - avoiding redundant tool call" "INFO" - # Output cached result so tool execution can be skipped - $result = @{ - cached = $true - toolName = $toolName - cachedOutput = $cachedData.content[0].text - } | ConvertTo-Json -Depth 10 -Compress - Write-Output $result - return + $blockResponse = @{ + continue = $false + stopReason = "cache hit" + hookSpecificOutput = @{ + hookEventName = "PreToolUse" + cached = $true + toolName = $toolName + cachedOutput = $cachedData.content[0].text + } + } | ConvertTo-Json -Depth 10 -Compress + Write-Output $blockResponse + [Console]::Out.Flush(); [Console]::Error.Flush() + return 2 } } @@ - } catch { - Write-Log "PreToolUse optimization failed: $($_.Exception.Message)" "ERROR" - } + } catch { + Write-Log "PreToolUse optimization failed: $($_.Exception.Message)" "ERROR" + return 1 + } + return 0Also applies to: 1235-1238
1401-1403: Null‑guard compressedTokens to avoid NRE.- Write-Log "After optimize_text: $($optimizeData.metadata.compressedTokens) tokens" "DEBUG" + $ct = if ($optimizeData.metadata -and $optimizeData.metadata.compressedTokens) { $optimizeData.metadata.compressedTokens } else { "unknown" } + Write-Log "After optimize_text: $ct tokens" "DEBUG"
1514-1519: Don’t dump full smart_read results to logs; guard and truncate.- Write-Log "smart_read raw result: $($resultJson | Out-String)" "DEBUG" + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1" -and $resultJson) { + $raw = [string]$resultJson + $snippet = $raw.Substring(0, [Math]::Min(500, $raw.Length)) + Write-Log "smart_read result (first 500 chars): $snippet" "DEBUG" + }hooks/dispatcher.ps1 (3)
58-60: Avoid logging raw JSON; risk of PII/secrets leakage.Gate behind env flag and truncate. Use Write-Log.
- "[DEBUG] [$timestamp] RAW JSON: $($input_json.Substring(0, [Math]::Min(200, $input_json.Length)))" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 + if ($env:TOKEN_OPTIMIZER_DEBUG -eq "1" -and $input_json) { + $snippet = $input_json.Substring(0, [Math]::Min(200, $input_json.Length)) + Write-Log "RAW JSON (first 200 chars): $snippet" "DEBUG" + }
113-152: Git enforcer currently allows remote ops; tighten to local-only.Remove pull/push/fetch/remote from allow‑list and add URL/remote safety check.
- $localGitOps = @( + $localGitOps = @( "git\s+status", "git\s+branch", "git\s+checkout", "git\s+worktree", "git\s+add", "git\s+commit", "git\s+diff", "git\s+log", "git\s+stash", - "git\s+pull", - "git\s+push", - "git\s+fetch", - "git\s+remote", "git\s+rev-parse", "git\s+config", "git\s+merge", "git\s+reset", "git\s+clean" ) @@ - if ($isLocalOp) { + if ($isLocalOp -and $data.tool_input.command -notmatch '\b(--upload-pack|--receive-pack)\b' -and + $data.tool_input.command -notmatch '(https?://|ssh://|git@|origin\s|upstream\s)') { Write-Log "[ALLOW] Local git operation: $($data.tool_input.command)" exit 0 }
164-169: Capture PreToolUseOptimization result and exit on cache hit.Currently ignored; cache hits won’t short‑circuit.
- $startTime = Get-Date - Handle-PreToolUseOptimization -InputJson $input_json + $startTime = Get-Date + $preResult = Handle-PreToolUseOptimization -InputJson $input_json $duration = ((Get-Date) - $startTime).TotalMilliseconds Write-Log "[PERF] pretooluse-optimize took $([math]::Round($duration, 2))ms" "DEBUG" + if ($preResult -eq 2) { exit 2 }
🧹 Nitpick comments (5)
hooks/dispatcher.ps1 (3)
11-14: Avoid hardcoded dev path; derive from environment/user profile.Use USERPROFILE or script root; don’t bake “cheat” path.
-if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { - $env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" -} +if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { + $home = $env:USERPROFILE + if (-not $home) { $home = (Get-Item "~").FullName } + $env:TOKEN_OPTIMIZER_DEV_PATH = (Join-Path $home "source\repos\token-optimizer-mcp") +}
49-52: Use Write-Log instead of ad‑hoc Out‑File for dispatcher “invoked” message.Keeps format consistent and centralizes logging.
- $timestamp = Get-Date -Format "yyyy-MM-dd HH:mm:ss" - "[DEBUG] [$timestamp] [$Phase] DISPATCHER INVOKED" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 + Write-Log "DISPATCHER INVOKED" "DEBUG"
69-71: Unify tool-name debug logging via Write-Log; drop duplicate Out‑File.- Write-Log "Tool: $toolName" - "[DEBUG] [$timestamp] TOOL NAME: $toolName" | Out-File -FilePath $LOG_FILE -Append -Encoding UTF8 + Write-Log "Tool: $toolName" "DEBUG"hooks/handlers/token-optimizer-orchestrator.ps1 (2)
16-20: Avoid hardcoded dev path; derive from USERPROFILE.-if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { - $env:TOKEN_OPTIMIZER_DEV_PATH = "C:\Users\cheat\source\repos\token-optimizer-mcp" -} +if (-not $env:TOKEN_OPTIMIZER_DEV_PATH) { + $home = $env:USERPROFILE; if (-not $home) { $home = (Get-Item "~").FullName } + $env:TOKEN_OPTIMIZER_DEV_PATH = (Join-Path $home "source\repos\token-optimizer-mcp") +}
1561-1620: Direct‑run block lacks inputs; consider env fallbacks to keep it usable.Optional: read $env:HOOK_PHASE/$env:HOOK_ACTION and pipe stdin to handlers when invoked directly.
-if ($MyInvocation.InvocationName -ne '.') { +if ($MyInvocation.InvocationName -ne '.') { try { - Write-Log "Phase: $Phase, Action: $Action" "INFO" + if (-not $Phase) { $Phase = $env:HOOK_PHASE } + if (-not $Action) { $Action = $env:HOOK_ACTION } + Write-Log "Phase: $Phase, Action: $Action" "INFO" + $stdin = "" + try { [Console]::InputEncoding = [Text.Encoding]::UTF8 } catch {} + if ($Host.Name -ne "ServerRemoteHost") { $stdin = [Console]::In.ReadToEnd() } @@ - "smart-read" { - Handle-SmartRead + "smart-read" { + Handle-SmartRead -InputJson $stdin } @@ - "optimize-tool-output" { - Handle-OptimizeToolOutput + "optimize-tool-output" { + Handle-OptimizeToolOutput -InputJson $stdin } @@ - "precompact-optimize" { - Handle-PreCompactOptimization + "precompact-optimize" { + Handle-PreCompactOptimization -InputJson $stdin } @@ - "user-prompt-optimize" { - Handle-UserPromptOptimization + "user-prompt-optimize" { + Handle-UserPromptOptimization -InputJson $stdin }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hooks/dispatcher.ps1(9 hunks)hooks/handlers/token-optimizer-orchestrator.ps1(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test (Node 18)
- GitHub Check: Test (Node 20)
🔇 Additional comments (3)
hooks/dispatcher.ps1 (2)
16-19: Dot‑sourcing orchestrator to keep a single PS process is solid.
83-90: SmartRead short‑circuit path looks correct.Captures return, times it, exits 2 on cache hit.
hooks/handlers/token-optimizer-orchestrator.ps1 (1)
5-9: Good fix: removed param block to stop stdin binding when dot‑sourced.
| # NOTE: pretooluse-optimize is DISABLED due to PowerShell/Node.js stdin EOF handling bug | ||
| # causing 60+ second hangs on every MCP call. Will re-enable after fixing invoke-mcp.ps1 | ||
| # $input_json | & powershell -NoProfile -ExecutionPolicy Bypass -File $ORCHESTRATOR -Phase "PreToolUse" -Action "pretooluse-optimize" | ||
|
|
There was a problem hiding this comment.
Comment says PreToolUseOptimization is disabled, but it’s invoked later.
Clarify intent: either remove the comment or actually disable the call below.
-# NOTE: pretooluse-optimize is DISABLED due to PowerShell/Node.js stdin EOF handling bug
-# causing 60+ second hangs on every MCP call. Will re-enable after fixing invoke-mcp.ps1
-# $input_json | & powershell -NoProfile -ExecutionPolicy Bypass -File $ORCHESTRATOR -Phase "PreToolUse" -Action "pretooluse-optimize"
+# NOTE: Direct process spawn path is disabled; dot-sourced function remains enabled.🤖 Prompt for AI Agents
In hooks/dispatcher.ps1 around lines 107 to 110, the comment states
pretooluse-optimize is disabled due to a PowerShell/Node stdin EOF bug but the
PreToolUseOptimization invocation later in the file is still active; either
remove the misleading comment or actually disable the downstream call. Update
the file so that intent matches behavior: if you want it disabled, comment out
or remove the PreToolUse invocation (and leave the explanatory note), or if you
want it enabled, remove the disabling note and ensure the invocation remains;
keep the comment and code consistent and include a brief note referencing the
invoke-mcp.ps1 bug if you keep it disabled.
| } | ||
| $warmupJson = $warmupArgs | ConvertTo-Json -Compress | ||
| & "$HELPERS_DIR\invoke-mcp.ps1" -Tool "cache_warmup" -ArgumentsJson $warmupJson | ||
| Write-Log "Cache warmup scheduled" "INFO" | ||
| } catch { | ||
| Write-Log "Cache warmup failed: $($_.Exception.Message)" "WARN" | ||
| } | ||
|
|
||
| # Get cache stats | ||
| try { | ||
| $statsResult = & "$HELPERS_DIR\invoke-mcp.ps1" -Tool "get_cache_stats" -ArgumentsJson "{}" | ||
| $statsData = if ($statsResult) { $statsResult | ConvertFrom-Json } else { $null } | ||
| if ($statsData -and $statsData.content) { | ||
| $stats = $statsData.content[0].text | ConvertFrom-Json | ||
| Write-Log "Cache stats: Hit rate: $($stats.hitRate), Entries: $($stats.entries)" "INFO" | ||
| } | ||
| } catch { | ||
| Write-Log "Failed to get cache stats: $($_.Exception.Message)" "WARN" | ||
| } | ||
|
|
||
| # Health monitor check | ||
| try { | ||
| $healthArgs = @{ operation = "check-health" } | ||
| $healthJson = $healthArgs | ConvertTo-Json -Compress | ||
| & "$HELPERS_DIR\invoke-mcp.ps1" -Tool "health_monitor" -ArgumentsJson $healthJson | ||
| Write-Log "Health monitor check completed" "INFO" | ||
| } catch { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Tool ID consistency: mixed prefixes (“mcp__token-optimizer__*” vs bare names). Verify and standardize.
Ensure invoke-mcp.ps1 expects the exact IDs used here.
🏁 Script executed:
#!/bin/bash
# Enumerate all MCP tool IDs referenced in ps1 files to spot inconsistencies
rg -nP --type=powershell -o '\-Tool\s+"([^"]+)"' -C1 | sed -E 's/.*-Tool\s+"([^"]+)".*/\1/' | sort | uniq -c | sort -nrLength of output: 34
🏁 Script executed:
#!/bin/bash
# Find all PowerShell files and extract Tool IDs for consistency check
fd -e ps1 -x rg -n '\-Tool\s+"([^"]+)"' {} \; | sed -E 's/.*-Tool\s+"([^"]+)".*/\1/' | sort | uniq -c | sort -nrLength of output: 808
Standardize Tool ID naming: mixed prefixes confirm inconsistency ("mcp__token-optimizer__" vs bare names).
Tool IDs used at lines 444-470 (cache_warmup, get_cache_stats, health_monitor) are bare names, but the codebase also uses prefixed variants like mcp__token-optimizer__cache_warmup and mcp__token-optimizer__optimize_session. Verify invoke-mcp.ps1 handles both forms or refactor all Tool IDs to use consistent naming.
Performance Benchmark Results |
1 participant
🚨 CRITICAL BUG FIX: Hooks Not Receiving JSON from Claude Code
Problem
After implementing Phase 2 performance optimizations (dot-sourcing orchestrator), all hooks stopped receiving JSON input from Claude Code. Logs showed "No JSON input" despite hooks being invoked correctly.
Root Cause
The
token-optimizer-orchestrator.ps1had aparamblock withMandatory=$trueparameters:When
dispatcher.ps1dot-sourced the orchestrator:PowerShell parsed the param block and attempted to bind mandatory parameters from stdin, consuming ALL JSON input BEFORE
dispatcher.ps1could read it.Investigation Journey
Initial Hypothesis (WRONG): UTF-8 encoding issue
Breakthrough: Created minimal test script without dot-sourcing
Root Cause Found: Used Gemini CLI (2M token context) to analyze full codebase
The Fix
Removed param block entirely from
token-optimizer-orchestrator.ps1(lines 5-11):Mandatory=$truecauses PowerShell to consume stdin during parseif ($MyInvocation.InvocationName -ne '.')conditional doesn't prevent param parsingReverted dispatcher.ps1 to use simple
[Console]::In.ReadToEnd():Performance Optimizations Preserved ✅
All Phase 1-5 optimizations remain intact:
[PERF]timing measurementsTesting Evidence
Timeline:
Minimal Test Script Results:
[Console]::In.ReadToEnd()works perfectlyFiles Changed
hooks/dispatcher.ps1: Reverted to simple[Console]::In.ReadToEnd()hooks/handlers/token-optimizer-orchestrator.ps1: Removed param blockImpact
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com