diff --git a/docs/docs/features/agents-and-models.md b/docs/docs/features/agents-and-models.md index 055d86cbd..81096b790 100644 --- a/docs/docs/features/agents-and-models.md +++ b/docs/docs/features/agents-and-models.md @@ -68,7 +68,7 @@ Reasoning flags require Claude Code >= 2.1.68 and Codex CLI >= 0.144.0. Saving a ## Model Labels -Every model has a GitHub label of the form `llm--`. Add one to an issue (together with your trigger label, such as `AI`) to route that issue to the model. The same identity follows the run through the system: task records show the selected agent and model, and branch names include the model identifier for traceability. +Every model has a GitHub label of the form `llm--`. For ordinary issues, add the trigger label (such as `AI`) by itself and ProPR uses the configured default model. Add one model label only when you want an explicit override. The same identity follows the run through the system: task records show the selected agent and model, and branch names include the model identifier for traceability. Adding **several** model labels to one issue fans the work out into one job per model label: each model gets its own run, its own worktree and branch, and its own pull request. Compare the PRs, merge the best one, and close the rest. If a `base-` label is also present, the fan-out is per base × per model. @@ -84,7 +84,7 @@ The same aliases work in PR comments (the `llm-` prefix is optional; the raw cat ``` /switch claude-opus5 # future follow-ups on this PR use this model -/use codex-gpt56-sol # one follow-up with this model +/use codex-gpt56-sol # change this PR's model label /review claude-opus5 codex-gpt56-sol # independent reviews from two models ``` diff --git a/docs/docs/features/pr-commands.md b/docs/docs/features/pr-commands.md index bea863978..c0c9fde73 100644 --- a/docs/docs/features/pr-commands.md +++ b/docs/docs/features/pr-commands.md @@ -23,15 +23,15 @@ To **take over an existing PR** for ongoing work (so that natural follow-up comm | `/fix` | You want to apply a `/review`'s pending suggestions | Yes | [`/fix`](#fix) | | `/merge` | You want the base branch merged into the PR branch | Maybe, if conflicts need resolution | [`/merge`](#merge) | | `/switch ` | You want future PR work to use a different model | No, unless you include follow-up instructions | [`/switch`](#switch) | -| `/use ` | You want one immediate follow-up run with a temporary model | Yes | [`/use`](#use) | +| `/use ` | You want to change the PR's model label | No | [`/use`](#use) | | `/ultrafix` | You want an automated review-fix loop | Yes | [`/ultrafix`](#ultrafix) | ## Syntax Rules - The slash command must be on the first line of the PR comment. A comment with leading blank lines or text before the command is treated as a normal follow-up comment. - Arguments go on the same line as the command (for example `/review llm-claude-opus5` or `/ultrafix goal=8 max=10`). -- Lines below the command become extra instructions for the run. -- Both top-level PR comments and line-level review comments are processed; line-level comments carry their file, line, and diff context to the agent. +- Lines below the command become extra instructions when the command queues work. `/use` ignores trailing text because it only changes a label. +- Both top-level PR comments and line-level review comments are processed; when a command queues work, line-level comments carry their file, line, and diff context to the agent. ## Model IDs @@ -170,14 +170,13 @@ Without instructions, `/switch` only updates the label and makes no code changes ### `/use` -`/use` runs one immediate follow-up task with a temporary model: +`/use` changes the PR's managed model label: ```text /use -Please investigate the flaky test failure and update the PR. ``` -The PR's model label keeps its current value. Later work returns to the PR's configured model unless you use `/switch` or another `/use`. Like `/switch`, `/use` takes one model argument, and the agent sees only your instructions, without the command syntax. +ProPR resolves a supported short alias or full `llm-*` label, removes the PR's other managed model labels, and adds the selected canonical label. All unrelated labels are preserved. `/use` does not queue work; any trailing text is ignored. Post a separate follow-up comment when you want the selected model to do work. ### Choosing A Model @@ -187,7 +186,7 @@ Use routing when: - A model is better suited to the task - The current model is stuck -- You want a one-off second opinion +- You want future work on the PR to use a different model - You need to work around provider capacity or rate limits ## Ultrafix And Branch Updates diff --git a/docs/docs/operations/troubleshooting.md b/docs/docs/operations/troubleshooting.md index aaddd6ba7..3763d4c1f 100644 --- a/docs/docs/operations/troubleshooting.md +++ b/docs/docs/operations/troubleshooting.md @@ -99,7 +99,7 @@ Then check credentials, branch settings, and agent configuration — the usual c Recovery runs through the PR conversation: - Add a clearer follow-up comment with stronger instructions. -- `/switch ` to change the PR's model going forward, or `/use ` for a one-off task with a different model. +- `/use ` to change the PR's model label without queuing work, or `/switch ` when you also need its optional follow-up behavior. - `/review` then `/fix`, or `/ultrafix` for an automated review-fix loop (remove the `ultrafix` PR label to stop it). - Re-run with a smaller scope — see [Work Splitting](../features/work-splitting.md). - Undo a bad commit with `propr task revert owner/repo `, which runs a signed system task (authorized via `SYSTEM_TASK_SECRET`) that resets the branch and force-pushes. diff --git a/docs/docs/tutorials/usage.md b/docs/docs/tutorials/usage.md index e29c179e4..2eb503ec0 100644 --- a/docs/docs/tutorials/usage.md +++ b/docs/docs/tutorials/usage.md @@ -79,7 +79,7 @@ Use slash commands only for specific actions: - `/fix` applies unprocessed AI review comments generated by `/review`. - `/merge` merges the base branch into the PR branch, attempts automatic conflict resolution, and reports back. - `/switch ` changes the PR's model label going forward. -- `/use ` runs one follow-up task with that model without changing the PR's model. +- `/use ` changes the PR's managed model label without queuing work. - `/ultrafix` runs a review-fix loop. Parameters: `goal=`, `max=`, `pause=`, `model=`, for example `/ultrafix goal=9 max=5`. It waits for CI checks and PR inactivity between cycles. The `ultrafix` PR label is the circuit breaker — remove it to stop the loop. See [PR Slash Commands](../features/pr-commands.md). diff --git a/packages/core/src/webhook/commentEventHandler.ts b/packages/core/src/webhook/commentEventHandler.ts index f4a7560e6..335cf9f62 100644 --- a/packages/core/src/webhook/commentEventHandler.ts +++ b/packages/core/src/webhook/commentEventHandler.ts @@ -17,11 +17,12 @@ import { handleMergeCommand } from './mergeConflictDetector.js'; import { parseSlashCommand, buildCommandMeta } from './slashCommandParser.js'; import type { CommandMeta, UltrafixCommandMeta } from './slashCommandParser.js'; import { safeUpdateLabels } from '../utils/github/labelOperations.js'; -import { resolveModelAlias } from '../config/modelAliases.js'; +import { resolveModelAlias, resolveReviewModels } from '../config/modelAliases.js'; import { MODEL_INFO_MAP } from '../config/modelDefinitions.js'; import { getBotUsername } from '../daemon/configLoader.js'; import { AgentRegistry } from '../agents/AgentRegistry.js'; import type { DeliveryDisposition } from '../intake/routingWebSocketProtocol.js'; +import { buildAgentModelLlmLabel, buildDynamicLlmLabel, MAX_GITHUB_LABEL_LENGTH, shortHash } from '@propr/shared'; export interface UltrafixDeps { loadUltrafixRatingGoal: () => Promise; @@ -283,19 +284,11 @@ async function handleSlashCommand(opts: SlashCommandHandlerOptions): Promise 0) { - const resolvedModel = resolveModelAlias(commandMeta.models[0]); - if (!await isKnownOrConfiguredModel(resolvedModel)) { - correlatedLogger.warn({ pullRequestNumber: prNumber, invalidModels: [resolvedModel] }, '/use command contains unrecognized model(s), ignoring'); - return; - } - } - const manualTakeover = await fenceManualCommand({ commandMeta, comment, eventContext, config, correlatedLogger }); correlatedLogger.info({ pullRequestNumber: prNumber, commentId: comment.id, commentAuthor, command: commandMeta.mode }, `/${commandMeta.mode} command detected, enqueuing job`); @@ -325,6 +318,184 @@ async function handleSlashCommand(opts: SlashCommandHandlerOptions): Promise & { commandMeta: CommandMeta & { mode: 'use' } }; + +function buildPrefixedDynamicModelLabel(prefix: string, agentAlias: string, modelId: string): string | null { + const canonicalLabel = `${prefix}${agentAlias}~${modelId}`; + if (canonicalLabel.length <= MAX_GITHUB_LABEL_LENGTH) return canonicalLabel; + + const hash = shortHash(modelId); + const maxAliasLength = Math.max(1, MAX_GITHUB_LABEL_LENGTH - `${prefix}~-x-${hash}`.length); + const sanitizedAlias = agentAlias + .replace(/[^a-zA-Z0-9_.-]/g, '-') + .slice(0, maxAliasLength) + .replace(/[^a-zA-Z0-9]+$/, ''); + const labelAlias = sanitizedAlias || 'agent'.slice(0, maxAliasLength); + const modelPrefixBudget = MAX_GITHUB_LABEL_LENGTH - `${prefix}${labelAlias}~-${hash}`.length; + const fallbackPrefix = 'model'.slice(0, Math.max(1, modelPrefixBudget)); + const modelPrefix = modelId + .replace(/[^a-zA-Z0-9_.-]/g, '-') + .slice(0, Math.max(1, modelPrefixBudget)) + .replace(/[^a-zA-Z0-9]+$/, ''); + const hashedLabel = `${prefix}${labelAlias}~${modelPrefix || fallbackPrefix}-${hash}`; + return hashedLabel.length <= MAX_GITHUB_LABEL_LENGTH ? hashedLabel : null; +} + +function applyModelLabelPrefix(defaultLabel: string, prefix: string, agentAlias: string, modelId: string): string | null { + if (!defaultLabel.startsWith('llm-')) { + return defaultLabel.length <= MAX_GITHUB_LABEL_LENGTH ? defaultLabel : null; + } + + const suffix = defaultLabel.slice('llm-'.length); + if (suffix.includes('~')) { + return buildPrefixedDynamicModelLabel(prefix, agentAlias, modelId); + } + + const staticLabel = `${prefix}${suffix}`; + return staticLabel.length <= MAX_GITHUB_LABEL_LENGTH + ? staticLabel + : buildPrefixedDynamicModelLabel(prefix, agentAlias, modelId); +} + +async function resolveCanonicalModelLabel( + target: string, + modelLabelPattern: string, + correlatedLogger: ReturnType, + prNumber: number, +): Promise { + try { + const modelLabelRegex = new RegExp(modelLabelPattern); + const targetMatch = modelLabelRegex.exec(target); + const routingTarget = targetMatch?.[1] || target; + const [resolution] = await resolveReviewModels([routingTarget]); + const registry = AgentRegistry.getInstance(); + const agent = registry.getAgentByAlias(resolution.agentAlias); + if (!agent) { + correlatedLogger.warn( + { pullRequestNumber: prNumber, target }, + '/use target is unknown, disabled, or unsupported; model label was not changed', + ); + return null; + } + + const modelInfo = MODEL_INFO_MAP[resolution.model]; + const defaultLabel = modelInfo + ? buildAgentModelLlmLabel(agent.config.type, agent.config.alias, modelInfo) + : buildDynamicLlmLabel(agent.config.alias, resolution.model); + const { prefix, derived } = modelLabelPrefix(modelLabelPattern); + if (!derived) { + correlatedLogger.warn( + { pullRequestNumber: prNumber, modelLabelPattern }, + 'Could not derive label prefix from MODEL_LABEL_PATTERN; /use cannot safely select a model label', + ); + return null; + } + + const canonicalLabel = applyModelLabelPrefix(defaultLabel, prefix, agent.config.alias, resolution.model); + if (!canonicalLabel) { + correlatedLogger.error( + { pullRequestNumber: prNumber, modelLabelPattern }, + '/use could not build a canonical model label within GitHub\'s label length limit', + ); + return null; + } + const canonicalMatch = modelLabelRegex.exec(canonicalLabel); + const canonicalRoutingToken = canonicalMatch?.[1]; + if (!canonicalRoutingToken) { + correlatedLogger.error( + { pullRequestNumber: prNumber, canonicalLabel, modelLabelPattern }, + '/use resolved a canonical label that does not match MODEL_LABEL_PATTERN', + ); + return null; + } + + let routedResolution: Awaited>[number]; + try { + [routedResolution] = await resolveReviewModels([canonicalRoutingToken]); + } catch (error) { + correlatedLogger.error( + { pullRequestNumber: prNumber, canonicalLabel, canonicalRoutingToken, error: (error as Error).message }, + '/use resolved a canonical label that cannot route back to the selected model', + ); + return null; + } + if (routedResolution.agentAlias !== resolution.agentAlias || routedResolution.model !== resolution.model) { + correlatedLogger.error( + { + pullRequestNumber: prNumber, + canonicalLabel, + selectedAgentAlias: resolution.agentAlias, + selectedModel: resolution.model, + routedAgentAlias: routedResolution.agentAlias, + routedModel: routedResolution.model, + }, + '/use resolved a canonical label that routes to a different model', + ); + return null; + } + return canonicalLabel; + } catch (error) { + correlatedLogger.warn( + { pullRequestNumber: prNumber, target, error: (error as Error).message }, + '/use target is unknown, disabled, or unsupported; model label was not changed', + ); + return null; + } +} + +async function handleUseCommand(opts: UseCommandOptions): Promise { + const { commandMeta, comment, commentAuthor, eventContext, config, correlatedLogger } = opts; + const { prNumber, owner, repo } = eventContext; + + if (commandMeta.models.length === 0) { + correlatedLogger.warn({ pullRequestNumber: prNumber, commentId: comment.id, commentAuthor }, '/use command requires a model argument, ignoring'); + return; + } + + const modelLabelPattern = config.MODEL_LABEL_PATTERN || '^llm-(.+)$'; + const canonicalLabel = await resolveCanonicalModelLabel(commandMeta.models[0], modelLabelPattern, correlatedLogger, prNumber); + if (!canonicalLabel) return; + + const { prLabels } = await getLivePRBranchAndLabels({ owner, repo, prNumber }); + const canonicalLabelIdentity = canonicalLabel.toLowerCase(); + const existingModelLabels = prLabels.filter(label => label.name.startsWith('llm-')).map(label => label.name); + const labelsToRemove = existingModelLabels.filter(label => label.toLowerCase() !== canonicalLabelIdentity); + const targetPresent = prLabels.some(label => label.name.toLowerCase() === canonicalLabelIdentity); + + if (labelsToRemove.length === 0 && targetPresent) { + correlatedLogger.debug({ pullRequestNumber: prNumber, modelLabel: canonicalLabel }, '/use model label is already active'); + return; + } + + const labels = [ + ...prLabels + .filter(label => !label.name.startsWith('llm-') && label.name.toLowerCase() !== canonicalLabelIdentity) + .map(label => label.name), + canonicalLabel, + ]; + + try { + const octokit = await getAuthenticatedOctokit(); + await octokit.request('PUT /repos/{owner}/{repo}/issues/{issue_number}/labels', { + owner, + repo, + issue_number: prNumber, + labels, + }); + } catch (error) { + correlatedLogger.error( + { pullRequestNumber: prNumber, modelLabel: canonicalLabel, error: (error as Error).message }, + '/use failed to update the PR model label', + ); + return; + } + + correlatedLogger.info( + { pullRequestNumber: prNumber, modelLabel: canonicalLabel }, + '/use updated the PR model label', + ); +} + type SwitchCommandOptions = Omit & { commandMeta: CommandMeta & { mode: 'switch' } }; async function handleSwitchCommand(opts: SwitchCommandOptions): Promise { diff --git a/packages/core/src/webhook/slashCommandParser.ts b/packages/core/src/webhook/slashCommandParser.ts index a13a23389..d059c4d9c 100644 --- a/packages/core/src/webhook/slashCommandParser.ts +++ b/packages/core/src/webhook/slashCommandParser.ts @@ -46,7 +46,7 @@ export interface SwitchCommandMeta { export interface UseCommandMeta { mode: 'use'; - /** Target model labels for single-run override */ + /** Target model label to make active on the PR */ models: string[]; /** Extra instructions from lines below the command */ instructions: string; @@ -120,7 +120,8 @@ function normalizeModelLabel(label: string): string { * For `/fix`: captures everything after `/fix` as instructions. * For `/merge`: returns a simple merge marker. * For `/switch`: extracts single model target and optional instructions. - * For `/use`: extracts single model for one-time override and optional instructions. + * For `/use`: extracts the single model label target. Trailing text is parsed for + * compatibility but ignored by the webhook handler. * For `/ultrafix`: parses positional goal or named key=value arguments. */ export function buildCommandMeta(parsed: ParsedSlashCommand): CommandMeta { diff --git a/src/shared/slashCommandsBlock.ts b/src/shared/slashCommandsBlock.ts index 809a62b62..cfbdc0a19 100644 --- a/src/shared/slashCommandsBlock.ts +++ b/src/shared/slashCommandsBlock.ts @@ -13,7 +13,7 @@ export function buildSlashCommandsBlock(): string { '| `/review` | Request an AI code review | `/review` or `/review claude-sonnet` |', '| `/fix` | Implement fixes for issues found by `/review` | `/fix` or `/fix address the null check issue` |', '| `/switch` | Change the AI model for this PR | `/switch claude-opus` |', - '| `/use` | Override the model for a single follow-up run | `/use claude-sonnet` |', + '| `/use` | Change this PR\'s model label | `/use claude-sonnet` |', '| `/ultrafix` | Loop review→fix cycles until score goal is met | `/ultrafix` or `/ultrafix goal=8 max=10` |', '', '', diff --git a/test/commentEventHandler.switch-use.test.ts b/test/commentEventHandler.switch-use.test.ts index 86d160d88..dc59a1e3e 100644 --- a/test/commentEventHandler.switch-use.test.ts +++ b/test/commentEventHandler.switch-use.test.ts @@ -2,6 +2,7 @@ import { test, mock, describe, beforeEach, after } from 'node:test'; import assert from 'node:assert'; import { createHash } from 'node:crypto'; import type { IssueCommentEvent, Label } from '@octokit/webhooks-types'; +import { buildDynamicLlmLabel, shortHash } from '@propr/shared'; import { createWebhookIssueCommentCreatedEvent, createWebhookPRReviewCommentCreatedEvent, createMockLabel } from './testHelpers.js'; function manualRevisionIdentity(updatedAt: string, body: string, eventType = 'issue_comment'): string { @@ -19,6 +20,18 @@ const mockOctokit = { request: mock.fn(async () => ({ data: {} })), }; +function getLabelReplacementCalls() { + return mockOctokit.request.mock.calls.filter( + (call: { arguments: unknown[] }) => call.arguments[0] === 'PUT /repos/{owner}/{repo}/issues/{issue_number}/labels', + ); +} + +function assertSingleLabelReplacement(labels: string[]) { + const replacementCalls = getLabelReplacementCalls(); + assert.strictEqual(replacementCalls.length, 1); + assert.deepStrictEqual((replacementCalls[0].arguments[1] as { labels: string[] }).labels, labels); +} + // Mock simple-git await mock.module('simple-git', { namedExports: { @@ -106,6 +119,57 @@ await mock.module('../packages/core/src/utils/logger.js', { }, }); +// Keep model resolution isolated from the full agent runtime. This mock must be +// installed before configManager is imported because that graph loads the +// canonical model resolver. +type MockAgent = { + config: { + alias: string; + type: 'claude' | 'codex' | 'antigravity' | 'opencode' | 'vibe'; + enabled: boolean; + supportedModels: string[]; + defaultModel?: string; + }; +}; + +const defaultMockAgents: MockAgent[] = [ + { + config: { + alias: 'claude', + type: 'claude' as const, + enabled: true, + supportedModels: ['claude-opus-5', 'claude-sonnet-5', 'claude-haiku-4-5-20251001'], + defaultModel: 'claude-sonnet-5', + }, + }, + { + config: { + alias: 'codex', + type: 'codex' as const, + enabled: true, + supportedModels: ['gpt-5.6-sol'], + defaultModel: 'gpt-5.6-sol', + }, + }, +]; +let mockAgents = defaultMockAgents; +const mockAgentRegistry = { + ensureInitialized: mock.fn(async () => {}), + getAllAgents: mock.fn(() => mockAgents), + getAgentByAlias: mock.fn((alias: string) => mockAgents.find(agent => agent.config.alias === alias)), + getDefaultAgent: mock.fn(() => mockAgents.find(agent => agent.config.enabled)), +}; +await mock.module('../packages/core/src/agents/AgentRegistry.js', { + namedExports: { + AgentRegistry: class AgentRegistry { + static getInstance() { + return mockAgentRegistry; + } + }, + getAgentRegistry: mock.fn(() => mockAgentRegistry), + }, +}); + // Mock configManager const actualConfigManager = await import('../packages/core/src/config/configManager.js'); await mock.module('../packages/core/src/config/configManager.js', { @@ -122,22 +186,6 @@ await mock.module('../packages/core/src/config/configManager.js', { }, }); -// Keep the model-validation fallback isolated from the full agent runtime. -const mockAgentRegistry = { - ensureInitialized: mock.fn(async () => {}), - getAllAgents: mock.fn(() => []), -}; -await mock.module('../packages/core/src/agents/AgentRegistry.js', { - namedExports: { - AgentRegistry: class AgentRegistry { - static getInstance() { - return mockAgentRegistry; - } - }, - getAgentRegistry: mock.fn(() => mockAgentRegistry), - }, -}); - // Mock commentFilters await mock.module('../packages/core/src/utils/commentFilters.js', { namedExports: { @@ -148,7 +196,12 @@ await mock.module('../packages/core/src/utils/commentFilters.js', { }); // Mock safeUpdateLabels — capture calls for assertions -const mockSafeUpdateLabels = mock.fn(async () => {}); +const mockSafeUpdateLabels = mock.fn(async (_context: unknown, labelsToRemove: string[] = [], labelsToAdd: string[] = []) => ({ + success: true, + removed: labelsToRemove, + added: labelsToAdd, + errors: [], +})); await mock.module('../packages/core/src/utils/github/labelOperations.js', { namedExports: { safeRemoveLabel: mock.fn(async () => true), @@ -182,6 +235,7 @@ await mock.module('../packages/core/src/utils/retryHandler.js', { const { processCommentEvent, handleCommentDeleted, handleCommentEdited, setUltrafixDeps } = await import( '../packages/core/src/webhook/commentEventHandler.js' ); +const { resolveLlmLabel } = await import('../packages/core/src/config/modelAliases.js'); const { closeConnection } = await import('../packages/core/src/db/connection.js'); const { shutdownQueue } = await import('../packages/core/src/queue/taskQueue.js'); const { applyPendingCommentCommandContext } = await import( @@ -204,6 +258,13 @@ setUltrafixDeps({ }); beforeEach(() => { + mockAgents = defaultMockAgents; + mockSafeUpdateLabels.mock.mockImplementation(async (_context: unknown, labelsToRemove: string[] = [], labelsToAdd: string[] = []) => ({ + success: true, + removed: labelsToRemove, + added: labelsToAdd, + errors: [], + })); mockInvalidateAutomaticWork.mock.resetCalls(); mockInvalidateAutomaticWork.mock.mockImplementation(async () => ({ workEpoch: 1, hadAutomaticWork: false })); mockHasAutomaticWork.mock.resetCalls(); @@ -530,6 +591,7 @@ describe('commentEventHandler — /use command', () => { mockOctokit.request.mock.resetCalls(); mockLoggerInstance.info.mock.resetCalls(); mockLoggerInstance.warn.mock.resetCalls(); + mockLoggerInstance.error.mock.resetCalls(); mockActiveJobs = []; mockWaitingJobs = []; mockDelayedJobs = []; @@ -542,38 +604,247 @@ describe('commentEventHandler — /use command', () => { })); }); - test('/use enqueues a job without updating labels', async () => { + test('/use resolves a short alias to its canonical label and preserves unrelated labels', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [ + createMockLabel({ name: 'AI' }), + createMockLabel({ name: 'bug' }), + createMockLabel({ name: 'release:next' }), + createMockLabel({ name: 'llm-claude-sonnet5' }), + ], + }, + })); const event = createPRCommentEvent('/use opus'); const config = createTestConfig(); - await processCommentEvent(event, 'issue_comment', 'corr-10', config); + await processCommentEvent(event, 'issue_comment', 'corr-use-alias', config); - // /use should NOT update labels + assertSingleLabelReplacement(['AI', 'bug', 'release:next', 'llm-claude-opus5']); assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); - // /use SHOULD enqueue a job - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + + test('/use removes all llm-prefixed labels regardless of MODEL_LABEL_PATTERN', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [ + createMockLabel({ name: 'AI' }), + createMockLabel({ name: 'llm-claude-sonnet5' }), + createMockLabel({ name: 'release:next' }), + ], + }, + })); + const event = createPRCommentEvent('/use llm-codex-gpt56-sol'); + const config = createTestConfig({ MODEL_LABEL_PATTERN: '^llm-(codex-.+)$' }); + + await processCommentEvent(event, 'issue_comment', 'corr-use-literal-llm-prefix', config); + + assertSingleLabelReplacement(['AI', 'release:next', 'llm-codex-gpt56-sol']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); - test('/use sets commandMode to "use" in job data', async () => { - const event = createPRCommentEvent('/use sonnet'); + test('/use on a review comment replaces labels from the live PR instead of the stale payload', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [ + createMockLabel({ name: 'AI' }), + createMockLabel({ name: 'llm-claude-sonnet5' }), + ], + }, + })); + const event = createPRReviewCommentEvent('/use opus'); + event.pull_request.labels = [createMockLabel({ name: 'llm-payload-stale' })]; const config = createTestConfig(); - await processCommentEvent(event, 'issue_comment', 'corr-11', config); + await processCommentEvent(event, 'pull_request_review_comment', 'corr-use-review-live-labels', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - assert.strictEqual(jobData.commandMode, 'use'); + assert.strictEqual(mockOctokit.request.mock.callCount(), 2); + assert.strictEqual( + mockOctokit.request.mock.calls[0].arguments[0], + 'GET /repos/{owner}/{repo}/pulls/{pull_number}', + ); + const replacementCalls = getLabelReplacementCalls(); + assert.strictEqual(replacementCalls.length, 1); + assert.deepStrictEqual((replacementCalls[0].arguments[1] as { labels: string[] }).labels, [ + 'AI', + 'llm-claude-opus5', + ]); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); - test('/use resolves model alias for LLM override in job data', async () => { - const event = createPRCommentEvent('/use opus'); + test('/use accepts a full model label and selects the same canonical label', async () => { + const event = createPRCommentEvent('/use llm-codex-gpt56-sol'); const config = createTestConfig(); - await processCommentEvent(event, 'issue_comment', 'corr-12', config); + await processCommentEvent(event, 'issue_comment', 'corr-use-full-label', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - assert.strictEqual(jobData.llm, 'claude-opus-5'); + assertSingleLabelReplacement(['llm-codex-gpt56-sol']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + + test('/use accepts an active full model label with a configured prefix as an idempotent no-op', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [createMockLabel({ name: 'ai-model-codex-gpt56-sol' })], + }, + })); + const event = createPRCommentEvent('/use ai-model-codex-gpt56-sol'); + const config = createTestConfig({ MODEL_LABEL_PATTERN: '^ai-model-(.+)$' }); + + await processCommentEvent(event, 'issue_comment', 'corr-use-custom-prefix-full-label', config); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + + test('/use resolves a configured agent alias to its default model label', async () => { + mockAgents = defaultMockAgents.map(agent => agent.config.type === 'codex' + ? { config: { ...agent.config, alias: 'custom-codex' } } + : agent); + const event = createPRCommentEvent('/use custom-codex'); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-agent-alias', config); + + assertSingleLabelReplacement(['llm-custom-codex-gpt56-sol']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + + test('/use resolves supported model IDs case-insensitively', async () => { + const event = createPRCommentEvent('/use GPT-5.6-SOL'); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-model-id-case', config); + + assertSingleLabelReplacement(['llm-codex-gpt56-sol']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + + test('/use round-trips a dynamic full label through the canonical resolver', async () => { + const dynamicModel = 'opencode-openai/gpt-5.5'; + const dynamicLabel = buildDynamicLlmLabel('custom-opencode', dynamicModel); + mockAgents = [{ + config: { + alias: 'custom-opencode', + type: 'opencode', + enabled: true, + supportedModels: [dynamicModel], + defaultModel: dynamicModel, + }, + }]; + const event = createPRCommentEvent(`/use ${dynamicLabel}`); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-dynamic-label', config); + + assertSingleLabelReplacement([dynamicLabel]); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + + test('/use round-trips a hashed dynamic full label through the canonical resolver', async () => { + const dynamicModel = 'opencode-provider-with-an-extremely-long-name/model-with-an-extremely-long-name'; + const dynamicLabel = buildDynamicLlmLabel('custom-opencode', dynamicModel); + assert.ok(dynamicLabel.length <= 50); + mockAgents = [{ + config: { + alias: 'custom-opencode', + type: 'opencode', + enabled: true, + supportedModels: [dynamicModel], + defaultModel: dynamicModel, + }, + }]; + const event = createPRCommentEvent(`/use ${dynamicLabel}`); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-hashed-label', config); + + assertSingleLabelReplacement([dynamicLabel]); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + + test('/use budgets a hashed dynamic label against a longer custom prefix', async () => { + const dynamicModel = 'opencode-provider-with-an-extremely-long-name/model-with-an-extremely-long-name'; + const dynamicLabel = buildDynamicLlmLabel('custom-opencode', dynamicModel); + mockAgents = [{ + config: { + alias: 'custom-opencode', + type: 'opencode', + enabled: true, + supportedModels: [dynamicModel], + defaultModel: dynamicModel, + }, + }]; + const event = createPRCommentEvent(`/use ${dynamicLabel}`); + const config = createTestConfig({ MODEL_LABEL_PATTERN: '^ai-model-(.+)$' }); + + await processCommentEvent(event, 'issue_comment', 'corr-use-custom-prefix-hashed-label', config); + + const replacementCalls = getLabelReplacementCalls(); + assert.strictEqual(replacementCalls.length, 1); + const canonicalLabel = ((replacementCalls[0].arguments[1] as { labels: string[] }).labels)[0]; + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.ok(canonicalLabel.length <= 50, `Label should fit in 50 chars, got ${canonicalLabel.length}`); + assert.match(canonicalLabel, /^ai-model-(.+)$/); + assert.ok(canonicalLabel.endsWith(`-${shortHash(dynamicModel)}`), 'Expected the stable model hash to be preserved'); + + const routingToken = canonicalLabel.match(/^ai-model-(.+)$/)?.[1]; + assert.ok(routingToken); + const roundTrip = await resolveLlmLabel(routingToken); + assert.deepStrictEqual(roundTrip, { agentAlias: 'custom-opencode', model: dynamicModel }); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + + test('/use rejects a generated label with an ambiguous truncated agent alias', async () => { + const dynamicModel = 'opencode-openai/gpt-5.5'; + const sharedAliasPrefix = 'custom-opencode-agent-with-a-shared-prefix-'; + const selectedAlias = `${sharedAliasPrefix}alpha`; + const collidingAlias = `${sharedAliasPrefix}beta`; + assert.strictEqual( + buildDynamicLlmLabel(selectedAlias, dynamicModel), + buildDynamicLlmLabel(collidingAlias, dynamicModel), + ); + mockAgents = [ + { + config: { + alias: selectedAlias, + type: 'opencode', + enabled: true, + supportedModels: [dynamicModel], + defaultModel: dynamicModel, + }, + }, + { + config: { + alias: collidingAlias, + type: 'opencode', + enabled: true, + supportedModels: [dynamicModel], + defaultModel: dynamicModel, + }, + }, + ]; + const event = createPRCommentEvent(`/use ${selectedAlias}`); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-ambiguous-truncated-alias', config); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockOctokit.request.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); test('/use without model argument warns and returns early', async () => { @@ -592,58 +863,69 @@ describe('commentEventHandler — /use command', () => { assert.ok(useWarn, 'Expected a warning about missing model argument'); }); - test('/use with instructions includes them in job data', async () => { - const event = createPRCommentEvent('/use haiku\nFix the login bug'); + test('/use with trailing text only changes the label', async () => { + const event = createPRCommentEvent('/use sonnet extra inline text\nRefactor the utils'); const config = createTestConfig(); - await processCommentEvent(event, 'issue_comment', 'corr-13', config); + await processCommentEvent(event, 'issue_comment', 'corr-use-trailing-text', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - assert.strictEqual(jobData.commandInstructions, 'Fix the login bug'); + assertSingleLabelReplacement(['llm-claude-sonnet5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); - test('/use with instructions passes stripped comment body without command text', async () => { - const event = createPRCommentEvent('/use sonnet\nRefactor the utils'); + test('/use selecting the active label is an idempotent no-op', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [createMockLabel({ name: 'AI' }), createMockLabel({ name: 'llm-claude-opus5' })], + }, + })); + const event = createPRCommentEvent('/use opus'); const config = createTestConfig(); - await processCommentEvent(event, 'issue_comment', 'corr-use-body', config); + await processCommentEvent(event, 'issue_comment', 'corr-use-idempotent', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - const comments = jobData.comments as Array<{ body: string }>; - // /use body is stripped like /switch — only user instructions remain - assert.ok(comments.length > 0); - assert.strictEqual(comments[0].body, 'Refactor the utils'); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); - test('/use with llm- prefixed argument strips prefix before resolving', async () => { - const event = createPRCommentEvent('/use llm-opus'); + test('/use selecting a differently-cased active label is an idempotent no-op', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [createMockLabel({ name: 'llm-Claude-Opus5' })], + }, + })); + const event = createPRCommentEvent('/use opus'); const config = createTestConfig(); - await processCommentEvent(event, 'issue_comment', 'corr-use-llm-prefix', config); + await processCommentEvent(event, 'issue_comment', 'corr-use-idempotent-case-insensitive', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - // "llm-opus" is normalized before resolving the current alias. - assert.strictEqual(jobData.llm, 'claude-opus-5'); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); - test('/use without instructions still enqueues a job with empty body', async () => { + test('/use removes stale managed labels when the target label is already present', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [ + createMockLabel({ name: 'llm-claude-opus5' }), + createMockLabel({ name: 'llm-claude-sonnet5' }), + createMockLabel({ name: 'reviewed' }), + ], + }, + })); const event = createPRCommentEvent('/use opus'); const config = createTestConfig(); - await processCommentEvent(event, 'issue_comment', 'corr-use-noinstructions', config); + await processCommentEvent(event, 'issue_comment', 'corr-use-converge', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - assert.strictEqual(jobData.commandMode, 'use'); - // commandInstructions should be empty - assert.strictEqual(jobData.commandInstructions, ''); - // The queued comment body must NOT contain the slash command text - const comments = jobData.comments as Array<{ body: string }>; - assert.ok(comments.length > 0, 'Expected at least one comment in job data'); - assert.strictEqual(comments[0].body, '', 'Bare /use should queue an empty body, not the command text'); + assertSingleLabelReplacement(['reviewed', 'llm-claude-opus5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); test('/use with unrecognized model warns and returns early', async () => { @@ -656,26 +938,101 @@ describe('commentEventHandler — /use command', () => { assert.strictEqual(mockQueueAdd.mock.callCount(), 0); const warnCalls = mockLoggerInstance.warn.mock.calls; const invalidWarn = warnCalls.find( - (c: { arguments: unknown[] }) => typeof c.arguments[1] === 'string' && c.arguments[1].includes('unrecognized model') + (c: { arguments: unknown[] }) => typeof c.arguments[1] === 'string' && c.arguments[1].includes('unknown, disabled, or unsupported') ); - assert.ok(invalidWarn, 'Expected a warning about unrecognized model'); + assert.ok(invalidWarn, 'Expected a warning about an unknown model'); }); - test('/use with extra models logs warning but uses first model', async () => { - const event = createPRCommentEvent('/use opus sonnet'); + test('/use rejects a target supported only by a disabled agent', async () => { + mockAgents = defaultMockAgents.map(agent => agent.config.type === 'claude' + ? { config: { ...agent.config, enabled: false } } + : agent); + const event = createPRCommentEvent('/use opus'); const config = createTestConfig(); - await processCommentEvent(event, 'issue_comment', 'corr-use-extra', config); + await processCommentEvent(event, 'issue_comment', 'corr-use-disabled', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - assert.strictEqual(jobData.llm, 'claude-opus-5'); - // Warning should be logged - const warnCalls = mockLoggerInstance.warn.mock.calls; - const extraWarn = warnCalls.find( - (c: { arguments: unknown[] }) => typeof c.arguments[1] === 'string' && c.arguments[1].includes('extra arguments were ignored') + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + + test('/use label-update failure does not enqueue work or claim success', async () => { + mockOctokit.request.mock.mockImplementation(async (endpoint: string) => { + if (endpoint === 'PUT /repos/{owner}/{repo}/issues/{issue_number}/labels') { + throw new Error('GitHub unavailable'); + } + return { data: { head: { ref: 'feature-branch' }, labels: [] } }; + }); + const event = createPRCommentEvent('/use opus'); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-update-failure', config); + + assert.strictEqual(getLabelReplacementCalls().length, 1); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + const successLog = mockLoggerInstance.info.mock.calls.find( + (call: { arguments: unknown[] }) => call.arguments[1] === '/use updated the PR model label', ); - assert.ok(extraWarn, 'Expected a warning about extra arguments'); + assert.strictEqual(successLog, undefined); + }); + + test('/use replacement failure does not remove the existing managed label', async () => { + mockOctokit.request.mock.mockImplementation(async (endpoint: string) => { + if (endpoint === 'PUT /repos/{owner}/{repo}/issues/{issue_number}/labels') { + throw new Error('Failed to replace labels'); + } + return { + data: { + head: { ref: 'feature-branch' }, + labels: [createMockLabel({ name: 'llm-claude-sonnet5' })], + }, + }; + }); + const event = createPRCommentEvent('/use opus'); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-add-failure-preserves-old', config); + + assertSingleLabelReplacement(['llm-claude-opus5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual( + mockOctokit.request.mock.calls.some( + (call: { arguments: unknown[] }) => call.arguments[0] === 'DELETE /repos/{owner}/{repo}/issues/{issue_number}/labels/{name}', + ), + false, + ); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); + const failureLog = mockLoggerInstance.error.mock.calls.find( + (call: { arguments: unknown[] }) => call.arguments[1] === '/use failed to update the PR model label', + ); + assert.ok(failureLog, 'Expected the failed target addition to be logged'); + const successLog = mockLoggerInstance.info.mock.calls.find( + (call: { arguments: unknown[] }) => call.arguments[1] === '/use updated the PR model label', + ); + assert.strictEqual(successLog, undefined); + }); + + test('/use converges multiple managed labels to only the selected label in one replacement', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [ + createMockLabel({ name: 'AI' }), + createMockLabel({ name: 'llm-claude-sonnet5' }), + createMockLabel({ name: 'llm-codex-gpt56-sol' }), + ], + }, + })); + const event = createPRCommentEvent('/use opus'); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-single-managed-label', config); + + assertSingleLabelReplacement(['AI', 'llm-claude-opus5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); }); @@ -716,21 +1073,16 @@ describe('commentEventHandler — commandMode serialization in job data', () => assert.strictEqual(jobData.commandInstructions, 'Do a review'); }); - test('/use job has commandMode "use" and commandMeta with resolved model', async () => { + test('/use does not serialize command or model fields into a job', async () => { const event = createPRCommentEvent('/use haiku\nSummarize changes'); const config = createTestConfig(); await processCommentEvent(event, 'issue_comment', 'corr-mode-use', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - assert.strictEqual(jobData.commandMode, 'use'); - const meta = jobData.commandMeta as { mode: string; models: string[]; instructions: string }; - assert.strictEqual(meta.mode, 'use'); - assert.deepStrictEqual(meta.models, ['haiku']); - assert.strictEqual(jobData.commandInstructions, 'Summarize changes'); - // LLM should be resolved from /use command - assert.strictEqual(jobData.llm, 'claude-haiku-4-5-20251001'); + assertSingleLabelReplacement(['llm-claude-haiku']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); test('/switch follow-up job does not include requestedModels (only /review uses that)', async () => { @@ -744,17 +1096,16 @@ describe('commentEventHandler — commandMode serialization in job data', () => assert.strictEqual(jobData.requestedModels, undefined); }); - test('/use job preserves its explicit model as requestedModels', async () => { + test('/use does not persist requestedModels or command context', async () => { const event = createPRCommentEvent('/use sonnet\nDo something'); const config = createTestConfig(); await processCommentEvent(event, 'issue_comment', 'corr-use-no-req', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - assert.deepStrictEqual(jobData.requestedModels, ['claude-sonnet-5']); - assert.strictEqual(jobData.commandCommentCreatedAt, event.comment.created_at); - assert.strictEqual(jobData.commandCommentType, 'issue'); + assertSingleLabelReplacement(['llm-claude-sonnet5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); test('/review job includes requestedModels from command args', async () => { @@ -833,14 +1184,17 @@ describe('commentEventHandler — slash command dedup protection', () => { const event = createPRCommentEvent('/use opus'); const config = createTestConfig(); - // First delivery — should enqueue + // First delivery changes the label without enqueuing. await processCommentEvent(event, 'issue_comment', 'corr-dedup-1', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); + assertSingleLabelReplacement(['llm-claude-opus5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); // Simulate redelivery — same event, same comment id await processCommentEvent(event, 'issue_comment', 'corr-dedup-2', config); - // Should NOT enqueue a second job - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); + assert.strictEqual(getLabelReplacementCalls().length, 1); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); test('redelivered /switch webhook is skipped and labels are not mutated again', async () => { @@ -888,7 +1242,7 @@ describe('commentEventHandler — slash command batching/concurrency guard', () })); }); - test('/use is batched when an existing job is active for the same PR', async () => { + test('/use only changes the label when an existing job is active for the same PR', async () => { // Simulate an active job for PR 42 mockActiveJobs = [{ name: 'processPullRequestComment', @@ -900,15 +1254,10 @@ describe('commentEventHandler — slash command batching/concurrency guard', () await processCommentEvent(event, 'issue_comment', 'corr-batch-1', config); - // Should NOT enqueue a new job + assertSingleLabelReplacement(['llm-claude-opus5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); - // Should store comment for batch via rpush - assert.strictEqual(config.redisClient.rpush.mock.callCount(), 1); - const pendingComment = JSON.parse(config.redisClient.rpush.mock.calls[0].arguments[1] as string) as Record; - assert.strictEqual(pendingComment.body, 'Fix the bug'); - assert.strictEqual(pendingComment.commandMode, 'use'); - assert.strictEqual(pendingComment.commandInstructions, 'Fix the bug'); - assert.strictEqual(pendingComment.llmOverride, 'claude-opus-5'); + assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); test('/switch with instructions is batched when an existing job is active', async () => { @@ -934,14 +1283,16 @@ describe('commentEventHandler — slash command batching/concurrency guard', () assert.strictEqual(pendingComment.llmOverride, 'claude-opus-5'); }); - test('/use enqueues normally when no existing job is active', async () => { + test('/use changes only the label when no existing job is active', async () => { // No active jobs (default) const event = createPRCommentEvent('/use opus'); const config = createTestConfig(); await processCommentEvent(event, 'issue_comment', 'corr-batch-3', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); + assertSingleLabelReplacement(['llm-claude-opus5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); @@ -1156,7 +1507,7 @@ describe('commentEventHandler — slash command batching/concurrency guard', () assert.strictEqual(pendingComment.commandMode, 'review'); }); - test('batched slash commands on review comments preserve code-review context', async () => { + test('/use on a review comment changes only the label and ignores review context', async () => { mockActiveJobs = [{ name: 'processPullRequestComment', data: { pullRequestNumber: 42, repoOwner: 'testowner', repoName: 'testrepo' }, @@ -1167,17 +1518,10 @@ describe('commentEventHandler — slash command batching/concurrency guard', () await processCommentEvent(event, 'pull_request_review_comment', 'corr-batch-review', config); + assertSingleLabelReplacement(['llm-claude-opus5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); - assert.strictEqual(config.redisClient.rpush.mock.callCount(), 1); - const pendingComment = JSON.parse(config.redisClient.rpush.mock.calls[0].arguments[1] as string) as Record; - assert.strictEqual(pendingComment.type, 'review'); - assert.strictEqual(pendingComment.createdAt, event.comment.created_at); - assert.strictEqual(pendingComment.hasCodeContext, true); - assert.match(pendingComment.body as string, /Please fix this line/); - assert.match(pendingComment.body as string, /--- Review Comment Context ---/); - assert.match(pendingComment.body as string, /File: src\/auth\.ts/); - assert.match(pendingComment.body as string, /Line: 27/); - assert.match(pendingComment.body as string, /@@ -1,5 \+1,10 @@/); + assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); }); diff --git a/test/slashCommandsBlock.test.ts b/test/slashCommandsBlock.test.ts index e1cb4d7c0..88e562191 100644 --- a/test/slashCommandsBlock.test.ts +++ b/test/slashCommandsBlock.test.ts @@ -49,10 +49,10 @@ describe('buildSlashCommandsBlock', () => { assert.ok(switchLine.toLowerCase().includes('change') || switchLine.toLowerCase().includes('model')); }); - test('/use description mentions single or override', () => { + test('/use description says it changes the PR model label', () => { const result = buildSlashCommandsBlock(); const useLine = result.split('\n').find(l => l.includes('`/use`')); assert.ok(useLine); - assert.ok(useLine.toLowerCase().includes('single') || useLine.toLowerCase().includes('override')); + assert.ok(useLine.toLowerCase().includes('model label')); }); });