diff --git a/packages/cli/src/generated/signals/intent.ts b/packages/cli/src/generated/signals/intent.ts index 02180c8e..36c91968 100644 --- a/packages/cli/src/generated/signals/intent.ts +++ b/packages/cli/src/generated/signals/intent.ts @@ -170,7 +170,11 @@ function parseExplicitEngineIds(input: string): string[] { .forEach(add); }; - const withMatch = input.match(/\bwith\s+([a-z0-9@_.-]+(?:\s*(?:,|and|or|plus)\s*[a-z0-9@_.-]+)*)/i); + // Accept comma/and/or/plus separators AND plain spaces between engine + // names, so "with codex claude agy" captures all three (not just codex). + // Over-capturing trailing prose is harmless: normalizeEngineToken() drops + // any token that isn't a known engine id. + const withMatch = input.match(/\bwith\s+([a-z0-9@_.-]+(?:(?:\s*,\s*|\s+and\s+|\s+or\s+|\s+plus\s+|\s+)[a-z0-9@_.-]+)*)/i); if (withMatch) collect(withMatch[1]); const askMatch = input.match(/\b(?:ask|have|get)\s+([a-z0-9@_.-]+(?:\s*(?:,|and|or|plus)\s*[a-z0-9@_.-]+)*)\s+(?:to\s+)?(?:review|check|audit|look|inspect|compare|weigh|think|debate)\b/i); @@ -179,7 +183,7 @@ function parseExplicitEngineIds(input: string): string[] { return engineIds; } -// @kern-source: intent:148 +// @kern-source: intent:152 function parseSemanticReviewShortcut(input: string): Intent|null { const lower = input.toLowerCase(); const reviewVerb = /\b(?:review|check|audit|inspect|look\s+over)\b/i.test(input); @@ -209,17 +213,17 @@ function parseSemanticReviewShortcut(input: string): Intent|null { return { type: 'review', engineId: engineIds[0], engineIds: engineIds, target: target } as Intent; } -// @kern-source: intent:172 +// @kern-source: intent:176 function stripCollaborationLeadIn(input: string): string { return input.replace(/^(?:can\s+you\s+|could\s+you\s+|please\s+)?(?:ask|have|get)\s+(?:the\s+)?(?:others|other\s+engines|team|engines|models|everyone|all\s+engines)\s+(?:to\s+)?/i, '').replace(/^(?:can\s+you\s+|could\s+you\s+|please\s+)?what\s+do\s+(?:the\s+)?(?:others|other\s+engines|team|engines|models|everyone|all\s+engines)\s+(?:think\s+about\s+|say\s+about\s+|recommend\s+for\s+)?/i, '').replace(/^(?:can\s+you\s+|could\s+you\s+|please\s+)?(?:talk|think)\s+(?:it|this)?\s*(?:through\s+)?with\s+(?:the\s+)?(?:others|other\s+engines|team|engines|models|everyone|all\s+engines)\s*/i, '').trim(); } -// @kern-source: intent:176 +// @kern-source: intent:180 function hasCollaborationAskShape(input: string): boolean { return /^(?:can\s+you\s+|could\s+you\s+|please\s+)?(?:ask|have|get)\s+(?:the\s+)?(?:others|other\s+engines|team|engines|models|everyone|all\s+engines)\b/i.test(input) || /^(?:can\s+you\s+|could\s+you\s+|please\s+)?what\s+do\s+(?:the\s+)?(?:others|other\s+engines|team|engines|models|everyone|all\s+engines)\s+(?:think|say|recommend)\b/i.test(input) || /^(?:can\s+you\s+|could\s+you\s+|please\s+)?(?:brainstorm|compare|weigh\s+in)\s+(?:this|it)?\s*(?:with\s+)?(?:the\s+)?(?:others|other\s+engines|team|engines|models|everyone|all\s+engines)\b/i.test(input); } -// @kern-source: intent:180 +// @kern-source: intent:184 function parseSemanticCollaborationShortcut(input: string): Intent|null { const question = stripCollaborationLeadIn(input); if (/\b(?:debate|argue|tribunal|red-team|red\s+team)\b/i.test(input)) { @@ -235,7 +239,7 @@ function parseSemanticCollaborationShortcut(input: string): Intent|null { return null; } -// @kern-source: intent:192 +// @kern-source: intent:196 function parseSemanticForgeShortcut(input: string): Intent|null { const explicitForgeImperative = /^(?:can\s+you\s+|could\s+you\s+|please\s+)?forge\b/i.test(input) && !/^(?:can\s+you\s+|could\s+you\s+|please\s+)?forge\s+(?:is|was|seems?|looks?|does|did|can|should|would|will|not|still)\b/i.test(input); const hasForgeShape = explicitForgeImperative || /\b(?:forge\s+this|forge\s+it|have\s+(?:the\s+)?(?:engines|models|team|others)\s+compete|make\s+(?:the\s+)?(?:engines|models|team|others)\s+compete|competitive\s+(?:build|implementation|fix))\b/i.test(input); @@ -250,23 +254,23 @@ function parseSemanticForgeShortcut(input: string): Intent|null { /** * Plain text must not start orchestration. Brainstorm, tribunal, campfire, forge, and review are slash-only from chat input; mention words like 'tribunal' or 'forge' should reach Cesar as normal text unless the user uses /tribunal, /forge, etc. */ -// @kern-source: intent:202 +// @kern-source: intent:206 function parseSemanticDelegationShortcut(input: string): Intent|null { return null; } -// @kern-source: intent:207 +// @kern-source: intent:211 function splitReviewArgs(input: string): string[] { return input.split(/\s+/).flatMap((part) => part.split(',')).map((part) => part.trim()).filter(Boolean); } -// @kern-source: intent:211 +// @kern-source: intent:215 function isReviewTargetArg(part: string): boolean { const lower = part.toLowerCase(); return lower === 'uncommitted' || lower.startsWith('branch:') || lower.startsWith('commit:'); } -// @kern-source: intent:216 +// @kern-source: intent:220 function isImplicitReviewSubjectArg(part: string): boolean { const lower = part.toLowerCase(); return lower === 'it' || lower === 'this' || lower === 'that' || lower === 'them' || lower === 'changes' || lower === 'diff'; @@ -275,7 +279,7 @@ function isImplicitReviewSubjectArg(part: string): boolean { /** * Parse review args into target + engine list. When bareWordsAreEngines is true (the explicit /review slash path), any bare word that isn't a target (uncommitted/branch:/commit:) or a keyword is treated as an engine name — so `/review codex claude` reviews with BOTH, no `with` needed. The natural-language shortcut path leaves it false so prose like `review this code` doesn't mis-bind `code` as an engine. */ -// @kern-source: intent:221 +// @kern-source: intent:225 function parseReviewInput(input: string, bareWordsAreEngines?: boolean): Intent { const reviewParts = splitReviewArgs(input); const engineIds: string[] = []; @@ -310,7 +314,7 @@ function parseReviewInput(input: string, bareWordsAreEngines?: boolean): Intent return { type: 'review', engineId, engineIds: engineIds.length > 0 ? engineIds : undefined, target } as Intent; } -// @kern-source: intent:257 +// @kern-source: intent:261 function parseReviewShortcut(input: string): Intent|null { const match = input.match(/^(?:review|cr)(?:\s+([\s\S]+))?$/i); if (!match) { @@ -338,7 +342,7 @@ function parseReviewShortcut(input: string): Intent|null { return null; } -// @kern-source: intent:279 +// @kern-source: intent:283 function parseSlashCommand(input: string, commandRegistry?: any): Intent { const stripped = input.slice(1).trim(); if (!stripped) return { type: 'slash-list' } as Intent; @@ -698,7 +702,7 @@ function parseSlashCommand(input: string, commandRegistry?: any): Intent { } } -// @kern-source: intent:639 +// @kern-source: intent:643 export function detectIntent(raw: string, commandRegistry?: any): Intent { const input = raw.trim(); if (!input) { @@ -721,6 +725,18 @@ export function detectIntent(raw: string, commandRegistry?: any): Intent { if (delegationShortcut) { return delegationShortcut; } + // Deterministic review dispatch for an UNAMBIGUOUS chat request: + // a review verb + "with " (politeness prefixes ok, + // compound "fix then review" excluded, engine tokens validated). This + // grounds "review with codex claude agy" / "can you review with ..." so it + // dispatches directly (target defaults to uncommitted) instead of relying + // on the Cesar model to call the Review tool — which a weak engine may fake + // or mis-target to the current branch. Plain mentions still reach Cesar: + // parseSemanticReviewShortcut returns null when no known engine is named. + const reviewShortcut = parseSemanticReviewShortcut(input); + if (reviewShortcut) { + return reviewShortcut; + } // Natural-language toggle for autoCredit (German + English) if (AUTOCREDIT_OFF_KEYWORDS.test(input)) { return { type: 'toggleAutoCredit', autoCredit: false, input: input } as Intent; diff --git a/packages/cli/src/kern/signals/intent.kern b/packages/cli/src/kern/signals/intent.kern index 33588f4b..90f9a537 100644 --- a/packages/cli/src/kern/signals/intent.kern +++ b/packages/cli/src/kern/signals/intent.kern @@ -136,7 +136,11 @@ module name=IntentParsing .forEach(add); }; - const withMatch = input.match(/\bwith\s+([a-z0-9@_.-]+(?:\s*(?:,|and|or|plus)\s*[a-z0-9@_.-]+)*)/i); + // Accept comma/and/or/plus separators AND plain spaces between engine + // names, so "with codex claude agy" captures all three (not just codex). + // Over-capturing trailing prose is harmless: normalizeEngineToken() drops + // any token that isn't a known engine id. + const withMatch = input.match(/\bwith\s+([a-z0-9@_.-]+(?:(?:\s*,\s*|\s+and\s+|\s+or\s+|\s+plus\s+|\s+)[a-z0-9@_.-]+)*)/i); if (withMatch) collect(withMatch[1]); const askMatch = input.match(/\b(?:ask|have|get)\s+([a-z0-9@_.-]+(?:\s*(?:,|and|or|plus)\s*[a-z0-9@_.-]+)*)\s+(?:to\s+)?(?:review|check|audit|look|inspect|compare|weigh|think|debate)\b/i); @@ -653,6 +657,17 @@ module name=IntentParsing let name=delegationShortcut value="parseSemanticDelegationShortcut(input)" if cond="delegationShortcut" return value="delegationShortcut" + comment raw="// Deterministic review dispatch for an UNAMBIGUOUS chat request:" + comment raw="// a review verb + \"with \" (politeness prefixes ok," + comment raw="// compound \"fix then review\" excluded, engine tokens validated). This" + comment raw="// grounds \"review with codex claude agy\" / \"can you review with ...\" so it" + comment raw="// dispatches directly (target defaults to uncommitted) instead of relying" + comment raw="// on the Cesar model to call the Review tool — which a weak engine may fake" + comment raw="// or mis-target to the current branch. Plain mentions still reach Cesar:" + comment raw="// parseSemanticReviewShortcut returns null when no known engine is named." + let name=reviewShortcut value="parseSemanticReviewShortcut(input)" + if cond="reviewShortcut" + return value="reviewShortcut" comment raw="// Natural-language toggle for autoCredit (German + English)" if cond="AUTOCREDIT_OFF_KEYWORDS.test(input)" return value="{ type: 'toggleAutoCredit', autoCredit: false, input } as Intent" diff --git a/tests/unit/auto-router.test.ts b/tests/unit/auto-router.test.ts index 80ca8a20..c368c62f 100644 --- a/tests/unit/auto-router.test.ts +++ b/tests/unit/auto-router.test.ts @@ -80,8 +80,9 @@ describe('Auto-Router', () => { it('plain orchestration words stay in auto instead of starting jobs', () => { expect(detectIntent('forge fix it').type).toBe('auto'); expect(detectIntent('debate whether REST fits').type).toBe('auto'); - expect(detectIntent('review with codex').type).toBe('auto'); expect(detectIntent('can you ask others what they think').type).toBe('auto'); + // NOTE: an explicit "review with " now dispatches a review + // directly (see intent.test.ts) — forge/tribunal/campfire remain slash-only. }); it('keyword shortcuts still work', () => { diff --git a/tests/unit/intent.test.ts b/tests/unit/intent.test.ts index 651e92ce..031b2df4 100644 --- a/tests/unit/intent.test.ts +++ b/tests/unit/intent.test.ts @@ -194,6 +194,14 @@ describe('Intent Detection — Slash Commands', () => { expect(withTarget.engineIds).toEqual(['codex', 'claude']); expect(withTarget.target).toBe('branch:main'); } + + // three space-separated engines, no target → defaults to uncommitted + const three = detectIntent('/review codex claude agy'); + expect(three.type).toBe('review'); + if (three.type === 'review') { + expect(three.engineIds).toEqual(['codex', 'claude', 'agy']); + expect(three.target).toBeUndefined(); + } }); it('/leaderboard', () => { @@ -447,17 +455,22 @@ describe('Intent Detection — Natural Language', () => { if (ambiguous.type === 'auto') expect(ambiguous.taskClass).toBe('ambiguous'); }); - it('keeps plain review text as auto; /review is required to start review', () => { - const r = detectIntent('review with gemini'); - expect(r.type).toBe('auto'); + it('dispatches an explicit "review with " from chat', () => { + const r = detectIntent('review with claude'); + expect(r.type).toBe('review'); const branch = detectIntent('review branch:main with claude'); - expect(branch.type).toBe('auto'); + expect(branch.type).toBe('review'); + if (branch.type === 'review') expect(branch.target).toBe('branch:main'); }); - it('keeps plain multi-engine review text as auto; /review is required', () => { - const r = detectIntent('review with codex gemini'); - expect(r.type).toBe('auto'); + it('dispatches plain multi-engine "review with " from chat', () => { + const r = detectIntent('review with codex claude'); + expect(r.type).toBe('review'); + if (r.type === 'review') expect(r.engineIds).toEqual(['codex', 'claude']); + + // unknown engine token ("gemini" is now "agy") → not a dispatch, stays auto + expect(detectIntent('review with gemini').type).toBe('auto'); }); it('does not short-circuit compound instructions starting with implementation verbs', () => { @@ -477,18 +490,19 @@ describe('Intent Detection — Natural Language', () => { expect(canYouFix.type).not.toBe('review'); }); - it('routes all plain review delegation text through Cesar instead of starting review', () => { + it('dispatches explicit review delegation text from chat (with known engines)', () => { const r = detectIntent('review it with codex'); - expect(r.type).toBe('auto'); + expect(r.type).toBe('review'); - const withAnd = detectIntent('review it with codex and gemini'); - expect(withAnd.type).toBe('auto'); + const withAnd = detectIntent('review it with codex and claude'); + expect(withAnd.type).toBe('review'); + if (withAnd.type === 'review') expect(withAnd.engineIds).toEqual(['codex', 'claude']); - const ask = detectIntent('ask codex and gemini to review it'); - expect(ask.type).toBe('auto'); + const ask = detectIntent('ask codex and claude to review it'); + expect(ask.type).toBe('review'); const canYou = detectIntent('can you review with claude and codex'); - expect(canYou.type).toBe('auto'); + expect(canYou.type).toBe('review'); }); it('keeps collaboration phrases as auto; slash commands are required for buddy flows', () => { @@ -600,3 +614,42 @@ describe('Intent Detection — Edge Cases', () => { } }); }); + +describe('Intent Detection — natural-language review dispatch (no slash)', () => { + it('dispatches "review with " deterministically, target defaults to uncommitted', () => { + const r = detectIntent('review with codex claude agy'); + expect(r.type).toBe('review'); + if (r.type === 'review') { + expect(r.engineIds).toEqual(['codex', 'claude', 'agy']); + expect(r.target).toBeUndefined(); // → uncommitted downstream + } + }); + + it('tolerates a politeness prefix and "and"-joined engines', () => { + const r = detectIntent('can you review with codex claude and agy'); + expect(r.type).toBe('review'); + if (r.type === 'review') { + expect(r.engineIds).toEqual(['codex', 'claude', 'agy']); + } + }); + + it('honours an explicit target in a chat request', () => { + const r = detectIntent('review branch:main with codex'); + expect(r.type).toBe('review'); + if (r.type === 'review') { + expect(r.engineIds).toEqual(['codex']); + expect(r.target).toBe('branch:main'); + } + }); + + it('does NOT hijack prose that merely mentions review (→ Cesar)', () => { + expect(detectIntent('review this code').type).not.toBe('review'); + expect(detectIntent('can you review whether this approach is correct').type).not.toBe('review'); + // "with" but no known engine names → not a dispatch + expect(detectIntent('review with me the options').type).not.toBe('review'); + }); + + it('does NOT hijack compound "fix then review" (→ Cesar handles the sequence)', () => { + expect(detectIntent('fix the bug then review with codex').type).not.toBe('review'); + }); +});