diff --git a/scripts/lib/context-selection.js b/scripts/lib/context-selection.js index 0a1a8745e..6a312f78a 100644 --- a/scripts/lib/context-selection.js +++ b/scripts/lib/context-selection.js @@ -30,10 +30,15 @@ const FALLBACK_MARGIN = 1.1; // longer overrides declines at the launch/selection call sites. // v5: indirect name citations cannot bypass review through exact or scored admission. // v6: ordered directives and singular named references share the same admission guard. -const ROUTING_POLICY_VERSION = 6; +// v7: independent directives scope negation and imperatives allow question punctuation. +const ROUTING_POLICY_VERSION = 7; const TASK_KEYS = new Set(['sessionId', 'taskId', 'revision', 'phase', 'query', 'explicitIds', 'proposedIds', 'noWorkflow']); const DIRECTIVE_VERB = /\b(use|apply|invoke|run|follow|load)\s+(the\s+)?/i; +const DIRECTIVE_REQUEST = /^(?:please )?(?:(?:can|could|would|will) you (?:please )?|do )?(?:use|apply|invoke|run|follow|load)\b/; +const QUESTION_START = /^(?!do not\b)(can|could|would|should|may|might|do|does|did|is|are|why|how|what|when|where|which)\b/; +const INSTRUCTION_BOUNDARY = /(?:,\s*(?:(?:and|but)\s+)?|\s+(?:and|but)\s+)(?=(?:please\s+)?(?:do\s+(?:not\s+)?|not\s+|never\s+)?(?:use|apply|invoke|run|follow|load)\b|(?:can|could|would|should|may|might|do|does|did|is|are|why|how|what|when|where|which)\b)/i; +/** Normalize citation names while preserving contracted negation as a separate word. */ const normalizedQueryName = text => text.replace(/n['\u2019]t\b/gi, ' not') .replace(/([a-z0-9])([A-Z])/g, '$1 $2').replace(/_/g, ' ') .toLowerCase().replace(/[^a-z0-9]+/g, ' ').trim(); @@ -67,19 +72,23 @@ function citationFor(candidate, query, nativeName) { return part.replace(/[^.!?;\n]/g, ' '); }); for (const clause of unquoted.match(/[^.!?;\n]+[.!?;\n]*/g) || []) { - const text = normalizedQueryName(clause); + // Keep comma boundaries for directive scope without changing alias normalization. + const text = clause.split(',').map(normalizedQueryName).join(', '); const mentions = aliases.flatMap(({ name, exact }) => [...text.matchAll(new RegExp('\\b' + name + '\\b', 'g'))] .map(mention => ({ index: mention.index, exact }))).sort((a, b) => a.index - b.index); - const polite = /^(?:(?:please )?(?:can|could|would|will) you (?:please )?|do )(?:use|apply|invoke|run|follow|load)\b/.test(text); - const question = !polite && (clause.includes('?') - || (!/^do not\b/.test(text) - && /^(can|could|would|should|may|might|do|does|did|is|are|why|how|what|when|where|which)\b/.test(text))); + const request = DIRECTIVE_REQUEST.test(text); + const interrogative = !request && QUESTION_START.test(text); for (const mention of mentions) { const prefix = text.slice(0, mention.index); + // A directive or question after a connector starts its own scope. + // Lists without another verb and reported speech keep their guards. + const instructionPrefix = prefix.split(INSTRUCTION_BOUNDARY).at(-1); + const question = interrogative || (!DIRECTIVE_REQUEST.test(instructionPrefix) + && (QUESTION_START.test(instructionPrefix) || (!request && clause.includes('?')))); if (question || /^\s*["'\u201c\u2018\x60]/.test(clause) || /\b(say|says|said|reads|told|mentions?|quoted?|document)\b/.test(prefix)) { if (citation === 'none') citation = 'indirect'; - } else if (/\b(do not|never|no|not|avoid)\b/.test(prefix)) { + } else if (/\b(do not|never|no|not|avoid)\b/.test(instructionPrefix)) { citation = 'indirect'; } else if (mention.exact && candidate.exact && new RegExp(DIRECTIVE_VERB.source + '(skill\\s*:?\\s*)?$', 'i').test(prefix)) { diff --git a/tests/lib/context-profile-launch.test.js b/tests/lib/context-profile-launch.test.js index 2edc088a1..681da8fd0 100644 --- a/tests/lib/context-profile-launch.test.js +++ b/tests/lib/context-profile-launch.test.js @@ -157,7 +157,19 @@ test('spawn failures and timeout signals remain unsuccessful without a native ex })); for (const query of ["Don't use the feature skill.", 'Don\u2019t use the feature skill.', - 'Can I use the feature skill?', 'The README says use the feature skill.', 'The docs say use feature.']) { + 'Can I use the feature skill?', 'The README says use the feature skill.', 'The docs say use feature.', + 'The docs say do not use feature and use shared guidance.', + 'The docs say do not use feature, use shared guidance, okay?', + 'Should we avoid feature and use shared guidance?', + 'Should we avoid feature, use shared guidance?', + 'Use standard tools, should we use feature?', + 'Please use the existing code, can I use feature?', + 'Use standard tools and should we use feature?', + 'Use standard tools but should we use feature?', + 'Do not use feature and shared guidance.', + "Use feature, don't use feature.", + "Use feature and don't use feature.", + "Use feature but don't use feature."]) { test('indirect citation uses the proposal path and honors decline: ' + query, () => withFixture(repoRoot => { const phases = []; const result = launchTaskContext({ repoRoot, task: { ...input, query, explicitIds: [] }, @@ -165,6 +177,7 @@ for (const query of ["Don't use the feature skill.", 'Don\u2019t use the feature phases.push(options.phase); if (options.phase === 'selection') return { status: 0, stdout: '{"selectedIds":[]}' }; assert.doesNotMatch(options.input, /# feature/); + assert.doesNotMatch(options.input, /# shared/); return { status: 0, stdout: 'ok' }; } }); assert.deepEqual(phases, ['selection', 'task']); @@ -174,6 +187,38 @@ for (const query of ["Don't use the feature skill.", 'Don\u2019t use the feature })); } +for (const [query, skill] of [ + ["Don't use feature, use shared guidance.", 'shared'], + ["Don't use feature and use shared guidance.", 'shared'], + ["Don't use feature but use shared guidance.", 'shared'], + ["Don't use feature, use shared guidance, okay?", 'shared'], + ["Don't use feature and use shared guidance, okay?", 'shared'], + ["Don't use feature but use shared guidance, okay?", 'shared'], + ['Do not use feature, please use shared guidance?', 'shared'], + ['Do not use feature and please use shared guidance?', 'shared'], + ['Do not use feature but please use shared guidance?', 'shared'], + ["Don't use feature, use feature.", 'feature'], + ['Use feature?', 'feature'], + ['Please use feature?', 'feature'], + ['Use feature to fix the bug, okay?', 'feature'], +]) { + test('an unambiguous final directive injects context without a proposal: ' + query, () => withFixture(repoRoot => { + const phases = []; + const result = launchTaskContext({ repoRoot, task: { ...input, query, explicitIds: [] }, + execute(command, args, options) { + phases.push(options.phase); + if (options.phase === 'selection') return { status: 0, stdout: '{"selectedIds":[]}' }; + assert.match(options.input, new RegExp('# ' + skill)); + assert.doesNotMatch(options.input, new RegExp('# ' + (skill === 'shared' ? 'feature' : 'shared'))); + return { status: 0, stdout: 'ok' }; + } }); + assert.deepEqual(phases, ['task']); + assert.equal(result.routingCalls, 0); + assert.deepEqual(result.selection.loadedIds, ['skill:' + skill]); + assert.equal(result.selection.reason, 'auto-selection'); + })); +} + test('a final directive after a question reaches the task with its context', () => withFixture(repoRoot => { const phases = []; const result = launchTaskContext({ repoRoot, diff --git a/tests/lib/context-selection.test.js b/tests/lib/context-selection.test.js index 682615c81..d55b50868 100644 --- a/tests/lib/context-selection.test.js +++ b/tests/lib/context-selection.test.js @@ -344,6 +344,74 @@ test('a negated citation does not suppress a separate affirmative directive', () } })); +for (const query of [ + "Don't use feature, use shared guidance.", + "Don't use feature and use shared guidance.", + "Don't use feature but use shared guidance.", + 'Do not use feature, please use shared guidance.', + 'Don\u2019t use feature, and please use shared guidance.', + "Don't use feature, do use shared guidance.", + "Don't use feature, use shared guidance, okay?", + "Don't use feature and use shared guidance, okay?", + "Don't use feature but use shared guidance, okay?", + 'Do not use feature, please use shared guidance?', + 'Do not use feature and please use shared guidance?', + 'Do not use feature but please use shared guidance?', +]) { + test('a new directive scopes an earlier rejection: ' + query, () => withFixture(repoRoot => { + const result = resolve(repoRoot, { query }, { load: true }); + assert.deepEqual(result.selectedIds, ['skill:shared']); + assert.deepEqual(result.loadedIds, ['skill:shared']); + assert.equal(result.reason, 'auto-selection'); + assert.equal(result.receipt.autoSelection.exact, true); + })); +} + +for (const query of [ + 'The docs say do not use feature and use shared guidance.', + 'The docs say do not use feature, use shared guidance.', + 'The docs say do not use feature, use shared guidance, okay?', + 'Should we avoid feature and use shared guidance?', + 'Should we avoid feature, use shared guidance?', + 'Do not use feature and shared guidance.', + 'Do not use feature, shared guidance.', +]) { + test('a connector preserves indirect or negated context: ' + query, () => withFixture(repoRoot => { + const result = resolve(repoRoot, { query }, { load: true }); + assert.ok(result.candidates.some(candidate => candidate.id === 'skill:shared')); + assert.deepEqual(result.selectedIds, []); + assert.deepEqual(result.loadedIds, []); + assert.equal(result.reason, 'agent-selection-required'); + assert.equal(result.fallback, null); + })); +} + +for (const query of [ + 'Use standard tools, should we use feature?', + 'Please use the existing code, can I use feature?', + 'Use standard tools and should we use feature?', + 'Use standard tools but should we use feature?', +]) { + test('a generic directive does not authorize a later question: ' + query, () => withFixture(repoRoot => { + const result = resolve(repoRoot, { query }, { load: true }); + assert.ok(result.candidates.some(candidate => candidate.id === 'skill:feature')); + assert.deepEqual(result.selectedIds, []); + assert.deepEqual(result.loadedIds, []); + assert.equal(result.reason, 'agent-selection-required'); + assert.equal(result.fallback, null); + })); +} + +for (const query of ['Use feature?', 'Please use feature?', 'Use feature to fix the bug, okay?']) { + test('an imperative with question punctuation remains a directive: ' + query, () => withFixture(repoRoot => { + const result = resolve(repoRoot, { query }, { load: true }); + assert.deepEqual(result.selectedIds, ['skill:feature']); + assert.deepEqual(result.loadedIds, ['skill:feature']); + assert.equal(result.reason, 'auto-selection'); + assert.equal(result.receipt.autoSelection.exact, true); + })); +} + test('ordinary and polite directives preserve automatic and explicit selection', () => withFixture(repoRoot => { for (const query of ['Please use the feature skill.', 'Can you use the feature skill?', 'Use feature without changing the code.', 'Use feature to document the change.', @@ -375,7 +443,7 @@ test('routing policy changes invalidate receipts created by the old citation pol const { compileContextProfile } = require('../../scripts/lib/context-profiles'); const plan = compileContextProfile({ repoRoot, selectionMode: 'auto' }); const bindingDigest = digestObject({ sessionId: 'session-1', taskId: 'task-1', revision: 1, - phase: 'implement', planDigest: plan.planDigest, routingPolicyVersion: 5, + phase: 'implement', planDigest: plan.planDigest, routingPolicyVersion: 6, triggersDigest: digestObject({}), queryDigest: digestObject(query) }); const { receiptDigest: _receiptDigest, ...receipt } = first.receipt; const oldReceipt = { ...receipt, bindingDigest, explicitIds: [], @@ -386,9 +454,30 @@ test('routing policy changes invalidate receipts created by the old citation pol assert.deepEqual(result.loadedIds, []); })); +test('an old empty selection is reconsidered when directive policy changes', () => withFixture(repoRoot => { + const query = "Don't use feature, use shared guidance."; + const first = resolve(repoRoot, { query }); + const { digestObject } = require('../../scripts/lib/context-profile-support'); + const { compileContextProfile } = require('../../scripts/lib/context-profiles'); + const plan = compileContextProfile({ repoRoot, selectionMode: 'auto' }); + const bindingDigest = digestObject({ sessionId: 'session-1', taskId: 'task-1', revision: 1, + phase: 'implement', planDigest: plan.planDigest, routingPolicyVersion: 6, + triggersDigest: digestObject({}), queryDigest: digestObject(query) }); + const { receiptDigest: _receiptDigest, autoSelection: _autoSelection, ...receipt } = first.receipt; + const oldReceipt = { ...receipt, bindingDigest, decision: 'none', selectedIds: [], explicitIds: [], + loadedIds: [], resources: [], selectionDigest: digestObject({ bindingDigest, selectedIds: [], explicitIds: [] }) }; + const previous = { ...oldReceipt, receiptDigest: digestObject(oldReceipt) }; + const result = resolve(repoRoot, { query }, { previous, load: true }); + assert.equal(result.reused, false); + assert.notEqual(result.receipt.bindingDigest, previous.bindingDigest); + assert.deepEqual(result.loadedIds, ['skill:shared']); + assert.equal(result.reason, 'auto-selection'); +})); + for (const query of [ 'Should we use feature? Use feature.', 'Use feature. Should we use feature?', + 'Use feature, should we use feature?', 'The docs say use feature. Use feature.', 'Use feature. The docs say use feature.', 'The phrase "Use feature." is an example. Use feature.', @@ -397,6 +486,9 @@ for (const query of [ `Use feature. The docs say "don’t use feature."`, 'Use feature. The phrase "Use feature." is an example.', "Don't use feature. Use feature.", + "Don't use feature, use feature.", + "Don't use feature and use feature.", + "Don't use feature but use feature.", 'The docs say never use feature. Use feature.', 'Use feature. The docs say never use feature.', 'Use feature to inspect the essay.', @@ -411,6 +503,9 @@ for (const query of [ for (const query of [ "Use feature. Don't use feature.", + "Use feature, don't use feature.", + "Use feature and don't use feature.", + "Use feature but don't use feature.", 'Use feature. Never use feature.', "Use feature. Don't use feature. Should we use feature?", ]) {