From 0d8d4936a49c542a2d9f952f4492fe79046e7c48 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Thu, 18 Jun 2026 11:45:01 +0300 Subject: [PATCH 1/5] fix(review-memory): enforce structured model output --- apps/web/src/app/api/trpc/[trpc]/route.ts | 3 +- .../apply-provider-specific-logic.test.ts | 53 +- .../apply-provider-specific-logic.ts | 19 + .../ai-gateway/providers/openrouter/types.ts | 1 + .../review-memory/aggregation.test.ts | 9 +- .../code-reviews/review-memory/aggregation.ts | 41 +- .../review-memory/change-request.test.ts | 309 +++++++++++- .../review-memory/change-request.ts | 268 +++++++++- .../code-reviews/review-memory/llm.test.ts | 411 +++++++++++++++ .../src/lib/code-reviews/review-memory/llm.ts | 468 +++++++++++++++++- .../review-md-integration.test.ts | 65 +++ .../review-memory/review-md-integration.ts | 24 +- apps/web/src/lib/trpc/init.ts | 44 +- .../code-reviews/review-memory-router.ts | 2 + apps/web/src/routers/test-utils.ts | 2 + 15 files changed, 1665 insertions(+), 54 deletions(-) create mode 100644 apps/web/src/lib/code-reviews/review-memory/llm.test.ts create mode 100644 apps/web/src/lib/code-reviews/review-memory/review-md-integration.test.ts diff --git a/apps/web/src/app/api/trpc/[trpc]/route.ts b/apps/web/src/app/api/trpc/[trpc]/route.ts index f32c374ac4..86f9d2161e 100644 --- a/apps/web/src/app/api/trpc/[trpc]/route.ts +++ b/apps/web/src/app/api/trpc/[trpc]/route.ts @@ -9,7 +9,8 @@ const handler = (req: Request) => endpoint: '/api/trpc', req, router: rootRouter, - createContext: createTRPCContext, + createContext: () => + createTRPCContext({ requestCorrelationId: req.headers.get('x-vercel-id') }), }); export { handler as GET, handler as POST }; diff --git a/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.test.ts b/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.test.ts index 1e091d6c3c..196c023c22 100644 --- a/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.test.ts +++ b/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from '@jest/globals'; import { CLAUDE_OPUS_CURRENT_MODEL_ID } from '@/lib/ai-gateway/providers/anthropic.constants'; -import { applyGatewayModelsFallback } from '@/lib/ai-gateway/providers/apply-provider-specific-logic'; +import { + applyGatewayModelsFallback, + requireStructuredOutputParameters, +} from '@/lib/ai-gateway/providers/apply-provider-specific-logic'; import type { GatewayRequest } from '@/lib/ai-gateway/providers/openrouter/types'; import type { ProviderId } from '@/lib/ai-gateway/providers/types'; @@ -46,3 +49,51 @@ describe('applyGatewayModelsFallback', () => { expect(request.body.models).toBeUndefined(); }); }); + +describe('requireStructuredOutputParameters', () => { + it('requires structured-output support from OpenRouter providers', () => { + const request = makeStructuredOutputRequest(); + request.body.provider = { only: ['anthropic'] }; + + requireStructuredOutputParameters('openrouter', request); + + expect(request.body.provider).toEqual({ + only: ['anthropic'], + require_parameters: true, + }); + }); + + it.each(['direct-byok', 'custom', 'experiment', 'vercel'])( + 'does not send OpenRouter routing controls to %s', + providerId => { + const request = makeStructuredOutputRequest(); + + requireStructuredOutputParameters(providerId, request); + + expect(request.body.provider).toBeUndefined(); + } + ); +}); + +function makeStructuredOutputRequest(): GatewayRequest { + return { + kind: 'chat_completions', + body: { + model: 'anthropic/claude-sonnet-4.6', + messages: [{ role: 'user', content: 'hello' }], + response_format: { + type: 'json_schema', + json_schema: { + name: 'result', + strict: true, + schema: { + type: 'object', + properties: { result: { type: 'string' } }, + required: ['result'], + additionalProperties: false, + }, + }, + }, + }, + }; +} diff --git a/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.ts b/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.ts index 584e42f5bb..a6981b6e33 100644 --- a/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.ts +++ b/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.ts @@ -113,6 +113,24 @@ export function applyGatewayModelsFallback( delete requestToMutate.body.models; } +export function requireStructuredOutputParameters( + providerId: ProviderId, + requestToMutate: GatewayRequest +): void { + if ( + providerId !== 'openrouter' || + requestToMutate.kind !== 'chat_completions' || + requestToMutate.body.response_format?.type !== 'json_schema' + ) { + return; + } + + requestToMutate.body.provider = { + ...requestToMutate.body.provider, + require_parameters: true, + }; +} + export function applyProviderSpecificLogic( provider: Provider, requestedModel: string, @@ -124,6 +142,7 @@ export function applyProviderSpecificLogic( taskId: string | null ) { applyGatewayModelsFallback(provider.id, requestedModel, requestToMutate); + requireStructuredOutputParameters(provider.id, requestToMutate); applyTrackingIds(requestToMutate, provider, userId, taskId); sanitizeBinaryToolResults(requestToMutate); diff --git a/apps/web/src/lib/ai-gateway/providers/openrouter/types.ts b/apps/web/src/lib/ai-gateway/providers/openrouter/types.ts index 3656e00757..fddec5da95 100644 --- a/apps/web/src/lib/ai-gateway/providers/openrouter/types.ts +++ b/apps/web/src/lib/ai-gateway/providers/openrouter/types.ts @@ -14,6 +14,7 @@ export type OpenRouterProviderConfig = { ignore?: string[]; data_collection?: 'allow' | 'deny'; zdr?: boolean; + require_parameters?: boolean; }; export type VercelInferenceProviderConfig = { apiKey: string; baseURL?: string } | AwsCredentials; diff --git a/apps/web/src/lib/code-reviews/review-memory/aggregation.test.ts b/apps/web/src/lib/code-reviews/review-memory/aggregation.test.ts index 59649cd6ac..47a8c25b47 100644 --- a/apps/web/src/lib/code-reviews/review-memory/aggregation.test.ts +++ b/apps/web/src/lib/code-reviews/review-memory/aggregation.test.ts @@ -26,6 +26,7 @@ describe('review memory aggregation', () => { owner, platform: 'github', repoFullName: 'acme/widgets', + requestCorrelationId: 'request-proposal-created', generate: async () => ({ draft: { status: 'propose', @@ -66,6 +67,7 @@ describe('review memory aggregation', () => { owner, platform: 'github', repoFullName: 'acme/widgets', + requestCorrelationId: 'request-no-change', generate: async () => ({ draft: { status: 'no_change', @@ -74,7 +76,12 @@ describe('review memory aggregation', () => { }) ).resolves.toEqual({ status: 'no_change' }); await expect( - runReviewMemoryAnalysis({ owner, platform: 'github', repoFullName: 'acme/empty' }) + runReviewMemoryAnalysis({ + owner, + platform: 'github', + repoFullName: 'acme/empty', + requestCorrelationId: 'request-no-feedback', + }) ).resolves.toEqual({ status: 'no_feedback' }); await expect(db.select().from(code_review_memory_proposals)).resolves.toHaveLength(0); }); diff --git a/apps/web/src/lib/code-reviews/review-memory/aggregation.ts b/apps/web/src/lib/code-reviews/review-memory/aggregation.ts index 5483f39eab..33a2e40625 100644 --- a/apps/web/src/lib/code-reviews/review-memory/aggregation.ts +++ b/apps/web/src/lib/code-reviews/review-memory/aggregation.ts @@ -1,4 +1,3 @@ -import { generateText } from 'ai'; import * as z from 'zod'; import type { CodeReviewFeedbackEvent } from '@kilocode/db/schema'; @@ -7,7 +6,7 @@ import type { ReviewMemoryOwner } from './db'; import { listRecentFeedbackEvents, upsertScopeProposal } from './db'; import { createReviewMemoryGatewayProvider, - extractReviewMemoryJsonObject, + generateReviewMemoryStructuredOutput, resolveReviewMemoryActor, resolveReviewMemoryModel, } from './llm'; @@ -16,7 +15,7 @@ import { reviewMemoryRetentionCutoff } from './retention'; const REVIEW_MEMORY_FORBIDDEN_PROPOSAL_PATTERN = /\b(?:review\s+memory|kilo|feedback\s+systems?|this\s+analysis|llms?)\b/i; -const ReviewMemoryProposalDraftSchema = z.discriminatedUnion('status', [ +export const ReviewMemoryProposalDraftSchema = z.discriminatedUnion('status', [ z.object({ status: z.literal('no_change') }), z.object({ status: z.literal('propose'), @@ -30,6 +29,17 @@ const ReviewMemoryProposalDraftSchema = z.discriminatedUnion('status', [ }), ]); +export const ReviewMemoryProposalWireSchema = z.object({ + status: z.enum(['no_change', 'propose']), + title: z.string().nullable(), + rationale: z.string().nullable(), + proposedMarkdown: z.string().nullable(), + positiveCount: z.number().int(), + negativeCount: z.number().int(), + neutralCount: z.number().int(), + evidenceEventIds: z.array(z.string()), +}); + export type ReviewMemoryProposalDraft = z.infer; export type GenerateReviewMemoryProposal = (input: { @@ -37,6 +47,7 @@ export type GenerateReviewMemoryProposal = (input: { platform: ReviewMemoryPlatform; repoFullName: string; events: CodeReviewFeedbackEvent[]; + requestCorrelationId: string; }) => Promise<{ draft: ReviewMemoryProposalDraft; tokensIn?: number | null; @@ -48,6 +59,7 @@ export async function generateReviewMemoryProposalWithGateway(input: { platform: ReviewMemoryPlatform; repoFullName: string; events: CodeReviewFeedbackEvent[]; + requestCorrelationId: string; }): Promise<{ draft: ReviewMemoryProposalDraft; tokensIn?: number | null; @@ -63,18 +75,23 @@ export async function generateReviewMemoryProposalWithGateway(input: { actor, userAgent: 'Kilo Review Memory Analyzer', }); - const result = await generateText({ + const result = await generateReviewMemoryStructuredOutput({ model: provider.chatModel(modelSlug), + modelSlug, + operation: 'proposal_analysis', + requestCorrelationId: input.requestCorrelationId, prompt: buildReviewMemoryAnalysisPrompt(input), maxOutputTokens: 4_000, + schemaName: 'review_memory_proposal', + schema: ReviewMemoryProposalDraftSchema, + wireSchema: ReviewMemoryProposalWireSchema, + validate: validateReviewMemoryProposalDraft, }); return { - draft: validateReviewMemoryProposalDraft( - ReviewMemoryProposalDraftSchema.parse(extractReviewMemoryJsonObject(result.text)) - ), - tokensIn: result.usage.inputTokens ?? null, - tokensOut: result.usage.outputTokens ?? null, + draft: result.output, + tokensIn: result.diagnostics.inputTokens, + tokensOut: result.diagnostics.outputTokens, }; } @@ -82,6 +99,7 @@ export async function runReviewMemoryAnalysis(input: { owner: ReviewMemoryOwner; platform: ReviewMemoryPlatform; repoFullName: string; + requestCorrelationId: string; generate?: GenerateReviewMemoryProposal; now?: Date; }): Promise<{ status: 'proposed' | 'no_change' | 'no_feedback'; proposalId?: string }> { @@ -101,6 +119,7 @@ export async function runReviewMemoryAnalysis(input: { platform: input.platform, repoFullName: input.repoFullName, events, + requestCorrelationId: input.requestCorrelationId, }); if (draft.status === 'no_change') return { status: 'no_change' }; @@ -144,7 +163,7 @@ function buildReviewMemoryAnalysisPrompt(input: { return `You analyze maintainer replies to Kilo's automated code-review comments for one repository. Return strict JSON in one of these shapes: -{"status":"no_change"} +{"status":"no_change","title":null,"rationale":null,"proposedMarkdown":null,"positiveCount":0,"negativeCount":0,"neutralCount":0,"evidenceEventIds":[]} {"status":"propose","title":"short proposal title","rationale":"why this guidance is justified","proposedMarkdown":"standalone REVIEW.md guidance","positiveCount":0,"negativeCount":0,"neutralCount":0,"evidenceEventIds":["event ids"]} Rules: @@ -164,7 +183,7 @@ Feedback events: ${JSON.stringify(events, null, 2)}`; } -function validateReviewMemoryProposalDraft( +export function validateReviewMemoryProposalDraft( draft: ReviewMemoryProposalDraft ): ReviewMemoryProposalDraft { if (draft.status === 'no_change') return draft; diff --git a/apps/web/src/lib/code-reviews/review-memory/change-request.test.ts b/apps/web/src/lib/code-reviews/review-memory/change-request.test.ts index b7646df474..071bcae044 100644 --- a/apps/web/src/lib/code-reviews/review-memory/change-request.test.ts +++ b/apps/web/src/lib/code-reviews/review-memory/change-request.test.ts @@ -4,6 +4,7 @@ import { insertTestUser } from '@/tests/helpers/user.helper'; import { code_review_memory_proposals, kilocode_users, + platform_integrations, type CodeReviewMemoryProposal, } from '@kilocode/db/schema'; @@ -15,11 +16,24 @@ import { upsertScopeProposal, type ReviewMemoryOwner, } from './db'; -import { buildChangeRequestBody, isStaleOpeningChangeRequest } from './change-request'; +import { + approveAndOpenReviewMemoryChangeRequest, + buildChangeRequestBody, + isStaleOpeningChangeRequest, + ReviewMemoryChangeRequestError, + type ReviewMemoryChangeRequestOperations, +} from './change-request'; +import { ReviewMemoryModelBoundaryError, type ReviewMemoryModelDiagnostics } from './llm'; + +const REQUEST_CORRELATION_ID = 'request-correlation-change-request'; +const RAW_OUTPUT_SENTINEL = 'RAW_MODEL_OUTPUT_SECRET'; +const REVIEW_MD_SENTINEL = 'EXISTING_REVIEW_MD_SECRET'; +const REPOSITORY_SENTINEL = 'sensitive-owner/sensitive-repository'; describe('review memory change requests', () => { afterEach(async () => { await db.delete(code_review_memory_proposals); + await db.delete(platform_integrations); await db.delete(kilocode_users); }); @@ -73,6 +87,210 @@ describe('review memory change requests', () => { expect(isStaleOpeningChangeRequest('2026-06-01T11:30:00.000Z', now)).toBe(true); expect(isStaleOpeningChangeRequest('not-a-date', now)).toBe(false); }); + + it('reports a structured-output failure safely before GitHub mutations', async () => { + const user = await insertTestUser(); + const owner = { type: 'user' as const, id: user.id }; + const proposal = await seedProposal(owner, REPOSITORY_SENTINEL); + await seedGitHubIntegration(owner); + const operations = buildOperations(); + operations.fetchReviewMd.mockResolvedValue(REVIEW_MD_SENTINEL); + const modelFailure = buildModelBoundaryError(); + Object.assign(modelFailure, { + text: RAW_OUTPUT_SENTINEL, + cause: new Error(`Cause containing ${RAW_OUTPUT_SENTINEL}`), + }); + operations.integrateReviewMd.mockRejectedValue(modelFailure); + + const error = await captureError(() => + approveAndOpenReviewMemoryChangeRequest( + { + owner, + proposalId: proposal.id, + requestCorrelationId: REQUEST_CORRELATION_ID, + }, + operations + ) + ); + + expect(error).toBeInstanceOf(ReviewMemoryChangeRequestError); + expect(error).toMatchObject({ + code: 'BAD_REQUEST', + message: 'Unable to open the REVIEW.md change request. The proposal can be retried.', + }); + const proposals = await db.select().from(code_review_memory_proposals); + expect(proposals).toEqual([ + expect.objectContaining({ id: proposal.id, status: 'change_request_failed' }), + ]); + expect(operations.createBranch).not.toHaveBeenCalled(); + expect(operations.writeReviewMd).not.toHaveBeenCalled(); + expect(operations.createPullRequest).not.toHaveBeenCalled(); + expect(operations.captureException).toHaveBeenCalledTimes(1); + + const [capturedError, captureContext] = operations.captureException.mock.calls[0]; + expect(capturedError).toEqual( + expect.objectContaining({ + name: 'ReviewMemoryChangeRequestFailure', + message: 'Review Memory model returned invalid structured output.', + }) + ); + expect(captureContext).toEqual( + expect.objectContaining({ + tags: { + source: 'review-memory', + operation: 'open-change-request', + stage: 'review_md_integration', + }, + extra: expect.objectContaining({ + requestCorrelationId: REQUEST_CORRELATION_ID, + proposalId: proposal.id, + providerRequestId: 'provider-request-model-failure', + modelDiagnostics: expect.objectContaining({ + failureClassification: 'invalid_structured_output', + }), + }), + }) + ); + const serializedCapture = JSON.stringify({ + name: capturedError.name, + message: capturedError.message, + stack: capturedError.stack, + captureContext, + }); + expect(serializedCapture).not.toContain(RAW_OUTPUT_SENTINEL); + expect(serializedCapture).not.toContain(REVIEW_MD_SENTINEL); + expect(serializedCapture).not.toContain(REPOSITORY_SENTINEL); + }); + + it('does not capture expected pre-claim domain errors', async () => { + const user = await insertTestUser(); + const owner = { type: 'user' as const, id: user.id }; + const proposal = await seedProposal(owner, 'acme/no-integration'); + const operations = buildOperations(); + + const error = await captureError(() => + approveAndOpenReviewMemoryChangeRequest( + { + owner, + proposalId: proposal.id, + requestCorrelationId: REQUEST_CORRELATION_ID, + }, + operations + ) + ); + + expect(error).toMatchObject({ + code: 'BAD_REQUEST', + message: 'No active GitHub integration has access to this repository.', + }); + const proposals = await db.select().from(code_review_memory_proposals); + expect(proposals).toEqual([expect.objectContaining({ id: proposal.id, status: 'open' })]); + expect(operations.generateInstallationToken).not.toHaveBeenCalled(); + expect(operations.captureException).not.toHaveBeenCalled(); + }); + + it('supersedes already-present guidance without GitHub mutations', async () => { + const user = await insertTestUser(); + const owner = { type: 'user' as const, id: user.id }; + const proposal = await seedProposal(owner, 'acme/already-present'); + await seedGitHubIntegration(owner); + const operations = buildOperations(); + operations.integrateReviewMd.mockResolvedValue({ + status: 'already_present', + updatedReviewMd: null, + integrationSummary: 'Equivalent guidance already exists.', + tokensIn: 12, + tokensOut: 5, + }); + + const result = await approveAndOpenReviewMemoryChangeRequest( + { + owner, + proposalId: proposal.id, + requestCorrelationId: REQUEST_CORRELATION_ID, + }, + operations + ); + + expect(result.status).toBe('superseded'); + expect(operations.createBranch).not.toHaveBeenCalled(); + expect(operations.writeReviewMd).not.toHaveBeenCalled(); + expect(operations.createPullRequest).not.toHaveBeenCalled(); + expect(operations.captureException).not.toHaveBeenCalled(); + expect(operations.log).toHaveBeenCalledWith( + expect.stringContaining('"result":"already_present"') + ); + }); + + it('opens and persists a change request after validated model output', async () => { + const user = await insertTestUser(); + const owner = { type: 'user' as const, id: user.id }; + const proposal = await seedProposal(owner, 'acme/success'); + await seedGitHubIntegration(owner); + const operations = buildOperations(); + + const result = await approveAndOpenReviewMemoryChangeRequest( + { + owner, + proposalId: proposal.id, + requestCorrelationId: REQUEST_CORRELATION_ID, + }, + operations + ); + + expect(result).toEqual( + expect.objectContaining({ + status: 'change_request_opened', + change_request_url: 'https://github.com/acme/widgets/pull/10', + }) + ); + expect(operations.createBranch).toHaveBeenCalledTimes(1); + expect(operations.writeReviewMd).toHaveBeenCalledTimes(1); + expect(operations.createPullRequest).toHaveBeenCalledTimes(1); + expect(operations.captureException).not.toHaveBeenCalled(); + expect(operations.log).toHaveBeenCalledWith( + expect.stringContaining('"result":"change_request_opened"') + ); + }); + + it('retains the original failure when failure-state persistence also fails', async () => { + const user = await insertTestUser(); + const owner = { type: 'user' as const, id: user.id }; + const proposal = await seedProposal(owner, 'acme/persistence-failure'); + await seedGitHubIntegration(owner); + const operations = buildOperations(); + operations.integrateReviewMd.mockRejectedValue(buildModelBoundaryError()); + const markFailed = jest.fn< + ReturnType, + Parameters + >(async () => { + throw new Error('database unavailable'); + }); + + const error = await captureError(() => + approveAndOpenReviewMemoryChangeRequest( + { + owner, + proposalId: proposal.id, + requestCorrelationId: REQUEST_CORRELATION_ID, + }, + { ...operations, markFailed } + ) + ); + + expect(error).toMatchObject({ + code: 'BAD_REQUEST', + message: 'Unable to open the REVIEW.md change request. The proposal can be retried.', + }); + expect(operations.captureException).toHaveBeenCalledTimes(2); + expect(operations.captureException.mock.calls[0]?.[1].tags.stage).toBe('review_md_integration'); + expect(operations.captureException.mock.calls[1]?.[1]).toEqual( + expect.objectContaining({ + tags: expect.objectContaining({ stage: 'failure_persistence' }), + extra: expect.objectContaining({ failedAtStage: 'review_md_integration' }), + }) + ); + }); }); function buildProposal(input: { @@ -114,3 +332,92 @@ async function seedProposal(owner: ReviewMemoryOwner, repoFullName: string) { neutralCount: 0, }); } + +async function seedGitHubIntegration(owner: ReviewMemoryOwner): Promise { + if (owner.type !== 'user') throw new Error('Test helper requires a user owner.'); + await db.insert(platform_integrations).values({ + owned_by_user_id: owner.id, + owned_by_organization_id: null, + platform: 'github', + integration_type: 'app', + platform_installation_id: 'installation-id', + github_app_type: 'standard', + integration_status: 'active', + repository_access: 'all', + permissions: { + contents: 'write', + pull_requests: 'write', + }, + }); +} + +function buildOperations() { + const fetchReviewMd = jest.fn< + ReturnType, + Parameters + >(async () => null); + const integrateReviewMd = jest.fn< + ReturnType, + Parameters + >(async () => ({ + status: 'updated', + updatedReviewMd: '# Code review\n\nCheck generated fixtures against their source.', + integrationSummary: 'Added generated fixture guidance.', + tokensIn: 12, + tokensOut: 5, + })); + + return { + generateInstallationToken: jest.fn(async () => ({ + token: 'installation-token', + expires_at: '2099-01-01T00:00:00.000Z', + })), + fetchDefaultBranch: jest.fn(async () => 'main'), + fetchReviewMd, + integrateReviewMd, + createBranch: jest.fn(async () => undefined), + writeReviewMd: jest.fn(async () => undefined), + createPullRequest: jest.fn(async () => ({ + number: 10, + url: 'https://github.com/acme/widgets/pull/10', + })), + markFailed: markProposalChangeRequestFailed, + log: jest.fn(), + captureException: jest.fn< + ReturnType, + Parameters + >(), + } satisfies ReviewMemoryChangeRequestOperations; +} + +function buildModelBoundaryError(): ReviewMemoryModelBoundaryError { + const diagnostics: ReviewMemoryModelDiagnostics = { + operation: 'review_md_integration', + stage: 'structured_output_validation', + outcome: 'failure', + requestCorrelationId: REQUEST_CORRELATION_ID, + modelSlug: 'test/model', + finishReason: 'stop', + rawFinishReason: 'stop', + generatedTextCharacterCount: 120, + structuredParseCandidateCharacterCount: 120, + inputTokens: 12, + outputTokens: 5, + totalTokens: 17, + providerRequestId: 'provider-request-model-failure', + responseId: 'response-model-failure', + failureClassification: 'invalid_structured_output', + errorClass: 'NoObjectGeneratedError', + errorMessage: 'Review Memory model returned invalid structured output.', + }; + return new ReviewMemoryModelBoundaryError(diagnostics); +} + +async function captureError(run: () => Promise): Promise { + try { + await run(); + } catch (error) { + return error; + } + throw new Error('Expected operation to reject.'); +} diff --git a/apps/web/src/lib/code-reviews/review-memory/change-request.ts b/apps/web/src/lib/code-reviews/review-memory/change-request.ts index 48b3b1eb5b..27cd31e8db 100644 --- a/apps/web/src/lib/code-reviews/review-memory/change-request.ts +++ b/apps/web/src/lib/code-reviews/review-memory/change-request.ts @@ -1,8 +1,9 @@ +import { captureException } from '@sentry/nextjs'; import type { CodeReviewMemoryProposal, PlatformIntegration } from '@kilocode/db/schema'; import type { ReviewMemoryProposalStatus } from '@kilocode/db/schema-types'; -import { getAllIntegrationsForOwner } from '@/lib/integrations/db/platform-integrations'; import { INTEGRATION_STATUS, PLATFORM } from '@/lib/integrations/core/constants'; +import { getAllIntegrationsForOwner } from '@/lib/integrations/db/platform-integrations'; import { createGitHubBranch, createGitHubPullRequest, @@ -11,6 +12,7 @@ import { fetchGitHubRootTextFileAtRef, generateGitHubInstallationToken, } from '@/lib/integrations/platforms/github/adapter'; +import { logExceptInTest } from '@/lib/utils.server'; import { getProposal, markProposalChangeRequestFailed, @@ -19,11 +21,14 @@ import { markProposalSuperseded, type ReviewMemoryOwner, } from './db'; +import { ReviewMemoryModelBoundaryError, sanitizeReviewMemoryDiagnosticMessage } from './llm'; import { generateIntegratedReviewGuidanceWithGateway } from './review-md-integration'; const REVIEW_MEMORY_TARGET_FILE_PATH = 'REVIEW.md'; const REVIEW_MEMORY_CHANGE_REQUEST_TITLE = 'docs(review): update REVIEW.md guidance'; const REVIEW_MEMORY_CHANGE_REQUEST_MARKER = ''; +const REVIEW_MEMORY_CHANGE_REQUEST_RETRY_MESSAGE = + 'Unable to open the REVIEW.md change request. The proposal can be retried.'; const OPENING_CHANGE_REQUEST_STALE_MS = 30 * 60 * 1000; const APPROVABLE_STATUSES: ReadonlySet = new Set([ 'open', @@ -31,6 +36,56 @@ const APPROVABLE_STATUSES: ReadonlySet = new Set([ 'change_request_failed', ]); +type ReviewMemoryChangeRequestStage = + | 'repository_validation' + | 'installation_token_creation' + | 'github_reads' + | 'review_md_integration' + | 'branch_creation' + | 'file_writing' + | 'pull_request_creation' + | 'final_persistence' + | 'failure_persistence'; + +type CaptureReviewMemoryException = ( + error: Error, + context: { + level: 'error'; + tags: { + source: 'review-memory'; + operation: 'open-change-request'; + stage: ReviewMemoryChangeRequestStage; + }; + extra: Record; + } +) => unknown; + +export type ReviewMemoryChangeRequestOperations = { + generateInstallationToken: typeof generateGitHubInstallationToken; + fetchDefaultBranch: typeof fetchGitHubRepositoryDefaultBranch; + fetchReviewMd: typeof fetchGitHubRootTextFileAtRef; + integrateReviewMd: typeof generateIntegratedReviewGuidanceWithGateway; + createBranch: typeof createGitHubBranch; + writeReviewMd: typeof createOrUpdateGitHubRootTextFile; + createPullRequest: typeof createGitHubPullRequest; + markFailed: typeof markProposalChangeRequestFailed; + log: (record: string) => void; + captureException: CaptureReviewMemoryException; +}; + +const defaultOperations: ReviewMemoryChangeRequestOperations = { + generateInstallationToken: generateGitHubInstallationToken, + fetchDefaultBranch: fetchGitHubRepositoryDefaultBranch, + fetchReviewMd: fetchGitHubRootTextFileAtRef, + integrateReviewMd: generateIntegratedReviewGuidanceWithGateway, + createBranch: createGitHubBranch, + writeReviewMd: createOrUpdateGitHubRootTextFile, + createPullRequest: createGitHubPullRequest, + markFailed: markProposalChangeRequestFailed, + log: logExceptInTest, + captureException, +}; + export class ReviewMemoryChangeRequestError extends Error { constructor( public readonly code: 'NOT_FOUND' | 'BAD_REQUEST' | 'CONFLICT', @@ -55,15 +110,26 @@ ${proposal.rationale} Kilo analyzed maintainer replies to recent review comments and found repeated feedback that this repository guidance should address. Review and edit the proposed REVIEW.md changes before merging.`; } -export async function approveAndOpenReviewMemoryChangeRequest(input: { - owner: ReviewMemoryOwner; - proposalId: string; -}): Promise { +export async function approveAndOpenReviewMemoryChangeRequest( + input: { + owner: ReviewMemoryOwner; + proposalId: string; + requestCorrelationId: string; + }, + operationOverrides: Partial = {} +): Promise { + const operations = { ...defaultOperations, ...operationOverrides }; let proposal = await getProposal({ owner: input.owner, proposalId: input.proposalId }); if (!proposal) { throw new ReviewMemoryChangeRequestError('NOT_FOUND', 'Review memory proposal not found.'); } if (proposal.status === 'change_request_opened' && proposal.change_request_url) { + emitChangeRequestRecord(operations.log, { + requestCorrelationId: input.requestCorrelationId, + stage: 'final_persistence', + outcome: 'success', + result: 'change_request_opened', + }); return proposal; } if (proposal.status === 'opening_change_request') { @@ -93,43 +159,57 @@ export async function approveAndOpenReviewMemoryChangeRequest(input: { ); } + let stage: ReviewMemoryChangeRequestStage = 'repository_validation'; try { const [repoOwner, repo] = proposal.repo_full_name.split('/'); if (!repoOwner || !repo) { - throw new Error(`Invalid repository name: ${proposal.repo_full_name}`); + throw new Error('Invalid repository name.'); } + + stage = 'installation_token_creation'; if (!integration.platform_installation_id) { throw new Error('GitHub installation id is missing.'); } - - const tokenData = await generateGitHubInstallationToken( + const tokenData = await operations.generateInstallationToken( integration.platform_installation_id, integration.github_app_type ?? 'standard' ); const token = tokenData.token; - const defaultBranch = await fetchGitHubRepositoryDefaultBranch({ + + stage = 'github_reads'; + const defaultBranch = await operations.fetchDefaultBranch({ token, owner: repoOwner, repo, }); - const existingReviewMd = await fetchGitHubRootTextFileAtRef({ + const existingReviewMd = await operations.fetchReviewMd({ token, owner: repoOwner, repo, path: REVIEW_MEMORY_TARGET_FILE_PATH, ref: defaultBranch, }); - const integrationResult = await generateIntegratedReviewGuidanceWithGateway({ + + stage = 'review_md_integration'; + const integrationResult = await operations.integrateReviewMd({ owner: input.owner, platform: 'github', repoFullName: proposal.repo_full_name, existingReviewMd, proposal, + requestCorrelationId: input.requestCorrelationId, }); if (integrationResult.status === 'already_present') { + stage = 'final_persistence'; const superseded = await markProposalSuperseded({ proposalId: proposal.id }); if (!superseded) throw new Error('Failed to mark proposal superseded.'); + emitChangeRequestRecord(operations.log, { + requestCorrelationId: input.requestCorrelationId, + stage, + outcome: 'success', + result: 'already_present', + }); return superseded; } if (!integrationResult.updatedReviewMd) { @@ -137,14 +217,17 @@ export async function approveAndOpenReviewMemoryChangeRequest(input: { } const branchName = `kilo/review-memory/${proposal.id.slice(0, 8)}`; - await createGitHubBranch({ + stage = 'branch_creation'; + await operations.createBranch({ token, owner: repoOwner, repo, branchName, baseBranch: defaultBranch, }); - await createOrUpdateGitHubRootTextFile({ + + stage = 'file_writing'; + await operations.writeReviewMd({ token, owner: repoOwner, repo, @@ -153,7 +236,9 @@ export async function approveAndOpenReviewMemoryChangeRequest(input: { message: 'Update REVIEW.md guidance', content: integrationResult.updatedReviewMd, }); - const pullRequest = await createGitHubPullRequest({ + + stage = 'pull_request_creation'; + const pullRequest = await operations.createPullRequest({ token, owner: repoOwner, repo, @@ -162,17 +247,46 @@ export async function approveAndOpenReviewMemoryChangeRequest(input: { headBranch: branchName, baseBranch: defaultBranch, }); + + stage = 'final_persistence'; const opened = await markProposalChangeRequestOpened({ proposalId: proposal.id, changeRequestUrl: pullRequest.url, }); if (!opened) throw new Error('Failed to mark proposal change request opened.'); + emitChangeRequestRecord(operations.log, { + requestCorrelationId: input.requestCorrelationId, + stage, + outcome: 'success', + result: 'change_request_opened', + }); return opened; } catch (error) { - await markProposalChangeRequestFailed({ proposalId: proposal.id }); + reportChangeRequestFailure({ + error, + stage, + requestCorrelationId: input.requestCorrelationId, + proposalId: proposal.id, + operations, + }); + + try { + const failed = await operations.markFailed({ proposalId: proposal.id }); + if (!failed) throw new Error('Failed to mark proposal change request failed.'); + } catch (persistenceError) { + reportChangeRequestFailure({ + error: persistenceError, + stage: 'failure_persistence', + failedAtStage: stage, + requestCorrelationId: input.requestCorrelationId, + proposalId: proposal.id, + operations, + }); + } + throw new ReviewMemoryChangeRequestError( 'BAD_REQUEST', - redactSensitiveErrorMessage(error instanceof Error ? error.message : String(error)) + REVIEW_MEMORY_CHANGE_REQUEST_RETRY_MESSAGE ); } } @@ -229,6 +343,124 @@ function assertGitHubPermissions(integration: PlatformIntegration): void { } } -function redactSensitiveErrorMessage(message: string): string { - return message.replace(/(?:ghs|ghu|ghp|github_pat)_[A-Za-z0-9_]+/g, '[redacted-token]'); +function reportChangeRequestFailure(input: { + error: unknown; + stage: ReviewMemoryChangeRequestStage; + failedAtStage?: ReviewMemoryChangeRequestStage; + requestCorrelationId: string; + proposalId: string; + operations: ReviewMemoryChangeRequestOperations; +}): void { + const modelDiagnostics = + input.error instanceof ReviewMemoryModelBoundaryError ? input.error.diagnostics : null; + const errorClass = getSafeErrorClass(input.error); + const errorMessage = modelDiagnostics + ? modelDiagnostics.errorMessage + : input.stage === 'failure_persistence' + ? 'Failed to persist the Review Memory change-request failure state.' + : 'Review Memory change-request operation failed.'; + + emitChangeRequestRecord(input.operations.log, { + requestCorrelationId: input.requestCorrelationId, + stage: input.stage, + outcome: 'failure', + errorClass, + errorMessage, + }); + + try { + const capturedError = createSanitizedChangeRequestError({ + sourceError: input.error, + message: errorMessage ?? 'Review Memory change-request operation failed.', + }); + input.operations.captureException(capturedError, { + level: 'error', + tags: { + source: 'review-memory', + operation: 'open-change-request', + stage: input.stage, + }, + extra: { + requestCorrelationId: input.requestCorrelationId, + proposalId: input.proposalId, + errorClass, + errorMessage, + ...(input.failedAtStage ? { failedAtStage: input.failedAtStage } : {}), + ...(modelDiagnostics ? { modelDiagnostics } : {}), + ...(modelDiagnostics?.providerRequestId + ? { providerRequestId: modelDiagnostics.providerRequestId } + : {}), + }, + }); + } catch { + // Reporting must not replace the original change-request failure. + } +} + +function emitChangeRequestRecord( + log: (record: string) => void, + input: { + requestCorrelationId: string; + stage: ReviewMemoryChangeRequestStage; + outcome: 'success' | 'failure'; + result?: 'already_present' | 'change_request_opened'; + errorClass?: string; + errorMessage?: string | null; + } +): void { + try { + log( + JSON.stringify({ + event: 'review_memory_change_request', + operation: 'open_change_request', + request_correlation_id: input.requestCorrelationId, + stage: input.stage, + outcome: input.outcome, + result: input.result ?? null, + error_class: input.errorClass ?? null, + error_message: input.errorMessage ?? null, + }) + ); + } catch { + // Telemetry must not affect change-request state transitions. + } +} + +function createSanitizedChangeRequestError(input: { + sourceError: unknown; + message: string; +}): Error { + const capturedError = new Error(sanitizeReviewMemoryDiagnosticMessage(input.message)); + capturedError.name = 'ReviewMemoryChangeRequestFailure'; + const stackFrames = getSafeStackFrames(input.sourceError); + if (stackFrames) { + capturedError.stack = `${capturedError.name}: ${capturedError.message}\n${stackFrames}`; + } + return capturedError; +} + +function getSafeStackFrames(error: unknown): string | null { + if (!(error instanceof Error) || !error.stack) return null; + const stackFrames = error.stack + .split('\n') + .slice(1) + .filter(line => /^\s*at\s/.test(line)) + .slice(0, 20) + .join('\n'); + return stackFrames ? sanitizeReviewMemoryDiagnosticMessage(stackFrames) : null; +} + +function getSafeErrorClass(error: unknown): string { + let name = 'UnknownError'; + if (error instanceof Error) { + name = error.name; + } else if ( + typeof error === 'object' && + error !== null && + 'name' in error && + typeof error.name === 'string' + ) { + name = error.name; + } + return name.replace(/[^A-Za-z0-9_.-]/g, '').slice(0, 100) || 'UnknownError'; } diff --git a/apps/web/src/lib/code-reviews/review-memory/llm.test.ts b/apps/web/src/lib/code-reviews/review-memory/llm.test.ts new file mode 100644 index 0000000000..4e3f80604d --- /dev/null +++ b/apps/web/src/lib/code-reviews/review-memory/llm.test.ts @@ -0,0 +1,411 @@ +import type { User } from '@kilocode/db/schema'; +import { MockLanguageModelV3 } from 'ai/test'; +import * as z from 'zod'; + +import { + ReviewMemoryProposalDraftSchema, + ReviewMemoryProposalWireSchema, + validateReviewMemoryProposalDraft, +} from './aggregation'; +import { + createReviewMemoryGatewayProvider, + generateReviewMemoryStructuredOutput, + ReviewMemoryModelBoundaryError, +} from './llm'; +import { + ReviewMdIntegrationOutputSchema, + validateReviewMdIntegrationOutput, +} from './review-md-integration'; + +const TestOutputSchema = z.object({ + status: z.literal('ok'), + value: z.string(), +}); + +const RAW_OUTPUT_SENTINEL = 'RAW_GENERATED_OUTPUT_SECRET'; +const PROMPT_SENTINEL = 'PROMPT_CONTENT_SECRET'; + +function buildModel(input: { + text: string; + finishReason?: { + unified: 'stop' | 'length'; + raw: string; + }; + responseId?: string; + providerRequestId?: string; +}) { + return new MockLanguageModelV3({ + provider: 'test-provider', + modelId: 'test-model', + doGenerate: { + content: [{ type: 'text', text: input.text }], + finishReason: input.finishReason ?? { unified: 'stop', raw: 'stop' }, + usage: { + inputTokens: { + total: 12, + noCache: 12, + cacheRead: 0, + cacheWrite: 0, + }, + outputTokens: { + total: 5, + text: 5, + reasoning: 0, + }, + }, + response: { + id: input.responseId ?? 'response-123', + timestamp: new Date('2026-06-18T00:00:00.000Z'), + modelId: 'test-model', + headers: { + 'request-id': input.providerRequestId ?? 'provider-request-123', + }, + }, + warnings: [], + }, + }); +} + +function generateWithModel(model: MockLanguageModelV3, log: (record: string) => void = () => {}) { + return generateReviewMemoryStructuredOutput({ + model, + modelSlug: 'test/model', + operation: 'proposal_analysis', + requestCorrelationId: 'request-correlation-123', + prompt: `Analyze without recording ${PROMPT_SENTINEL}`, + maxOutputTokens: 100, + schemaName: 'test_review_memory_output', + schema: TestOutputSchema, + validate: output => output, + log, + }); +} + +describe('Review Memory structured model output', () => { + it('returns typed JSON output with token and provider metadata', async () => { + const model = buildModel({ text: '{"status":"ok","value":"accepted"}' }); + const records: string[] = []; + + const result = await generateWithModel(model, record => records.push(record)); + + expect(result.output).toEqual({ status: 'ok', value: 'accepted' }); + expect(result.diagnostics).toEqual( + expect.objectContaining({ + outcome: 'success', + inputTokens: 12, + outputTokens: 5, + totalTokens: 17, + responseId: 'response-123', + providerRequestId: 'provider-request-123', + }) + ); + expect(model.doGenerateCalls).toHaveLength(1); + expect(model.doGenerateCalls[0]?.responseFormat).toEqual( + expect.objectContaining({ + type: 'json', + schema: expect.any(Object), + }) + ); + expect(records).toHaveLength(1); + }); + + it('transmits a provider-compatible proposal schema while validating the source Zod schema', async () => { + const model = buildModel({ + text: JSON.stringify({ + status: 'propose', + title: 'Generated fixtures', + rationale: 'Maintainers repeatedly corrected generated fixture comments.', + proposedMarkdown: 'Ignore generated fixtures unless their source behavior changes.', + positiveCount: 0, + negativeCount: 2, + neutralCount: 0, + evidenceEventIds: [], + }), + }); + + await generateReviewMemoryStructuredOutput({ + model, + modelSlug: 'test/model', + operation: 'proposal_analysis', + requestCorrelationId: 'request-proposal-schema', + prompt: 'Analyze feedback.', + maxOutputTokens: 100, + schemaName: 'review_memory_proposal', + schema: ReviewMemoryProposalDraftSchema, + wireSchema: ReviewMemoryProposalWireSchema, + validate: validateReviewMemoryProposalDraft, + log: () => {}, + }); + + const responseFormat = model.doGenerateCalls[0]?.responseFormat; + expect(responseFormat).toEqual( + expect.objectContaining({ + type: 'json', + schema: expect.objectContaining({ type: 'object' }), + }) + ); + const serializedResponseFormat = JSON.stringify(responseFormat); + expect(serializedResponseFormat).not.toContain('"oneOf"'); + expect(serializedResponseFormat).not.toMatch( + /"(?:minimum|maximum|minLength|maxLength|maxItems|uniqueItems)"/ + ); + }); + + it('transmits a provider-compatible REVIEW.md integration schema', async () => { + const model = buildModel({ + text: JSON.stringify({ + status: 'updated', + updatedReviewMd: '# Code review\n\nCheck generated fixtures against their source.', + integrationSummary: 'Added generated fixture guidance.', + }), + }); + + await generateReviewMemoryStructuredOutput({ + model, + modelSlug: 'test/model', + operation: 'review_md_integration', + requestCorrelationId: 'request-integration-schema', + prompt: 'Integrate guidance.', + maxOutputTokens: 100, + schemaName: 'review_md_integration', + schema: ReviewMdIntegrationOutputSchema, + validate: validateReviewMdIntegrationOutput, + log: () => {}, + }); + + const responseFormat = JSON.stringify(model.doGenerateCalls[0]?.responseFormat); + expect(responseFormat).toContain('"type":"json"'); + expect(responseFormat).not.toMatch(/"(?:minLength|maxLength)"/); + }); + + it('sends JSON Schema and requires compatible gateway routing', async () => { + let requestBody: unknown; + const gatewayFetch: typeof fetch = async (_request, init) => { + if (typeof init?.body !== 'string') throw new Error('Expected a JSON request body.'); + requestBody = JSON.parse(init.body); + return new Response( + JSON.stringify({ + id: 'gateway-response-id', + object: 'chat.completion', + created: 1_750_204_800, + model: 'test/model', + choices: [ + { + index: 0, + message: { + role: 'assistant', + content: '{"status":"ok","value":"accepted"}', + }, + finish_reason: 'stop', + }, + ], + usage: { + prompt_tokens: 12, + completion_tokens: 5, + total_tokens: 17, + }, + }), + { + status: 200, + headers: { + 'content-type': 'application/json', + 'request-id': 'gateway-request-id', + }, + } + ); + }; + const actor = { + id: 'test-user-id', + api_token_pepper: null, + } as User; + const provider = createReviewMemoryGatewayProvider({ + owner: { type: 'user', id: actor.id }, + actor, + userAgent: 'Review Memory structured output test', + fetch: gatewayFetch, + }); + const model = provider.chatModel('test/model'); + + await generateReviewMemoryStructuredOutput({ + model, + modelSlug: 'test/model', + operation: 'proposal_analysis', + requestCorrelationId: 'request-wire-format', + prompt: 'Return a test result.', + maxOutputTokens: 100, + schemaName: 'test_review_memory_output', + schema: TestOutputSchema, + validate: output => output, + log: () => {}, + }); + + expect(Reflect.get(model, 'supportsStructuredOutputs')).toBe(true); + expect(requestBody).toEqual( + expect.objectContaining({ + response_format: expect.objectContaining({ + type: 'json_schema', + json_schema: expect.objectContaining({ strict: true }), + }), + }) + ); + expect(requestBody).not.toEqual(expect.objectContaining({ provider: expect.anything() })); + }); + + it.each([ + ['fenced JSON', `\`\`\`json\n{"status":"ok","value":"${RAW_OUTPUT_SENTINEL}"}\n\`\`\``], + ['prose-wrapped JSON', `Result: {"status":"ok","value":"${RAW_OUTPUT_SENTINEL}"}`], + ['malformed JSON', `{"status":"ok","value":"${RAW_OUTPUT_SENTINEL}`], + ['schema-invalid JSON', `{"status":"wrong","value":"${RAW_OUTPUT_SENTINEL}"}`], + ])('rejects %s without recovering text', async (_name, text) => { + const records: string[] = []; + const model = buildModel({ text }); + + const error = await captureError(() => + generateWithModel(model, record => records.push(record)) + ); + + expect(error).toBeInstanceOf(ReviewMemoryModelBoundaryError); + if (!(error instanceof ReviewMemoryModelBoundaryError)) return; + expect(error.diagnostics).toEqual( + expect.objectContaining({ + stage: 'structured_output_validation', + outcome: 'failure', + failureClassification: 'invalid_structured_output', + }) + ); + expect(records).toHaveLength(1); + const serialized = JSON.stringify({ + message: error.message, + diagnostics: error.diagnostics, + records, + }); + expect(serialized).not.toContain(RAW_OUTPUT_SENTINEL); + expect(serialized).not.toContain(PROMPT_SENTINEL); + }); + + it('sanitizes gateway-generation failures without retaining provider content', async () => { + const records: string[] = []; + const model = new MockLanguageModelV3({ + provider: 'test-provider', + modelId: 'test-model', + doGenerate: async () => { + throw new Error( + `Provider failed at https://provider.example/path?token=secret with ${RAW_OUTPUT_SENTINEL}` + ); + }, + }); + + const error = await captureError(() => + generateWithModel(model, record => records.push(record)) + ); + + expect(error).toBeInstanceOf(ReviewMemoryModelBoundaryError); + if (!(error instanceof ReviewMemoryModelBoundaryError)) return; + expect(error.diagnostics).toEqual( + expect.objectContaining({ + stage: 'gateway_generation', + failureClassification: 'generation_failed', + errorMessage: 'Review Memory model generation failed.', + }) + ); + const serialized = JSON.stringify({ + message: error.message, + diagnostics: error.diagnostics, + records, + }); + expect(serialized).not.toContain(RAW_OUTPUT_SENTINEL); + expect(serialized).not.toContain('provider.example'); + expect(serialized).not.toContain(PROMPT_SENTINEL); + }); + + it('classifies length-limited output as incomplete before output access', async () => { + const records: string[] = []; + const model = buildModel({ + text: `{"status":"ok","value":"${RAW_OUTPUT_SENTINEL}`, + finishReason: { unified: 'length', raw: 'max_tokens' }, + responseId: 'length-response-id', + providerRequestId: 'length-provider-request-id', + }); + + const error = await captureError(() => + generateWithModel(model, record => records.push(record)) + ); + + expect(error).toBeInstanceOf(ReviewMemoryModelBoundaryError); + if (!(error instanceof ReviewMemoryModelBoundaryError)) return; + expect(error.diagnostics).toEqual( + expect.objectContaining({ + failureClassification: 'incomplete_output', + finishReason: 'length', + rawFinishReason: 'max_tokens', + generatedTextCharacterCount: expect.any(Number), + inputTokens: 12, + outputTokens: 5, + totalTokens: 17, + responseId: 'length-response-id', + providerRequestId: 'length-provider-request-id', + }) + ); + expect(records).toHaveLength(1); + const record = JSON.parse(records[0]); + expect(record).toEqual( + expect.objectContaining({ + event: 'review_memory_model_generation', + operation: 'proposal_analysis', + stage: 'structured_output_validation', + outcome: 'failure', + model_slug: 'test/model', + finish_reason: 'length', + raw_finish_reason: 'max_tokens', + provider_request_id: 'length-provider-request-id', + response_id: 'length-response-id', + }) + ); + expect(records[0]).not.toContain(RAW_OUTPUT_SENTINEL); + expect(records[0]).not.toContain(PROMPT_SENTINEL); + }); + + it('does not include generated values from semantic validation errors', async () => { + const records: string[] = []; + const model = buildModel({ + text: `{"status":"ok","value":"${RAW_OUTPUT_SENTINEL}"}`, + }); + + const error = await captureError(() => + generateReviewMemoryStructuredOutput({ + model, + modelSlug: 'test/model', + operation: 'review_md_integration', + requestCorrelationId: 'request-correlation-semantic', + prompt: PROMPT_SENTINEL, + maxOutputTokens: 100, + schemaName: 'test_semantic_output', + schema: TestOutputSchema, + validate: output => { + throw new Error(`Rejected generated value: ${output.value}`); + }, + log: record => records.push(record), + }) + ); + + expect(error).toBeInstanceOf(ReviewMemoryModelBoundaryError); + if (!(error instanceof ReviewMemoryModelBoundaryError)) return; + expect(error.diagnostics.failureClassification).toBe('invalid_semantic_output'); + const serialized = JSON.stringify({ + message: error.message, + diagnostics: error.diagnostics, + records, + }); + expect(serialized).not.toContain(RAW_OUTPUT_SENTINEL); + expect(serialized).not.toContain(PROMPT_SENTINEL); + }); +}); + +async function captureError(run: () => Promise): Promise { + try { + await run(); + } catch (error) { + return error; + } + throw new Error('Expected operation to reject.'); +} diff --git a/apps/web/src/lib/code-reviews/review-memory/llm.ts b/apps/web/src/lib/code-reviews/review-memory/llm.ts index 8f23c333fc..f6674a10ed 100644 --- a/apps/web/src/lib/code-reviews/review-memory/llm.ts +++ b/apps/web/src/lib/code-reviews/review-memory/llm.ts @@ -1,4 +1,13 @@ import { createOpenAICompatible } from '@ai-sdk/openai-compatible'; +import { + generateText, + jsonSchema, + NoObjectGeneratedError, + NoOutputGeneratedError, + Output, + zodSchema, + type LanguageModel, +} from 'ai'; import * as z from 'zod'; import { getAgentConfigForOwner } from '@/lib/agent-config/db/agent-configs'; @@ -8,14 +17,123 @@ import { APP_URL } from '@/lib/constants'; import { FEATURE_HEADER } from '@/lib/feature-detection'; import { generateApiToken } from '@/lib/tokens'; import { findUserById } from '@/lib/user'; +import { logExceptInTest } from '@/lib/utils.server'; import type { User } from '@kilocode/db/schema'; import type { ReviewMemoryPlatform } from '@kilocode/db/schema-types'; import type { ReviewMemoryOwner } from './db'; +const MAX_DIAGNOSTIC_TEXT_LENGTH = 500; +const MAX_DIAGNOSTIC_IDENTIFIER_LENGTH = 200; +// Claude rejects these constraints at the provider boundary; the source Zod schema still validates output locally. +const UNSUPPORTED_WIRE_SCHEMA_KEYWORDS = new Set([ + 'minimum', + 'maximum', + 'exclusiveMinimum', + 'exclusiveMaximum', + 'multipleOf', + 'minLength', + 'maxLength', + 'maxItems', + 'uniqueItems', + 'contains', + 'minContains', + 'maxContains', + 'minProperties', + 'maxProperties', + 'patternProperties', + 'propertyNames', + 'dependencies', + 'dependentRequired', + 'dependentSchemas', + 'unevaluatedProperties', + 'unevaluatedItems', + 'not', + 'if', + 'then', + 'else', +]); + const ReviewMemoryModelConfigSchema = z.object({ model_slug: z.string().optional(), }); +export type ReviewMemoryModelOperation = 'proposal_analysis' | 'review_md_integration'; +export type ReviewMemoryModelStage = 'gateway_generation' | 'structured_output_validation'; +export type ReviewMemoryModelFailureClassification = + | 'generation_failed' + | 'incomplete_output' + | 'invalid_structured_output' + | 'invalid_semantic_output'; + +export type ReviewMemoryModelDiagnostics = { + operation: ReviewMemoryModelOperation; + stage: ReviewMemoryModelStage; + outcome: 'success' | 'failure'; + requestCorrelationId: string; + modelSlug: string; + finishReason: string | null; + rawFinishReason: string | null; + generatedTextCharacterCount: number | null; + structuredParseCandidateCharacterCount: number | null; + inputTokens: number | null; + outputTokens: number | null; + totalTokens: number | null; + providerRequestId: string | null; + responseId: string | null; + failureClassification: ReviewMemoryModelFailureClassification | null; + errorClass: string | null; + errorMessage: string | null; +}; + +export class ReviewMemoryModelBoundaryError extends Error { + constructor(public readonly diagnostics: ReviewMemoryModelDiagnostics) { + super(diagnostics.errorMessage ?? 'Review Memory model generation failed.'); + this.name = 'ReviewMemoryModelBoundaryError'; + } +} + +type GenerationMetadata = Pick< + ReviewMemoryModelDiagnostics, + | 'finishReason' + | 'rawFinishReason' + | 'generatedTextCharacterCount' + | 'structuredParseCandidateCharacterCount' + | 'inputTokens' + | 'outputTokens' + | 'totalTokens' + | 'providerRequestId' + | 'responseId' +>; + +type GenerationMetadataSource = { + text?: string; + finishReason?: string; + rawFinishReason?: string; + usage?: { + inputTokens?: number; + outputTokens?: number; + totalTokens?: number; + }; + response?: { + id?: string; + headers?: Record; + }; +}; + +type GenerateReviewMemoryStructuredOutputInput = { + model: LanguageModel; + modelSlug: string; + operation: ReviewMemoryModelOperation; + requestCorrelationId: string; + prompt: string; + maxOutputTokens: number; + schemaName: string; + schema: z.ZodType; + wireSchema?: z.ZodType; + validate: (output: OUTPUT) => OUTPUT; + log?: (record: string) => void; +}; + export async function resolveReviewMemoryActor(owner: ReviewMemoryOwner): Promise { if (owner.type === 'org') { return await ensureBotUserForOrg(owner.id, 'code-review'); @@ -43,6 +161,7 @@ export function createReviewMemoryGatewayProvider(input: { owner: ReviewMemoryOwner; actor: User; userAgent: string; + fetch?: typeof globalThis.fetch; }) { const headers: Record = { 'User-Agent': input.userAgent, @@ -57,16 +176,347 @@ export function createReviewMemoryGatewayProvider(input: { baseURL: `${APP_URL}/api/openrouter`, apiKey: generateApiToken(input.actor, { internalApiUse: true }), headers, + fetch: input.fetch, + supportsStructuredOutputs: true, }); } -export function extractReviewMemoryJsonObject(text: string): unknown { - const trimmed = text.trim(); - if (trimmed.startsWith('{') && trimmed.endsWith('}')) return JSON.parse(trimmed); - const fenced = trimmed.match(/```(?:json)?\s*([\s\S]*?)```/i); - if (fenced?.[1]) return JSON.parse(fenced[1]); - const start = trimmed.indexOf('{'); - const end = trimmed.lastIndexOf('}'); - if (start >= 0 && end > start) return JSON.parse(trimmed.slice(start, end + 1)); - throw new Error('Review Memory model did not return JSON'); +export async function generateReviewMemoryStructuredOutput( + input: GenerateReviewMemoryStructuredOutputInput +): Promise<{ + output: OUTPUT; + diagnostics: ReviewMemoryModelDiagnostics; +}> { + const log = input.log ?? logExceptInTest; + let finishMetadata: GenerationMetadata | undefined; + const sourceSchema = zodSchema(input.schema); + const providerSchema = zodSchema(input.wireSchema ?? input.schema); + const wireSchema = jsonSchema( + Promise.resolve(providerSchema.jsonSchema).then(schema => { + const transformed = structuredClone(schema); + transformReviewMemoryWireSchema(transformed); + return transformed; + }), + { validate: sourceSchema.validate } + ); + const outputSpecification = Output.object({ + schema: wireSchema, + name: input.schemaName, + }); + const generate = () => + generateText({ + model: input.model, + prompt: input.prompt, + maxOutputTokens: input.maxOutputTokens, + output: outputSpecification, + experimental_include: { + requestBody: false, + responseBody: false, + }, + onFinish: event => { + finishMetadata = getGenerationMetadata(event); + }, + }); + let result: Awaited>; + + try { + result = await generate(); + } catch (error) { + const isStructuredOutputError = + NoObjectGeneratedError.isInstance(error) || NoOutputGeneratedError.isInstance(error); + const metadata = NoObjectGeneratedError.isInstance(error) + ? mergeGenerationMetadata(getGenerationMetadata(error), finishMetadata) + : (finishMetadata ?? emptyGenerationMetadata()); + throwModelBoundaryError({ + input, + log, + metadata, + stage: isStructuredOutputError ? 'structured_output_validation' : 'gateway_generation', + classification: isStructuredOutputError ? 'invalid_structured_output' : 'generation_failed', + sourceError: error, + }); + } + + const metadata = getGenerationMetadata(result); + if (result.finishReason === 'length') { + throwModelBoundaryError({ + input, + log, + metadata, + stage: 'structured_output_validation', + classification: 'incomplete_output', + sourceError: new Error('Generation reached its output-token limit.'), + }); + } + if (result.finishReason !== 'stop') { + throwModelBoundaryError({ + input, + log, + metadata, + stage: 'structured_output_validation', + classification: 'invalid_structured_output', + sourceError: new NoOutputGeneratedError(), + }); + } + + let output: OUTPUT; + try { + output = result.output; + } catch (error) { + throwModelBoundaryError({ + input, + log, + metadata, + stage: 'structured_output_validation', + classification: 'invalid_structured_output', + sourceError: error, + }); + } + + let validated: OUTPUT; + try { + validated = input.validate(output); + } catch (error) { + throwModelBoundaryError({ + input, + log, + metadata, + stage: 'structured_output_validation', + classification: 'invalid_semantic_output', + sourceError: error, + }); + } + + const diagnostics = createDiagnostics({ + input, + metadata, + stage: 'structured_output_validation', + outcome: 'success', + classification: null, + sourceError: null, + }); + emitModelGenerationRecord(log, diagnostics); + return { output: validated, diagnostics }; +} + +export function sanitizeReviewMemoryDiagnosticMessage(message: string): string { + return message + .replace(/https?:\/\/[^\s)>'"]+/gi, '[redacted-url]') + .replace(/\b(?:Bearer|Basic)\s+[A-Za-z0-9._~+/=-]+/gi, '[redacted-authorization]') + .replace(/(?:ghs|ghu|ghp|github_pat)_[A-Za-z0-9_]+/g, '[redacted-token]') + .replace( + /\b(api[_ -]?key|authorization|password|secret|token)\s*[:=]\s*[^\s,;]+/gi, + '$1=[redacted]' + ) + .replace(/\b[A-Za-z0-9_-]{40,}\b/g, '[redacted-value]') + .slice(0, MAX_DIAGNOSTIC_TEXT_LENGTH); +} + +function transformReviewMemoryWireSchema(schema: unknown): void { + const pending: unknown[] = [schema]; + + while (pending.length > 0) { + const value = pending.pop(); + if (Array.isArray(value)) { + pending.push(...value); + continue; + } + if (!isRecord(value)) continue; + + if (Array.isArray(value.oneOf)) { + const existingAnyOf = Array.isArray(value.anyOf) ? value.anyOf : []; + value.anyOf = [...existingAnyOf, ...value.oneOf]; + delete value.oneOf; + } + if (value.type === 'object') { + value.additionalProperties = false; + } + + for (const [key, nestedValue] of Object.entries(value)) { + if ( + UNSUPPORTED_WIRE_SCHEMA_KEYWORDS.has(key) || + (key === 'minItems' && nestedValue !== 0 && nestedValue !== 1) + ) { + delete value[key]; + continue; + } + pending.push(nestedValue); + } + } +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} + +function throwModelBoundaryError(input: { + input: GenerateReviewMemoryStructuredOutputInput; + log: (record: string) => void; + metadata: GenerationMetadata; + stage: ReviewMemoryModelStage; + classification: ReviewMemoryModelFailureClassification; + sourceError: unknown; +}): never { + const diagnostics = createDiagnostics({ + input: input.input, + metadata: input.metadata, + stage: input.stage, + outcome: 'failure', + classification: input.classification, + sourceError: input.sourceError, + }); + emitModelGenerationRecord(input.log, diagnostics); + throw new ReviewMemoryModelBoundaryError(diagnostics); +} + +function createDiagnostics(input: { + input: GenerateReviewMemoryStructuredOutputInput; + metadata: GenerationMetadata; + stage: ReviewMemoryModelStage; + outcome: 'success' | 'failure'; + classification: ReviewMemoryModelFailureClassification | null; + sourceError: unknown; +}): ReviewMemoryModelDiagnostics { + return { + operation: input.input.operation, + stage: input.stage, + outcome: input.outcome, + requestCorrelationId: input.input.requestCorrelationId, + modelSlug: input.input.modelSlug, + ...input.metadata, + failureClassification: input.classification, + errorClass: input.sourceError === null ? null : getSafeErrorClass(input.sourceError), + errorMessage: + input.classification === null ? null : getSafeModelBoundaryMessage(input.classification), + }; +} + +function getSafeModelBoundaryMessage( + classification: ReviewMemoryModelFailureClassification +): string { + switch (classification) { + case 'generation_failed': + return 'Review Memory model generation failed.'; + case 'incomplete_output': + return 'Review Memory structured output was incomplete.'; + case 'invalid_structured_output': + return 'Review Memory model returned invalid structured output.'; + case 'invalid_semantic_output': + return 'Review Memory structured output failed semantic validation.'; + } +} + +function getSafeErrorClass(error: unknown): string { + let name = 'UnknownError'; + if (error instanceof Error) { + name = error.name; + } else if ( + typeof error === 'object' && + error !== null && + 'name' in error && + typeof error.name === 'string' + ) { + name = error.name; + } + return name.replace(/[^A-Za-z0-9_.-]/g, '').slice(0, 100) || 'UnknownError'; +} + +function getGenerationMetadata(source: GenerationMetadataSource): GenerationMetadata { + const textLength = source.text?.length ?? null; + return { + finishReason: boundedIdentifier(source.finishReason), + rawFinishReason: boundedIdentifier(source.rawFinishReason), + generatedTextCharacterCount: textLength, + structuredParseCandidateCharacterCount: textLength, + inputTokens: source.usage?.inputTokens ?? null, + outputTokens: source.usage?.outputTokens ?? null, + totalTokens: source.usage?.totalTokens ?? null, + providerRequestId: boundedIdentifier(getResponseHeader(source.response?.headers, 'request-id')), + responseId: boundedIdentifier(source.response?.id), + }; +} + +function emptyGenerationMetadata(): GenerationMetadata { + return { + finishReason: null, + rawFinishReason: null, + generatedTextCharacterCount: null, + structuredParseCandidateCharacterCount: null, + inputTokens: null, + outputTokens: null, + totalTokens: null, + providerRequestId: null, + responseId: null, + }; +} + +function mergeGenerationMetadata( + primary: GenerationMetadata, + fallback: GenerationMetadata | undefined +): GenerationMetadata { + if (!fallback) return primary; + return { + finishReason: primary.finishReason ?? fallback.finishReason, + rawFinishReason: primary.rawFinishReason ?? fallback.rawFinishReason, + generatedTextCharacterCount: + primary.generatedTextCharacterCount ?? fallback.generatedTextCharacterCount, + structuredParseCandidateCharacterCount: + primary.structuredParseCandidateCharacterCount ?? + fallback.structuredParseCandidateCharacterCount, + inputTokens: primary.inputTokens ?? fallback.inputTokens, + outputTokens: primary.outputTokens ?? fallback.outputTokens, + totalTokens: primary.totalTokens ?? fallback.totalTokens, + providerRequestId: primary.providerRequestId ?? fallback.providerRequestId, + responseId: primary.responseId ?? fallback.responseId, + }; +} + +function getResponseHeader( + headers: Record | undefined, + requestedName: string +): string | undefined { + if (!headers) return undefined; + const entry = Object.entries(headers).find( + ([name]) => name.toLowerCase() === requestedName.toLowerCase() + ); + return entry?.[1]; +} + +function boundedIdentifier(value: string | undefined): string | null { + return value?.slice(0, MAX_DIAGNOSTIC_IDENTIFIER_LENGTH) ?? null; +} + +function emitModelGenerationRecord( + log: (record: string) => void, + diagnostics: ReviewMemoryModelDiagnostics +): void { + try { + log( + JSON.stringify({ + event: 'review_memory_model_generation', + operation: diagnostics.operation, + stage: diagnostics.stage, + outcome: diagnostics.outcome, + request_correlation_id: diagnostics.requestCorrelationId, + vercel_deployment_id: process.env.VERCEL_DEPLOYMENT_ID ?? null, + vercel_region: process.env.VERCEL_REGION ?? process.env.VERCEL_FUNCTION_REGION ?? null, + model_slug: diagnostics.modelSlug, + finish_reason: diagnostics.finishReason, + raw_finish_reason: diagnostics.rawFinishReason, + generated_text_character_count: diagnostics.generatedTextCharacterCount, + structured_parse_candidate_character_count: + diagnostics.structuredParseCandidateCharacterCount, + input_tokens: diagnostics.inputTokens, + output_tokens: diagnostics.outputTokens, + total_tokens: diagnostics.totalTokens, + provider_request_id: diagnostics.providerRequestId, + response_id: diagnostics.responseId, + failure_classification: diagnostics.failureClassification, + error_class: diagnostics.errorClass, + error_message: diagnostics.errorMessage, + }) + ); + } catch { + // Telemetry must not affect model-boundary behavior. + } } diff --git a/apps/web/src/lib/code-reviews/review-memory/review-md-integration.test.ts b/apps/web/src/lib/code-reviews/review-memory/review-md-integration.test.ts new file mode 100644 index 0000000000..11f9f99f05 --- /dev/null +++ b/apps/web/src/lib/code-reviews/review-memory/review-md-integration.test.ts @@ -0,0 +1,65 @@ +import { ReviewMemoryProposalDraftSchema, validateReviewMemoryProposalDraft } from './aggregation'; +import { + ReviewMdIntegrationOutputSchema, + validateReviewMdIntegrationOutput, +} from './review-md-integration'; + +describe('Review Memory model output semantics', () => { + it('accepts a complete REVIEW.md update', () => { + const output = ReviewMdIntegrationOutputSchema.parse({ + status: 'updated', + updatedReviewMd: '# Code review\n\nCheck generated fixtures against their source.', + integrationSummary: 'Added generated fixture guidance.', + }); + + expect(validateReviewMdIntegrationOutput(output)).toEqual(output); + }); + + it.each([null, '', ' '])( + 'rejects updated status without non-empty REVIEW.md content (%p)', + updatedReviewMd => { + const parsed = ReviewMdIntegrationOutputSchema.safeParse({ + status: 'updated', + updatedReviewMd, + integrationSummary: 'Attempted update.', + }); + + if (!parsed.success) { + expect(updatedReviewMd).toBe(''); + return; + } + expect(() => validateReviewMdIntegrationOutput(parsed.data)).toThrow( + /without updatedReviewMd|empty REVIEW\.md/ + ); + } + ); + + it('rejects Review Memory branding in integrated guidance', () => { + const output = ReviewMdIntegrationOutputSchema.parse({ + status: 'updated', + updatedReviewMd: '# Review Memory\n\nRemember maintainer preferences.', + integrationSummary: 'Added guidance.', + }); + + expect(() => validateReviewMdIntegrationOutput(output)).toThrow( + 'Integrated REVIEW.md must not mention Review Memory.' + ); + }); + + it('retains proposal branding validation after structured parsing', () => { + const draft = ReviewMemoryProposalDraftSchema.parse({ + status: 'propose', + title: 'Generated fixtures', + rationale: 'Maintainers repeatedly corrected generated fixture comments.', + proposedMarkdown: 'Kilo should ignore generated fixtures.', + positiveCount: 0, + negativeCount: 2, + neutralCount: 0, + evidenceEventIds: [], + }); + + expect(() => validateReviewMemoryProposalDraft(draft)).toThrow( + 'Review Memory proposal must not mention Kilo, Review Memory, feedback systems, or LLMs.' + ); + }); +}); diff --git a/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts b/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts index e6b34e5bba..0dc3908ae8 100644 --- a/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts +++ b/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts @@ -1,4 +1,3 @@ -import { generateText } from 'ai'; import * as z from 'zod'; import type { CodeReviewMemoryProposal } from '@kilocode/db/schema'; @@ -6,7 +5,7 @@ import type { ReviewMemoryPlatform } from '@kilocode/db/schema-types'; import type { ReviewMemoryOwner } from './db'; import { createReviewMemoryGatewayProvider, - extractReviewMemoryJsonObject, + generateReviewMemoryStructuredOutput, resolveReviewMemoryActor, resolveReviewMemoryModel, } from './llm'; @@ -15,7 +14,7 @@ const MAX_REVIEW_MD_CHARS = 30_000; const REVIEW_MEMORY_BRANDING_PATTERN = /\b(?:kilo\s+)?review\s+memory\b/i; const REVIEW_MEMORY_HEADING_PATTERN = /^#{1,6}\s+(?:kilo\s+)?review\s+memory\b/im; -const ReviewMdIntegrationOutputSchema = z.object({ +export const ReviewMdIntegrationOutputSchema = z.object({ status: z.enum(['updated', 'already_present']), updatedReviewMd: z.string().min(1).max(MAX_REVIEW_MD_CHARS).nullable(), integrationSummary: z.string().min(1).max(1_000), @@ -35,6 +34,7 @@ export async function generateIntegratedReviewGuidanceWithGateway(input: { repoFullName: string; existingReviewMd: string | null; proposal: CodeReviewMemoryProposal; + requestCorrelationId: string; }): Promise { if (input.existingReviewMd && input.existingReviewMd.length > MAX_REVIEW_MD_CHARS) { throw new Error( @@ -53,22 +53,26 @@ export async function generateIntegratedReviewGuidanceWithGateway(input: { userAgent: 'Kilo Review Memory Integrator', }); - const result = await generateText({ + const result = await generateReviewMemoryStructuredOutput({ model: provider.chatModel(modelSlug), + modelSlug, + operation: 'review_md_integration', + requestCorrelationId: input.requestCorrelationId, prompt: buildReviewMdIntegrationPrompt(input), maxOutputTokens: 8_000, + schemaName: 'review_md_integration', + schema: ReviewMdIntegrationOutputSchema, + validate: validateReviewMdIntegrationOutput, }); - const parsed = ReviewMdIntegrationOutputSchema.parse(extractReviewMemoryJsonObject(result.text)); - const validated = validateReviewMdIntegrationOutput(parsed); return { - ...validated, - tokensIn: result.usage.inputTokens ?? null, - tokensOut: result.usage.outputTokens ?? null, + ...result.output, + tokensIn: result.diagnostics.inputTokens, + tokensOut: result.diagnostics.outputTokens, }; } -function validateReviewMdIntegrationOutput( +export function validateReviewMdIntegrationOutput( output: z.infer ): ReviewMdIntegrationResult { if (output.status === 'already_present') { diff --git a/apps/web/src/lib/trpc/init.ts b/apps/web/src/lib/trpc/init.ts index 2b1a40c523..6b18cc813b 100644 --- a/apps/web/src/lib/trpc/init.ts +++ b/apps/web/src/lib/trpc/init.ts @@ -1,19 +1,34 @@ import 'server-only'; +import { randomUUID } from 'node:crypto'; import { getUserFromAuth } from '@/lib/user/server'; import { initTRPC, TRPCError } from '@trpc/server'; import type { User } from '@kilocode/db/schema'; import * as z from 'zod'; import { setTag, trpcMiddleware } from '@sentry/nextjs'; import { userCanManageCredits } from '@/lib/admin/credit-management'; +const MAX_REQUEST_CORRELATION_ID_LENGTH = 200; +const REQUEST_CORRELATION_ID_PATTERN = /^(?:[a-z]{3}\d::){1,2}[a-z0-9]{5}-\d{13}-[a-f0-9]{12}$/; + // Define the context type export type TRPCContext = { user: User; + requestCorrelationId?: string; +}; + +type CorrelatedTRPCContext = TRPCContext & { + requestCorrelationId: string; +}; + +type CreateTRPCContextInput = { + requestCorrelationId?: string | null; }; /** * @see: https://trpc.io/docs/server/context */ -export const createTRPCContext = async (): Promise => { +export const createTRPCContext = async ( + input: CreateTRPCContextInput = {} +): Promise => { const { user } = await getUserFromAuth({ adminOnly: false }); if (!user) { throw new TRPCError({ @@ -24,9 +39,22 @@ export const createTRPCContext = async (): Promise => { setTag('userId', user.id); return { user, + requestCorrelationId: normalizeRequestCorrelationId(input.requestCorrelationId), }; }; +function normalizeRequestCorrelationId(requestCorrelationId: string | null | undefined): string { + const normalized = requestCorrelationId?.trim(); + if ( + !normalized || + normalized.length > MAX_REQUEST_CORRELATION_ID_LENGTH || + !REQUEST_CORRELATION_ID_PATTERN.test(normalized) + ) { + return randomUUID(); + } + return normalized; +} + // Avoid exporting the entire t-object // since it's not very descriptive. // For instance, the use of a t variable @@ -69,6 +97,15 @@ const t = initTRPC.context().create({ }, }); +const requestCorrelationMiddleware = t.middleware(({ ctx, next }) => + next({ + ctx: { + ...ctx, + requestCorrelationId: ctx.requestCorrelationId ?? randomUUID(), + }, + }) +); + const sentryMiddleware = t.middleware( trpcMiddleware({ attachRpcInput: false, @@ -97,7 +134,10 @@ const timingMiddleware = t.middleware(async ({ path, type, ctx, next }) => { // Base router and procedure helpers export const createTRPCRouter = t.router; export const createCallerFactory = t.createCallerFactory; -export const baseProcedure = t.procedure.use(timingMiddleware).use(sentryMiddleware); +export const baseProcedure = t.procedure + .use(requestCorrelationMiddleware) + .use(timingMiddleware) + .use(sentryMiddleware); // Admin-only procedure export const adminProcedure = baseProcedure.use(async ({ ctx, next }) => { diff --git a/apps/web/src/routers/code-reviews/review-memory-router.ts b/apps/web/src/routers/code-reviews/review-memory-router.ts index dac9e6b934..9e9bf3ef13 100644 --- a/apps/web/src/routers/code-reviews/review-memory-router.ts +++ b/apps/web/src/routers/code-reviews/review-memory-router.ts @@ -105,6 +105,7 @@ export const reviewMemoryRouter = createTRPCRouter({ owner, platform: input.platform, repoFullName: input.repoFullName, + requestCorrelationId: ctx.requestCorrelationId, }); }), @@ -151,6 +152,7 @@ export const reviewMemoryRouter = createTRPCRouter({ return await approveAndOpenReviewMemoryChangeRequest({ owner, proposalId: input.proposalId, + requestCorrelationId: ctx.requestCorrelationId, }); } catch (error) { if (error instanceof ReviewMemoryChangeRequestError) { diff --git a/apps/web/src/routers/test-utils.ts b/apps/web/src/routers/test-utils.ts index d932d6b620..94ab4d0a77 100644 --- a/apps/web/src/routers/test-utils.ts +++ b/apps/web/src/routers/test-utils.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'node:crypto'; import type { TRPCContext } from '@/lib/trpc/init'; import { createCallerFactory } from '@/lib/trpc/init'; import { findUserById } from '@/lib/user'; @@ -13,6 +14,7 @@ const createTestTRPCContext = async (userId: string): Promise => { } return { user, + requestCorrelationId: randomUUID(), }; }; From 77a41600f7ea9418e15bbb077d53d6f99653c671 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Thu, 18 Jun 2026 14:52:06 +0300 Subject: [PATCH 2/5] refactor(review): share error class helper --- .../review-memory/change-request.ts | 21 +++++-------------- .../src/lib/code-reviews/review-memory/llm.ts | 2 +- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/apps/web/src/lib/code-reviews/review-memory/change-request.ts b/apps/web/src/lib/code-reviews/review-memory/change-request.ts index 27cd31e8db..b618a22fc9 100644 --- a/apps/web/src/lib/code-reviews/review-memory/change-request.ts +++ b/apps/web/src/lib/code-reviews/review-memory/change-request.ts @@ -21,7 +21,11 @@ import { markProposalSuperseded, type ReviewMemoryOwner, } from './db'; -import { ReviewMemoryModelBoundaryError, sanitizeReviewMemoryDiagnosticMessage } from './llm'; +import { + getSafeErrorClass, + ReviewMemoryModelBoundaryError, + sanitizeReviewMemoryDiagnosticMessage, +} from './llm'; import { generateIntegratedReviewGuidanceWithGateway } from './review-md-integration'; const REVIEW_MEMORY_TARGET_FILE_PATH = 'REVIEW.md'; @@ -449,18 +453,3 @@ function getSafeStackFrames(error: unknown): string | null { .join('\n'); return stackFrames ? sanitizeReviewMemoryDiagnosticMessage(stackFrames) : null; } - -function getSafeErrorClass(error: unknown): string { - let name = 'UnknownError'; - if (error instanceof Error) { - name = error.name; - } else if ( - typeof error === 'object' && - error !== null && - 'name' in error && - typeof error.name === 'string' - ) { - name = error.name; - } - return name.replace(/[^A-Za-z0-9_.-]/g, '').slice(0, 100) || 'UnknownError'; -} diff --git a/apps/web/src/lib/code-reviews/review-memory/llm.ts b/apps/web/src/lib/code-reviews/review-memory/llm.ts index f6674a10ed..bfe9f8b3b0 100644 --- a/apps/web/src/lib/code-reviews/review-memory/llm.ts +++ b/apps/web/src/lib/code-reviews/review-memory/llm.ts @@ -406,7 +406,7 @@ function getSafeModelBoundaryMessage( } } -function getSafeErrorClass(error: unknown): string { +export function getSafeErrorClass(error: unknown): string { let name = 'UnknownError'; if (error instanceof Error) { name = error.name; From 8d2fcc43f37cfd88504d65e7b0a9f0b4a9dd1f1d Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Thu, 18 Jun 2026 15:08:22 +0300 Subject: [PATCH 3/5] refactor(ai-gateway): rename json helper --- .../providers/apply-provider-specific-logic.test.ts | 8 ++++---- .../ai-gateway/providers/apply-provider-specific-logic.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.test.ts b/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.test.ts index 196c023c22..109650f47a 100644 --- a/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.test.ts +++ b/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.test.ts @@ -1,8 +1,8 @@ import { describe, expect, it } from '@jest/globals'; import { CLAUDE_OPUS_CURRENT_MODEL_ID } from '@/lib/ai-gateway/providers/anthropic.constants'; import { + applyOpenRouterStructuredOutputRouting, applyGatewayModelsFallback, - requireStructuredOutputParameters, } from '@/lib/ai-gateway/providers/apply-provider-specific-logic'; import type { GatewayRequest } from '@/lib/ai-gateway/providers/openrouter/types'; import type { ProviderId } from '@/lib/ai-gateway/providers/types'; @@ -50,12 +50,12 @@ describe('applyGatewayModelsFallback', () => { }); }); -describe('requireStructuredOutputParameters', () => { +describe('applyOpenRouterStructuredOutputRouting', () => { it('requires structured-output support from OpenRouter providers', () => { const request = makeStructuredOutputRequest(); request.body.provider = { only: ['anthropic'] }; - requireStructuredOutputParameters('openrouter', request); + applyOpenRouterStructuredOutputRouting('openrouter', request); expect(request.body.provider).toEqual({ only: ['anthropic'], @@ -68,7 +68,7 @@ describe('requireStructuredOutputParameters', () => { providerId => { const request = makeStructuredOutputRequest(); - requireStructuredOutputParameters(providerId, request); + applyOpenRouterStructuredOutputRouting(providerId, request); expect(request.body.provider).toBeUndefined(); } diff --git a/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.ts b/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.ts index a6981b6e33..bd2682365a 100644 --- a/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.ts +++ b/apps/web/src/lib/ai-gateway/providers/apply-provider-specific-logic.ts @@ -113,7 +113,7 @@ export function applyGatewayModelsFallback( delete requestToMutate.body.models; } -export function requireStructuredOutputParameters( +export function applyOpenRouterStructuredOutputRouting( providerId: ProviderId, requestToMutate: GatewayRequest ): void { @@ -142,7 +142,7 @@ export function applyProviderSpecificLogic( taskId: string | null ) { applyGatewayModelsFallback(provider.id, requestedModel, requestToMutate); - requireStructuredOutputParameters(provider.id, requestToMutate); + applyOpenRouterStructuredOutputRouting(provider.id, requestToMutate); applyTrackingIds(requestToMutate, provider, userId, taskId); sanitizeBinaryToolResults(requestToMutate); From af7f029481be21e55dc1884a7783e656dad77a69 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Thu, 18 Jun 2026 15:33:45 +0300 Subject: [PATCH 4/5] refactor(review-memory): rely on schemas --- apps/web/src/lib/code-reviews/review-memory/aggregation.ts | 4 ---- .../lib/code-reviews/review-memory/review-md-integration.ts | 3 --- 2 files changed, 7 deletions(-) diff --git a/apps/web/src/lib/code-reviews/review-memory/aggregation.ts b/apps/web/src/lib/code-reviews/review-memory/aggregation.ts index 33a2e40625..89fbc13ad0 100644 --- a/apps/web/src/lib/code-reviews/review-memory/aggregation.ts +++ b/apps/web/src/lib/code-reviews/review-memory/aggregation.ts @@ -162,10 +162,6 @@ function buildReviewMemoryAnalysisPrompt(input: { return `You analyze maintainer replies to Kilo's automated code-review comments for one repository. -Return strict JSON in one of these shapes: -{"status":"no_change","title":null,"rationale":null,"proposedMarkdown":null,"positiveCount":0,"negativeCount":0,"neutralCount":0,"evidenceEventIds":[]} -{"status":"propose","title":"short proposal title","rationale":"why this guidance is justified","proposedMarkdown":"standalone REVIEW.md guidance","positiveCount":0,"negativeCount":0,"neutralCount":0,"evidenceEventIds":["event ids"]} - Rules: - Classify each maintainer reply as positive, negative, or neutral. - Propose the smallest possible REVIEW.md change only when there is a clear, repeated pattern. diff --git a/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts b/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts index 0dc3908ae8..301ee9e145 100644 --- a/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts +++ b/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts @@ -107,9 +107,6 @@ function buildReviewMdIntegrationPrompt(input: { return `You are a repository maintainer editing REVIEW.md, the repository-maintained instructions for automated code review. -Return strict JSON with this shape: -{"status":"updated|already_present","updatedReviewMd":"complete updated REVIEW.md or null","integrationSummary":"short summary"} - Rules: - Preserve existing guidance, ordering, and voice as much as possible. - Integrate the proposal into the most relevant existing section when one exists. From 83096c946f8d63fbca72d2a789a01fe838c41cd3 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Thu, 18 Jun 2026 16:25:13 +0300 Subject: [PATCH 5/5] refactor(review-memory): trim schema PR --- apps/web/src/app/api/trpc/[trpc]/route.ts | 3 +- .../review-memory/aggregation.test.ts | 9 +- .../code-reviews/review-memory/aggregation.ts | 17 +- .../review-memory/change-request.test.ts | 309 +------------- .../review-memory/change-request.ts | 257 +----------- .../code-reviews/review-memory/llm.test.ts | 352 +--------------- .../src/lib/code-reviews/review-memory/llm.ts | 378 +----------------- .../review-md-integration.test.ts | 65 --- .../review-memory/review-md-integration.ts | 12 +- apps/web/src/lib/trpc/init.ts | 44 +- .../code-reviews/review-memory-router.ts | 2 - apps/web/src/routers/test-utils.ts | 2 - 12 files changed, 58 insertions(+), 1392 deletions(-) delete mode 100644 apps/web/src/lib/code-reviews/review-memory/review-md-integration.test.ts diff --git a/apps/web/src/app/api/trpc/[trpc]/route.ts b/apps/web/src/app/api/trpc/[trpc]/route.ts index 86f9d2161e..f32c374ac4 100644 --- a/apps/web/src/app/api/trpc/[trpc]/route.ts +++ b/apps/web/src/app/api/trpc/[trpc]/route.ts @@ -9,8 +9,7 @@ const handler = (req: Request) => endpoint: '/api/trpc', req, router: rootRouter, - createContext: () => - createTRPCContext({ requestCorrelationId: req.headers.get('x-vercel-id') }), + createContext: createTRPCContext, }); export { handler as GET, handler as POST }; diff --git a/apps/web/src/lib/code-reviews/review-memory/aggregation.test.ts b/apps/web/src/lib/code-reviews/review-memory/aggregation.test.ts index 47a8c25b47..59649cd6ac 100644 --- a/apps/web/src/lib/code-reviews/review-memory/aggregation.test.ts +++ b/apps/web/src/lib/code-reviews/review-memory/aggregation.test.ts @@ -26,7 +26,6 @@ describe('review memory aggregation', () => { owner, platform: 'github', repoFullName: 'acme/widgets', - requestCorrelationId: 'request-proposal-created', generate: async () => ({ draft: { status: 'propose', @@ -67,7 +66,6 @@ describe('review memory aggregation', () => { owner, platform: 'github', repoFullName: 'acme/widgets', - requestCorrelationId: 'request-no-change', generate: async () => ({ draft: { status: 'no_change', @@ -76,12 +74,7 @@ describe('review memory aggregation', () => { }) ).resolves.toEqual({ status: 'no_change' }); await expect( - runReviewMemoryAnalysis({ - owner, - platform: 'github', - repoFullName: 'acme/empty', - requestCorrelationId: 'request-no-feedback', - }) + runReviewMemoryAnalysis({ owner, platform: 'github', repoFullName: 'acme/empty' }) ).resolves.toEqual({ status: 'no_feedback' }); await expect(db.select().from(code_review_memory_proposals)).resolves.toHaveLength(0); }); diff --git a/apps/web/src/lib/code-reviews/review-memory/aggregation.ts b/apps/web/src/lib/code-reviews/review-memory/aggregation.ts index 89fbc13ad0..69bee7d136 100644 --- a/apps/web/src/lib/code-reviews/review-memory/aggregation.ts +++ b/apps/web/src/lib/code-reviews/review-memory/aggregation.ts @@ -15,7 +15,7 @@ import { reviewMemoryRetentionCutoff } from './retention'; const REVIEW_MEMORY_FORBIDDEN_PROPOSAL_PATTERN = /\b(?:review\s+memory|kilo|feedback\s+systems?|this\s+analysis|llms?)\b/i; -export const ReviewMemoryProposalDraftSchema = z.discriminatedUnion('status', [ +const ReviewMemoryProposalDraftSchema = z.discriminatedUnion('status', [ z.object({ status: z.literal('no_change') }), z.object({ status: z.literal('propose'), @@ -29,7 +29,7 @@ export const ReviewMemoryProposalDraftSchema = z.discriminatedUnion('status', [ }), ]); -export const ReviewMemoryProposalWireSchema = z.object({ +const ReviewMemoryProposalWireSchema = z.object({ status: z.enum(['no_change', 'propose']), title: z.string().nullable(), rationale: z.string().nullable(), @@ -47,7 +47,6 @@ export type GenerateReviewMemoryProposal = (input: { platform: ReviewMemoryPlatform; repoFullName: string; events: CodeReviewFeedbackEvent[]; - requestCorrelationId: string; }) => Promise<{ draft: ReviewMemoryProposalDraft; tokensIn?: number | null; @@ -59,7 +58,6 @@ export async function generateReviewMemoryProposalWithGateway(input: { platform: ReviewMemoryPlatform; repoFullName: string; events: CodeReviewFeedbackEvent[]; - requestCorrelationId: string; }): Promise<{ draft: ReviewMemoryProposalDraft; tokensIn?: number | null; @@ -77,9 +75,6 @@ export async function generateReviewMemoryProposalWithGateway(input: { }); const result = await generateReviewMemoryStructuredOutput({ model: provider.chatModel(modelSlug), - modelSlug, - operation: 'proposal_analysis', - requestCorrelationId: input.requestCorrelationId, prompt: buildReviewMemoryAnalysisPrompt(input), maxOutputTokens: 4_000, schemaName: 'review_memory_proposal', @@ -90,8 +85,8 @@ export async function generateReviewMemoryProposalWithGateway(input: { return { draft: result.output, - tokensIn: result.diagnostics.inputTokens, - tokensOut: result.diagnostics.outputTokens, + tokensIn: result.tokensIn, + tokensOut: result.tokensOut, }; } @@ -99,7 +94,6 @@ export async function runReviewMemoryAnalysis(input: { owner: ReviewMemoryOwner; platform: ReviewMemoryPlatform; repoFullName: string; - requestCorrelationId: string; generate?: GenerateReviewMemoryProposal; now?: Date; }): Promise<{ status: 'proposed' | 'no_change' | 'no_feedback'; proposalId?: string }> { @@ -119,7 +113,6 @@ export async function runReviewMemoryAnalysis(input: { platform: input.platform, repoFullName: input.repoFullName, events, - requestCorrelationId: input.requestCorrelationId, }); if (draft.status === 'no_change') return { status: 'no_change' }; @@ -179,7 +172,7 @@ Feedback events: ${JSON.stringify(events, null, 2)}`; } -export function validateReviewMemoryProposalDraft( +function validateReviewMemoryProposalDraft( draft: ReviewMemoryProposalDraft ): ReviewMemoryProposalDraft { if (draft.status === 'no_change') return draft; diff --git a/apps/web/src/lib/code-reviews/review-memory/change-request.test.ts b/apps/web/src/lib/code-reviews/review-memory/change-request.test.ts index 071bcae044..b7646df474 100644 --- a/apps/web/src/lib/code-reviews/review-memory/change-request.test.ts +++ b/apps/web/src/lib/code-reviews/review-memory/change-request.test.ts @@ -4,7 +4,6 @@ import { insertTestUser } from '@/tests/helpers/user.helper'; import { code_review_memory_proposals, kilocode_users, - platform_integrations, type CodeReviewMemoryProposal, } from '@kilocode/db/schema'; @@ -16,24 +15,11 @@ import { upsertScopeProposal, type ReviewMemoryOwner, } from './db'; -import { - approveAndOpenReviewMemoryChangeRequest, - buildChangeRequestBody, - isStaleOpeningChangeRequest, - ReviewMemoryChangeRequestError, - type ReviewMemoryChangeRequestOperations, -} from './change-request'; -import { ReviewMemoryModelBoundaryError, type ReviewMemoryModelDiagnostics } from './llm'; - -const REQUEST_CORRELATION_ID = 'request-correlation-change-request'; -const RAW_OUTPUT_SENTINEL = 'RAW_MODEL_OUTPUT_SECRET'; -const REVIEW_MD_SENTINEL = 'EXISTING_REVIEW_MD_SECRET'; -const REPOSITORY_SENTINEL = 'sensitive-owner/sensitive-repository'; +import { buildChangeRequestBody, isStaleOpeningChangeRequest } from './change-request'; describe('review memory change requests', () => { afterEach(async () => { await db.delete(code_review_memory_proposals); - await db.delete(platform_integrations); await db.delete(kilocode_users); }); @@ -87,210 +73,6 @@ describe('review memory change requests', () => { expect(isStaleOpeningChangeRequest('2026-06-01T11:30:00.000Z', now)).toBe(true); expect(isStaleOpeningChangeRequest('not-a-date', now)).toBe(false); }); - - it('reports a structured-output failure safely before GitHub mutations', async () => { - const user = await insertTestUser(); - const owner = { type: 'user' as const, id: user.id }; - const proposal = await seedProposal(owner, REPOSITORY_SENTINEL); - await seedGitHubIntegration(owner); - const operations = buildOperations(); - operations.fetchReviewMd.mockResolvedValue(REVIEW_MD_SENTINEL); - const modelFailure = buildModelBoundaryError(); - Object.assign(modelFailure, { - text: RAW_OUTPUT_SENTINEL, - cause: new Error(`Cause containing ${RAW_OUTPUT_SENTINEL}`), - }); - operations.integrateReviewMd.mockRejectedValue(modelFailure); - - const error = await captureError(() => - approveAndOpenReviewMemoryChangeRequest( - { - owner, - proposalId: proposal.id, - requestCorrelationId: REQUEST_CORRELATION_ID, - }, - operations - ) - ); - - expect(error).toBeInstanceOf(ReviewMemoryChangeRequestError); - expect(error).toMatchObject({ - code: 'BAD_REQUEST', - message: 'Unable to open the REVIEW.md change request. The proposal can be retried.', - }); - const proposals = await db.select().from(code_review_memory_proposals); - expect(proposals).toEqual([ - expect.objectContaining({ id: proposal.id, status: 'change_request_failed' }), - ]); - expect(operations.createBranch).not.toHaveBeenCalled(); - expect(operations.writeReviewMd).not.toHaveBeenCalled(); - expect(operations.createPullRequest).not.toHaveBeenCalled(); - expect(operations.captureException).toHaveBeenCalledTimes(1); - - const [capturedError, captureContext] = operations.captureException.mock.calls[0]; - expect(capturedError).toEqual( - expect.objectContaining({ - name: 'ReviewMemoryChangeRequestFailure', - message: 'Review Memory model returned invalid structured output.', - }) - ); - expect(captureContext).toEqual( - expect.objectContaining({ - tags: { - source: 'review-memory', - operation: 'open-change-request', - stage: 'review_md_integration', - }, - extra: expect.objectContaining({ - requestCorrelationId: REQUEST_CORRELATION_ID, - proposalId: proposal.id, - providerRequestId: 'provider-request-model-failure', - modelDiagnostics: expect.objectContaining({ - failureClassification: 'invalid_structured_output', - }), - }), - }) - ); - const serializedCapture = JSON.stringify({ - name: capturedError.name, - message: capturedError.message, - stack: capturedError.stack, - captureContext, - }); - expect(serializedCapture).not.toContain(RAW_OUTPUT_SENTINEL); - expect(serializedCapture).not.toContain(REVIEW_MD_SENTINEL); - expect(serializedCapture).not.toContain(REPOSITORY_SENTINEL); - }); - - it('does not capture expected pre-claim domain errors', async () => { - const user = await insertTestUser(); - const owner = { type: 'user' as const, id: user.id }; - const proposal = await seedProposal(owner, 'acme/no-integration'); - const operations = buildOperations(); - - const error = await captureError(() => - approveAndOpenReviewMemoryChangeRequest( - { - owner, - proposalId: proposal.id, - requestCorrelationId: REQUEST_CORRELATION_ID, - }, - operations - ) - ); - - expect(error).toMatchObject({ - code: 'BAD_REQUEST', - message: 'No active GitHub integration has access to this repository.', - }); - const proposals = await db.select().from(code_review_memory_proposals); - expect(proposals).toEqual([expect.objectContaining({ id: proposal.id, status: 'open' })]); - expect(operations.generateInstallationToken).not.toHaveBeenCalled(); - expect(operations.captureException).not.toHaveBeenCalled(); - }); - - it('supersedes already-present guidance without GitHub mutations', async () => { - const user = await insertTestUser(); - const owner = { type: 'user' as const, id: user.id }; - const proposal = await seedProposal(owner, 'acme/already-present'); - await seedGitHubIntegration(owner); - const operations = buildOperations(); - operations.integrateReviewMd.mockResolvedValue({ - status: 'already_present', - updatedReviewMd: null, - integrationSummary: 'Equivalent guidance already exists.', - tokensIn: 12, - tokensOut: 5, - }); - - const result = await approveAndOpenReviewMemoryChangeRequest( - { - owner, - proposalId: proposal.id, - requestCorrelationId: REQUEST_CORRELATION_ID, - }, - operations - ); - - expect(result.status).toBe('superseded'); - expect(operations.createBranch).not.toHaveBeenCalled(); - expect(operations.writeReviewMd).not.toHaveBeenCalled(); - expect(operations.createPullRequest).not.toHaveBeenCalled(); - expect(operations.captureException).not.toHaveBeenCalled(); - expect(operations.log).toHaveBeenCalledWith( - expect.stringContaining('"result":"already_present"') - ); - }); - - it('opens and persists a change request after validated model output', async () => { - const user = await insertTestUser(); - const owner = { type: 'user' as const, id: user.id }; - const proposal = await seedProposal(owner, 'acme/success'); - await seedGitHubIntegration(owner); - const operations = buildOperations(); - - const result = await approveAndOpenReviewMemoryChangeRequest( - { - owner, - proposalId: proposal.id, - requestCorrelationId: REQUEST_CORRELATION_ID, - }, - operations - ); - - expect(result).toEqual( - expect.objectContaining({ - status: 'change_request_opened', - change_request_url: 'https://github.com/acme/widgets/pull/10', - }) - ); - expect(operations.createBranch).toHaveBeenCalledTimes(1); - expect(operations.writeReviewMd).toHaveBeenCalledTimes(1); - expect(operations.createPullRequest).toHaveBeenCalledTimes(1); - expect(operations.captureException).not.toHaveBeenCalled(); - expect(operations.log).toHaveBeenCalledWith( - expect.stringContaining('"result":"change_request_opened"') - ); - }); - - it('retains the original failure when failure-state persistence also fails', async () => { - const user = await insertTestUser(); - const owner = { type: 'user' as const, id: user.id }; - const proposal = await seedProposal(owner, 'acme/persistence-failure'); - await seedGitHubIntegration(owner); - const operations = buildOperations(); - operations.integrateReviewMd.mockRejectedValue(buildModelBoundaryError()); - const markFailed = jest.fn< - ReturnType, - Parameters - >(async () => { - throw new Error('database unavailable'); - }); - - const error = await captureError(() => - approveAndOpenReviewMemoryChangeRequest( - { - owner, - proposalId: proposal.id, - requestCorrelationId: REQUEST_CORRELATION_ID, - }, - { ...operations, markFailed } - ) - ); - - expect(error).toMatchObject({ - code: 'BAD_REQUEST', - message: 'Unable to open the REVIEW.md change request. The proposal can be retried.', - }); - expect(operations.captureException).toHaveBeenCalledTimes(2); - expect(operations.captureException.mock.calls[0]?.[1].tags.stage).toBe('review_md_integration'); - expect(operations.captureException.mock.calls[1]?.[1]).toEqual( - expect.objectContaining({ - tags: expect.objectContaining({ stage: 'failure_persistence' }), - extra: expect.objectContaining({ failedAtStage: 'review_md_integration' }), - }) - ); - }); }); function buildProposal(input: { @@ -332,92 +114,3 @@ async function seedProposal(owner: ReviewMemoryOwner, repoFullName: string) { neutralCount: 0, }); } - -async function seedGitHubIntegration(owner: ReviewMemoryOwner): Promise { - if (owner.type !== 'user') throw new Error('Test helper requires a user owner.'); - await db.insert(platform_integrations).values({ - owned_by_user_id: owner.id, - owned_by_organization_id: null, - platform: 'github', - integration_type: 'app', - platform_installation_id: 'installation-id', - github_app_type: 'standard', - integration_status: 'active', - repository_access: 'all', - permissions: { - contents: 'write', - pull_requests: 'write', - }, - }); -} - -function buildOperations() { - const fetchReviewMd = jest.fn< - ReturnType, - Parameters - >(async () => null); - const integrateReviewMd = jest.fn< - ReturnType, - Parameters - >(async () => ({ - status: 'updated', - updatedReviewMd: '# Code review\n\nCheck generated fixtures against their source.', - integrationSummary: 'Added generated fixture guidance.', - tokensIn: 12, - tokensOut: 5, - })); - - return { - generateInstallationToken: jest.fn(async () => ({ - token: 'installation-token', - expires_at: '2099-01-01T00:00:00.000Z', - })), - fetchDefaultBranch: jest.fn(async () => 'main'), - fetchReviewMd, - integrateReviewMd, - createBranch: jest.fn(async () => undefined), - writeReviewMd: jest.fn(async () => undefined), - createPullRequest: jest.fn(async () => ({ - number: 10, - url: 'https://github.com/acme/widgets/pull/10', - })), - markFailed: markProposalChangeRequestFailed, - log: jest.fn(), - captureException: jest.fn< - ReturnType, - Parameters - >(), - } satisfies ReviewMemoryChangeRequestOperations; -} - -function buildModelBoundaryError(): ReviewMemoryModelBoundaryError { - const diagnostics: ReviewMemoryModelDiagnostics = { - operation: 'review_md_integration', - stage: 'structured_output_validation', - outcome: 'failure', - requestCorrelationId: REQUEST_CORRELATION_ID, - modelSlug: 'test/model', - finishReason: 'stop', - rawFinishReason: 'stop', - generatedTextCharacterCount: 120, - structuredParseCandidateCharacterCount: 120, - inputTokens: 12, - outputTokens: 5, - totalTokens: 17, - providerRequestId: 'provider-request-model-failure', - responseId: 'response-model-failure', - failureClassification: 'invalid_structured_output', - errorClass: 'NoObjectGeneratedError', - errorMessage: 'Review Memory model returned invalid structured output.', - }; - return new ReviewMemoryModelBoundaryError(diagnostics); -} - -async function captureError(run: () => Promise): Promise { - try { - await run(); - } catch (error) { - return error; - } - throw new Error('Expected operation to reject.'); -} diff --git a/apps/web/src/lib/code-reviews/review-memory/change-request.ts b/apps/web/src/lib/code-reviews/review-memory/change-request.ts index b618a22fc9..48b3b1eb5b 100644 --- a/apps/web/src/lib/code-reviews/review-memory/change-request.ts +++ b/apps/web/src/lib/code-reviews/review-memory/change-request.ts @@ -1,9 +1,8 @@ -import { captureException } from '@sentry/nextjs'; import type { CodeReviewMemoryProposal, PlatformIntegration } from '@kilocode/db/schema'; import type { ReviewMemoryProposalStatus } from '@kilocode/db/schema-types'; -import { INTEGRATION_STATUS, PLATFORM } from '@/lib/integrations/core/constants'; import { getAllIntegrationsForOwner } from '@/lib/integrations/db/platform-integrations'; +import { INTEGRATION_STATUS, PLATFORM } from '@/lib/integrations/core/constants'; import { createGitHubBranch, createGitHubPullRequest, @@ -12,7 +11,6 @@ import { fetchGitHubRootTextFileAtRef, generateGitHubInstallationToken, } from '@/lib/integrations/platforms/github/adapter'; -import { logExceptInTest } from '@/lib/utils.server'; import { getProposal, markProposalChangeRequestFailed, @@ -21,18 +19,11 @@ import { markProposalSuperseded, type ReviewMemoryOwner, } from './db'; -import { - getSafeErrorClass, - ReviewMemoryModelBoundaryError, - sanitizeReviewMemoryDiagnosticMessage, -} from './llm'; import { generateIntegratedReviewGuidanceWithGateway } from './review-md-integration'; const REVIEW_MEMORY_TARGET_FILE_PATH = 'REVIEW.md'; const REVIEW_MEMORY_CHANGE_REQUEST_TITLE = 'docs(review): update REVIEW.md guidance'; const REVIEW_MEMORY_CHANGE_REQUEST_MARKER = ''; -const REVIEW_MEMORY_CHANGE_REQUEST_RETRY_MESSAGE = - 'Unable to open the REVIEW.md change request. The proposal can be retried.'; const OPENING_CHANGE_REQUEST_STALE_MS = 30 * 60 * 1000; const APPROVABLE_STATUSES: ReadonlySet = new Set([ 'open', @@ -40,56 +31,6 @@ const APPROVABLE_STATUSES: ReadonlySet = new Set([ 'change_request_failed', ]); -type ReviewMemoryChangeRequestStage = - | 'repository_validation' - | 'installation_token_creation' - | 'github_reads' - | 'review_md_integration' - | 'branch_creation' - | 'file_writing' - | 'pull_request_creation' - | 'final_persistence' - | 'failure_persistence'; - -type CaptureReviewMemoryException = ( - error: Error, - context: { - level: 'error'; - tags: { - source: 'review-memory'; - operation: 'open-change-request'; - stage: ReviewMemoryChangeRequestStage; - }; - extra: Record; - } -) => unknown; - -export type ReviewMemoryChangeRequestOperations = { - generateInstallationToken: typeof generateGitHubInstallationToken; - fetchDefaultBranch: typeof fetchGitHubRepositoryDefaultBranch; - fetchReviewMd: typeof fetchGitHubRootTextFileAtRef; - integrateReviewMd: typeof generateIntegratedReviewGuidanceWithGateway; - createBranch: typeof createGitHubBranch; - writeReviewMd: typeof createOrUpdateGitHubRootTextFile; - createPullRequest: typeof createGitHubPullRequest; - markFailed: typeof markProposalChangeRequestFailed; - log: (record: string) => void; - captureException: CaptureReviewMemoryException; -}; - -const defaultOperations: ReviewMemoryChangeRequestOperations = { - generateInstallationToken: generateGitHubInstallationToken, - fetchDefaultBranch: fetchGitHubRepositoryDefaultBranch, - fetchReviewMd: fetchGitHubRootTextFileAtRef, - integrateReviewMd: generateIntegratedReviewGuidanceWithGateway, - createBranch: createGitHubBranch, - writeReviewMd: createOrUpdateGitHubRootTextFile, - createPullRequest: createGitHubPullRequest, - markFailed: markProposalChangeRequestFailed, - log: logExceptInTest, - captureException, -}; - export class ReviewMemoryChangeRequestError extends Error { constructor( public readonly code: 'NOT_FOUND' | 'BAD_REQUEST' | 'CONFLICT', @@ -114,26 +55,15 @@ ${proposal.rationale} Kilo analyzed maintainer replies to recent review comments and found repeated feedback that this repository guidance should address. Review and edit the proposed REVIEW.md changes before merging.`; } -export async function approveAndOpenReviewMemoryChangeRequest( - input: { - owner: ReviewMemoryOwner; - proposalId: string; - requestCorrelationId: string; - }, - operationOverrides: Partial = {} -): Promise { - const operations = { ...defaultOperations, ...operationOverrides }; +export async function approveAndOpenReviewMemoryChangeRequest(input: { + owner: ReviewMemoryOwner; + proposalId: string; +}): Promise { let proposal = await getProposal({ owner: input.owner, proposalId: input.proposalId }); if (!proposal) { throw new ReviewMemoryChangeRequestError('NOT_FOUND', 'Review memory proposal not found.'); } if (proposal.status === 'change_request_opened' && proposal.change_request_url) { - emitChangeRequestRecord(operations.log, { - requestCorrelationId: input.requestCorrelationId, - stage: 'final_persistence', - outcome: 'success', - result: 'change_request_opened', - }); return proposal; } if (proposal.status === 'opening_change_request') { @@ -163,57 +93,43 @@ export async function approveAndOpenReviewMemoryChangeRequest( ); } - let stage: ReviewMemoryChangeRequestStage = 'repository_validation'; try { const [repoOwner, repo] = proposal.repo_full_name.split('/'); if (!repoOwner || !repo) { - throw new Error('Invalid repository name.'); + throw new Error(`Invalid repository name: ${proposal.repo_full_name}`); } - - stage = 'installation_token_creation'; if (!integration.platform_installation_id) { throw new Error('GitHub installation id is missing.'); } - const tokenData = await operations.generateInstallationToken( + + const tokenData = await generateGitHubInstallationToken( integration.platform_installation_id, integration.github_app_type ?? 'standard' ); const token = tokenData.token; - - stage = 'github_reads'; - const defaultBranch = await operations.fetchDefaultBranch({ + const defaultBranch = await fetchGitHubRepositoryDefaultBranch({ token, owner: repoOwner, repo, }); - const existingReviewMd = await operations.fetchReviewMd({ + const existingReviewMd = await fetchGitHubRootTextFileAtRef({ token, owner: repoOwner, repo, path: REVIEW_MEMORY_TARGET_FILE_PATH, ref: defaultBranch, }); - - stage = 'review_md_integration'; - const integrationResult = await operations.integrateReviewMd({ + const integrationResult = await generateIntegratedReviewGuidanceWithGateway({ owner: input.owner, platform: 'github', repoFullName: proposal.repo_full_name, existingReviewMd, proposal, - requestCorrelationId: input.requestCorrelationId, }); if (integrationResult.status === 'already_present') { - stage = 'final_persistence'; const superseded = await markProposalSuperseded({ proposalId: proposal.id }); if (!superseded) throw new Error('Failed to mark proposal superseded.'); - emitChangeRequestRecord(operations.log, { - requestCorrelationId: input.requestCorrelationId, - stage, - outcome: 'success', - result: 'already_present', - }); return superseded; } if (!integrationResult.updatedReviewMd) { @@ -221,17 +137,14 @@ export async function approveAndOpenReviewMemoryChangeRequest( } const branchName = `kilo/review-memory/${proposal.id.slice(0, 8)}`; - stage = 'branch_creation'; - await operations.createBranch({ + await createGitHubBranch({ token, owner: repoOwner, repo, branchName, baseBranch: defaultBranch, }); - - stage = 'file_writing'; - await operations.writeReviewMd({ + await createOrUpdateGitHubRootTextFile({ token, owner: repoOwner, repo, @@ -240,9 +153,7 @@ export async function approveAndOpenReviewMemoryChangeRequest( message: 'Update REVIEW.md guidance', content: integrationResult.updatedReviewMd, }); - - stage = 'pull_request_creation'; - const pullRequest = await operations.createPullRequest({ + const pullRequest = await createGitHubPullRequest({ token, owner: repoOwner, repo, @@ -251,46 +162,17 @@ export async function approveAndOpenReviewMemoryChangeRequest( headBranch: branchName, baseBranch: defaultBranch, }); - - stage = 'final_persistence'; const opened = await markProposalChangeRequestOpened({ proposalId: proposal.id, changeRequestUrl: pullRequest.url, }); if (!opened) throw new Error('Failed to mark proposal change request opened.'); - emitChangeRequestRecord(operations.log, { - requestCorrelationId: input.requestCorrelationId, - stage, - outcome: 'success', - result: 'change_request_opened', - }); return opened; } catch (error) { - reportChangeRequestFailure({ - error, - stage, - requestCorrelationId: input.requestCorrelationId, - proposalId: proposal.id, - operations, - }); - - try { - const failed = await operations.markFailed({ proposalId: proposal.id }); - if (!failed) throw new Error('Failed to mark proposal change request failed.'); - } catch (persistenceError) { - reportChangeRequestFailure({ - error: persistenceError, - stage: 'failure_persistence', - failedAtStage: stage, - requestCorrelationId: input.requestCorrelationId, - proposalId: proposal.id, - operations, - }); - } - + await markProposalChangeRequestFailed({ proposalId: proposal.id }); throw new ReviewMemoryChangeRequestError( 'BAD_REQUEST', - REVIEW_MEMORY_CHANGE_REQUEST_RETRY_MESSAGE + redactSensitiveErrorMessage(error instanceof Error ? error.message : String(error)) ); } } @@ -347,109 +229,6 @@ function assertGitHubPermissions(integration: PlatformIntegration): void { } } -function reportChangeRequestFailure(input: { - error: unknown; - stage: ReviewMemoryChangeRequestStage; - failedAtStage?: ReviewMemoryChangeRequestStage; - requestCorrelationId: string; - proposalId: string; - operations: ReviewMemoryChangeRequestOperations; -}): void { - const modelDiagnostics = - input.error instanceof ReviewMemoryModelBoundaryError ? input.error.diagnostics : null; - const errorClass = getSafeErrorClass(input.error); - const errorMessage = modelDiagnostics - ? modelDiagnostics.errorMessage - : input.stage === 'failure_persistence' - ? 'Failed to persist the Review Memory change-request failure state.' - : 'Review Memory change-request operation failed.'; - - emitChangeRequestRecord(input.operations.log, { - requestCorrelationId: input.requestCorrelationId, - stage: input.stage, - outcome: 'failure', - errorClass, - errorMessage, - }); - - try { - const capturedError = createSanitizedChangeRequestError({ - sourceError: input.error, - message: errorMessage ?? 'Review Memory change-request operation failed.', - }); - input.operations.captureException(capturedError, { - level: 'error', - tags: { - source: 'review-memory', - operation: 'open-change-request', - stage: input.stage, - }, - extra: { - requestCorrelationId: input.requestCorrelationId, - proposalId: input.proposalId, - errorClass, - errorMessage, - ...(input.failedAtStage ? { failedAtStage: input.failedAtStage } : {}), - ...(modelDiagnostics ? { modelDiagnostics } : {}), - ...(modelDiagnostics?.providerRequestId - ? { providerRequestId: modelDiagnostics.providerRequestId } - : {}), - }, - }); - } catch { - // Reporting must not replace the original change-request failure. - } -} - -function emitChangeRequestRecord( - log: (record: string) => void, - input: { - requestCorrelationId: string; - stage: ReviewMemoryChangeRequestStage; - outcome: 'success' | 'failure'; - result?: 'already_present' | 'change_request_opened'; - errorClass?: string; - errorMessage?: string | null; - } -): void { - try { - log( - JSON.stringify({ - event: 'review_memory_change_request', - operation: 'open_change_request', - request_correlation_id: input.requestCorrelationId, - stage: input.stage, - outcome: input.outcome, - result: input.result ?? null, - error_class: input.errorClass ?? null, - error_message: input.errorMessage ?? null, - }) - ); - } catch { - // Telemetry must not affect change-request state transitions. - } -} - -function createSanitizedChangeRequestError(input: { - sourceError: unknown; - message: string; -}): Error { - const capturedError = new Error(sanitizeReviewMemoryDiagnosticMessage(input.message)); - capturedError.name = 'ReviewMemoryChangeRequestFailure'; - const stackFrames = getSafeStackFrames(input.sourceError); - if (stackFrames) { - capturedError.stack = `${capturedError.name}: ${capturedError.message}\n${stackFrames}`; - } - return capturedError; -} - -function getSafeStackFrames(error: unknown): string | null { - if (!(error instanceof Error) || !error.stack) return null; - const stackFrames = error.stack - .split('\n') - .slice(1) - .filter(line => /^\s*at\s/.test(line)) - .slice(0, 20) - .join('\n'); - return stackFrames ? sanitizeReviewMemoryDiagnosticMessage(stackFrames) : null; +function redactSensitiveErrorMessage(message: string): string { + return message.replace(/(?:ghs|ghu|ghp|github_pat)_[A-Za-z0-9_]+/g, '[redacted-token]'); } diff --git a/apps/web/src/lib/code-reviews/review-memory/llm.test.ts b/apps/web/src/lib/code-reviews/review-memory/llm.test.ts index 4e3f80604d..ebac2baadd 100644 --- a/apps/web/src/lib/code-reviews/review-memory/llm.test.ts +++ b/apps/web/src/lib/code-reviews/review-memory/llm.test.ts @@ -1,184 +1,15 @@ import type { User } from '@kilocode/db/schema'; -import { MockLanguageModelV3 } from 'ai/test'; import * as z from 'zod'; -import { - ReviewMemoryProposalDraftSchema, - ReviewMemoryProposalWireSchema, - validateReviewMemoryProposalDraft, -} from './aggregation'; -import { - createReviewMemoryGatewayProvider, - generateReviewMemoryStructuredOutput, - ReviewMemoryModelBoundaryError, -} from './llm'; -import { - ReviewMdIntegrationOutputSchema, - validateReviewMdIntegrationOutput, -} from './review-md-integration'; +import { createReviewMemoryGatewayProvider, generateReviewMemoryStructuredOutput } from './llm'; const TestOutputSchema = z.object({ status: z.literal('ok'), value: z.string(), }); -const RAW_OUTPUT_SENTINEL = 'RAW_GENERATED_OUTPUT_SECRET'; -const PROMPT_SENTINEL = 'PROMPT_CONTENT_SECRET'; - -function buildModel(input: { - text: string; - finishReason?: { - unified: 'stop' | 'length'; - raw: string; - }; - responseId?: string; - providerRequestId?: string; -}) { - return new MockLanguageModelV3({ - provider: 'test-provider', - modelId: 'test-model', - doGenerate: { - content: [{ type: 'text', text: input.text }], - finishReason: input.finishReason ?? { unified: 'stop', raw: 'stop' }, - usage: { - inputTokens: { - total: 12, - noCache: 12, - cacheRead: 0, - cacheWrite: 0, - }, - outputTokens: { - total: 5, - text: 5, - reasoning: 0, - }, - }, - response: { - id: input.responseId ?? 'response-123', - timestamp: new Date('2026-06-18T00:00:00.000Z'), - modelId: 'test-model', - headers: { - 'request-id': input.providerRequestId ?? 'provider-request-123', - }, - }, - warnings: [], - }, - }); -} - -function generateWithModel(model: MockLanguageModelV3, log: (record: string) => void = () => {}) { - return generateReviewMemoryStructuredOutput({ - model, - modelSlug: 'test/model', - operation: 'proposal_analysis', - requestCorrelationId: 'request-correlation-123', - prompt: `Analyze without recording ${PROMPT_SENTINEL}`, - maxOutputTokens: 100, - schemaName: 'test_review_memory_output', - schema: TestOutputSchema, - validate: output => output, - log, - }); -} - describe('Review Memory structured model output', () => { - it('returns typed JSON output with token and provider metadata', async () => { - const model = buildModel({ text: '{"status":"ok","value":"accepted"}' }); - const records: string[] = []; - - const result = await generateWithModel(model, record => records.push(record)); - - expect(result.output).toEqual({ status: 'ok', value: 'accepted' }); - expect(result.diagnostics).toEqual( - expect.objectContaining({ - outcome: 'success', - inputTokens: 12, - outputTokens: 5, - totalTokens: 17, - responseId: 'response-123', - providerRequestId: 'provider-request-123', - }) - ); - expect(model.doGenerateCalls).toHaveLength(1); - expect(model.doGenerateCalls[0]?.responseFormat).toEqual( - expect.objectContaining({ - type: 'json', - schema: expect.any(Object), - }) - ); - expect(records).toHaveLength(1); - }); - - it('transmits a provider-compatible proposal schema while validating the source Zod schema', async () => { - const model = buildModel({ - text: JSON.stringify({ - status: 'propose', - title: 'Generated fixtures', - rationale: 'Maintainers repeatedly corrected generated fixture comments.', - proposedMarkdown: 'Ignore generated fixtures unless their source behavior changes.', - positiveCount: 0, - negativeCount: 2, - neutralCount: 0, - evidenceEventIds: [], - }), - }); - - await generateReviewMemoryStructuredOutput({ - model, - modelSlug: 'test/model', - operation: 'proposal_analysis', - requestCorrelationId: 'request-proposal-schema', - prompt: 'Analyze feedback.', - maxOutputTokens: 100, - schemaName: 'review_memory_proposal', - schema: ReviewMemoryProposalDraftSchema, - wireSchema: ReviewMemoryProposalWireSchema, - validate: validateReviewMemoryProposalDraft, - log: () => {}, - }); - - const responseFormat = model.doGenerateCalls[0]?.responseFormat; - expect(responseFormat).toEqual( - expect.objectContaining({ - type: 'json', - schema: expect.objectContaining({ type: 'object' }), - }) - ); - const serializedResponseFormat = JSON.stringify(responseFormat); - expect(serializedResponseFormat).not.toContain('"oneOf"'); - expect(serializedResponseFormat).not.toMatch( - /"(?:minimum|maximum|minLength|maxLength|maxItems|uniqueItems)"/ - ); - }); - - it('transmits a provider-compatible REVIEW.md integration schema', async () => { - const model = buildModel({ - text: JSON.stringify({ - status: 'updated', - updatedReviewMd: '# Code review\n\nCheck generated fixtures against their source.', - integrationSummary: 'Added generated fixture guidance.', - }), - }); - - await generateReviewMemoryStructuredOutput({ - model, - modelSlug: 'test/model', - operation: 'review_md_integration', - requestCorrelationId: 'request-integration-schema', - prompt: 'Integrate guidance.', - maxOutputTokens: 100, - schemaName: 'review_md_integration', - schema: ReviewMdIntegrationOutputSchema, - validate: validateReviewMdIntegrationOutput, - log: () => {}, - }); - - const responseFormat = JSON.stringify(model.doGenerateCalls[0]?.responseFormat); - expect(responseFormat).toContain('"type":"json"'); - expect(responseFormat).not.toMatch(/"(?:minLength|maxLength)"/); - }); - - it('sends JSON Schema and requires compatible gateway routing', async () => { + it('sends JSON Schema through the gateway and returns typed output', async () => { let requestBody: unknown; const gatewayFetch: typeof fetch = async (_request, init) => { if (typeof init?.body !== 'string') throw new Error('Expected a JSON request body.'); @@ -207,10 +38,7 @@ describe('Review Memory structured model output', () => { }), { status: 200, - headers: { - 'content-type': 'application/json', - 'request-id': 'gateway-request-id', - }, + headers: { 'content-type': 'application/json' }, } ); }; @@ -224,22 +52,21 @@ describe('Review Memory structured model output', () => { userAgent: 'Review Memory structured output test', fetch: gatewayFetch, }); - const model = provider.chatModel('test/model'); - await generateReviewMemoryStructuredOutput({ - model, - modelSlug: 'test/model', - operation: 'proposal_analysis', - requestCorrelationId: 'request-wire-format', + const result = await generateReviewMemoryStructuredOutput({ + model: provider.chatModel('test/model'), prompt: 'Return a test result.', maxOutputTokens: 100, schemaName: 'test_review_memory_output', schema: TestOutputSchema, validate: output => output, - log: () => {}, }); - expect(Reflect.get(model, 'supportsStructuredOutputs')).toBe(true); + expect(result).toEqual({ + output: { status: 'ok', value: 'accepted' }, + tokensIn: 12, + tokensOut: 5, + }); expect(requestBody).toEqual( expect.objectContaining({ response_format: expect.objectContaining({ @@ -248,164 +75,5 @@ describe('Review Memory structured model output', () => { }), }) ); - expect(requestBody).not.toEqual(expect.objectContaining({ provider: expect.anything() })); - }); - - it.each([ - ['fenced JSON', `\`\`\`json\n{"status":"ok","value":"${RAW_OUTPUT_SENTINEL}"}\n\`\`\``], - ['prose-wrapped JSON', `Result: {"status":"ok","value":"${RAW_OUTPUT_SENTINEL}"}`], - ['malformed JSON', `{"status":"ok","value":"${RAW_OUTPUT_SENTINEL}`], - ['schema-invalid JSON', `{"status":"wrong","value":"${RAW_OUTPUT_SENTINEL}"}`], - ])('rejects %s without recovering text', async (_name, text) => { - const records: string[] = []; - const model = buildModel({ text }); - - const error = await captureError(() => - generateWithModel(model, record => records.push(record)) - ); - - expect(error).toBeInstanceOf(ReviewMemoryModelBoundaryError); - if (!(error instanceof ReviewMemoryModelBoundaryError)) return; - expect(error.diagnostics).toEqual( - expect.objectContaining({ - stage: 'structured_output_validation', - outcome: 'failure', - failureClassification: 'invalid_structured_output', - }) - ); - expect(records).toHaveLength(1); - const serialized = JSON.stringify({ - message: error.message, - diagnostics: error.diagnostics, - records, - }); - expect(serialized).not.toContain(RAW_OUTPUT_SENTINEL); - expect(serialized).not.toContain(PROMPT_SENTINEL); - }); - - it('sanitizes gateway-generation failures without retaining provider content', async () => { - const records: string[] = []; - const model = new MockLanguageModelV3({ - provider: 'test-provider', - modelId: 'test-model', - doGenerate: async () => { - throw new Error( - `Provider failed at https://provider.example/path?token=secret with ${RAW_OUTPUT_SENTINEL}` - ); - }, - }); - - const error = await captureError(() => - generateWithModel(model, record => records.push(record)) - ); - - expect(error).toBeInstanceOf(ReviewMemoryModelBoundaryError); - if (!(error instanceof ReviewMemoryModelBoundaryError)) return; - expect(error.diagnostics).toEqual( - expect.objectContaining({ - stage: 'gateway_generation', - failureClassification: 'generation_failed', - errorMessage: 'Review Memory model generation failed.', - }) - ); - const serialized = JSON.stringify({ - message: error.message, - diagnostics: error.diagnostics, - records, - }); - expect(serialized).not.toContain(RAW_OUTPUT_SENTINEL); - expect(serialized).not.toContain('provider.example'); - expect(serialized).not.toContain(PROMPT_SENTINEL); - }); - - it('classifies length-limited output as incomplete before output access', async () => { - const records: string[] = []; - const model = buildModel({ - text: `{"status":"ok","value":"${RAW_OUTPUT_SENTINEL}`, - finishReason: { unified: 'length', raw: 'max_tokens' }, - responseId: 'length-response-id', - providerRequestId: 'length-provider-request-id', - }); - - const error = await captureError(() => - generateWithModel(model, record => records.push(record)) - ); - - expect(error).toBeInstanceOf(ReviewMemoryModelBoundaryError); - if (!(error instanceof ReviewMemoryModelBoundaryError)) return; - expect(error.diagnostics).toEqual( - expect.objectContaining({ - failureClassification: 'incomplete_output', - finishReason: 'length', - rawFinishReason: 'max_tokens', - generatedTextCharacterCount: expect.any(Number), - inputTokens: 12, - outputTokens: 5, - totalTokens: 17, - responseId: 'length-response-id', - providerRequestId: 'length-provider-request-id', - }) - ); - expect(records).toHaveLength(1); - const record = JSON.parse(records[0]); - expect(record).toEqual( - expect.objectContaining({ - event: 'review_memory_model_generation', - operation: 'proposal_analysis', - stage: 'structured_output_validation', - outcome: 'failure', - model_slug: 'test/model', - finish_reason: 'length', - raw_finish_reason: 'max_tokens', - provider_request_id: 'length-provider-request-id', - response_id: 'length-response-id', - }) - ); - expect(records[0]).not.toContain(RAW_OUTPUT_SENTINEL); - expect(records[0]).not.toContain(PROMPT_SENTINEL); - }); - - it('does not include generated values from semantic validation errors', async () => { - const records: string[] = []; - const model = buildModel({ - text: `{"status":"ok","value":"${RAW_OUTPUT_SENTINEL}"}`, - }); - - const error = await captureError(() => - generateReviewMemoryStructuredOutput({ - model, - modelSlug: 'test/model', - operation: 'review_md_integration', - requestCorrelationId: 'request-correlation-semantic', - prompt: PROMPT_SENTINEL, - maxOutputTokens: 100, - schemaName: 'test_semantic_output', - schema: TestOutputSchema, - validate: output => { - throw new Error(`Rejected generated value: ${output.value}`); - }, - log: record => records.push(record), - }) - ); - - expect(error).toBeInstanceOf(ReviewMemoryModelBoundaryError); - if (!(error instanceof ReviewMemoryModelBoundaryError)) return; - expect(error.diagnostics.failureClassification).toBe('invalid_semantic_output'); - const serialized = JSON.stringify({ - message: error.message, - diagnostics: error.diagnostics, - records, - }); - expect(serialized).not.toContain(RAW_OUTPUT_SENTINEL); - expect(serialized).not.toContain(PROMPT_SENTINEL); }); }); - -async function captureError(run: () => Promise): Promise { - try { - await run(); - } catch (error) { - return error; - } - throw new Error('Expected operation to reject.'); -} diff --git a/apps/web/src/lib/code-reviews/review-memory/llm.ts b/apps/web/src/lib/code-reviews/review-memory/llm.ts index bfe9f8b3b0..e4f003f859 100644 --- a/apps/web/src/lib/code-reviews/review-memory/llm.ts +++ b/apps/web/src/lib/code-reviews/review-memory/llm.ts @@ -1,13 +1,5 @@ import { createOpenAICompatible } from '@ai-sdk/openai-compatible'; -import { - generateText, - jsonSchema, - NoObjectGeneratedError, - NoOutputGeneratedError, - Output, - zodSchema, - type LanguageModel, -} from 'ai'; +import { generateText, jsonSchema, Output, zodSchema, type LanguageModel } from 'ai'; import * as z from 'zod'; import { getAgentConfigForOwner } from '@/lib/agent-config/db/agent-configs'; @@ -17,13 +9,10 @@ import { APP_URL } from '@/lib/constants'; import { FEATURE_HEADER } from '@/lib/feature-detection'; import { generateApiToken } from '@/lib/tokens'; import { findUserById } from '@/lib/user'; -import { logExceptInTest } from '@/lib/utils.server'; import type { User } from '@kilocode/db/schema'; import type { ReviewMemoryPlatform } from '@kilocode/db/schema-types'; import type { ReviewMemoryOwner } from './db'; -const MAX_DIAGNOSTIC_TEXT_LENGTH = 500; -const MAX_DIAGNOSTIC_IDENTIFIER_LENGTH = 200; // Claude rejects these constraints at the provider boundary; the source Zod schema still validates output locally. const UNSUPPORTED_WIRE_SCHEMA_KEYWORDS = new Set([ 'minimum', @@ -57,81 +46,14 @@ const ReviewMemoryModelConfigSchema = z.object({ model_slug: z.string().optional(), }); -export type ReviewMemoryModelOperation = 'proposal_analysis' | 'review_md_integration'; -export type ReviewMemoryModelStage = 'gateway_generation' | 'structured_output_validation'; -export type ReviewMemoryModelFailureClassification = - | 'generation_failed' - | 'incomplete_output' - | 'invalid_structured_output' - | 'invalid_semantic_output'; - -export type ReviewMemoryModelDiagnostics = { - operation: ReviewMemoryModelOperation; - stage: ReviewMemoryModelStage; - outcome: 'success' | 'failure'; - requestCorrelationId: string; - modelSlug: string; - finishReason: string | null; - rawFinishReason: string | null; - generatedTextCharacterCount: number | null; - structuredParseCandidateCharacterCount: number | null; - inputTokens: number | null; - outputTokens: number | null; - totalTokens: number | null; - providerRequestId: string | null; - responseId: string | null; - failureClassification: ReviewMemoryModelFailureClassification | null; - errorClass: string | null; - errorMessage: string | null; -}; - -export class ReviewMemoryModelBoundaryError extends Error { - constructor(public readonly diagnostics: ReviewMemoryModelDiagnostics) { - super(diagnostics.errorMessage ?? 'Review Memory model generation failed.'); - this.name = 'ReviewMemoryModelBoundaryError'; - } -} - -type GenerationMetadata = Pick< - ReviewMemoryModelDiagnostics, - | 'finishReason' - | 'rawFinishReason' - | 'generatedTextCharacterCount' - | 'structuredParseCandidateCharacterCount' - | 'inputTokens' - | 'outputTokens' - | 'totalTokens' - | 'providerRequestId' - | 'responseId' ->; - -type GenerationMetadataSource = { - text?: string; - finishReason?: string; - rawFinishReason?: string; - usage?: { - inputTokens?: number; - outputTokens?: number; - totalTokens?: number; - }; - response?: { - id?: string; - headers?: Record; - }; -}; - type GenerateReviewMemoryStructuredOutputInput = { model: LanguageModel; - modelSlug: string; - operation: ReviewMemoryModelOperation; - requestCorrelationId: string; prompt: string; maxOutputTokens: number; schemaName: string; schema: z.ZodType; wireSchema?: z.ZodType; validate: (output: OUTPUT) => OUTPUT; - log?: (record: string) => void; }; export async function resolveReviewMemoryActor(owner: ReviewMemoryOwner): Promise { @@ -185,10 +107,9 @@ export async function generateReviewMemoryStructuredOutput( input: GenerateReviewMemoryStructuredOutputInput ): Promise<{ output: OUTPUT; - diagnostics: ReviewMemoryModelDiagnostics; + tokensIn: number | null; + tokensOut: number | null; }> { - const log = input.log ?? logExceptInTest; - let finishMetadata: GenerationMetadata | undefined; const sourceSchema = zodSchema(input.schema); const providerSchema = zodSchema(input.wireSchema ?? input.schema); const wireSchema = jsonSchema( @@ -199,117 +120,22 @@ export async function generateReviewMemoryStructuredOutput( }), { validate: sourceSchema.validate } ); - const outputSpecification = Output.object({ - schema: wireSchema, - name: input.schemaName, - }); - const generate = () => - generateText({ - model: input.model, - prompt: input.prompt, - maxOutputTokens: input.maxOutputTokens, - output: outputSpecification, - experimental_include: { - requestBody: false, - responseBody: false, - }, - onFinish: event => { - finishMetadata = getGenerationMetadata(event); - }, - }); - let result: Awaited>; - try { - result = await generate(); - } catch (error) { - const isStructuredOutputError = - NoObjectGeneratedError.isInstance(error) || NoOutputGeneratedError.isInstance(error); - const metadata = NoObjectGeneratedError.isInstance(error) - ? mergeGenerationMetadata(getGenerationMetadata(error), finishMetadata) - : (finishMetadata ?? emptyGenerationMetadata()); - throwModelBoundaryError({ - input, - log, - metadata, - stage: isStructuredOutputError ? 'structured_output_validation' : 'gateway_generation', - classification: isStructuredOutputError ? 'invalid_structured_output' : 'generation_failed', - sourceError: error, - }); - } - - const metadata = getGenerationMetadata(result); - if (result.finishReason === 'length') { - throwModelBoundaryError({ - input, - log, - metadata, - stage: 'structured_output_validation', - classification: 'incomplete_output', - sourceError: new Error('Generation reached its output-token limit.'), - }); - } - if (result.finishReason !== 'stop') { - throwModelBoundaryError({ - input, - log, - metadata, - stage: 'structured_output_validation', - classification: 'invalid_structured_output', - sourceError: new NoOutputGeneratedError(), - }); - } - - let output: OUTPUT; - try { - output = result.output; - } catch (error) { - throwModelBoundaryError({ - input, - log, - metadata, - stage: 'structured_output_validation', - classification: 'invalid_structured_output', - sourceError: error, - }); - } - - let validated: OUTPUT; - try { - validated = input.validate(output); - } catch (error) { - throwModelBoundaryError({ - input, - log, - metadata, - stage: 'structured_output_validation', - classification: 'invalid_semantic_output', - sourceError: error, - }); - } - - const diagnostics = createDiagnostics({ - input, - metadata, - stage: 'structured_output_validation', - outcome: 'success', - classification: null, - sourceError: null, + const result = await generateText({ + model: input.model, + prompt: input.prompt, + maxOutputTokens: input.maxOutputTokens, + output: Output.object({ + schema: wireSchema, + name: input.schemaName, + }), }); - emitModelGenerationRecord(log, diagnostics); - return { output: validated, diagnostics }; -} -export function sanitizeReviewMemoryDiagnosticMessage(message: string): string { - return message - .replace(/https?:\/\/[^\s)>'"]+/gi, '[redacted-url]') - .replace(/\b(?:Bearer|Basic)\s+[A-Za-z0-9._~+/=-]+/gi, '[redacted-authorization]') - .replace(/(?:ghs|ghu|ghp|github_pat)_[A-Za-z0-9_]+/g, '[redacted-token]') - .replace( - /\b(api[_ -]?key|authorization|password|secret|token)\s*[:=]\s*[^\s,;]+/gi, - '$1=[redacted]' - ) - .replace(/\b[A-Za-z0-9_-]{40,}\b/g, '[redacted-value]') - .slice(0, MAX_DIAGNOSTIC_TEXT_LENGTH); + return { + output: input.validate(result.output), + tokensIn: result.usage.inputTokens ?? null, + tokensOut: result.usage.outputTokens ?? null, + }; } function transformReviewMemoryWireSchema(schema: unknown): void { @@ -348,175 +174,3 @@ function transformReviewMemoryWireSchema(schema: unknown): void { function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null; } - -function throwModelBoundaryError(input: { - input: GenerateReviewMemoryStructuredOutputInput; - log: (record: string) => void; - metadata: GenerationMetadata; - stage: ReviewMemoryModelStage; - classification: ReviewMemoryModelFailureClassification; - sourceError: unknown; -}): never { - const diagnostics = createDiagnostics({ - input: input.input, - metadata: input.metadata, - stage: input.stage, - outcome: 'failure', - classification: input.classification, - sourceError: input.sourceError, - }); - emitModelGenerationRecord(input.log, diagnostics); - throw new ReviewMemoryModelBoundaryError(diagnostics); -} - -function createDiagnostics(input: { - input: GenerateReviewMemoryStructuredOutputInput; - metadata: GenerationMetadata; - stage: ReviewMemoryModelStage; - outcome: 'success' | 'failure'; - classification: ReviewMemoryModelFailureClassification | null; - sourceError: unknown; -}): ReviewMemoryModelDiagnostics { - return { - operation: input.input.operation, - stage: input.stage, - outcome: input.outcome, - requestCorrelationId: input.input.requestCorrelationId, - modelSlug: input.input.modelSlug, - ...input.metadata, - failureClassification: input.classification, - errorClass: input.sourceError === null ? null : getSafeErrorClass(input.sourceError), - errorMessage: - input.classification === null ? null : getSafeModelBoundaryMessage(input.classification), - }; -} - -function getSafeModelBoundaryMessage( - classification: ReviewMemoryModelFailureClassification -): string { - switch (classification) { - case 'generation_failed': - return 'Review Memory model generation failed.'; - case 'incomplete_output': - return 'Review Memory structured output was incomplete.'; - case 'invalid_structured_output': - return 'Review Memory model returned invalid structured output.'; - case 'invalid_semantic_output': - return 'Review Memory structured output failed semantic validation.'; - } -} - -export function getSafeErrorClass(error: unknown): string { - let name = 'UnknownError'; - if (error instanceof Error) { - name = error.name; - } else if ( - typeof error === 'object' && - error !== null && - 'name' in error && - typeof error.name === 'string' - ) { - name = error.name; - } - return name.replace(/[^A-Za-z0-9_.-]/g, '').slice(0, 100) || 'UnknownError'; -} - -function getGenerationMetadata(source: GenerationMetadataSource): GenerationMetadata { - const textLength = source.text?.length ?? null; - return { - finishReason: boundedIdentifier(source.finishReason), - rawFinishReason: boundedIdentifier(source.rawFinishReason), - generatedTextCharacterCount: textLength, - structuredParseCandidateCharacterCount: textLength, - inputTokens: source.usage?.inputTokens ?? null, - outputTokens: source.usage?.outputTokens ?? null, - totalTokens: source.usage?.totalTokens ?? null, - providerRequestId: boundedIdentifier(getResponseHeader(source.response?.headers, 'request-id')), - responseId: boundedIdentifier(source.response?.id), - }; -} - -function emptyGenerationMetadata(): GenerationMetadata { - return { - finishReason: null, - rawFinishReason: null, - generatedTextCharacterCount: null, - structuredParseCandidateCharacterCount: null, - inputTokens: null, - outputTokens: null, - totalTokens: null, - providerRequestId: null, - responseId: null, - }; -} - -function mergeGenerationMetadata( - primary: GenerationMetadata, - fallback: GenerationMetadata | undefined -): GenerationMetadata { - if (!fallback) return primary; - return { - finishReason: primary.finishReason ?? fallback.finishReason, - rawFinishReason: primary.rawFinishReason ?? fallback.rawFinishReason, - generatedTextCharacterCount: - primary.generatedTextCharacterCount ?? fallback.generatedTextCharacterCount, - structuredParseCandidateCharacterCount: - primary.structuredParseCandidateCharacterCount ?? - fallback.structuredParseCandidateCharacterCount, - inputTokens: primary.inputTokens ?? fallback.inputTokens, - outputTokens: primary.outputTokens ?? fallback.outputTokens, - totalTokens: primary.totalTokens ?? fallback.totalTokens, - providerRequestId: primary.providerRequestId ?? fallback.providerRequestId, - responseId: primary.responseId ?? fallback.responseId, - }; -} - -function getResponseHeader( - headers: Record | undefined, - requestedName: string -): string | undefined { - if (!headers) return undefined; - const entry = Object.entries(headers).find( - ([name]) => name.toLowerCase() === requestedName.toLowerCase() - ); - return entry?.[1]; -} - -function boundedIdentifier(value: string | undefined): string | null { - return value?.slice(0, MAX_DIAGNOSTIC_IDENTIFIER_LENGTH) ?? null; -} - -function emitModelGenerationRecord( - log: (record: string) => void, - diagnostics: ReviewMemoryModelDiagnostics -): void { - try { - log( - JSON.stringify({ - event: 'review_memory_model_generation', - operation: diagnostics.operation, - stage: diagnostics.stage, - outcome: diagnostics.outcome, - request_correlation_id: diagnostics.requestCorrelationId, - vercel_deployment_id: process.env.VERCEL_DEPLOYMENT_ID ?? null, - vercel_region: process.env.VERCEL_REGION ?? process.env.VERCEL_FUNCTION_REGION ?? null, - model_slug: diagnostics.modelSlug, - finish_reason: diagnostics.finishReason, - raw_finish_reason: diagnostics.rawFinishReason, - generated_text_character_count: diagnostics.generatedTextCharacterCount, - structured_parse_candidate_character_count: - diagnostics.structuredParseCandidateCharacterCount, - input_tokens: diagnostics.inputTokens, - output_tokens: diagnostics.outputTokens, - total_tokens: diagnostics.totalTokens, - provider_request_id: diagnostics.providerRequestId, - response_id: diagnostics.responseId, - failure_classification: diagnostics.failureClassification, - error_class: diagnostics.errorClass, - error_message: diagnostics.errorMessage, - }) - ); - } catch { - // Telemetry must not affect model-boundary behavior. - } -} diff --git a/apps/web/src/lib/code-reviews/review-memory/review-md-integration.test.ts b/apps/web/src/lib/code-reviews/review-memory/review-md-integration.test.ts deleted file mode 100644 index 11f9f99f05..0000000000 --- a/apps/web/src/lib/code-reviews/review-memory/review-md-integration.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { ReviewMemoryProposalDraftSchema, validateReviewMemoryProposalDraft } from './aggregation'; -import { - ReviewMdIntegrationOutputSchema, - validateReviewMdIntegrationOutput, -} from './review-md-integration'; - -describe('Review Memory model output semantics', () => { - it('accepts a complete REVIEW.md update', () => { - const output = ReviewMdIntegrationOutputSchema.parse({ - status: 'updated', - updatedReviewMd: '# Code review\n\nCheck generated fixtures against their source.', - integrationSummary: 'Added generated fixture guidance.', - }); - - expect(validateReviewMdIntegrationOutput(output)).toEqual(output); - }); - - it.each([null, '', ' '])( - 'rejects updated status without non-empty REVIEW.md content (%p)', - updatedReviewMd => { - const parsed = ReviewMdIntegrationOutputSchema.safeParse({ - status: 'updated', - updatedReviewMd, - integrationSummary: 'Attempted update.', - }); - - if (!parsed.success) { - expect(updatedReviewMd).toBe(''); - return; - } - expect(() => validateReviewMdIntegrationOutput(parsed.data)).toThrow( - /without updatedReviewMd|empty REVIEW\.md/ - ); - } - ); - - it('rejects Review Memory branding in integrated guidance', () => { - const output = ReviewMdIntegrationOutputSchema.parse({ - status: 'updated', - updatedReviewMd: '# Review Memory\n\nRemember maintainer preferences.', - integrationSummary: 'Added guidance.', - }); - - expect(() => validateReviewMdIntegrationOutput(output)).toThrow( - 'Integrated REVIEW.md must not mention Review Memory.' - ); - }); - - it('retains proposal branding validation after structured parsing', () => { - const draft = ReviewMemoryProposalDraftSchema.parse({ - status: 'propose', - title: 'Generated fixtures', - rationale: 'Maintainers repeatedly corrected generated fixture comments.', - proposedMarkdown: 'Kilo should ignore generated fixtures.', - positiveCount: 0, - negativeCount: 2, - neutralCount: 0, - evidenceEventIds: [], - }); - - expect(() => validateReviewMemoryProposalDraft(draft)).toThrow( - 'Review Memory proposal must not mention Kilo, Review Memory, feedback systems, or LLMs.' - ); - }); -}); diff --git a/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts b/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts index 301ee9e145..80c196f3f1 100644 --- a/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts +++ b/apps/web/src/lib/code-reviews/review-memory/review-md-integration.ts @@ -14,7 +14,7 @@ const MAX_REVIEW_MD_CHARS = 30_000; const REVIEW_MEMORY_BRANDING_PATTERN = /\b(?:kilo\s+)?review\s+memory\b/i; const REVIEW_MEMORY_HEADING_PATTERN = /^#{1,6}\s+(?:kilo\s+)?review\s+memory\b/im; -export const ReviewMdIntegrationOutputSchema = z.object({ +const ReviewMdIntegrationOutputSchema = z.object({ status: z.enum(['updated', 'already_present']), updatedReviewMd: z.string().min(1).max(MAX_REVIEW_MD_CHARS).nullable(), integrationSummary: z.string().min(1).max(1_000), @@ -34,7 +34,6 @@ export async function generateIntegratedReviewGuidanceWithGateway(input: { repoFullName: string; existingReviewMd: string | null; proposal: CodeReviewMemoryProposal; - requestCorrelationId: string; }): Promise { if (input.existingReviewMd && input.existingReviewMd.length > MAX_REVIEW_MD_CHARS) { throw new Error( @@ -55,9 +54,6 @@ export async function generateIntegratedReviewGuidanceWithGateway(input: { const result = await generateReviewMemoryStructuredOutput({ model: provider.chatModel(modelSlug), - modelSlug, - operation: 'review_md_integration', - requestCorrelationId: input.requestCorrelationId, prompt: buildReviewMdIntegrationPrompt(input), maxOutputTokens: 8_000, schemaName: 'review_md_integration', @@ -67,12 +63,12 @@ export async function generateIntegratedReviewGuidanceWithGateway(input: { return { ...result.output, - tokensIn: result.diagnostics.inputTokens, - tokensOut: result.diagnostics.outputTokens, + tokensIn: result.tokensIn, + tokensOut: result.tokensOut, }; } -export function validateReviewMdIntegrationOutput( +function validateReviewMdIntegrationOutput( output: z.infer ): ReviewMdIntegrationResult { if (output.status === 'already_present') { diff --git a/apps/web/src/lib/trpc/init.ts b/apps/web/src/lib/trpc/init.ts index 6b18cc813b..2b1a40c523 100644 --- a/apps/web/src/lib/trpc/init.ts +++ b/apps/web/src/lib/trpc/init.ts @@ -1,34 +1,19 @@ import 'server-only'; -import { randomUUID } from 'node:crypto'; import { getUserFromAuth } from '@/lib/user/server'; import { initTRPC, TRPCError } from '@trpc/server'; import type { User } from '@kilocode/db/schema'; import * as z from 'zod'; import { setTag, trpcMiddleware } from '@sentry/nextjs'; import { userCanManageCredits } from '@/lib/admin/credit-management'; -const MAX_REQUEST_CORRELATION_ID_LENGTH = 200; -const REQUEST_CORRELATION_ID_PATTERN = /^(?:[a-z]{3}\d::){1,2}[a-z0-9]{5}-\d{13}-[a-f0-9]{12}$/; - // Define the context type export type TRPCContext = { user: User; - requestCorrelationId?: string; -}; - -type CorrelatedTRPCContext = TRPCContext & { - requestCorrelationId: string; -}; - -type CreateTRPCContextInput = { - requestCorrelationId?: string | null; }; /** * @see: https://trpc.io/docs/server/context */ -export const createTRPCContext = async ( - input: CreateTRPCContextInput = {} -): Promise => { +export const createTRPCContext = async (): Promise => { const { user } = await getUserFromAuth({ adminOnly: false }); if (!user) { throw new TRPCError({ @@ -39,22 +24,9 @@ export const createTRPCContext = async ( setTag('userId', user.id); return { user, - requestCorrelationId: normalizeRequestCorrelationId(input.requestCorrelationId), }; }; -function normalizeRequestCorrelationId(requestCorrelationId: string | null | undefined): string { - const normalized = requestCorrelationId?.trim(); - if ( - !normalized || - normalized.length > MAX_REQUEST_CORRELATION_ID_LENGTH || - !REQUEST_CORRELATION_ID_PATTERN.test(normalized) - ) { - return randomUUID(); - } - return normalized; -} - // Avoid exporting the entire t-object // since it's not very descriptive. // For instance, the use of a t variable @@ -97,15 +69,6 @@ const t = initTRPC.context().create({ }, }); -const requestCorrelationMiddleware = t.middleware(({ ctx, next }) => - next({ - ctx: { - ...ctx, - requestCorrelationId: ctx.requestCorrelationId ?? randomUUID(), - }, - }) -); - const sentryMiddleware = t.middleware( trpcMiddleware({ attachRpcInput: false, @@ -134,10 +97,7 @@ const timingMiddleware = t.middleware(async ({ path, type, ctx, next }) => { // Base router and procedure helpers export const createTRPCRouter = t.router; export const createCallerFactory = t.createCallerFactory; -export const baseProcedure = t.procedure - .use(requestCorrelationMiddleware) - .use(timingMiddleware) - .use(sentryMiddleware); +export const baseProcedure = t.procedure.use(timingMiddleware).use(sentryMiddleware); // Admin-only procedure export const adminProcedure = baseProcedure.use(async ({ ctx, next }) => { diff --git a/apps/web/src/routers/code-reviews/review-memory-router.ts b/apps/web/src/routers/code-reviews/review-memory-router.ts index 9e9bf3ef13..dac9e6b934 100644 --- a/apps/web/src/routers/code-reviews/review-memory-router.ts +++ b/apps/web/src/routers/code-reviews/review-memory-router.ts @@ -105,7 +105,6 @@ export const reviewMemoryRouter = createTRPCRouter({ owner, platform: input.platform, repoFullName: input.repoFullName, - requestCorrelationId: ctx.requestCorrelationId, }); }), @@ -152,7 +151,6 @@ export const reviewMemoryRouter = createTRPCRouter({ return await approveAndOpenReviewMemoryChangeRequest({ owner, proposalId: input.proposalId, - requestCorrelationId: ctx.requestCorrelationId, }); } catch (error) { if (error instanceof ReviewMemoryChangeRequestError) { diff --git a/apps/web/src/routers/test-utils.ts b/apps/web/src/routers/test-utils.ts index 94ab4d0a77..d932d6b620 100644 --- a/apps/web/src/routers/test-utils.ts +++ b/apps/web/src/routers/test-utils.ts @@ -1,4 +1,3 @@ -import { randomUUID } from 'node:crypto'; import type { TRPCContext } from '@/lib/trpc/init'; import { createCallerFactory } from '@/lib/trpc/init'; import { findUserById } from '@/lib/user'; @@ -14,7 +13,6 @@ const createTestTRPCContext = async (userId: string): Promise => { } return { user, - requestCorrelationId: randomUUID(), }; };