fix(hooks): findOverrideAttempt + helpful diagnostic for silent-reject when justification missing. Resolves UX bug where enforce-verify-before-push silently rejected master overrides without justification line. Now emits explicit diagnostic. 132/132 hook tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -214,6 +214,27 @@ export function findOverride(userPrompt, ruleKey, vocab) {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Diagnostic variant: returns phrase object if substring matches AND rule
|
||||
* applies, regardless of justification presence. Use ONLY for error-message
|
||||
* generation in hooks — never to grant suppression.
|
||||
*
|
||||
* Fixes silent-reject bug where users see "no verification artifact" while
|
||||
* having typed the override phrase but missing the justification line.
|
||||
*/
|
||||
export function findOverrideAttempt(userPrompt, ruleKey, vocab) {
|
||||
if (!userPrompt || typeof userPrompt !== 'string') return null;
|
||||
const v = vocab || loadOverrideVocab();
|
||||
const lo = userPrompt.toLowerCase();
|
||||
for (const p of v.phrases || []) {
|
||||
if (!p.phrase || !Array.isArray(p.suppresses)) continue;
|
||||
if (!lo.includes(p.phrase.toLowerCase())) continue;
|
||||
if (!p.suppresses.includes(ruleKey)) continue;
|
||||
return p;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export function logOverride(ruleKey, phraseObj, sessionId) {
|
||||
try {
|
||||
const f = join(runtimeDir(), 'override-usage.jsonl');
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
loadOverrideVocab,
|
||||
_resetVocabCache,
|
||||
findOverride,
|
||||
findOverrideAttempt,
|
||||
isProductionCodePath,
|
||||
isMemoryPath,
|
||||
detectGitCommandKind,
|
||||
@@ -180,6 +181,57 @@ describe('findOverride — requires_justification (hole 7)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('findOverrideAttempt — diagnostic helper (silent-reject bug fix)', () => {
|
||||
const testVocab = {
|
||||
phrases: [
|
||||
{
|
||||
phrase: 'ремонт инфраструктуры',
|
||||
suppresses: ['verify-before-push', 'classifier-mismatch'],
|
||||
requires_justification: 'ремонт:',
|
||||
description: 'master kill — requires justification',
|
||||
},
|
||||
{
|
||||
phrase: 'срочно',
|
||||
suppresses: ['verify-before-push'],
|
||||
description: 'no justification required',
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
it('returns phrase even when justification line missing (so caller can emit helpful diagnostic)', () => {
|
||||
const r = findOverrideAttempt('ремонт инфраструктуры', 'verify-before-push', testVocab);
|
||||
expect(r).not.toBeNull();
|
||||
expect(r.phrase).toBe('ремонт инфраструктуры');
|
||||
expect(r.requires_justification).toBe('ремонт:');
|
||||
});
|
||||
|
||||
it('returns phrase when justification IS provided (same behaviour as findOverride for success path)', () => {
|
||||
const r = findOverrideAttempt('ремонт инфраструктуры\nремонт: observer refresh', 'verify-before-push', testVocab);
|
||||
expect(r).not.toBeNull();
|
||||
expect(r.phrase).toBe('ремонт инфраструктуры');
|
||||
});
|
||||
|
||||
it('returns phrase for non-justification overrides (e.g., срочно)', () => {
|
||||
const r = findOverrideAttempt('срочно надо', 'verify-before-push', testVocab);
|
||||
expect(r).not.toBeNull();
|
||||
expect(r.phrase).toBe('срочно');
|
||||
});
|
||||
|
||||
it('returns null when phrase substring not in prompt', () => {
|
||||
expect(findOverrideAttempt('hello world', 'verify-before-push', testVocab)).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null when rule key not in suppresses (phrase irrelevant)', () => {
|
||||
const r = findOverrideAttempt('ремонт инфраструктуры', 'tdd-gate-other', testVocab);
|
||||
expect(r).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null on empty / null prompt', () => {
|
||||
expect(findOverrideAttempt('', 'verify-before-push', testVocab)).toBeNull();
|
||||
expect(findOverrideAttempt(null, 'verify-before-push', testVocab)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('isProductionCodePath', () => {
|
||||
it('classifies tools/*.mjs as production', () => {
|
||||
expect(isProductionCodePath('tools/router-classifier.mjs')).toBe(true);
|
||||
|
||||
@@ -19,6 +19,7 @@ import {
|
||||
readTranscript,
|
||||
lastUserPromptText,
|
||||
findOverride,
|
||||
findOverrideAttempt,
|
||||
logOverride,
|
||||
exitDecision,
|
||||
detectGitCommandKind,
|
||||
@@ -30,12 +31,31 @@ const RULE_KEY_COMMIT = 'verify-before-commit';
|
||||
const RULE_KEY_PUSH = 'verify-before-push';
|
||||
const MAX_AGE_SEC = 30 * 60; // 30 min
|
||||
|
||||
export function decide({ toolName, command, sentinel, sentinelAge, override }) {
|
||||
export function decide({ toolName, command, sentinel, sentinelAge, override, overrideAttempt }) {
|
||||
if (toolName !== 'Bash' || typeof command !== 'string') return { block: false };
|
||||
const kind = detectGitCommandKind(command);
|
||||
if (kind !== 'commit' && kind !== 'push') return { block: false };
|
||||
if (override) return { block: false };
|
||||
|
||||
// Silent-reject bug fix (2026-05-26): when user typed an override phrase that
|
||||
// requires justification (e.g. "ремонт инфраструктуры") but forgot the
|
||||
// "<prefix> <reason>" line, emit an explicit diagnostic — not the generic
|
||||
// "no verification artifact" message that misled users into thinking the
|
||||
// override mechanism was broken.
|
||||
if (overrideAttempt && overrideAttempt.requires_justification) {
|
||||
return {
|
||||
block: true,
|
||||
message: [
|
||||
`[enforce-verify-before-push] Override phrase "${overrideAttempt.phrase}" found, but missing justification line.`,
|
||||
`Add a line "${overrideAttempt.requires_justification} <one-line причина>" in the SAME prompt.`,
|
||||
``,
|
||||
`Example:`,
|
||||
` ${overrideAttempt.phrase}`,
|
||||
` ${overrideAttempt.requires_justification} observer refresh after brainstorm session`,
|
||||
].join('\n'),
|
||||
};
|
||||
}
|
||||
|
||||
if (!sentinel) {
|
||||
return {
|
||||
block: true,
|
||||
@@ -82,11 +102,12 @@ async function main() {
|
||||
const ruleKey = kind === 'commit' ? RULE_KEY_COMMIT : RULE_KEY_PUSH;
|
||||
const override = findOverride(userPrompt, ruleKey);
|
||||
if (override) logOverride(ruleKey, override, event.session_id);
|
||||
const overrideAttempt = override ? null : findOverrideAttempt(userPrompt, ruleKey);
|
||||
|
||||
const sentinel = readSentinel('verify-pass', event.session_id);
|
||||
const age = sentinelAgeSec('verify-pass', event.session_id);
|
||||
|
||||
const result = decide({ toolName, command, sentinel, sentinelAge: age, override });
|
||||
const result = decide({ toolName, command, sentinel, sentinelAge: age, override, overrideAttempt });
|
||||
exitDecision(result);
|
||||
} catch {
|
||||
exitDecision({ block: false });
|
||||
|
||||
@@ -110,4 +110,53 @@ describe('enforce-verify-before-push / decide', () => {
|
||||
});
|
||||
expect(r.block).toBe(false);
|
||||
});
|
||||
|
||||
it('emits helpful diagnostic when override phrase matched but justification missing', () => {
|
||||
// Silent-reject bug fix: user typed "ремонт инфраструктуры" but forgot
|
||||
// the "ремонт: <reason>" line. Old behaviour: generic "No verification artifact".
|
||||
// New behaviour: explicit "phrase found but missing 'ремонт: <reason>' line".
|
||||
const r = decide({
|
||||
toolName: 'Bash', command: 'git commit -m "x"',
|
||||
sentinel: null,
|
||||
override: null,
|
||||
overrideAttempt: {
|
||||
phrase: 'ремонт инфраструктуры',
|
||||
requires_justification: 'ремонт:',
|
||||
suppresses: ['verify-before-push'],
|
||||
},
|
||||
});
|
||||
expect(r.block).toBe(true);
|
||||
expect(r.message).toMatch(/ремонт инфраструктуры/);
|
||||
expect(r.message).toMatch(/ремонт:/);
|
||||
expect(r.message).toMatch(/justification|причин/i);
|
||||
});
|
||||
|
||||
it('falls back to generic message when overrideAttempt is null (phrase not even typed)', () => {
|
||||
const r = decide({
|
||||
toolName: 'Bash', command: 'git commit -m "x"',
|
||||
sentinel: null,
|
||||
override: null,
|
||||
overrideAttempt: null,
|
||||
});
|
||||
expect(r.block).toBe(true);
|
||||
expect(r.message).toMatch(/No verification/);
|
||||
});
|
||||
|
||||
it('does NOT emit override-missing-justification diagnostic for overrides without requires_justification', () => {
|
||||
// "срочно" doesn't need justification — if it matched, override would've been set.
|
||||
// overrideAttempt without requires_justification means something else (logic bug),
|
||||
// fall through to normal sentinel checks.
|
||||
const r = decide({
|
||||
toolName: 'Bash', command: 'git commit -m "x"',
|
||||
sentinel: null,
|
||||
override: null,
|
||||
overrideAttempt: {
|
||||
phrase: 'срочно',
|
||||
suppresses: ['verify-before-push'],
|
||||
// no requires_justification
|
||||
},
|
||||
});
|
||||
expect(r.block).toBe(true);
|
||||
expect(r.message).toMatch(/No verification/);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user