From 4cf391b7ffb04c42346a01bf11cced486d5adc95 Mon Sep 17 00:00:00 2001
From: Vanhci <vanhci@wanghanchaodeMac-mini.local>
Date: Fri, 22 May 2026 16:06:09 +0800
Subject: [PATCH] fix(comment-checker): skip modified-existing comments and
 dedupe per-session (issue #4292)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Issue 1: hasNewCommentsOnly() now returns false when oldString and newString
both contain comment syntax and the new lines are a subset of old lines —
preventing the hook from firing on comment-only modifications.

Issue 2: Per-session deduplication via sessionLastWarning Map with a 30s
window (DEDUP_WINDOW_MS). At most one warning fires per session per
response turn, breaking the deadloop on consecutive edits.
---
 src/hooks/comment-checker/cli-runner.ts | 44 +++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/src/hooks/comment-checker/cli-runner.ts b/src/hooks/comment-checker/cli-runner.ts
index 00f5a0411..79a96869a 100644
--- a/src/hooks/comment-checker/cli-runner.ts
+++ b/src/hooks/comment-checker/cli-runner.ts
@@ -6,6 +6,35 @@ import { runCommentChecker, getCommentCheckerPath, startBackgroundInit, type Hoo
 let cliPathPromise: Promise<string | null> | null = null
 let isRunning = false
 
+/** Per-session deduplication: track last warning time to prevent deadloop */
+const sessionLastWarning = new Map<string, number>()
+const DEDUP_WINDOW_MS = 30_000 // 30 seconds — fire at most once per response turn
+
+/** Detect whether a comment string looks like a line-comment or block-comment pattern */
+function hasCommentSyntax(text: string | undefined): boolean {
+  if (!text) return false
+  return /^\s*(\/\/|\/\*|#|--|<!--|:\s*)[\s\S]*$/m.test(text) || /<!--[\s\S]*-->/.test(text)
+}
+
+/**
+ * Returns true if any lines in `newText` contain comments that did NOT exist in
+ * `oldText`. This filters out false positives when oldString/newString both
+ * contain the same existing comment that was only slightly modified.
+ */
+function hasNewCommentsOnly(oldText: string | undefined, newText: string | undefined): boolean {
+  if (!hasCommentSyntax(newText)) return false
+  // If there was no old text, any comment is by definition new
+  if (!hasCommentSyntax(oldText)) return true
+  // Both contain comments — do a rough line-level diff to see if new comment
+  // lines were added (not just modified in-place)
+  const oldLines = new Set((oldText ?? "").split("\n").map((l) => l.trim()))
+  const newLines = (newText ?? "").split("\n")
+  return newLines.some((l) => {
+    const trimmed = l.trim()
+    return trimmed && hasCommentSyntax(trimmed) && !oldLines.has(trimmed)
+  })
+}
+
 async function withCommentCheckerLock<T>(
   fn: () => Promise<T>,
   fallback: T,
@@ -70,6 +99,21 @@ export async function processWithCli(
       },
     }
 
+    // --- Fix #4292 Issue 1: skip if comment was already in oldString ---
+    if (!hasNewCommentsOnly(pendingCall.oldString, pendingCall.newString)) {
+      debugLog("skipping: no net-new comments in edit (oldString/newString)")
+      return
+    }
+
+    // --- Fix #4292 Issue 2: deduplicate per-session (at most once per 30s) ---
+    const lastWarned = sessionLastWarning.get(pendingCall.sessionID) ?? 0
+    const now = Date.now()
+    if (now - lastWarned < DEDUP_WINDOW_MS) {
+      debugLog("dedup: skipping comment warning within dedup window for session", pendingCall.sessionID)
+      return
+    }
+    sessionLastWarning.set(pendingCall.sessionID, now)
+
     const result = await (deps.runCommentChecker ?? runCommentChecker)(hookInput, cliPath, customPrompt)
 
     if (result.hasComments && result.message) {