diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index cea0265b3..3c553fa01 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -2323,6 +2323,7 @@ export async function runStopHook({ stdinJson, env = {}, cwd = process.cwd(), no const freshGroups = []; let scanned = 0; + let cacheDirty = false; for (const filePath of touched) { if (scanned >= STOP_MAX_FILES) break; if (hasPathTraversal(filePath) || SENSITIVE_PATH.test(filePath)) continue; @@ -2343,29 +2344,39 @@ export async function runStopHook({ stdinJson, env = {}, cwd = process.cwd(), no try { content = fs.readFileSync(filePath, 'utf-8'); } catch { continue; } let findings; + let detectorThrew = false; const useHtmlEngine = configuredExt ? configuredExt.engine === 'html' : (ext === '.html' || ext === '.htm'); if (useHtmlEngine && typeof det.detectHtml === 'function') { - try { findings = await det.detectHtml(filePath, scanOptions); } catch { findings = []; } + try { findings = await det.detectHtml(filePath, scanOptions); } catch { findings = []; detectorThrew = true; } } else { - try { findings = await det.detectText(content, filePath, scanOptions); } catch { findings = []; } + try { findings = await det.detectText(content, filePath, scanOptions); } catch { findings = []; detectorThrew = true; } } + // A detector failure tells us nothing about the file. Leave whatever + // was remembered alone rather than recording an empty scan as truth. + if (detectorThrew) continue; + // Full rule set: no tier split here. Config/inline ignores still apply, // and the session dedupe drops everything the per-edit pass (or an // earlier Stop pass) already surfaced. const filtered = filterFindings(findings || [], content, ext, config); const fresh = dedupeAgainstCache(filtered, cache, sessionId, filePath); + // Sync to the live scan, including empty. Remembering only `fresh` + // (or skipping the write on a clean Stop) left stale keys in place, so + // a finding that was fixed and later reintroduced never fired again. + rememberFindings(cache, sessionId, filePath, filtered); + cacheDirty = true; if (fresh.length > 0) { - rememberFindings(cache, sessionId, filePath, fresh); freshGroups.push({ filePath, findings: fresh }); } } audit.scannedFiles = scanned; if (freshGroups.length === 0) { + if (cacheDirty) persistCache(projectCwd, cache); return result({ emitted: false, skipped: 'stop-clean', durationMs: Date.now() - started }); } @@ -2382,8 +2393,8 @@ export async function runStopHook({ stdinJson, env = {}, cwd = process.cwd(), no ); commitFooterShown(cache, sessionId, text); - // Fresh findings earn the cache write so the next Stop fire is silent - // unless new issues appear; the notice flags ride along. + // Persist the live finding set so the next Stop fire is silent unless + // new issues appear; the notice flags ride along. persistCache(projectCwd, cache); return { exitCode: 0, diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index 91e37370c..907de5f6c 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -4099,6 +4099,56 @@ describe('runStopHook()', () => { assert.match(out.hookSpecificOutput.additionalContext, /marketing-buzzword/); }); + it('Grok Stop re-emits a finding that was fixed then reintroduced', async () => { + // Grok PostToolUse only touches the file. Stop is the cache writer. + // A clean Stop must replace the remembered set with the empty scan so + // the same finding is not deduped away when it comes back. + const sid = 'grok-stop-reintro'; + const file = write('src/Card.tsx', 'noop'); + let current = [finding('dark-glow', 5)]; + const det = { + set(next) { current = next; }, + detectText: () => current.slice(), + detectHtml: () => current.slice(), + }; + + await runHook({ stdinJson: JSON.stringify(grokEditEvent(file, sid)), env: {}, cwd, detector: det }); + const first = await runStopHook({ stdinJson: JSON.stringify(grokStopEvent(sid)), env: {}, cwd, detector: det }); + assert.match(first.stdout, /dark-glow/); + + det.set([]); + const clean = await runStopHook({ stdinJson: JSON.stringify(grokStopEvent(sid)), env: {}, cwd, detector: det }); + assert.equal(clean.stdout, ''); + assert.equal(clean.audit.skipped, 'stop-clean'); + assert.deepEqual(readCache(cwd).sessions[sid].files[file].findings, []); + + det.set([finding('dark-glow', 5)]); + const again = await runStopHook({ stdinJson: JSON.stringify(grokStopEvent(sid)), env: {}, cwd, detector: det }); + assert.equal(again.audit.emitted, true); + assert.match(again.stdout, /dark-glow/, 'a finding fixed then reintroduced must fire at Stop again'); + }); + + it('Stop remembers the live scan, not only newly emitted findings', async () => { + // Per-edit already remembered dark-glow. Stop then emits the deferred + // remainder. The cache must keep both keys so a second Stop stays silent + // instead of re-firing the immediate-tier finding. + const sid = 'stop-sync-full-set'; + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([ + finding('dark-glow', 5), + finding('marketing-buzzword', 3), + ]); + + await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); + const first = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); + assert.match(first.stdout, /marketing-buzzword/); + assert.doesNotMatch(first.stdout, /dark-glow/); + + const second = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); + assert.equal(second.stdout, ''); + assert.equal(second.audit.skipped, 'stop-clean'); + }); + it('Grok Stop shutdown is observe-only and does not emit a second deep pass (#646)', async () => { const sid = 'grok-shutdown'; const file = write('src/Card.tsx', 'noop');