diff --git a/src/agent/core/domain/agent-events/types.ts b/src/agent/core/domain/agent-events/types.ts index 976cd6b4f..b3543f868 100644 --- a/src/agent/core/domain/agent-events/types.ts +++ b/src/agent/core/domain/agent-events/types.ts @@ -442,6 +442,7 @@ export interface AgentEventMap { callId?: string error?: string errorType?: ToolErrorType + lifecycleResult?: unknown metadata?: Record result?: unknown sessionId: string @@ -787,6 +788,7 @@ export interface SessionEventMap { callId?: string error?: string errorType?: ToolErrorType + lifecycleResult?: unknown metadata?: Record result?: unknown success: boolean diff --git a/src/agent/infra/agent/cipher-agent.ts b/src/agent/infra/agent/cipher-agent.ts index d4e57683a..b93497de2 100644 --- a/src/agent/infra/agent/cipher-agent.ts +++ b/src/agent/infra/agent/cipher-agent.ts @@ -893,8 +893,14 @@ export class CipherAgent extends BaseAgent implements ICipherAgent { const data = payload as {sessionId?: string} if (data.sessionId !== sessionId) return + const streamData = eventName === 'llmservice:toolResult' + ? (({lifecycleResult: _lifecycleResult, ...publicData}) => publicData)( + data as {lifecycleResult?: unknown; sessionId?: string}, + ) + : data + // Add event to queue with name discriminant - eventQueue.push({name: eventName, ...data} as StreamingEvent) + eventQueue.push({name: eventName, ...streamData} as StreamingEvent) // Close iterator on run:complete if (eventName === 'run:complete') { diff --git a/src/agent/infra/llm/agent-llm-service.ts b/src/agent/infra/llm/agent-llm-service.ts index 1b2de44c7..6e36eb4c8 100644 --- a/src/agent/infra/llm/agent-llm-service.ts +++ b/src/agent/infra/llm/agent-llm-service.ts @@ -73,6 +73,20 @@ export function buildDateTimePrefix(now: Date = new Date()): string { return `Current date and time: ${now.toISOString()}\n\n` } +/** + * Select the bounded curation result carried to lifecycle hooks before display truncation. + * code_exec intentionally exposes only its curate accumulator, never stdout, locals, or return values. + */ +export function selectLifecycleResult(toolName: string, result: unknown, commandType?: string): unknown { + if (commandType !== 'curate') return undefined + if (toolName === 'curate') return result + if (toolName !== 'code_exec' || !result || typeof result !== 'object' || Array.isArray(result)) return undefined + + const resultRecord = result as Record + if (!Object.hasOwn(resultRecord, 'curateResults')) return undefined + return {curateResults: resultRecord.curateResults} +} + /** * Result of parallel tool execution (before adding to context). * Contains all information needed to add the result to context in order. @@ -1171,6 +1185,7 @@ export class AgentLLMService implements ILLMService { // Process output (truncation and file saving if needed, with per-command overrides) const processedOutput = await this.outputProcessor.processStructuredOutput(toolName, result.content, executionContext?.commandType) + const lifecycleResult = selectLifecycleResult(toolName, result.content, executionContext?.commandType) // Emit truncation event if output was truncated if (processedOutput.metadata?.truncated) { @@ -1191,6 +1206,7 @@ export class AgentLLMService implements ILLMService { ...result.metadata, ...processedOutput.metadata, }, + ...(lifecycleResult === undefined ? {} : {lifecycleResult}), result: processedOutput.content, success: result.success, taskId: taskId || undefined, diff --git a/src/agent/infra/tools/implementations/code-exec-tool.ts b/src/agent/infra/tools/implementations/code-exec-tool.ts index 34d41949f..8d809a1d8 100644 --- a/src/agent/infra/tools/implementations/code-exec-tool.ts +++ b/src/agent/infra/tools/implementations/code-exec-tool.ts @@ -127,6 +127,7 @@ export function createCodeExecTool(sandboxService: ISandboxService): Tool { } return { + ...(result.curateResults === undefined ? {} : {curateResults: result.curateResults}), executionTime: result.executionTime, finalResult: result.finalResult, locals: result.locals, @@ -146,7 +147,7 @@ export function createCodeExecTool(sandboxService: ISandboxService): Tool { } return { - ...(result.curateResults ? {curateResults: result.curateResults} : {}), + ...(result.curateResults === undefined ? {} : {curateResults: result.curateResults}), executionTime: result.executionTime, finalResult: result.finalResult, locals: result.locals, diff --git a/src/server/core/domain/entities/curate-log-entry.ts b/src/server/core/domain/entities/curate-log-entry.ts index b2c015717..30da84a25 100644 --- a/src/server/core/domain/entities/curate-log-entry.ts +++ b/src/server/core/domain/entities/curate-log-entry.ts @@ -27,6 +27,11 @@ export type CurateLogSummary = { updated: number } +export type ReviewIntegrity = { + reason?: string + status: 'unresolved' | 'verified' +} + /** * Curate-side latency tiers . All optional for back-compat with * pre-telemetry entries. No `searchMs` — curate has no BM25 search phase. @@ -71,6 +76,8 @@ type CurateLogBase = { operations: CurateLogOperation[] /** Tokens emitted for the completion across all curate sub-phases. */ outputTokens?: number + /** Whether the structured curation operation channel was validated for this run. */ + reviewIntegrity?: ReviewIntegrity startedAt: number summary: CurateLogSummary taskId: string diff --git a/src/server/core/domain/transport/schemas.ts b/src/server/core/domain/transport/schemas.ts index 2f9e3b928..f3f45759d 100644 --- a/src/server/core/domain/transport/schemas.ts +++ b/src/server/core/domain/transport/schemas.ts @@ -186,6 +186,7 @@ export const ToolResultPayloadSchema = z.object({ callId: z.string().optional(), error: z.string().optional(), errorType: ToolErrorTypeSchema.optional(), + lifecycleResult: z.unknown().optional(), metadata: z.record(z.unknown()).optional(), result: z.unknown().optional(), sessionId: z.string(), @@ -746,6 +747,7 @@ export const LlmToolResultEventSchema = z.object({ callId: z.string().optional(), error: z.string().optional(), errorType: ToolErrorTypeSchema.optional(), + lifecycleResult: z.unknown().optional(), metadata: z.record(z.unknown()).optional(), result: z.unknown().optional(), sessionId: z.string(), diff --git a/src/server/infra/process/curate-log-handler.ts b/src/server/infra/process/curate-log-handler.ts index bd29ad1fd..7ea4e6e24 100644 --- a/src/server/infra/process/curate-log-handler.ts +++ b/src/server/infra/process/curate-log-handler.ts @@ -1,10 +1,10 @@ -import type {CurateLogEntry, CurateLogOperation, CurateLogSummary, CurateLogTiming, CurateUsageRecord} from '../../core/domain/entities/curate-log-entry.js' +import type {CurateLogEntry, CurateLogOperation, CurateLogSummary, CurateLogTiming, CurateUsageRecord, ReviewIntegrity} from '../../core/domain/entities/curate-log-entry.js' import type {LlmToolResultEvent} from '../../core/domain/transport/schemas.js' import type {TaskInfo} from '../../core/domain/transport/task-info.js' import type {ITaskLifecycleHook} from '../../core/interfaces/process/i-task-lifecycle-hook.js' import type {ICurateLogStore} from '../../core/interfaces/storage/i-curate-log-store.js' -import {extractCurateOperations} from '../../utils/curate-result-parser.js' +import {extractCurateOperationCapture} from '../../utils/curate-result-parser.js' import {getProjectDataDir} from '../../utils/path-utils.js' import {transportLog} from '../../utils/process-logger.js' import {FileCurateLogStore} from '../storage/file-curate-log-store.js' @@ -26,6 +26,8 @@ type TaskState = { * daemon stamps once at the task-create boundary. */ reviewDisabled: boolean + reviewIntegrity: ReviewIntegrity + reviewIntegrityFailed: boolean /** Telemetry from the executor . Set by `setCurateUsage`. */ usage?: CurateUsageRecord } @@ -175,6 +177,7 @@ export class CurateLogHandler implements ITaskLifecycleHook { ...telemetryFields(state.usage), completedAt: Date.now(), operations: state.operations, + reviewIntegrity: state.reviewIntegrity, status: 'cancelled', summary: computeSummary(state.operations), } @@ -198,6 +201,7 @@ export class CurateLogHandler implements ITaskLifecycleHook { completedAt: Date.now(), operations: state.operations, response: result || undefined, + reviewIntegrity: state.reviewIntegrity, status: 'completed', summary: computeSummary(state.operations), } @@ -237,6 +241,7 @@ export class CurateLogHandler implements ITaskLifecycleHook { ...(task.folderPath ? {folders: [task.folderPath]} : {}), }, operations: [], + reviewIntegrity: {reason: 'No valid curation operation channel captured', status: 'unresolved'}, startedAt: task.createdAt, status: 'processing', summary: {added: 0, deleted: 0, failed: 0, merged: 0, updated: 0}, @@ -248,7 +253,14 @@ export class CurateLogHandler implements ITaskLifecycleHook { // without a getById round-trip — so completion is never lost even if this initial // save fails. const reviewDisabled = task.reviewDisabled ?? false - this.tasks.set(task.taskId, {entry, operations: [], projectPath: task.projectPath, reviewDisabled}) + this.tasks.set(task.taskId, { + entry, + operations: [], + projectPath: task.projectPath, + reviewDisabled, + reviewIntegrity: entry.reviewIntegrity!, + reviewIntegrityFailed: false, + }) this.activeTaskCount.set(task.projectPath, (this.activeTaskCount.get(task.projectPath) ?? 0) + 1) // Fire-and-forget: logId is already known, save is best-effort. @@ -277,6 +289,7 @@ export class CurateLogHandler implements ITaskLifecycleHook { completedAt: Date.now(), error: errorMessage, operations: state.operations, + reviewIntegrity: state.reviewIntegrity, status: 'error', summary: computeSummary(state.operations), ...telemetryFields(state.usage), @@ -293,8 +306,15 @@ export class CurateLogHandler implements ITaskLifecycleHook { const state = this.tasks.get(taskId) if (!state) return - const ops = extractCurateOperations(payload) - for (const op of ops) { + const capture = extractCurateOperationCapture(payload) + if (capture.reviewIntegrity?.status === 'unresolved') { + state.reviewIntegrity = capture.reviewIntegrity + state.reviewIntegrityFailed = true + } else if (capture.reviewIntegrity?.status === 'verified' && !state.reviewIntegrityFailed) { + state.reviewIntegrity = capture.reviewIntegrity + } + + for (const op of capture.operations) { if (op.needsReview && op.status === 'success' && !state.reviewDisabled) { op.reviewStatus = 'pending' } @@ -311,6 +331,17 @@ export class CurateLogHandler implements ITaskLifecycleHook { state.operations.push(op) } + + if ( + !state.reviewDisabled + && capture.operations.some((op) => op.needsReview === true && op.status === 'success' && op.reviewStatus !== 'pending') + ) { + state.reviewIntegrity = { + reason: 'Successful review-required operation was not marked pending', + status: 'unresolved', + } + state.reviewIntegrityFailed = true + } } /** diff --git a/src/server/infra/process/task-router.ts b/src/server/infra/process/task-router.ts index 46f251904..95e12645e 100644 --- a/src/server/infra/process/task-router.ts +++ b/src/server/infra/process/task-router.ts @@ -1803,6 +1803,7 @@ export class TaskRouter { */ private routeLlmEvent(eventName: string, data: {[key: string]: unknown; taskId: string}): void { const {taskId, ...rest} = data + Reflect.deleteProperty(rest, 'lifecycleResult') const activeTask = this.tasks.get(taskId) const task = activeTask ?? this.completedTasks.get(taskId)?.task @@ -1815,7 +1816,7 @@ export class TaskRouter { // Only mutates for ACTIVE tasks — grace-period entries already had their // terminal save persisted by the lifecycle hook. if (activeTask) { - this.accumulateLlmEvent(taskId, eventName, data) + this.accumulateLlmEvent(taskId, eventName, {taskId, ...rest}) } // Notify onToolResult hooks only for active tasks diff --git a/src/server/infra/storage/file-curate-log-store.ts b/src/server/infra/storage/file-curate-log-store.ts index 9ad1339ce..d6a8d392d 100644 --- a/src/server/infra/storage/file-curate-log-store.ts +++ b/src/server/infra/storage/file-curate-log-store.ts @@ -34,6 +34,11 @@ const CurateLogSummaryFileSchema = z.object({ updated: z.number(), }) +const ReviewIntegrityFileSchema = z.object({ + reason: z.string().optional(), + status: z.enum(['unresolved', 'verified']), +}) + const CurateLogTimingFileSchema = z.object({ llmMs: z.number().optional(), totalMs: z.number().optional(), @@ -52,6 +57,7 @@ const CurateLogEntryBaseSchema = z.object({ inputTokens: z.number().optional(), operations: z.array(CurateLogOperationFileSchema), outputTokens: z.number().optional(), + reviewIntegrity: ReviewIntegrityFileSchema.optional(), startedAt: z.number(), summary: CurateLogSummaryFileSchema, taskId: z.string(), diff --git a/src/server/utils/curate-result-parser.ts b/src/server/utils/curate-result-parser.ts index 833d6b722..a2789f3ea 100644 --- a/src/server/utils/curate-result-parser.ts +++ b/src/server/utils/curate-result-parser.ts @@ -1,6 +1,6 @@ import {z} from 'zod' -import type {CurateLogOperation} from '../core/domain/entities/curate-log-entry.js' +import type {CurateLogOperation, ReviewIntegrity} from '../core/domain/entities/curate-log-entry.js' import type {LlmToolResultEvent} from '../core/domain/transport/schemas.js' // ── Zod schemas ────────────────────────────────────────────────────────────── @@ -34,6 +34,36 @@ export const CurateResultSchema = z.object({ .optional(), }) +const CapturedCurateOperationSchema = CurateOperationSchema.extend({ + confidence: z.enum(['high', 'low']), + impact: z.enum(['high', 'low']), + needsReview: z.boolean(), + reason: z.string(), +}).superRefine((operation, context) => { + if (operation.status !== 'success') return + const policyRequiresReview = operation.type === 'DELETE' || operation.impact === 'high' + if (operation.needsReview !== policyRequiresReview) { + context.addIssue({ + code: z.ZodIssueCode.custom, + message: 'needsReview contradicts ByteRover 3.16.1 review policy', + path: ['needsReview'], + }) + } +}) + +const CapturedCurateResultSchema = z.object({ + applied: z.array(CapturedCurateOperationSchema), +}) + +const CapturedCodeExecResultSchema = z.object({ + curateResults: z.array(CapturedCurateResultSchema).min(1), +}) + +export type CurateOperationCapture = { + operations: CurateLogOperation[] + reviewIntegrity?: ReviewIntegrity +} + // ── Internal helpers ────────────────────────────────────────────────────────── /** @@ -114,10 +144,15 @@ export function extractCurateResultFromCodeExec(resultData: Record, + payload: Pick, filter?: (op: CurateLogOperation) => boolean, ): CurateLogOperation[] { - const {result: rawPayload, toolName} = payload + const {lifecycleResult, result: rawPayload, toolName} = payload + + if (lifecycleResult !== undefined) { + const capture = extractCurateOperationCapture(payload) + return filter ? capture.operations.filter((op) => filter(op)) : capture.operations + } // ToolOutputProcessor always stringifies tool output — parse if string let result: unknown = rawPayload @@ -144,3 +179,40 @@ export function extractCurateOperations( const ops: CurateLogOperation[] = parsed.data.applied return filter ? ops.filter((op) => filter(op)) : ops } + +/** Validate the internal, pre-truncation curation operation channel. */ +export function extractCurateOperationCapture( + payload: Pick, +): CurateOperationCapture { + const {lifecycleResult, toolName} = payload + + if (toolName === 'curate') { + const parsed = CapturedCurateResultSchema.safeParse(lifecycleResult) + if (!parsed.success) { + return { + operations: [], + reviewIntegrity: {reason: 'Invalid direct curate lifecycle result', status: 'unresolved'}, + } + } + + return {operations: parsed.data.applied, reviewIntegrity: {status: 'verified'}} + } + + if (toolName === 'code_exec') { + if (lifecycleResult === undefined) return {operations: []} + const parsed = CapturedCodeExecResultSchema.safeParse(lifecycleResult) + if (!parsed.success) { + return { + operations: [], + reviewIntegrity: {reason: 'Invalid code_exec curate lifecycle result', status: 'unresolved'}, + } + } + + return { + operations: parsed.data.curateResults.flatMap((curateResult) => curateResult.applied), + reviewIntegrity: {status: 'verified'}, + } + } + + return {operations: []} +} diff --git a/test/unit/agent/cipher-agent.test.ts b/test/unit/agent/cipher-agent.test.ts index 087ab60c1..518343de0 100644 --- a/test/unit/agent/cipher-agent.test.ts +++ b/test/unit/agent/cipher-agent.test.ts @@ -52,6 +52,59 @@ describe('CipherAgent', () => { restore() }) + describe('stream', () => { + it('keeps lifecycleResult on the internal event bus but not the public stream', async () => { + const agent = new CipherAgent(agentConfig) + const abortController = new AbortController() + + try { + await agent.start() + const internalEvents: unknown[] = [] + agent.agentEventBus!.on('llmservice:toolResult', (payload) => internalEvents.push(payload)) + const iterator = await agent.stream('inspect stream privacy', {signal: abortController.signal}) + const lifecycleResult = { + applied: [{ + confidence: 'high', + impact: 'high', + needsReview: true, + path: '/private-operation.md', + reason: 'Internal lifecycle metadata', + status: 'success', + type: 'UPSERT', + }], + } + + agent.agentEventBus!.emit('llmservice:toolResult', { + lifecycleResult, + result: 'public display result', + sessionId: agent.sessionId!, + success: true, + toolName: 'curate', + } as never) + + const readPublicToolResult = async (attemptsRemaining: number): Promise | undefined> => { + if (attemptsRemaining === 0) return undefined + const next = await iterator.next() + if (next.done) return undefined + if (next.value.name === 'llmservice:toolResult') { + return next.value as unknown as Record + } + + return readPublicToolResult(attemptsRemaining - 1) + } + + const publicToolResult = await readPublicToolResult(10) + + expect(internalEvents[0]).to.deep.include({lifecycleResult}) + expect(publicToolResult).to.include({result: 'public display result'}) + expect(publicToolResult).to.not.have.property('lifecycleResult') + } finally { + abortController.abort() + await agent.stop() + } + }) + }) + describe('constructor', () => { it('should create instance with valid agent config', () => { const agent = new CipherAgent(agentConfig) diff --git a/test/unit/agent/tools/code-exec-tool.test.ts b/test/unit/agent/tools/code-exec-tool.test.ts new file mode 100644 index 000000000..031b1db87 --- /dev/null +++ b/test/unit/agent/tools/code-exec-tool.test.ts @@ -0,0 +1,76 @@ +import {expect} from 'chai' + +import type {ISandboxService} from '../../../../src/agent/core/interfaces/i-sandbox-service.js' + +import {createCodeExecTool} from '../../../../src/agent/infra/tools/implementations/code-exec-tool.js' + +describe('code_exec tool', () => { + it('preserves curateResults when large stdout is redirected', async () => { + const curateResults = [{ + applied: [{ + confidence: 'high', + impact: 'high', + needsReview: true, + path: '/review-integrity.md', + reason: 'A core decision changed', + status: 'success', + type: 'UPSERT', + }], + }] + const sandboxService = { + async executeCode() { + return { + curateResults, + executionTime: 12, + finalResult: undefined, + locals: {}, + returnValue: undefined, + stderr: '', + stdout: 'x'.repeat(3000), + } + }, + setSandboxVariable() {}, + } as unknown as ISandboxService + const tool = createCodeExecTool(sandboxService) + + const result = await tool.execute( + {code: 'console.log("x")'}, + {commandType: 'curate', sessionId: 'curate-session'}, + ) as Record + + expect(result.curateResults).to.deep.equal(curateResults) + expect(result.stdout).to.match(/stored in variable/i) + }) + + it('preserves an own curateResults property for every defined value in small and silent results', async () => { + const cases = [false, true].flatMap((silent) => ( + [null, false, 0, ''].map((curateResults) => ({curateResults, silent})) + )) + + await Promise.all(cases.map(async ({curateResults, silent}) => { + const sandboxService = { + async executeCode() { + return { + curateResults, + executionTime: 1, + finalResult: undefined, + locals: {}, + returnValue: undefined, + stderr: '', + stdout: 'small output', + } + }, + } as unknown as ISandboxService + const tool = createCodeExecTool(sandboxService) + + const result = await tool.execute( + {code: 'return 1', silent}, + {commandType: 'curate', sessionId: 'curate-session'}, + ) as Record + + expect(Object.hasOwn(result, 'curateResults')).to.equal(true) + expect(result.curateResults).to.equal(curateResults) + expect(result.stdout).to.equal(silent ? '' : 'small output') + })) + }) +}) diff --git a/test/unit/core/domain/transport/schemas.test.ts b/test/unit/core/domain/transport/schemas.test.ts index 14bd34091..95d254d51 100644 --- a/test/unit/core/domain/transport/schemas.test.ts +++ b/test/unit/core/domain/transport/schemas.test.ts @@ -1,6 +1,7 @@ import {expect} from 'chai' import { + LlmToolResultEventSchema, TaskClearCompletedRequestSchema, TaskCreatedSchema, TaskDeleteBulkRequestSchema, @@ -14,6 +15,24 @@ import { } from '../../../../../src/server/core/domain/transport/schemas.js' describe('task transport schemas', () => { + it('accepts lifecycleResult on tool-result events', () => { + const lifecycleResult = { + curateResults: [{applied: [{path: '/a.md', status: 'success', type: 'ADD'}]}], + } + const result = LlmToolResultEventSchema.safeParse({ + lifecycleResult, + sessionId: 'session-1', + success: true, + taskId: 'task-1', + toolName: 'code_exec', + }) + + expect(result.success).to.equal(true) + if (result.success) { + expect((result.data as typeof result.data & {lifecycleResult?: unknown}).lifecycleResult).to.deep.equal(lifecycleResult) + } + }) + describe('TaskListItemSchema', () => { const baseEntry = { content: 'test', diff --git a/test/unit/infra/llm/internal-llm-service-gemini.test.ts b/test/unit/infra/llm/internal-llm-service-gemini.test.ts index 6e88ca7a2..6292d92e6 100644 --- a/test/unit/infra/llm/internal-llm-service-gemini.test.ts +++ b/test/unit/infra/llm/internal-llm-service-gemini.test.ts @@ -6,7 +6,7 @@ import type {GenerateContentResponse} from '../../../../src/agent/core/interface import {ToolErrorType} from '../../../../src/agent/core/domain/tools/tool-error.js' import {SessionEventBus} from '../../../../src/agent/infra/events/event-emitter.js' import {ByteRoverLlmHttpService} from '../../../../src/agent/infra/http/internal-llm-http-service.js' -import {AgentLLMService} from '../../../../src/agent/infra/llm/agent-llm-service.js' +import {AgentLLMService, selectLifecycleResult} from '../../../../src/agent/infra/llm/agent-llm-service.js' import {ByteRoverContentGenerator} from '../../../../src/agent/infra/llm/generators/byterover-content-generator.js' import {SystemPromptManager} from '../../../../src/agent/infra/system-prompt/system-prompt-manager.js' import {ToolManager} from '../../../../src/agent/infra/tools/tool-manager.js' @@ -47,6 +47,7 @@ describe('AgentLLMService - Gemini Integration', () => { getAllTools: sandbox.stub().returns({}), getAvailableMarkers: sandbox.stub().returns(new Set()), getToolNames: sandbox.stub().returns([]), + hasTool: sandbox.stub().returns(true), }) // eslint-disable-next-line @typescript-eslint/no-explicit-any toolManager = new ToolManager(mockToolProvider as any) @@ -355,6 +356,80 @@ describe('AgentLLMService - Gemini Integration', () => { expect(result).to.equal('I found 5 TypeScript files') }) + it('preserves malformed code_exec curateResults presence for integrity validation', () => { + expect(selectLifecycleResult( + 'code_exec', + {curateResults: 'malformed producer value', stdout: 'do not preserve'}, + 'curate', + )).to.deep.equal({curateResults: 'malformed producer value'}) + }) + + it('preserves only code_exec curateResults when the display result is truncated', async () => { + const generator = createContentGenerator('gemini-2.5-flash') + const service = new AgentLLMService( + 'test-session', + generator, + {model: 'gemini-2.5-flash'}, + {sessionEventBus, systemPromptManager, toolManager}, + ) + const highImpactOperation = { + impact: 'high', + needsReview: true, + path: '/topics/review-integrity.md', + status: 'success', + type: 'UPSERT', + } + const curateResults = [{applied: [highImpactOperation]}] + const rawResult = { + curateResults, + diagnostics: Array.from({length: 500}, (_, index) => `${index}:${'x'.repeat(300)}`), + stdout: 'full memory body must not enter the lifecycle channel', + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + sandbox.stub(service.getContextManager() as any, 'addUserMessage').resolves() + // eslint-disable-next-line @typescript-eslint/no-explicit-any + sandbox.stub(service.getContextManager() as any, 'getFormattedMessagesWithCompression').resolves({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + formattedMessages: [{parts: [{text: 'curate'}], role: 'user'} as any], + }) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + sandbox.stub(service.getContextManager() as any, 'addAssistantMessage').resolves() + // eslint-disable-next-line @typescript-eslint/no-explicit-any + sandbox.stub(service.getContextManager() as any, 'addToolResult').resolves('tool-result') + sandbox.stub(toolManager, 'executeTool').resolves({content: rawResult, metadata: {}, success: true}) + + const generateStub = sandbox.stub(generator, 'generateContent') + generateStub.onFirstCall().resolves({ + content: '', + finishReason: 'tool_calls', + toolCalls: [{ + function: {arguments: '{}', name: 'code_exec'}, + id: 'call-1', + type: 'function', + }], + }) + generateStub.onSecondCall().resolves({content: 'done', finishReason: 'stop', toolCalls: []}) + + const toolResultEvents: unknown[] = [] + sessionEventBus.on('llmservice:toolResult', (event) => toolResultEvents.push(event)) + await service.completeTask('curate', { + executionContext: {commandType: 'curate'}, + taskId: 'task-review-integrity', + }) + + expect(toolResultEvents).to.have.lengthOf(1) + const event = toolResultEvents[0] as { + lifecycleResult?: unknown + metadata?: {truncated?: boolean} + result?: unknown + } + expect(event.metadata?.truncated).to.equal(true) + expect(event.lifecycleResult).to.deep.equal({curateResults}) + expect(event.lifecycleResult).to.not.have.property('stdout') + expect(event.lifecycleResult).to.not.have.property('diagnostics') + }) + it('should handle multiple parallel tool calls with Gemini', async () => { const generator = createContentGenerator('gemini-2.5-flash') const service = new AgentLLMService( diff --git a/test/unit/infra/process/curate-log-handler.test.ts b/test/unit/infra/process/curate-log-handler.test.ts index f3b8defd6..81fb7e379 100644 --- a/test/unit/infra/process/curate-log-handler.test.ts +++ b/test/unit/infra/process/curate-log-handler.test.ts @@ -23,6 +23,26 @@ function makeTask(overrides: Partial = {}): TaskInfo { } } +type NativeCapturedOperation = CurateLogOperation & { + confidence: 'high' | 'low' + impact: 'high' | 'low' + needsReview: boolean + reason: string +} + +function makeCapturedOperation(overrides: Partial = {}): NativeCapturedOperation { + return { + confidence: 'high', + impact: 'low', + needsReview: false, + path: '/captured.md', + reason: 'Native operation metadata', + status: 'success', + type: 'ADD', + ...overrides, + } +} + function makeStore(sandbox: SinonSandbox): ICurateLogStore & { batchUpdateOperationReviewStatus: SinonStub getById: SinonStub @@ -97,7 +117,7 @@ describe('computeSummary', () => { expect(summary.added).to.equal(0) expect(summary.deleted).to.equal(0) }) -}) + }) // ============================================================================ // CurateLogHandler @@ -213,9 +233,9 @@ describe('CurateLogHandler', () => { it('should collect curate operations from tool result', () => { handler.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [ - {path: '/topics/auth.md', status: 'success', type: 'ADD'}, + makeCapturedOperation({path: '/topics/auth.md'}), ], }, sessionId: 'sess-1', @@ -230,7 +250,7 @@ describe('CurateLogHandler', () => { it('should set reviewStatus=pending for operations with needsReview=true', async () => { handler.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [ {confidence: 'low', impact: 'high', needsReview: true, path: '/a.md', reason: 'uncertain', status: 'success', type: 'UPDATE'}, {confidence: 'high', impact: 'low', needsReview: false, path: '/b.md', reason: 'clear', status: 'success', type: 'ADD'}, @@ -253,7 +273,7 @@ describe('CurateLogHandler', () => { it('should NOT set reviewStatus=pending for failed operations even with needsReview=true', async () => { handler.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [ {confidence: 'low', impact: 'high', needsReview: true, path: '/a.md', reason: 'uncertain', status: 'failed', type: 'UPDATE'}, {confidence: 'high', impact: 'high', needsReview: true, path: '/b.md', reason: 'irreversible', status: 'success', type: 'DELETE'}, @@ -278,9 +298,9 @@ describe('CurateLogHandler', () => { it('should not set reviewStatus for operations without needsReview', async () => { handler.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [ - {confidence: 'high', impact: 'low', needsReview: false, path: '/a.md', status: 'success', type: 'ADD'}, + makeCapturedOperation({path: '/a.md'}), ], }, sessionId: 'sess-1', @@ -298,12 +318,12 @@ describe('CurateLogHandler', () => { it('should deduplicate operations by filePath, keeping the latest', async () => { // First tool result: initial UPSERT for a file handler.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [{ confidence: 'low', filePath: '/app/.brv/context-tree/design/caching/caching_strategy.md', impact: 'low', - needsReview: true, + needsReview: false, path: 'design/caching', reason: 'first pass', status: 'success', @@ -318,7 +338,7 @@ describe('CurateLogHandler', () => { // Second tool result: same file updated again handler.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [{ confidence: 'low', filePath: '/app/.brv/context-tree/design/caching/caching_strategy.md', @@ -347,10 +367,10 @@ describe('CurateLogHandler', () => { it('should keep separate operations for different filePaths', async () => { handler.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [ - {filePath: '/app/.brv/context-tree/design/caching/redis.md', path: 'design/caching', status: 'success', type: 'ADD'}, - {filePath: '/app/.brv/context-tree/design/caching/memcache.md', path: 'design/caching', status: 'success', type: 'ADD'}, + makeCapturedOperation({filePath: '/app/.brv/context-tree/design/caching/redis.md', path: 'design/caching'}), + makeCapturedOperation({filePath: '/app/.brv/context-tree/design/caching/memcache.md', path: 'design/caching'}), ], }, sessionId: 'sess-1', @@ -367,9 +387,9 @@ describe('CurateLogHandler', () => { it('should not deduplicate operations without filePath', async () => { handler.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [ - {path: 'design/caching', status: 'failed', type: 'ADD'}, + makeCapturedOperation({path: 'design/caching', status: 'failed'}), ], }, sessionId: 'sess-1', @@ -379,9 +399,9 @@ describe('CurateLogHandler', () => { } as never) handler.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [ - {path: 'design/caching', status: 'failed', type: 'ADD'}, + makeCapturedOperation({path: 'design/caching', status: 'failed'}), ], }, sessionId: 'sess-1', @@ -400,7 +420,7 @@ describe('CurateLogHandler', () => { it('should silently skip unknown taskId', () => { expect(() => { handler.onToolResult('unknown-task', { - result: {applied: [{path: '/a.md', status: 'success', type: 'ADD'}]}, + lifecycleResult: {applied: [{path: '/a.md', status: 'success', type: 'ADD'}]}, sessionId: 'sess-1', success: true, taskId: 'unknown-task', @@ -420,7 +440,7 @@ describe('CurateLogHandler', () => { // Inject operations via onToolResult handler.onToolResult('task-abc', { - result: {applied: [{path: '/a.md', status: 'success', type: 'ADD'}]}, + lifecycleResult: {applied: [makeCapturedOperation({path: '/a.md'})]}, sessionId: 'sess-1', success: true, taskId: 'task-abc', @@ -441,7 +461,7 @@ describe('CurateLogHandler', () => { it('should compute correct summary from collected operations', async () => { handler.onToolResult('task-abc', { - result: {applied: [{path: '/b.md', status: 'failed', type: 'UPDATE'}]}, + lifecycleResult: {applied: [makeCapturedOperation({path: '/b.md', status: 'failed', type: 'UPDATE'})]}, sessionId: 'sess-1', success: true, taskId: 'task-abc', @@ -462,6 +482,50 @@ describe('CurateLogHandler', () => { }) }) + describe('review integrity capture', () => { + it('records verified structured operations and pending status', async () => { + await handler.onTaskCreate(makeTask()) + handler.onToolResult('task-abc', { + lifecycleResult: { + applied: [makeCapturedOperation({impact: 'high', needsReview: true, path: '/high.md', type: 'UPSERT'})], + }, + result: 'truncated display output', + sessionId: 'sess-1', + success: true, + taskId: 'task-abc', + toolName: 'curate', + } as never) + + await handler.onTaskCompleted('task-abc', 'done', makeTask()) + const completed = store.save.secondCall.args[0] as CurateLogEntry & {reviewIntegrity?: {status: string}} + expect(completed.operations[0].reviewStatus).to.equal('pending') + expect(completed.reviewIntegrity).to.deep.equal({status: 'verified'}) + }) + + it('keeps integrity unresolved after malformed lifecycle data follows valid capture', async () => { + await handler.onTaskCreate(makeTask()) + handler.onToolResult('task-abc', { + lifecycleResult: {applied: [makeCapturedOperation({path: '/valid.md'})]}, + sessionId: 'sess-1', + success: true, + taskId: 'task-abc', + toolName: 'curate', + } as never) + handler.onToolResult('task-abc', { + lifecycleResult: {applied: [{path: '/invalid.md', status: 'success', type: 'UPSERT'}]}, + sessionId: 'sess-1', + success: true, + taskId: 'task-abc', + toolName: 'curate', + } as never) + + await handler.onTaskCompleted('task-abc', 'done', makeTask()) + const completed = store.save.secondCall.args[0] as CurateLogEntry & {reviewIntegrity?: {status: string}} + expect(completed.operations).to.have.lengthOf(1) + expect(completed.reviewIntegrity?.status).to.equal('unresolved') + }) + }) + // ========================================================================== // onTaskCancelled // ========================================================================== @@ -577,12 +641,13 @@ describe('CurateLogHandler', () => { // Inject operation with reviewStatus=pending handlerWithCallback.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [{ confidence: 'low', impact: 'high', needsReview: true, path: '/a.md', + reason: 'Irreversible deletion', status: 'success', type: 'DELETE', }], @@ -613,7 +678,7 @@ describe('CurateLogHandler', () => { // Inject operation without needsReview handlerWithCallback.onToolResult('task-abc', { - result: {applied: [{path: '/a.md', status: 'success', type: 'ADD'}]}, + lifecycleResult: {applied: [makeCapturedOperation({path: '/a.md'})]}, sessionId: 'sess-1', success: true, taskId: 'task-abc', @@ -634,8 +699,8 @@ describe('CurateLogHandler', () => { await handlerWithBadCallback.onTaskCreate(makeTask()) handlerWithBadCallback.onToolResult('task-abc', { - result: { - applied: [{needsReview: true, path: '/a.md', status: 'success', type: 'DELETE'}], + lifecycleResult: { + applied: [makeCapturedOperation({impact: 'high', needsReview: true, path: '/a.md', type: 'DELETE'})], }, sessionId: 'sess-1', success: true, @@ -658,7 +723,7 @@ describe('CurateLogHandler', () => { await handlerWithToggle.onTaskCreate(makeTask({reviewDisabled: true})) handlerWithToggle.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [ {confidence: 'low', impact: 'high', needsReview: true, path: '/a.md', reason: 'uncertain', status: 'success', type: 'UPDATE'}, {confidence: 'high', impact: 'high', needsReview: true, path: '/b.md', reason: 'irreversible', status: 'success', type: 'DELETE'}, @@ -672,7 +737,9 @@ describe('CurateLogHandler', () => { await handlerWithToggle.onTaskCompleted('task-abc', 'done', makeTask({reviewDisabled: true})) - const completed: CurateLogEntry = store.save.secondCall.args[0] + const completed = store.save.secondCall.args[0] as CurateLogEntry & { + reviewIntegrity: {reason?: string; status: 'unresolved' | 'verified'} + } expect(completed.operations[0].reviewStatus).to.be.undefined expect(completed.operations[1].reviewStatus).to.be.undefined }) @@ -682,9 +749,9 @@ describe('CurateLogHandler', () => { await handlerWithToggle.onTaskCreate(makeTask({reviewDisabled: false})) handlerWithToggle.onToolResult('task-abc', { - result: { + lifecycleResult: { applied: [ - {confidence: 'low', impact: 'high', needsReview: true, path: '/a.md', status: 'success', type: 'UPDATE'}, + makeCapturedOperation({confidence: 'low', impact: 'high', needsReview: true, path: '/a.md', type: 'UPDATE'}), ], }, sessionId: 'sess-1', @@ -703,7 +770,7 @@ describe('CurateLogHandler', () => { await handlerWithToggle.onTaskCreate(makeTask()) handlerWithToggle.onToolResult('task-abc', { - result: {applied: [{needsReview: true, path: '/a.md', status: 'success', type: 'DELETE'}]}, + lifecycleResult: {applied: [makeCapturedOperation({impact: 'high', needsReview: true, path: '/a.md', type: 'DELETE'})]}, sessionId: 'sess-1', success: true, taskId: 'task-abc', @@ -724,8 +791,8 @@ describe('CurateLogHandler', () => { await handlerWithToggle.onTaskCreate(makeTask({reviewDisabled: true})) handlerWithToggle.onToolResult('task-abc', { - result: { - applied: [{needsReview: true, path: '/a.md', status: 'success', type: 'DELETE'}], + lifecycleResult: { + applied: [makeCapturedOperation({impact: 'high', needsReview: true, path: '/a.md', type: 'DELETE'})], }, sessionId: 'sess-1', success: true, @@ -743,7 +810,7 @@ describe('CurateLogHandler', () => { await handlerWithToggle.onTaskCreate(makeTask({reviewDisabled: true})) handlerWithToggle.onToolResult('task-abc', { - result: {applied: [{needsReview: true, path: '/a.md', status: 'success', type: 'DELETE'}]}, + lifecycleResult: {applied: [makeCapturedOperation({impact: 'high', needsReview: true, path: '/a.md', type: 'DELETE'})]}, sessionId: 'sess-1', success: true, taskId: 'task-abc', @@ -764,7 +831,7 @@ describe('CurateLogHandler', () => { task.reviewDisabled = false handlerWithToggle.onToolResult('task-abc', { - result: {applied: [{needsReview: true, path: '/a.md', status: 'success', type: 'DELETE'}]}, + lifecycleResult: {applied: [makeCapturedOperation({impact: 'high', needsReview: true, path: '/a.md', type: 'DELETE'})]}, sessionId: 'sess-1', success: true, taskId: 'task-abc', @@ -777,5 +844,4 @@ describe('CurateLogHandler', () => { }) }) }) - -}) // end curate-log-handler +}) diff --git a/test/unit/infra/process/task-router-accumulator.test.ts b/test/unit/infra/process/task-router-accumulator.test.ts index ec7ca181c..8a78540cb 100644 --- a/test/unit/infra/process/task-router-accumulator.test.ts +++ b/test/unit/infra/process/task-router-accumulator.test.ts @@ -97,6 +97,7 @@ type StubHook = ITaskLifecycleHook & { onTaskCreate: SinonStub onTaskError: SinonStub onTaskUpdate: SinonStub + onToolResult: SinonStub } function makeStubLifecycleHook(sandbox: SinonSandbox): StubHook { @@ -107,6 +108,7 @@ function makeStubLifecycleHook(sandbox: SinonSandbox): StubHook { onTaskCreate: sandbox.stub().resolves(), onTaskError: sandbox.stub().resolves(), onTaskUpdate: sandbox.stub().resolves(), + onToolResult: sandbox.stub(), } } @@ -335,6 +337,45 @@ describe('TaskRouter — llmservice accumulator', () => { expect(task?.toolCalls?.[0].result).to.deep.equal({ok: true}) }) + it('keeps lifecycleResult internal to hooks and excludes it from history and client broadcasts', async () => { + const taskId = randomUUID() + const lifecycleResult = { + curateResults: [{applied: [{needsReview: true, path: '/high-impact.md', status: 'success', type: 'UPSERT'}]}], + } + await createTask(taskId) + + dispatchLlm(LlmEventNames.TOOL_CALL, { + args: {}, + callId: 'c1', + sessionId: 's1', + taskId, + toolName: 'code_exec', + }) + dispatchLlm(LlmEventNames.TOOL_RESULT, { + callId: 'c1', + lifecycleResult, + result: 'truncated display output', + sessionId: 's1', + success: true, + taskId, + toolName: 'code_exec', + }) + + expect(hook.onToolResult.calledOnce).to.equal(true) + expect(hook.onToolResult.firstCall.args[1].lifecycleResult).to.deep.equal(lifecycleResult) + + const task = getLiveTask(taskId) + expect(task?.toolCalls?.[0]).to.not.have.property('lifecycleResult') + + const directPayload = (transportHelper.transport.sendTo as SinonStub).getCalls() + .find((call) => call.args[1] === LlmEventNames.TOOL_RESULT)?.args[2] + expect(directPayload).to.not.have.property('lifecycleResult') + + const projectPayload = projectRouter.broadcastToProject.getCalls() + .find((call) => call.args[1] === LlmEventNames.TOOL_RESULT)?.args[2] + expect(projectPayload).to.not.have.property('lifecycleResult') + }) + it('llmservice:error / :unsupportedInput do NOT mutate TaskInfo', async () => { const taskId = randomUUID() await createTask(taskId) diff --git a/test/unit/infra/storage/file-curate-log-store.test.ts b/test/unit/infra/storage/file-curate-log-store.test.ts index aa5779d30..6bc5ee2ed 100644 --- a/test/unit/infra/storage/file-curate-log-store.test.ts +++ b/test/unit/infra/storage/file-curate-log-store.test.ts @@ -108,6 +108,7 @@ describe('FileCurateLogStore', () => { input: {context: 'test context', files: ['src/auth.ts']}, operations: [{path: '/a.md', status: 'success', type: 'ADD'}], response: 'Done!', + reviewIntegrity: {status: 'verified'}, startedAt: Date.now() - 1000, status: 'completed', summary: {added: 1, deleted: 0, failed: 0, merged: 0, updated: 0}, @@ -119,6 +120,7 @@ describe('FileCurateLogStore', () => { expect(retrieved).to.deep.equal(entry) expect(retrieved?.status).to.equal('completed') + expect(retrieved?.reviewIntegrity).to.deep.equal({status: 'verified'}) }) it('should save an error entry', async () => { diff --git a/test/unit/utils/curate-result-parser.test.ts b/test/unit/utils/curate-result-parser.test.ts index 895416910..5e8b6b751 100644 --- a/test/unit/utils/curate-result-parser.test.ts +++ b/test/unit/utils/curate-result-parser.test.ts @@ -3,6 +3,7 @@ import {expect} from 'chai' import { CurateOperationSchema, CurateResultSchema, + extractCurateOperationCapture, extractCurateOperations, extractCurateResultFromCodeExec, } from '../../../src/server/utils/curate-result-parser.js' @@ -191,6 +192,15 @@ describe('curate-result-parser', () => { describe('extractCurateOperations', () => { const validOp = {path: '/topics/auth.md', status: 'success' as const, type: 'ADD' as const} + const capturedOperation = { + confidence: 'high' as const, + impact: 'low' as const, + needsReview: false, + path: '/topics/auth.md', + reason: 'Capture a stable authentication rule', + status: 'success' as const, + type: 'ADD' as const, + } it('should extract operations from curate tool result', () => { const ops = extractCurateOperations({ @@ -257,5 +267,76 @@ describe('curate-result-parser', () => { }) expect(ops).to.have.lengthOf(0) }) + + it('should extract direct curate operations from lifecycleResult when display output is truncated', () => { + const ops = extractCurateOperations({ + lifecycleResult: {applied: [capturedOperation]}, + result: '{"applied":[{"path":"/topics/auth.md"... output truncated', + toolName: 'curate', + } as never) + + expect(ops).to.deep.equal([capturedOperation]) + }) + + it('should extract only code_exec curateResults from lifecycleResult', () => { + const ops = extractCurateOperations({ + lifecycleResult: { + curateResults: [{applied: [capturedOperation]}], + }, + result: 'truncated display output', + toolName: 'code_exec', + } as never) + + expect(ops).to.deep.equal([capturedOperation]) + }) + }) + + describe('extractCurateOperationCapture integrity policy', () => { + const capturedOperation = { + confidence: 'high' as const, + impact: 'low' as const, + needsReview: false, + path: '/topics/auth.md', + reason: 'Capture a stable authentication rule', + status: 'success' as const, + type: 'ADD' as const, + } + + it('verifies a complete low-impact non-delete operation', () => { + const capture = extractCurateOperationCapture({lifecycleResult: {applied: [capturedOperation]}, toolName: 'curate'}) + expect(capture.reviewIntegrity).to.deep.equal({status: 'verified'}) + expect(capture.operations).to.deep.equal([capturedOperation]) + }) + + it('verifies complete high-impact and delete operations that require review', () => { + for (const operation of [ + {...capturedOperation, impact: 'high' as const, needsReview: true, type: 'UPSERT' as const}, + {...capturedOperation, needsReview: true, type: 'DELETE' as const}, + ]) { + const capture = extractCurateOperationCapture({lifecycleResult: {applied: [operation]}, toolName: 'curate'}) + expect(capture.reviewIntegrity).to.deep.equal({status: 'verified'}) + expect(capture.operations).to.deep.equal([operation]) + } + }) + + it('marks missing native review-decision fields unresolved', () => { + const missingImpact: Partial = {...capturedOperation} + delete missingImpact.impact + const capture = extractCurateOperationCapture({lifecycleResult: {applied: [missingImpact]}, toolName: 'curate'}) + expect(capture.operations).to.deep.equal([]) + expect(capture.reviewIntegrity?.status).to.equal('unresolved') + }) + + it('marks successful high-impact and delete review contradictions unresolved', () => { + for (const operation of [ + {...capturedOperation, impact: 'high' as const, needsReview: false, type: 'UPSERT' as const}, + {...capturedOperation, needsReview: false, type: 'DELETE' as const}, + {...capturedOperation, needsReview: true}, + ]) { + const capture = extractCurateOperationCapture({lifecycleResult: {applied: [operation]}, toolName: 'curate'}) + expect(capture.operations).to.deep.equal([]) + expect(capture.reviewIntegrity?.status).to.equal('unresolved') + } + }) }) })