From 540baccffc720ea6720a0206f1418ab98b8b17c9 Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:07:51 +0000 Subject: [PATCH 1/7] fix(ai): Resolve issue #1905 - Make /use change only the PR model label Implemented by ProPR AI using gpt-5.6-sol model. Implementation completed successfully. --- docs/docs/features/agents-and-models.md | 4 +- docs/docs/features/pr-commands.md | 13 +- docs/docs/operations/troubleshooting.md | 2 +- docs/docs/tutorials/usage.md | 2 +- .../core/src/webhook/commentEventHandler.ts | 117 +++++++- .../core/src/webhook/slashCommandParser.ts | 5 +- src/shared/slashCommandsBlock.ts | 2 +- test/commentEventHandler.switch-use.test.ts | 264 ++++++++++-------- test/slashCommandsBlock.test.ts | 4 +- 9 files changed, 269 insertions(+), 144 deletions(-) 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..35998a449 100644 --- a/packages/core/src/webhook/commentEventHandler.ts +++ b/packages/core/src/webhook/commentEventHandler.ts @@ -22,6 +22,7 @@ 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 } 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,110 @@ async function handleSlashCommand(opts: SlashCommandHandlerOptions): Promise & { commandMeta: CommandMeta & { mode: 'use' } }; + +async function resolveCanonicalModelLabel( + target: string, + modelLabelPattern: string, + correlatedLogger: ReturnType, + prNumber: number, +): Promise { + try { + const registry = AgentRegistry.getInstance(); + await registry.ensureInitialized(); + const resolvedModel = resolveModelAlias(target); + const enabledAgents = registry.getAllAgents().filter(agent => agent.config.enabled); + const explicitlyNamedAgent = enabledAgents.find(agent => + target.toLowerCase().startsWith(`${agent.config.alias.toLowerCase()}-`) + || target.toLowerCase().startsWith(`${agent.config.alias.toLowerCase()}~`) + ); + const agent = explicitlyNamedAgent?.config.supportedModels.some(model => model.toLowerCase() === resolvedModel.toLowerCase()) + ? explicitlyNamedAgent + : enabledAgents.find(candidate => candidate.config.supportedModels.some( + model => model.toLowerCase() === resolvedModel.toLowerCase() + )); + if (!agent) { + correlatedLogger.warn( + { pullRequestNumber: prNumber, target, resolvedModel }, + '/use target is unknown, disabled, or unsupported; model label was not changed', + ); + return null; + } + + const modelInfo = MODEL_INFO_MAP[resolvedModel]; + const defaultLabel = modelInfo + ? buildAgentModelLlmLabel(agent.config.type, agent.config.alias, modelInfo) + : buildDynamicLlmLabel(agent.config.alias, resolvedModel); + 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 = defaultLabel.startsWith('llm-') + ? `${prefix}${defaultLabel.slice('llm-'.length)}` + : defaultLabel; + if (!new RegExp(modelLabelPattern).test(canonicalLabel)) { + correlatedLogger.error( + { pullRequestNumber: prNumber, canonicalLabel, modelLabelPattern }, + '/use resolved a canonical label that does not match MODEL_LABEL_PATTERN', + ); + 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, payload, config, correlatedLogger } = opts; + const { eventType, 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 getPRBranchAndLabels(eventType, payload, { owner, repo, prNumber }); + const modelLabelRegex = new RegExp(modelLabelPattern); + const existingModelLabels = prLabels.filter(label => modelLabelRegex.test(label.name)).map(label => label.name); + const labelsToRemove = existingModelLabels.filter(label => label !== canonicalLabel); + const labelsToAdd = existingModelLabels.includes(canonicalLabel) ? [] : [canonicalLabel]; + + if (labelsToRemove.length === 0 && labelsToAdd.length === 0) { + correlatedLogger.debug({ pullRequestNumber: prNumber, modelLabel: canonicalLabel }, '/use model label is already active'); + return; + } + + const octokit = await getAuthenticatedOctokit(); + const update = await safeUpdateLabels( + { octokit, owner, repo, issueNumber: prNumber, logger: correlatedLogger }, + labelsToRemove, + labelsToAdd, + ); + if (!update.success) { + correlatedLogger.error( + { pullRequestNumber: prNumber, modelLabel: canonicalLabel, errors: update.errors }, + '/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..524907f48 100644 --- a/test/commentEventHandler.switch-use.test.ts +++ b/test/commentEventHandler.switch-use.test.ts @@ -123,9 +123,32 @@ await mock.module('../packages/core/src/config/configManager.js', { }); // Keep the model-validation fallback isolated from the full agent runtime. +const defaultMockAgents = [ + { + 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(() => []), + 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: { @@ -148,7 +171,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), @@ -204,6 +232,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(); @@ -542,38 +577,39 @@ 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); - - // /use should NOT update labels - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); - // /use SHOULD enqueue a job - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - }); - - test('/use sets commandMode to "use" in job data', async () => { - const event = createPRCommentEvent('/use sonnet'); - const config = createTestConfig(); - - await processCommentEvent(event, 'issue_comment', 'corr-11', config); + await processCommentEvent(event, 'issue_comment', 'corr-use-alias', config); - assert.strictEqual(mockQueueAdd.mock.callCount(), 1); - const jobData = mockQueueAdd.mock.calls[0].arguments[1] as Record; - assert.strictEqual(jobData.commandMode, 'use'); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], ['llm-claude-sonnet5']); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-opus5']); + assert.strictEqual(mockQueueAdd.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'); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-codex-gpt56-sol']); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); test('/use without model argument warns and returns early', async () => { @@ -592,58 +628,54 @@ 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'); - const config = createTestConfig(); - - await processCommentEvent(event, 'issue_comment', 'corr-13', 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'); - }); - - test('/use with instructions passes stripped comment body without command text', async () => { - const event = createPRCommentEvent('/use sonnet\nRefactor the utils'); + 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-use-body', 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; - 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(), 1); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-sonnet5']); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); - test('/use with llm- prefixed argument strips prefix before resolving', async () => { - const event = createPRCommentEvent('/use llm-opus'); + 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-llm-prefix', 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; - // "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'); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], ['llm-claude-sonnet5']); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], []); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); test('/use with unrecognized model warns and returns early', async () => { @@ -656,26 +688,42 @@ 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 () => { + mockSafeUpdateLabels.mock.mockImplementationOnce(async () => ({ + success: false, + removed: [], + added: [], + errors: ['GitHub unavailable'], + })); + const event = createPRCommentEvent('/use opus'); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-update-failure', config); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + 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); }); }); @@ -716,21 +764,15 @@ 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'); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + 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 +786,15 @@ 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'); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + 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 +873,15 @@ 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); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + 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(mockSafeUpdateLabels.mock.callCount(), 1); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); test('redelivered /switch webhook is skipped and labels are not mutated again', async () => { @@ -888,7 +929,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 +941,9 @@ describe('commentEventHandler — slash command batching/concurrency guard', () await processCommentEvent(event, 'issue_comment', 'corr-batch-1', config); - // Should NOT enqueue a new job + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); 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 +969,15 @@ 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); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); @@ -1156,7 +1192,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 +1203,9 @@ describe('commentEventHandler — slash command batching/concurrency guard', () await processCommentEvent(event, 'pull_request_review_comment', 'corr-batch-review', config); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); 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')); }); }); From 28dab91f3e7538efb8ad693fd34d6a326f151130 Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:16:06 +0000 Subject: [PATCH 2/7] feat(ai): Implemented the narrow `/use` correction on the exact PR head. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implemented the narrow `/use` correction on the exact PR head. - Replaced manual target matching with `resolveReviewModels`, ProPR’s canonical enabled agent/model resolver. - Builds canonical static or dynamic labels from the resolved pair. - Preserves scoped `MODEL_LABEL_PATTERN` replacement behavior. - Added regressions for configured aliases, case-insensitive model IDs, dynamic labels, and hashed full labels. - Left `/switch`, queues, persistence, pending comments, retries, and task state untouched. Validation: - Focused suite: 61/61 passed. - Hashed-label suite: 3/3 passed. - Typecheck passed. - No commit created. PR: #1909 Comment by: @integry (ID: 5298204684) Model: gpt-5.6-sol --- .../core/src/webhook/commentEventHandler.ts | 22 +--- test/commentEventHandler.switch-use.test.ts | 117 +++++++++++++++--- 2 files changed, 106 insertions(+), 33 deletions(-) diff --git a/packages/core/src/webhook/commentEventHandler.ts b/packages/core/src/webhook/commentEventHandler.ts index 35998a449..5ebebce0b 100644 --- a/packages/core/src/webhook/commentEventHandler.ts +++ b/packages/core/src/webhook/commentEventHandler.ts @@ -17,7 +17,7 @@ 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'; @@ -327,31 +327,21 @@ async function resolveCanonicalModelLabel( prNumber: number, ): Promise { try { + const [resolution] = await resolveReviewModels([target]); const registry = AgentRegistry.getInstance(); - await registry.ensureInitialized(); - const resolvedModel = resolveModelAlias(target); - const enabledAgents = registry.getAllAgents().filter(agent => agent.config.enabled); - const explicitlyNamedAgent = enabledAgents.find(agent => - target.toLowerCase().startsWith(`${agent.config.alias.toLowerCase()}-`) - || target.toLowerCase().startsWith(`${agent.config.alias.toLowerCase()}~`) - ); - const agent = explicitlyNamedAgent?.config.supportedModels.some(model => model.toLowerCase() === resolvedModel.toLowerCase()) - ? explicitlyNamedAgent - : enabledAgents.find(candidate => candidate.config.supportedModels.some( - model => model.toLowerCase() === resolvedModel.toLowerCase() - )); + const agent = registry.getAgentByAlias(resolution.agentAlias); if (!agent) { correlatedLogger.warn( - { pullRequestNumber: prNumber, target, resolvedModel }, + { pullRequestNumber: prNumber, target }, '/use target is unknown, disabled, or unsupported; model label was not changed', ); return null; } - const modelInfo = MODEL_INFO_MAP[resolvedModel]; + const modelInfo = MODEL_INFO_MAP[resolution.model]; const defaultLabel = modelInfo ? buildAgentModelLlmLabel(agent.config.type, agent.config.alias, modelInfo) - : buildDynamicLlmLabel(agent.config.alias, resolvedModel); + : buildDynamicLlmLabel(agent.config.alias, resolution.model); const { prefix, derived } = modelLabelPrefix(modelLabelPattern); if (!derived) { correlatedLogger.warn( diff --git a/test/commentEventHandler.switch-use.test.ts b/test/commentEventHandler.switch-use.test.ts index 524907f48..c1c69140a 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 } from '@propr/shared'; import { createWebhookIssueCommentCreatedEvent, createWebhookPRReviewCommentCreatedEvent, createMockLabel } from './testHelpers.js'; function manualRevisionIdentity(updatedAt: string, body: string, eventType = 'issue_comment'): string { @@ -106,24 +107,20 @@ await mock.module('../packages/core/src/utils/logger.js', { }, }); -// Mock configManager -const actualConfigManager = await import('../packages/core/src/config/configManager.js'); -await mock.module('../packages/core/src/config/configManager.js', { - namedExports: { - ...actualConfigManager, - loadFollowupIgnoreKeywords: mock.fn(async () => []), - loadMonitoredRepos: mock.fn(async () => []), - loadAiPrimaryTag: mock.fn(async () => 'AI'), - loadPrimaryProcessingLabels: mock.fn(async () => ['AI']), - loadSettings: mock.fn(async () => ({})), - loadAgentTankSettings: mock.fn(async () => ({})), - getConfig: mock.fn(async () => null), - saveConfig: mock.fn(async () => true), - }, -}); +// 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; + }; +}; -// Keep the model-validation fallback isolated from the full agent runtime. -const defaultMockAgents = [ +const defaultMockAgents: MockAgent[] = [ { config: { alias: 'claude', @@ -161,6 +158,22 @@ await mock.module('../packages/core/src/agents/AgentRegistry.js', { }, }); +// Mock configManager +const actualConfigManager = await import('../packages/core/src/config/configManager.js'); +await mock.module('../packages/core/src/config/configManager.js', { + namedExports: { + ...actualConfigManager, + loadFollowupIgnoreKeywords: mock.fn(async () => []), + loadMonitoredRepos: mock.fn(async () => []), + loadAiPrimaryTag: mock.fn(async () => 'AI'), + loadPrimaryProcessingLabels: mock.fn(async () => ['AI']), + loadSettings: mock.fn(async () => ({})), + loadAgentTankSettings: mock.fn(async () => ({})), + getConfig: mock.fn(async () => null), + saveConfig: mock.fn(async () => true), + }, +}); + // Mock commentFilters await mock.module('../packages/core/src/utils/commentFilters.js', { namedExports: { @@ -612,6 +625,76 @@ describe('commentEventHandler — /use command', () => { 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); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-custom-codex-gpt56-sol']); + 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); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-codex-gpt56-sol']); + 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); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], [dynamicLabel]); + 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); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], [dynamicLabel]); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + test('/use without model argument warns and returns early', async () => { const event = createPRCommentEvent('/use'); const config = createTestConfig(); From 5899597e67c6cf61f37b34c0e940adb35873a1ab Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:31:15 +0000 Subject: [PATCH 3/7] feat(ai): Implemented the narrow `/use` correction on the exact PR head. Implemented the narrow `/use` correction on the exact PR head. - `/use` now always fetches live PR labels, including review-comment events. - Missing targets are added before stale managed labels are removed. - Failed target additions abort cleanup, preserving existing managed labels. - Target-present behavior removes only stale labels. - Failure logging, idempotency, and no-queue/pending-state behavior remain intact. - Added focused regressions for stale review payloads and failed-add preservation. Changed: - [commentEventHandler.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T21-25-35/packages/core/src/webhook/commentEventHandler.ts) - [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T21-25-35/test/commentEventHandler.switch-use.test.ts) Validation: - Focused suite: 63/63 passed - Typecheck passed - `git diff --check` passed - No commit created PR: #1909 Comment by: @integry (ID: 5298323060) Model: gpt-5.6-sol --- .../core/src/webhook/commentEventHandler.ts | 48 +++++++----- test/commentEventHandler.switch-use.test.ts | 74 ++++++++++++++++++- 2 files changed, 101 insertions(+), 21 deletions(-) diff --git a/packages/core/src/webhook/commentEventHandler.ts b/packages/core/src/webhook/commentEventHandler.ts index 5ebebce0b..05dffe899 100644 --- a/packages/core/src/webhook/commentEventHandler.ts +++ b/packages/core/src/webhook/commentEventHandler.ts @@ -372,8 +372,8 @@ async function resolveCanonicalModelLabel( } async function handleUseCommand(opts: UseCommandOptions): Promise { - const { commandMeta, comment, commentAuthor, eventContext, payload, config, correlatedLogger } = opts; - const { eventType, prNumber, owner, repo } = eventContext; + 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'); @@ -384,32 +384,46 @@ async function handleUseCommand(opts: UseCommandOptions): Promise { const canonicalLabel = await resolveCanonicalModelLabel(commandMeta.models[0], modelLabelPattern, correlatedLogger, prNumber); if (!canonicalLabel) return; - const { prLabels } = await getPRBranchAndLabels(eventType, payload, { owner, repo, prNumber }); + const { prLabels } = await getLivePRBranchAndLabels({ owner, repo, prNumber }); const modelLabelRegex = new RegExp(modelLabelPattern); const existingModelLabels = prLabels.filter(label => modelLabelRegex.test(label.name)).map(label => label.name); const labelsToRemove = existingModelLabels.filter(label => label !== canonicalLabel); - const labelsToAdd = existingModelLabels.includes(canonicalLabel) ? [] : [canonicalLabel]; + const targetPresent = existingModelLabels.includes(canonicalLabel); - if (labelsToRemove.length === 0 && labelsToAdd.length === 0) { + if (labelsToRemove.length === 0 && targetPresent) { correlatedLogger.debug({ pullRequestNumber: prNumber, modelLabel: canonicalLabel }, '/use model label is already active'); return; } const octokit = await getAuthenticatedOctokit(); - const update = await safeUpdateLabels( - { octokit, owner, repo, issueNumber: prNumber, logger: correlatedLogger }, - labelsToRemove, - labelsToAdd, - ); - if (!update.success) { - correlatedLogger.error( - { pullRequestNumber: prNumber, modelLabel: canonicalLabel, errors: update.errors }, - '/use failed to update the PR model label', - ); - return; + const labelContext = { octokit, owner, repo, issueNumber: prNumber, logger: correlatedLogger }; + + if (!targetPresent) { + const addUpdate = await safeUpdateLabels(labelContext, [], [canonicalLabel]); + if (!addUpdate.success) { + correlatedLogger.error( + { pullRequestNumber: prNumber, modelLabel: canonicalLabel, errors: addUpdate.errors }, + '/use failed to update the PR model label', + ); + return; + } } - correlatedLogger.info({ pullRequestNumber: prNumber, modelLabel: canonicalLabel }, '/use updated the PR model label'); + if (labelsToRemove.length > 0) { + const removalUpdate = await safeUpdateLabels(labelContext, labelsToRemove, []); + if (!removalUpdate.success) { + correlatedLogger.error( + { pullRequestNumber: prNumber, modelLabel: canonicalLabel, errors: removalUpdate.errors }, + '/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' } }; diff --git a/test/commentEventHandler.switch-use.test.ts b/test/commentEventHandler.switch-use.test.ts index c1c69140a..5415c5dce 100644 --- a/test/commentEventHandler.switch-use.test.ts +++ b/test/commentEventHandler.switch-use.test.ts @@ -578,6 +578,7 @@ describe('commentEventHandler — /use command', () => { mockOctokit.request.mock.resetCalls(); mockLoggerInstance.info.mock.resetCalls(); mockLoggerInstance.warn.mock.resetCalls(); + mockLoggerInstance.error.mock.resetCalls(); mockActiveJobs = []; mockWaitingJobs = []; mockDelayedJobs = []; @@ -607,12 +608,44 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-alias', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], ['llm-claude-sonnet5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 2); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-opus5']); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[1], ['llm-claude-sonnet5']); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[2], []); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); + 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, 'pull_request_review_comment', 'corr-use-review-live-labels', config); + + assert.strictEqual(mockOctokit.request.mock.callCount(), 1); + assert.strictEqual( + mockOctokit.request.mock.calls[0].arguments[0], + 'GET /repos/{owner}/{repo}/pulls/{pull_number}', + ); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 2); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-opus5']); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[1], ['llm-claude-sonnet5']); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[2], []); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); + }); + 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(); @@ -808,6 +841,39 @@ describe('commentEventHandler — /use command', () => { ); assert.strictEqual(successLog, undefined); }); + + test('/use target-add failure does not remove the existing managed label', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [createMockLabel({ name: 'llm-claude-sonnet5' })], + }, + })); + mockSafeUpdateLabels.mock.mockImplementationOnce(async (_context, labelsToRemove, labelsToAdd) => ({ + success: false, + removed: labelsToRemove, + added: [], + errors: [`Failed to add '${labelsToAdd[0]}'`], + })); + const event = createPRCommentEvent('/use opus'); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-add-failure-preserves-old', config); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-opus5']); + 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); + }); }); describe('commentEventHandler — commandMode serialization in job data', () => { @@ -853,7 +919,7 @@ describe('commentEventHandler — commandMode serialization in job data', () => await processCommentEvent(event, 'issue_comment', 'corr-mode-use', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 2); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); @@ -875,7 +941,7 @@ describe('commentEventHandler — commandMode serialization in job data', () => await processCommentEvent(event, 'issue_comment', 'corr-use-no-req', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 2); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); From a5bfb27f98e766728616c2f03b185f8cbfb213c5 Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:57:05 +0000 Subject: [PATCH 4/7] feat(ai): Implemented only F1 and F2, without committing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implemented only F1 and F2, without committing. - F1: stale-label removal failure now compensates by removing the newly added target label. - F2: canonical labels now use the configured prefix’s actual 50-character budget while retaining stable hashes and routing round trips. - Added focused regressions in [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T21-51-07/test/commentEventHandler.switch-use.test.ts:732). Verification passed: - 65 targeted tests - Repository-wide TypeScript check - Core package TypeScript check - Package-local ESLint - `git diff --check` PR: #1909 Comment by: @propr-ultrafix (ID: 0) Model: gpt-5.6-sol --- .../core/src/webhook/commentEventHandler.ts | 60 +++++++++++++-- test/commentEventHandler.switch-use.test.ts | 77 ++++++++++++++++++- 2 files changed, 131 insertions(+), 6 deletions(-) diff --git a/packages/core/src/webhook/commentEventHandler.ts b/packages/core/src/webhook/commentEventHandler.ts index 05dffe899..5f0901440 100644 --- a/packages/core/src/webhook/commentEventHandler.ts +++ b/packages/core/src/webhook/commentEventHandler.ts @@ -22,7 +22,7 @@ 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 } from '@propr/shared'; +import { buildAgentModelLlmLabel, buildDynamicLlmLabel, MAX_GITHUB_LABEL_LENGTH, shortHash } from '@propr/shared'; export interface UltrafixDeps { loadUltrafixRatingGoal: () => Promise; @@ -320,6 +320,43 @@ 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, @@ -351,9 +388,14 @@ async function resolveCanonicalModelLabel( return null; } - const canonicalLabel = defaultLabel.startsWith('llm-') - ? `${prefix}${defaultLabel.slice('llm-'.length)}` - : defaultLabel; + 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; + } if (!new RegExp(modelLabelPattern).test(canonicalLabel)) { correlatedLogger.error( { pullRequestNumber: prNumber, canonicalLabel, modelLabelPattern }, @@ -412,8 +454,16 @@ async function handleUseCommand(opts: UseCommandOptions): Promise { if (labelsToRemove.length > 0) { const removalUpdate = await safeUpdateLabels(labelContext, labelsToRemove, []); if (!removalUpdate.success) { + const rollbackUpdate = !targetPresent + ? await safeUpdateLabels(labelContext, [canonicalLabel], []) + : null; correlatedLogger.error( - { pullRequestNumber: prNumber, modelLabel: canonicalLabel, errors: removalUpdate.errors }, + { + pullRequestNumber: prNumber, + modelLabel: canonicalLabel, + errors: removalUpdate.errors, + rollbackErrors: rollbackUpdate?.success === false ? rollbackUpdate.errors : undefined, + }, '/use failed to update the PR model label', ); return; diff --git a/test/commentEventHandler.switch-use.test.ts b/test/commentEventHandler.switch-use.test.ts index 5415c5dce..e4a4a290f 100644 --- a/test/commentEventHandler.switch-use.test.ts +++ b/test/commentEventHandler.switch-use.test.ts @@ -2,7 +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 } from '@propr/shared'; +import { buildDynamicLlmLabel, shortHash } from '@propr/shared'; import { createWebhookIssueCommentCreatedEvent, createWebhookPRReviewCommentCreatedEvent, createMockLabel } from './testHelpers.js'; function manualRevisionIdentity(updatedAt: string, body: string, eventType = 'issue_comment'): string { @@ -223,6 +223,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( @@ -728,6 +729,36 @@ describe('commentEventHandler — /use command', () => { 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); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + const canonicalLabel = (mockSafeUpdateLabels.mock.calls[0].arguments[2] as string[])[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 without model argument warns and returns early', async () => { const event = createPRCommentEvent('/use'); const config = createTestConfig(); @@ -874,6 +905,50 @@ describe('commentEventHandler — /use command', () => { ); assert.strictEqual(successLog, undefined); }); + + test('/use removes a newly added target when stale-label removal fails', async () => { + mockOctokit.request.mock.mockImplementation(async () => ({ + data: { + head: { ref: 'feature-branch' }, + labels: [createMockLabel({ name: 'llm-claude-sonnet5' })], + }, + })); + let updateCall = 0; + mockSafeUpdateLabels.mock.mockImplementation(async (_context, labelsToRemove, labelsToAdd) => { + updateCall += 1; + if (updateCall === 2) { + return { + success: false, + removed: [], + added: [], + errors: ["Failed to remove 'llm-claude-sonnet5'"], + }; + } + return { + success: true, + removed: labelsToRemove, + added: labelsToAdd, + errors: [], + }; + }); + const event = createPRCommentEvent('/use opus'); + const config = createTestConfig(); + + await processCommentEvent(event, 'issue_comment', 'corr-use-removal-failure-rollback', config); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 3); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-opus5']); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[1], ['llm-claude-sonnet5']); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[2], []); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[2].arguments[1], ['llm-claude-opus5']); + assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[2].arguments[2], []); + 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.strictEqual(successLog, undefined); + }); }); describe('commentEventHandler — commandMode serialization in job data', () => { From 133f79a545332135084ab8484c3df8c2cc4bdf43 Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:18:30 +0000 Subject: [PATCH 5/7] feat(ai): Implemented F3, F4, and F5 only. Implemented F3, F4, and F5 only. - Validates generated routing tokens resolve to the exact selected agent/model. - Supports full labels using custom `MODEL_LABEL_PATTERN` prefixes. - Compares GitHub labels case-insensitively to preserve idempotency. - Added regressions for truncated alias collisions, custom-prefix labels, and mixed-case labels. Verification passed: - Focused suite: 68 tests - TypeScript typecheck - Core ESLint - `git diff --check` No commit created. PR: #1909 Comment by: @propr-ultrafix (ID: 0) Model: gpt-5.6-sol --- .../core/src/webhook/commentEventHandler.ts | 39 ++++++++-- test/commentEventHandler.switch-use.test.ts | 71 +++++++++++++++++++ 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/packages/core/src/webhook/commentEventHandler.ts b/packages/core/src/webhook/commentEventHandler.ts index 5f0901440..c4c64055b 100644 --- a/packages/core/src/webhook/commentEventHandler.ts +++ b/packages/core/src/webhook/commentEventHandler.ts @@ -364,7 +364,10 @@ async function resolveCanonicalModelLabel( prNumber: number, ): Promise { try { - const [resolution] = await resolveReviewModels([target]); + 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) { @@ -396,13 +399,40 @@ async function resolveCanonicalModelLabel( ); return null; } - if (!new RegExp(modelLabelPattern).test(canonicalLabel)) { + 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( @@ -429,8 +459,9 @@ async function handleUseCommand(opts: UseCommandOptions): Promise { const { prLabels } = await getLivePRBranchAndLabels({ owner, repo, prNumber }); const modelLabelRegex = new RegExp(modelLabelPattern); const existingModelLabels = prLabels.filter(label => modelLabelRegex.test(label.name)).map(label => label.name); - const labelsToRemove = existingModelLabels.filter(label => label !== canonicalLabel); - const targetPresent = existingModelLabels.includes(canonicalLabel); + const canonicalLabelIdentity = canonicalLabel.toLowerCase(); + const labelsToRemove = existingModelLabels.filter(label => label.toLowerCase() !== canonicalLabelIdentity); + const targetPresent = existingModelLabels.some(label => label.toLowerCase() === canonicalLabelIdentity); if (labelsToRemove.length === 0 && targetPresent) { correlatedLogger.debug({ pullRequestNumber: prNumber, modelLabel: canonicalLabel }, '/use model label is already active'); diff --git a/test/commentEventHandler.switch-use.test.ts b/test/commentEventHandler.switch-use.test.ts index e4a4a290f..cbbebc630 100644 --- a/test/commentEventHandler.switch-use.test.ts +++ b/test/commentEventHandler.switch-use.test.ts @@ -659,6 +659,22 @@ describe('commentEventHandler — /use command', () => { 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' } } @@ -759,6 +775,45 @@ describe('commentEventHandler — /use command', () => { 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 () => { const event = createPRCommentEvent('/use'); const config = createTestConfig(); @@ -803,6 +858,22 @@ describe('commentEventHandler — /use command', () => { assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); + 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-idempotent-case-insensitive', config); + + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); + assert.strictEqual(mockQueueAdd.mock.callCount(), 0); + }); + test('/use removes stale managed labels when the target label is already present', async () => { mockOctokit.request.mock.mockImplementation(async () => ({ data: { From 30651b3da6d5a09f9ea4cf8e94efc9d802104b4d Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:30:10 +0000 Subject: [PATCH 6/7] feat(ai): Implemented the follow-up correction. Implemented the follow-up correction. - `/use` now replaces the live PR label set in one request, preserving unrelated labels and leaving exactly one managed LLM label. - Failed replacement leaves existing labels untouched and does not claim success or enqueue work. - Updated focused regressions in [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T22-22-00/test/commentEventHandler.switch-use.test.ts). Validation passed: - 68/68 focused tests - TypeScript typecheck - Core ESLint - `git diff --check` No commit created. PR: #1909 Comment by: @integry (ID: 5298715670) Model: gpt-5.6-sol --- .../core/src/webhook/commentEventHandler.ts | 48 ++--- test/commentEventHandler.switch-use.test.ts | 182 +++++++++--------- 2 files changed, 109 insertions(+), 121 deletions(-) diff --git a/packages/core/src/webhook/commentEventHandler.ts b/packages/core/src/webhook/commentEventHandler.ts index c4c64055b..b986bc897 100644 --- a/packages/core/src/webhook/commentEventHandler.ts +++ b/packages/core/src/webhook/commentEventHandler.ts @@ -468,37 +468,25 @@ async function handleUseCommand(opts: UseCommandOptions): Promise { return; } - const octokit = await getAuthenticatedOctokit(); - const labelContext = { octokit, owner, repo, issueNumber: prNumber, logger: correlatedLogger }; + const labels = [ + ...prLabels.filter(label => !modelLabelRegex.test(label.name)).map(label => label.name), + canonicalLabel, + ]; - if (!targetPresent) { - const addUpdate = await safeUpdateLabels(labelContext, [], [canonicalLabel]); - if (!addUpdate.success) { - correlatedLogger.error( - { pullRequestNumber: prNumber, modelLabel: canonicalLabel, errors: addUpdate.errors }, - '/use failed to update the PR model label', - ); - return; - } - } - - if (labelsToRemove.length > 0) { - const removalUpdate = await safeUpdateLabels(labelContext, labelsToRemove, []); - if (!removalUpdate.success) { - const rollbackUpdate = !targetPresent - ? await safeUpdateLabels(labelContext, [canonicalLabel], []) - : null; - correlatedLogger.error( - { - pullRequestNumber: prNumber, - modelLabel: canonicalLabel, - errors: removalUpdate.errors, - rollbackErrors: rollbackUpdate?.success === false ? rollbackUpdate.errors : undefined, - }, - '/use failed to update the PR model label', - ); - return; - } + 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( diff --git a/test/commentEventHandler.switch-use.test.ts b/test/commentEventHandler.switch-use.test.ts index cbbebc630..31dc2d3ba 100644 --- a/test/commentEventHandler.switch-use.test.ts +++ b/test/commentEventHandler.switch-use.test.ts @@ -20,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: { @@ -609,11 +621,8 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-alias', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 2); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-opus5']); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[1], ['llm-claude-sonnet5']); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[2], []); + assertSingleLabelReplacement(['AI', 'bug', 'release:next', 'llm-claude-opus5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); @@ -633,16 +642,18 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'pull_request_review_comment', 'corr-use-review-live-labels', config); - assert.strictEqual(mockOctokit.request.mock.callCount(), 1); + assert.strictEqual(mockOctokit.request.mock.callCount(), 2); assert.strictEqual( mockOctokit.request.mock.calls[0].arguments[0], 'GET /repos/{owner}/{repo}/pulls/{pull_number}', ); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 2); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-opus5']); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[1], ['llm-claude-sonnet5']); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[2], []); + 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); }); @@ -653,9 +664,8 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-full-label', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-codex-gpt56-sol']); + assertSingleLabelReplacement(['llm-codex-gpt56-sol']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); @@ -684,8 +694,8 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-agent-alias', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-custom-codex-gpt56-sol']); + assertSingleLabelReplacement(['llm-custom-codex-gpt56-sol']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); @@ -695,8 +705,8 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-model-id-case', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-codex-gpt56-sol']); + assertSingleLabelReplacement(['llm-codex-gpt56-sol']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); @@ -717,8 +727,8 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-dynamic-label', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], [dynamicLabel]); + assertSingleLabelReplacement([dynamicLabel]); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); @@ -740,8 +750,8 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-hashed-label', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], [dynamicLabel]); + assertSingleLabelReplacement([dynamicLabel]); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); @@ -762,8 +772,10 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-custom-prefix-hashed-label', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - const canonicalLabel = (mockSafeUpdateLabels.mock.calls[0].arguments[2] as string[])[0]; + 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'); @@ -836,8 +848,8 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-trailing-text', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-sonnet5']); + assertSingleLabelReplacement(['llm-claude-sonnet5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); @@ -890,9 +902,8 @@ describe('commentEventHandler — /use command', () => { await processCommentEvent(event, 'issue_comment', 'corr-use-converge', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], ['llm-claude-sonnet5']); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], []); + assertSingleLabelReplacement(['reviewed', 'llm-claude-opus5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); @@ -925,18 +936,19 @@ describe('commentEventHandler — /use command', () => { }); test('/use label-update failure does not enqueue work or claim success', async () => { - mockSafeUpdateLabels.mock.mockImplementationOnce(async () => ({ - success: false, - removed: [], - added: [], - errors: ['GitHub unavailable'], - })); + 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(mockSafeUpdateLabels.mock.callCount(), 1); + 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', @@ -944,27 +956,31 @@ describe('commentEventHandler — /use command', () => { assert.strictEqual(successLog, undefined); }); - test('/use target-add failure does not remove the existing managed label', async () => { - mockOctokit.request.mock.mockImplementation(async () => ({ - data: { - head: { ref: 'feature-branch' }, - labels: [createMockLabel({ name: 'llm-claude-sonnet5' })], - }, - })); - mockSafeUpdateLabels.mock.mockImplementationOnce(async (_context, labelsToRemove, labelsToAdd) => ({ - success: false, - removed: labelsToRemove, - added: [], - errors: [`Failed to add '${labelsToAdd[0]}'`], - })); + 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); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-opus5']); + 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( @@ -977,48 +993,25 @@ describe('commentEventHandler — /use command', () => { assert.strictEqual(successLog, undefined); }); - test('/use removes a newly added target when stale-label removal fails', async () => { + 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: 'llm-claude-sonnet5' })], + labels: [ + createMockLabel({ name: 'AI' }), + createMockLabel({ name: 'llm-claude-sonnet5' }), + createMockLabel({ name: 'llm-codex-gpt56-sol' }), + ], }, })); - let updateCall = 0; - mockSafeUpdateLabels.mock.mockImplementation(async (_context, labelsToRemove, labelsToAdd) => { - updateCall += 1; - if (updateCall === 2) { - return { - success: false, - removed: [], - added: [], - errors: ["Failed to remove 'llm-claude-sonnet5'"], - }; - } - return { - success: true, - removed: labelsToRemove, - added: labelsToAdd, - errors: [], - }; - }); const event = createPRCommentEvent('/use opus'); const config = createTestConfig(); - await processCommentEvent(event, 'issue_comment', 'corr-use-removal-failure-rollback', config); + await processCommentEvent(event, 'issue_comment', 'corr-use-single-managed-label', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 3); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[1], []); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[0].arguments[2], ['llm-claude-opus5']); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[1], ['llm-claude-sonnet5']); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[1].arguments[2], []); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[2].arguments[1], ['llm-claude-opus5']); - assert.deepStrictEqual(mockSafeUpdateLabels.mock.calls[2].arguments[2], []); + assertSingleLabelReplacement(['AI', 'llm-claude-opus5']); + 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.strictEqual(successLog, undefined); }); }); @@ -1065,7 +1058,8 @@ describe('commentEventHandler — commandMode serialization in job data', () => await processCommentEvent(event, 'issue_comment', 'corr-mode-use', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 2); + assertSingleLabelReplacement(['llm-claude-haiku']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); @@ -1087,7 +1081,8 @@ describe('commentEventHandler — commandMode serialization in job data', () => await processCommentEvent(event, 'issue_comment', 'corr-use-no-req', config); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 2); + assertSingleLabelReplacement(['llm-claude-sonnet5']); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); assert.strictEqual(config.redisClient.rpush.mock.callCount(), 0); }); @@ -1170,12 +1165,14 @@ describe('commentEventHandler — slash command dedup protection', () => { // First delivery changes the label without enqueuing. await processCommentEvent(event, 'issue_comment', 'corr-dedup-1', config); - assert.strictEqual(mockSafeUpdateLabels.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); - assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 1); + assert.strictEqual(getLabelReplacementCalls().length, 1); + assert.strictEqual(mockSafeUpdateLabels.mock.callCount(), 0); assert.strictEqual(mockQueueAdd.mock.callCount(), 0); }); @@ -1236,7 +1233,8 @@ describe('commentEventHandler — slash command batching/concurrency guard', () await processCommentEvent(event, 'issue_comment', 'corr-batch-1', config); - assert.strictEqual(mockSafeUpdateLabels.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); }); @@ -1271,7 +1269,8 @@ describe('commentEventHandler — slash command batching/concurrency guard', () await processCommentEvent(event, 'issue_comment', 'corr-batch-3', config); - assert.strictEqual(mockSafeUpdateLabels.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); }); @@ -1498,7 +1497,8 @@ describe('commentEventHandler — slash command batching/concurrency guard', () await processCommentEvent(event, 'pull_request_review_comment', 'corr-batch-review', config); - assert.strictEqual(mockSafeUpdateLabels.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); }); From d86990499d5093f5d3f641a96f09154416a0ebbb Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:49:28 +0000 Subject: [PATCH 7/7] feat(ai): Implemented the narrow `/use` correction: Implemented the narrow `/use` correction: - Removes every live PR label prefixed with `llm-`. - Adds the resolved canonical target once. - Preserves unrelated labels and idempotent behavior. - Added focused regression coverage. Files changed: - [commentEventHandler.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T22-45-04/packages/core/src/webhook/commentEventHandler.ts:459) - [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T22-45-04/test/commentEventHandler.switch-use.test.ts:629) Validation passed: 69 focused tests, TypeScript typecheck, core ESLint, and `git diff --check`. No commit created. PR: #1909 Comment by: @integry (ID: 5298894213) Model: gpt-5.6-sol --- .../core/src/webhook/commentEventHandler.ts | 9 ++++---- test/commentEventHandler.switch-use.test.ts | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/core/src/webhook/commentEventHandler.ts b/packages/core/src/webhook/commentEventHandler.ts index b986bc897..335cf9f62 100644 --- a/packages/core/src/webhook/commentEventHandler.ts +++ b/packages/core/src/webhook/commentEventHandler.ts @@ -457,11 +457,10 @@ async function handleUseCommand(opts: UseCommandOptions): Promise { if (!canonicalLabel) return; const { prLabels } = await getLivePRBranchAndLabels({ owner, repo, prNumber }); - const modelLabelRegex = new RegExp(modelLabelPattern); - const existingModelLabels = prLabels.filter(label => modelLabelRegex.test(label.name)).map(label => label.name); 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 = existingModelLabels.some(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'); @@ -469,7 +468,9 @@ async function handleUseCommand(opts: UseCommandOptions): Promise { } const labels = [ - ...prLabels.filter(label => !modelLabelRegex.test(label.name)).map(label => label.name), + ...prLabels + .filter(label => !label.name.startsWith('llm-') && label.name.toLowerCase() !== canonicalLabelIdentity) + .map(label => label.name), canonicalLabel, ]; diff --git a/test/commentEventHandler.switch-use.test.ts b/test/commentEventHandler.switch-use.test.ts index 31dc2d3ba..dc59a1e3e 100644 --- a/test/commentEventHandler.switch-use.test.ts +++ b/test/commentEventHandler.switch-use.test.ts @@ -626,6 +626,27 @@ describe('commentEventHandler — /use command', () => { 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 on a review comment replaces labels from the live PR instead of the stale payload', async () => { mockOctokit.request.mock.mockImplementation(async () => ({ data: {