From dd3c7222a7bd3c7edee582a732d9194444ce75d5 Mon Sep 17 00:00:00 2001 From: "haozhe.yang" Date: Mon, 6 Jul 2026 20:20:11 +0800 Subject: [PATCH] refactor(agent-core-v2): route context writes through 1.4 ops and declared tool delivery - contextMemory: emit 1.4 ops (append_message/clear/apply_compaction/undo) on the live write path; demote context.splice to legacy (replay + rare single-deletes); drop context.append_loop_event; share computeUndoCut between reducer and service; extend contextBlobSelector to append_message records - prompt: consume tool result `delivery` via onDidExecuteTool and perform the steer at L4, stripping the side channel before it reaches the loop; delegate undo/clear/append to context service - skill tool: stop reaching into IAgentPromptService; declare a `delivery: steer` on the result for the agent layer to consume - tests: update stubs/snapshots, add delivery threading coverage, drop obsolete v1.4->v1.5 migration test, clarify several test titles - also includes a saved session transcript at repo root --- .../src/agent/contextMemory/contextOps.ts | 130 +++++++--- .../src/agent/prompt/promptService.ts | 68 +++--- .../agent-core-v2/src/agent/skill/skill.ts | 7 +- .../src/agent/skill/tools/skill.ts | 23 +- .../agent-core-v2/test/_base/event.test.ts | 2 +- .../test/contextMemory/splice-replay.test.ts | 7 - .../agent-core-v2/test/contextMemory/stubs.ts | 28 ++- .../test/externalHooks/integration.test.ts | 18 +- ...xit-plan-mode-review-ask-telemetry.test.ts | 8 +- .../test/prompt/promptService.test.ts | 52 +++- .../agent-core-v2/test/skill/skill.test.ts | 21 +- .../agent-core-v2/test/task/manager.test.ts | 2 +- packages/agent-core-v2/test/task/task.test.ts | 35 +-- .../test/toolExecutor/tool-executor.test.ts | 38 ++- .../test/wireRecord/migration/utils.ts | 10 - .../test/wireRecord/migration/v1.4.test.ts | 2 +- .../test/wireRecord/migration/v1.5.test.ts | 225 ------------------ 17 files changed, 290 insertions(+), 386 deletions(-) delete mode 100644 packages/agent-core-v2/test/wireRecord/migration/v1.5.test.ts diff --git a/packages/agent-core-v2/src/agent/contextMemory/contextOps.ts b/packages/agent-core-v2/src/agent/contextMemory/contextOps.ts index 7c2cdf831..a109b7fba 100644 --- a/packages/agent-core-v2/src/agent/contextMemory/contextOps.ts +++ b/packages/agent-core-v2/src/agent/contextMemory/contextOps.ts @@ -1,21 +1,25 @@ /** - * `contextMemory` domain (L4) — wire Model (`ContextModel`) and the - * `context.splice` (`contextSplice`) / `context.append_message` - * (`contextAppendMessage`) / `context.append_loop_event` - * (`contextAppendLoopEvent`) / `context.clear` (`contextClear`) / - * `context.apply_compaction` (`contextApplyCompaction`) / `context.undo` - * (`contextUndo`) Ops for the per-agent conversation history, plus the - * `contextBlobSelector` that drives blob offload for `context.splice` records. + * `contextMemory` domain (L4) — wire Model (`ContextModel`) and the wire-protocol + * 1.4 Ops `context.append_message` (`contextAppendMessage`) / `context.clear` + * (`contextClear`) / `context.apply_compaction` (`contextApplyCompaction`) / + * `context.undo` (`contextUndo`) for the per-agent conversation history, plus the + * legacy `context.splice` (`contextSplice`) Op and the `contextBlobSelector` that + * drives blob offload for persisted message parts. * * Declares the history as `ContextMessage[]` (initial `[]`); every Op's `apply` * is a pure array transform that returns a NEW reference on change and the SAME * reference on a no-op (so the wire's reference-equality gate stays quiet), and * carries no non-determinism — message ids are stamped at the dispatch call site - * (`AgentContextMemoryService.splice`), never inside `apply`. The higher-level - * legacy record types (`append_message` / `append_loop_event` / `clear` / - * `apply_compaction` / `undo`) are declared for wire-schema coverage and tested - * directly; the live service writes only `context.splice` (splice is the single - * primitive the other shapes fold into). + * (`AgentContextMemoryService.append`), never inside `apply`. + * + * The live write path emits the 1.4 Ops (`append_message` / `clear` / + * `apply_compaction` / `undo`); assistant and tool messages are persisted already + * folded (the loop appends whole messages, not raw loop events), so on-disk + * records use the 1.4 type names without reintroducing a stateful loop-event + * fold. `context.splice` (the pre-1.4 primitive) stays registered so + * sessions written at wire protocol 1.5 still replay (newer-version passthrough, + * no migration) and for the few internal single-delete mutations that have no 1.4 + * spelling. * * Blob handling uses two complementary mechanisms: * - `contextBlobSelector` (record-level): offloads oversized content parts to @@ -56,6 +60,7 @@ export interface ContextSplicePayload { readonly tokens?: number; } +/** @deprecated Legacy 1.5 record type; kept for replay of old sessions and rare internal single-deletes. */ export const contextSplice = defineOp(ContextModel, 'context.splice', { apply: (state, p: ContextSplicePayload): ContextMessage[] => { if (p.deleteCount === 0 && p.messages.length === 0) return state; @@ -73,10 +78,6 @@ export const contextAppendMessage = defineOp(ContextModel, 'context.append_messa apply: (state, p: ContextMessagePayload): ContextMessage[] => [...state, p.message], }); -export const contextAppendLoopEvent = defineOp(ContextModel, 'context.append_loop_event', { - apply: (state, p: ContextMessagePayload): ContextMessage[] => [...state, p.message], -}); - export const contextClear = defineOp(ContextModel, 'context.clear', { apply: (state): ContextMessage[] => (state.length === 0 ? state : []), }); @@ -97,32 +98,87 @@ export interface ContextUndoPayload { readonly count: number; } +export interface UndoCut { + readonly cutIndex: number; + readonly removedCount: number; + readonly stoppedAtCompaction: boolean; +} + +/** + * Locate the trailing cut for an undo of `count` real-user prompts: the oldest + * index of the Nth-from-tail real-user prompt (skipping `injection` messages and + * stopping at a `compaction_summary` boundary). `removedCount` is how many + * real-user prompts were found; `cutIndex` is where the trailing exchange begins + * (everything from there to the end is removed), or `-1` when none was found. + * Shared by the `context.undo` reducer and the live service so dispatch and + * replay produce identical state. + */ +export function computeUndoCut(state: readonly ContextMessage[], count: number): UndoCut { + let remaining = count; + let cutIndex = -1; + let removedCount = 0; + let stoppedAtCompaction = false; + for (let i = state.length - 1; i >= 0 && remaining > 0; i--) { + const message = state[i]; + if (message === undefined || message.origin?.kind === 'injection') continue; + if (message.origin?.kind === 'compaction_summary') { + stoppedAtCompaction = true; + break; + } + if (isRealUserPrompt(message)) { + remaining--; + removedCount++; + cutIndex = i; + } + } + return { cutIndex, removedCount, stoppedAtCompaction }; +} + export const contextUndo = defineOp(ContextModel, 'context.undo', { apply: (state, p: ContextUndoPayload): ContextMessage[] => { if (p.count <= 0 || state.length === 0) return state; - const drop = new Set(); - let remaining = p.count; - for (let i = state.length - 1; i >= 0 && remaining > 0; i--) { - if (state[i]!.role !== 'user') continue; - drop.add(i); - remaining--; - } - if (drop.size === 0) return state; - return state.filter((_, index) => !drop.has(index)); + const { cutIndex, removedCount } = computeUndoCut(state, p.count); + if (cutIndex < 0 || removedCount < p.count) return state; + return state.slice(0, cutIndex); }, }); +function isRealUserPrompt(message: ContextMessage): boolean { + if (message.role !== 'user') return false; + const origin = message.origin; + if (origin === undefined || origin.kind === 'user') return true; + return ( + (origin.kind === 'skill_activation' || origin.kind === 'plugin_command') && + origin.trigger === 'user-slash' + ); +} + export const contextBlobSelector: WireBlobSelector = (record) => { - if (record.type !== 'context.splice') return []; - const messages = record['messages']; - if (!Array.isArray(messages)) return []; - return (messages as readonly ContextMessage[]).map((message, index) => ({ - parts: message.content, - replace: (current, parts) => ({ - ...current, - messages: (current['messages'] as readonly ContextMessage[]).map((item, itemIndex) => - itemIndex === index ? { ...item, content: [...parts] } : item, - ), - }), - })); + if (record.type === 'context.splice') { + const messages = record['messages']; + if (!Array.isArray(messages)) return []; + return (messages as readonly ContextMessage[]).map((message, index) => ({ + parts: message.content, + replace: (current, parts) => ({ + ...current, + messages: (current['messages'] as readonly ContextMessage[]).map((item, itemIndex) => + itemIndex === index ? { ...item, content: [...parts] } : item, + ), + }), + })); + } + if (record.type === 'context.append_message') { + const message = record['message'] as ContextMessage | undefined; + if (message === undefined) return []; + return [ + { + parts: message.content, + replace: (current, parts) => ({ + ...current, + message: { ...(current['message'] as ContextMessage), content: [...parts] }, + }), + }, + ]; + } + return []; }; diff --git a/packages/agent-core-v2/src/agent/prompt/promptService.ts b/packages/agent-core-v2/src/agent/prompt/promptService.ts index a9412debf..42d7e23b7 100644 --- a/packages/agent-core-v2/src/agent/prompt/promptService.ts +++ b/packages/agent-core-v2/src/agent/prompt/promptService.ts @@ -8,7 +8,9 @@ import { type ContextMessage, } from '#/agent/contextMemory'; import { IAgentLoopService } from '#/agent/loop'; +import { IAgentToolExecutorService } from '#/agent/toolExecutor'; import { IAgentTurnService, type Turn } from '#/agent/turn'; +import type { ExecutableToolResult, ToolDidExecuteContext } from '#/agent/tool'; import { OrderedHookSlot } from '#/hooks'; import { IAgentPromptService, @@ -35,6 +37,7 @@ export class AgentPromptService implements IAgentPromptService { @IAgentContextMemoryService private readonly context: IAgentContextMemoryService, @IAgentTurnService private readonly turnService: IAgentTurnService, @IAgentLoopService loopService: IAgentLoopService, + @IAgentToolExecutorService toolExecutor: IAgentToolExecutorService, ) { loopService.hooks.beforeStep.register('prompt-service-steer-before-step', async (_ctx, next) => { this.flushSteerQueue(); @@ -46,6 +49,10 @@ export class AgentPromptService implements IAgentPromptService { } await next(); }); + toolExecutor.hooks.onDidExecuteTool.register('prompt-service-delivery', async (ctx, next) => { + await this.deliverToolResult(ctx); + await next(); + }); } async prompt(message: ContextMessage): Promise { @@ -77,6 +84,29 @@ export class AgentPromptService implements IAgentPromptService { }; } + private async deliverToolResult(ctx: ToolDidExecuteContext): Promise { + const delivery = ctx.result.delivery; + if (delivery === undefined) return; + + // Consume the side channel: strip it from the result so it never reaches the + // loop / persistence, then perform the declared delivery here on the agent + // (L4) side where `steer` lives (the L3 executor only threads it through). + const { delivery: _consumed, ...rest } = ctx.result; + ctx.result = rest as ExecutableToolResult; + + switch (delivery.kind) { + case 'steer': + // The tool built a full user `ContextMessage`; the L3 contract carries it + // as an opaque `ToolDeliveryMessage`, so restore the type at the L4 edge. + await this.steer(delivery.message as ContextMessage).launched; + return; + default: { + const _exhaustive: never = delivery.kind; + void _exhaustive; + } + } + } + retry(trigger?: string): Turn | undefined { return this.launch(); } @@ -84,26 +114,7 @@ export class AgentPromptService implements IAgentPromptService { undo(count: number): number { if (count <= 0) return 0; - const history = this.context.get(); - let removedCount = 0; - let stoppedAtCompaction = false; - for (let index = history.length - 1; index >= 0 && removedCount < count; index--) { - const message = history[index]; - if (message === undefined || message.origin?.kind === 'injection') continue; - if (message.origin?.kind === 'compaction_summary') { - stoppedAtCompaction = true; - break; - } - - this.context.splice(index, 1, []); - if (isRealUserPrompt(message)) { - removedCount++; - } - } - - // `undo` is only ever invoked live (user / RPC); the legacy `context.undo` - // record is migrated to `context.splice` and replayed by contextMemory, so - // this method never runs during restore and needs no restoring-phase guard. + const { removedCount, stoppedAtCompaction } = this.context.undo(count); if (removedCount < count) { throw new KimiError( ErrorCodes.REQUEST_INVALID, @@ -123,14 +134,11 @@ export class AgentPromptService implements IAgentPromptService { clear(): void { this.discardQueuedSteers(); - const historyLength = this.context.get().length; - if (historyLength > 0) { - this.context.splice(0, historyLength, []); - } + this.context.clear(); } private append(...messages: ContextMessage[]): void { - this.context.splice(this.context.get().length, 0, messages); + this.context.append(...messages); } private launch(): Turn { @@ -208,16 +216,6 @@ function steerAlreadyEmittedError(): KimiError { ); } -function isRealUserPrompt(message: ContextMessage): boolean { - if (message.role !== 'user') return false; - const origin = message.origin; - if (origin === undefined || origin.kind === 'user') return true; - return ( - (origin.kind === 'skill_activation' || origin.kind === 'plugin_command') && - origin.trigger === 'user-slash' - ); -} - function formatUndoUnavailableMessage( requestedCount: number, undoableCount: number, diff --git a/packages/agent-core-v2/src/agent/skill/skill.ts b/packages/agent-core-v2/src/agent/skill/skill.ts index fc7fb2cee..334a6db22 100644 --- a/packages/agent-core-v2/src/agent/skill/skill.ts +++ b/packages/agent-core-v2/src/agent/skill/skill.ts @@ -13,9 +13,10 @@ export interface IAgentSkillService { activate(input: SkillActivationInput): Promise; /** * Records a model-tool skill activation (an inline skill loaded through the - * `Skill` tool) without opening a new turn — the tool builds and steers its - * own message into the current turn. Publishes the activation and emits - * telemetry, matching the user-slash `activate` path's side effects. + * `Skill` tool) without opening a new turn — the tool returns a + * `delivery: 'steer'` for the executor to inject into the current turn. + * Publishes the activation and emits telemetry, matching the user-slash + * `activate` path's side effects. */ recordModelToolActivation(origin: SkillActivationOrigin): void; } diff --git a/packages/agent-core-v2/src/agent/skill/tools/skill.ts b/packages/agent-core-v2/src/agent/skill/tools/skill.ts index ce680ca20..f0e553473 100644 --- a/packages/agent-core-v2/src/agent/skill/tools/skill.ts +++ b/packages/agent-core-v2/src/agent/skill/tools/skill.ts @@ -8,10 +8,13 @@ * * The model-facing wrapping lives here on purpose: resolving the skill from * the catalog, the inline-only / `disableModelInvocation` gates, the `isError` - * tool result, and the `prompt.steer` delivery into the *current* turn all + * tool result, and the declared `delivery: 'steer'` into the *current* turn all * assume the caller is already inside a turn — which is exactly the edge a - * tool runs at. `IAgentSkillService` keeps only the user-slash `activate` - * primitive (it opens a fresh turn) and the shared activation recording. + * tool runs at. The tool only declares the `delivery`; the agent (L4) layer + * performs the actual steer, so the tool never reaches into + * `IAgentPromptService`. `IAgentSkillService` keeps only the user-slash + * `activate` primitive (it opens a fresh turn) and the shared activation + * recording. * * Anti-loop: `MAX_SKILL_QUERY_DEPTH` caps Skill→Skill recursion so a * skill that re-invokes itself (or chains into another) cannot recurse @@ -22,12 +25,11 @@ import { randomUUID } from 'node:crypto'; import { z } from 'zod'; -import type { ContextMessage, SkillActivationOrigin } from '#/agent/contextMemory'; -import { IAgentPromptService } from '#/agent/prompt'; +import type { SkillActivationOrigin } from '#/agent/contextMemory'; import { IAgentSkillService } from '#/agent/skill/skill'; import { renderModelToolSkillPrompt } from '#/agent/skill/prompt'; import type { BuiltinTool } from '#/agent/tool'; -import type { ExecutableToolResult, ToolExecution } from '#/agent/tool'; +import type { ExecutableToolResult, ToolDeliveryMessage, ToolExecution } from '#/agent/tool'; import { registerTool } from '#/agent/toolRegistry'; import { isInlineSkillType } from '#/app/skillCatalog/types'; import { ISessionSkillCatalog } from '#/session/sessionSkillCatalog'; @@ -80,7 +82,6 @@ export class SkillTool implements BuiltinTool { constructor( @ISessionSkillCatalog private readonly catalog: ISessionSkillCatalog, - @IAgentPromptService private readonly prompt: IAgentPromptService, @IAgentSkillService private readonly skill: IAgentSkillService, @ISessionContext private readonly sessionContext: ISessionContext, ) {} @@ -96,7 +97,7 @@ export class SkillTool implements BuiltinTool { } withInitialQueryDepth(initialQueryDepth: number): SkillTool { - const clone = new SkillTool(this.catalog, this.prompt, this.skill, this.sessionContext); + const clone = new SkillTool(this.catalog, this.skill, this.sessionContext); clone.queryDepth = initialQueryDepth; return clone; } @@ -104,7 +105,6 @@ export class SkillTool implements BuiltinTool { private async execution(args: SkillToolInput): Promise { return executeModelSkill( this.catalog, - this.prompt, this.skill, args, this.queryDepth, @@ -117,7 +117,6 @@ registerTool(SkillTool); export async function executeModelSkill( catalog: ISessionSkillCatalog, - prompt: IAgentPromptService, skillService: IAgentSkillService, args: SkillToolInput, queryDepth: number, @@ -164,7 +163,7 @@ export async function executeModelSkill( skillSource: skill.source, }; const skillContent = catalog.catalog.renderSkillPrompt(skill, skillArgs, { sessionId }); - const message: ContextMessage = { + const message: ToolDeliveryMessage = { role: 'user', content: [ { @@ -183,9 +182,9 @@ export async function executeModelSkill( origin, }; skillService.recordModelToolActivation(origin); - await prompt.steer(message).launched; return { output: `Skill "${skill.name}" loaded inline. Follow its instructions.`, + delivery: { kind: 'steer', message }, }; } diff --git a/packages/agent-core-v2/test/_base/event.test.ts b/packages/agent-core-v2/test/_base/event.test.ts index 7443d6ee9..08d175c25 100644 --- a/packages/agent-core-v2/test/_base/event.test.ts +++ b/packages/agent-core-v2/test/_base/event.test.ts @@ -48,7 +48,7 @@ describe('Emitter / Event', () => { emitter.dispose(); }); - it('thisArg binds the listener correctly', () => { + it('binds thisArg so the listener sees the supplied context', () => { const emitter = new Emitter(); const context = { tag: 'ctx', got: [] as string[] }; diff --git a/packages/agent-core-v2/test/contextMemory/splice-replay.test.ts b/packages/agent-core-v2/test/contextMemory/splice-replay.test.ts index 4ff9af03a..9e02b41f4 100644 --- a/packages/agent-core-v2/test/contextMemory/splice-replay.test.ts +++ b/packages/agent-core-v2/test/contextMemory/splice-replay.test.ts @@ -18,7 +18,6 @@ import { AgentContextMemoryService, contextBlobSelector, ContextModel, - contextAppendLoopEvent, contextAppendMessage, contextApplyCompaction, contextClear, @@ -214,11 +213,6 @@ describe('AgentContextMemoryService (wire-backed)', () => { expect(model()).not.toBe(prev); expect(model()).toHaveLength(0); - prev = model(); - host.wire.dispatch(contextAppendLoopEvent({ message: userMessage('d') })); - expect(model()).not.toBe(prev); - expect(model()).toHaveLength(1); - await host.wire.flush(); const records = await readRecords(host.log); expect(records.every((record) => 'payload' in record === false)).toBe(true); @@ -228,7 +222,6 @@ describe('AgentContextMemoryService (wire-backed)', () => { 'context.undo', 'context.apply_compaction', 'context.clear', - 'context.append_loop_event', ]); }); diff --git a/packages/agent-core-v2/test/contextMemory/stubs.ts b/packages/agent-core-v2/test/contextMemory/stubs.ts index 3ae21e9f2..51eb695c4 100644 --- a/packages/agent-core-v2/test/contextMemory/stubs.ts +++ b/packages/agent-core-v2/test/contextMemory/stubs.ts @@ -11,7 +11,7 @@ import { toDisposable } from '#/_base/di'; import type { ServiceRegistration } from '#/_base/di/test'; import { createHooks } from '#/hooks'; import type { Hooks } from '#/hooks'; -import { ensureMessageId, IAgentContextMemoryService, type ContextMessage } from '#/agent/contextMemory'; +import { computeUndoCut, ensureMessageId, IAgentContextMemoryService, type ContextMessage } from '#/agent/contextMemory'; import { IAgentWireRecordService } from '#/agent/wireRecord'; /** @@ -64,6 +64,32 @@ export function stubContextMemory(): StubContextMemory { return messages; }, get: () => [...messages], + append: (...inserted) => { + const stamped = inserted.map(ensureMessageId); + const start = messages.length; + messages.push(...stamped); + void hooks.onSpliced.run({ start, deleteCount: 0, messages: [...stamped] }); + }, + clear: () => { + const deleteCount = messages.length; + if (deleteCount === 0) return; + messages.splice(0, deleteCount); + void hooks.onSpliced.run({ start: 0, deleteCount, messages: [] }); + }, + undo: (count) => { + const cut = computeUndoCut(messages, count); + if (cut.cutIndex >= 0 && cut.removedCount >= count) { + const deleteCount = messages.length - cut.cutIndex; + messages.splice(cut.cutIndex, deleteCount); + void hooks.onSpliced.run({ start: cut.cutIndex, deleteCount, messages: [] }); + } + return cut; + }, + applyCompaction: ({ count, summary, tokens }) => { + const stamped = ensureMessageId(summary); + messages.splice(0, count, stamped); + void hooks.onSpliced.run({ start: 0, deleteCount: count, messages: [stamped], tokens }); + }, splice: (start, deleteCount, inserted, tokens) => { const stamped = inserted.map(ensureMessageId); messages.splice(start, deleteCount, ...stamped); diff --git a/packages/agent-core-v2/test/externalHooks/integration.test.ts b/packages/agent-core-v2/test/externalHooks/integration.test.ts index 6522a15c9..b8baab699 100644 --- a/packages/agent-core-v2/test/externalHooks/integration.test.ts +++ b/packages/agent-core-v2/test/externalHooks/integration.test.ts @@ -8,7 +8,7 @@ import { } from '#/_base/di/test'; import { Event } from '#/_base/event'; import { emptyUsage } from '#/app/llmProtocol'; -import { IAgentContextMemoryService, type ContextMessage } from '#/agent/contextMemory'; +import { computeUndoCut, ensureMessageId, IAgentContextMemoryService, type ContextMessage } from '#/agent/contextMemory'; import { IAgentTaskService } from '#/agent/task'; import { AgentExternalHooksService, @@ -77,6 +77,22 @@ function stubContextMemory(): IAgentContextMemoryService & { return { _serviceBrand: undefined, get: () => [...messages], + append: (...inserted) => { + messages.push(...inserted.map(ensureMessageId)); + }, + clear: () => { + messages.splice(0, messages.length); + }, + undo: (count) => { + const cut = computeUndoCut(messages, count); + if (cut.cutIndex >= 0 && cut.removedCount >= count) { + messages.splice(cut.cutIndex, messages.length - cut.cutIndex); + } + return cut; + }, + applyCompaction: ({ count, summary }) => { + messages.splice(0, count, ensureMessageId(summary)); + }, splice: (start, deleteCount, inserted) => { messages.splice(start, deleteCount, ...inserted); }, diff --git a/packages/agent-core-v2/test/permissionPolicy/exit-plan-mode-review-ask-telemetry.test.ts b/packages/agent-core-v2/test/permissionPolicy/exit-plan-mode-review-ask-telemetry.test.ts index 9062e22df..794b414b9 100644 --- a/packages/agent-core-v2/test/permissionPolicy/exit-plan-mode-review-ask-telemetry.test.ts +++ b/packages/agent-core-v2/test/permissionPolicy/exit-plan-mode-review-ask-telemetry.test.ts @@ -166,7 +166,7 @@ describe('ExitPlanModeReviewAskPermissionPolicyService telemetry', () => { }); }); - it('handles revision requests with feedback through plan resolution telemetry', async () => { + it('records a revise outcome with feedback and keeps plan mode active when the user requests changes', async () => { const exitPlanMode = vi.fn(); const result = await makePolicy(exitPlanMode).evaluate(policyContext(planReviewDisplay())); if (result?.kind !== 'ask') throw new Error('expected ask'); @@ -194,7 +194,7 @@ describe('ExitPlanModeReviewAskPermissionPolicyService telemetry', () => { }); }); - it('handles plain rejections without exiting plan mode', async () => { + it('keeps plan mode active and records a rejected outcome when the user rejects the plan', async () => { const exitPlanMode = vi.fn(); const result = await makePolicy(exitPlanMode).evaluate(policyContext(planReviewDisplay())); if (result?.kind !== 'ask') throw new Error('expected ask'); @@ -215,7 +215,7 @@ describe('ExitPlanModeReviewAskPermissionPolicyService telemetry', () => { }); }); - it('handles dismissed approval dialogs without exiting plan mode', async () => { + it('keeps plan mode active and records a dismissed outcome when the approval dialog is cancelled', async () => { const exitPlanMode = vi.fn(); const result = await makePolicy(exitPlanMode).evaluate(policyContext(planReviewDisplay())); if (result?.kind !== 'ask') throw new Error('expected ask'); @@ -236,7 +236,7 @@ describe('ExitPlanModeReviewAskPermissionPolicyService telemetry', () => { }); }); - it('handles reject-and-exit and exits plan mode', async () => { + it('exits plan mode and records a rejected_and_exited outcome when the user chooses reject and exit', async () => { const exitPlanMode = vi.fn(); const result = await makePolicy(exitPlanMode).evaluate(policyContext(planReviewDisplay())); if (result?.kind !== 'ask') throw new Error('expected ask'); diff --git a/packages/agent-core-v2/test/prompt/promptService.test.ts b/packages/agent-core-v2/test/prompt/promptService.test.ts index 8793ac376..6d9b08806 100644 --- a/packages/agent-core-v2/test/prompt/promptService.test.ts +++ b/packages/agent-core-v2/test/prompt/promptService.test.ts @@ -6,10 +6,12 @@ import { IAgentLoopService } from '#/agent/loop'; import { AgentPromptService, IAgentPromptService } from '#/agent/prompt'; import type { PromptSubmitContext } from '#/agent/prompt'; import { IAgentContextMemoryService, type ContextMessage } from '#/agent/contextMemory'; +import type { ToolDidExecuteContext } from '#/agent/tool'; +import { IAgentToolExecutorService } from '#/agent/toolExecutor'; import { IAgentTurnService, type Turn } from '#/agent/turn'; import { stubContextMemory } from '../contextMemory/stubs'; -import { stubLoopWithHooks, stubTurn } from '../turn/stubs'; +import { stubLoopWithHooks, stubToolExecutor, stubTurn } from '../turn/stubs'; function userMessage(text: string, origin: ContextMessage['origin']): ContextMessage { return { @@ -27,12 +29,14 @@ function createHarness(options: { readonly hasActiveTurn?: boolean } = {}) { const context = stubContextMemory(); const loop = stubLoopWithHooks(); const turn = stubTurn({ hasActiveTurn: options.hasActiveTurn }); + const toolExecutor = stubToolExecutor(); const ix = createServices(disposables, { strict: true, additionalServices: (reg) => { reg.defineInstance(IAgentContextMemoryService, context); reg.defineInstance(IAgentTurnService, turn); reg.defineInstance(IAgentLoopService, loop); + reg.defineInstance(IAgentToolExecutorService, toolExecutor); reg.define(IAgentPromptService, AgentPromptService); }, }); @@ -41,6 +45,7 @@ function createHarness(options: { readonly hasActiveTurn?: boolean } = {}) { context, loop, prompt: ix.get(IAgentPromptService), + toolExecutor, turn, }; } @@ -151,4 +156,49 @@ describe('AgentPromptService', () => { expect(turn.launches).toEqual([]); expect(context.messages).toHaveLength(1); }); + + it('delivers a declared steer through onDidExecuteTool and strips delivery', async () => { + const { context, loop, turn, toolExecutor } = createHarness({ hasActiveTurn: true }); + const activeTurn = turn.launch(); + + const origin = { + kind: 'skill_activation', + activationId: 'a1', + skillName: 'commit', + trigger: 'model-tool', + } as const; + const didCtx: ToolDidExecuteContext = { + turnId: activeTurn.id, + signal: activeTurn.abortController.signal, + toolCall: { type: 'function', id: 'call_skill', name: 'Skill', arguments: '{}' }, + toolCalls: [], + args: {}, + result: { + output: 'ack', + delivery: { + kind: 'steer', + message: { + role: 'user', + content: [{ type: 'text', text: 'injected skill body' }], + toolCalls: [], + origin, + }, + }, + }, + }; + + await toolExecutor.hooks.onDidExecuteTool.run(didCtx); + + // The hook consumes the side channel so it never reaches the loop/persistence. + expect(didCtx.result.delivery).toBeUndefined(); + + await flushSteers(loop, activeTurn); + expect(context.messages.map((message) => message.content[0])).toMatchObject([ + { type: 'text', text: 'injected skill body' }, + ]); + expect(context.messages[0]?.origin).toMatchObject({ + kind: 'skill_activation', + skillName: 'commit', + }); + }); }); diff --git a/packages/agent-core-v2/test/skill/skill.test.ts b/packages/agent-core-v2/test/skill/skill.test.ts index f16461590..816f710c3 100644 --- a/packages/agent-core-v2/test/skill/skill.test.ts +++ b/packages/agent-core-v2/test/skill/skill.test.ts @@ -229,7 +229,6 @@ describe('SkillTool', () => { function makeTool(ix: TestInstantiationService, depth?: number): SkillTool { const tool = new SkillTool( ix.get(ISessionSkillCatalog), - ix.get(IAgentPromptService), stubSkillService(), stubSessionContext(), ); @@ -303,40 +302,42 @@ describe('SkillTool', () => { output: 'Skill "commit" loaded inline. Follow its instructions.', }); expect(result.output).not.toContain('# Commit'); - expect(prompted).toHaveLength(1); - expect(prompted[0]!.origin).toMatchObject({ + // The tool only declares a `delivery`; the agent (L4) layer performs the steer. + expect(prompted).toHaveLength(0); + expect(result.delivery?.kind).toBe('steer'); + expect(result.delivery?.message.origin).toMatchObject({ kind: 'skill_activation', skillName: 'commit', trigger: 'model-tool', }); - expect(prompted[0]!.content[0]).toMatchObject({ + expect(result.delivery?.message.content[0]).toMatchObject({ type: 'text', text: expect.stringContaining( '', ), }); - expect(prompted[0]!.content[0]).toMatchObject({ + expect(result.delivery?.message.content[0]).toMatchObject({ type: 'text', text: expect.stringContaining('ARGUMENTS: src/app.ts'), }); }); it('honors initialQueryDepth as an alias for queryDepth', async () => { - await executeTool( + const nested = await executeTool( makeTool(ix, 2), toolContext({ skill: 'commit' }), ); - await executeTool( + const root = await executeTool( makeTool(ix, 0), toolContext({ skill: 'commit' }), ); - expect(prompted).toHaveLength(2); - expect(prompted[0]!.origin).toMatchObject({ + expect(prompted).toHaveLength(0); + expect(nested.delivery?.message.origin).toMatchObject({ kind: 'skill_activation', trigger: 'nested-skill', }); - expect(prompted[1]!.origin).toMatchObject({ + expect(root.delivery?.message.origin).toMatchObject({ kind: 'skill_activation', trigger: 'model-tool', }); diff --git a/packages/agent-core-v2/test/task/manager.test.ts b/packages/agent-core-v2/test/task/manager.test.ts index 1b090e757..9c42cb5f8 100644 --- a/packages/agent-core-v2/test/task/manager.test.ts +++ b/packages/agent-core-v2/test/task/manager.test.ts @@ -565,7 +565,7 @@ describe('AgentTaskService', () => { }); }); - it('handles process stream errors before process wait settles', async () => { + it('fails the process task once wait settles after an earlier stream error', async () => { const { manager } = createAgentTaskService(); const { proc, failStdout, resolveWait } = processWithStdoutErrorBeforeWait(); const taskId = registerProcess( diff --git a/packages/agent-core-v2/test/task/task.test.ts b/packages/agent-core-v2/test/task/task.test.ts index 9c1999a2a..271abadc1 100644 --- a/packages/agent-core-v2/test/task/task.test.ts +++ b/packages/agent-core-v2/test/task/task.test.ts @@ -39,20 +39,6 @@ describe('TaskService', () => { expect(handle.state).toBe('failed'); }); - it('delivers output through onDidOutput', async () => { - const chunks: string[] = []; - const handle = svc.run(async (_signal, output) => { - output('hello'); - output('world'); - }); - handle.onDidOutput((data) => chunks.push(data)); - // Output fires synchronously within the executor, but the executor - // runs in a microtask. Wait for settlement. - await handle.result; - // The listener was registered after run() but the output calls happen - // within the same microtask — retest with pre-registered listener. - }); - it('delivers output to pre-registered listeners', async () => { const chunks: string[] = []; const handle = svc.run(async (_signal, output) => { @@ -174,19 +160,6 @@ describe('TaskService', () => { // 'running' was already fired before listener was attached }); - it('captures full transition sequence when listener is pre-registered', async () => { - const states: TaskState[] = []; - // Create the service fresh to attach listener before run - const handle = svc.run(async () => 'ok'); - // We need to register before the microtask fires - handle.onDidChangeState((s) => states.push(s)); - await handle.result; - // 'running' fires synchronously in the constructor, so by the time - // we register the listener it has already fired. 'completed' fires - // when the promise resolves. - expect(states).toEqual(['completed']); - }); - it('resolve/reject after settlement is ignored on deferred', () => { const states: TaskState[] = []; const handle = svc.defer(); @@ -202,14 +175,14 @@ describe('TaskService', () => { // ── Four consumption patterns ───────────────────────────── describe('consumption patterns', () => { - it('sync: await handle.result', async () => { + it('resolves the value and completes when awaiting handle.result', async () => { const handle = svc.run(async () => 'value'); const result = await handle.result; expect(result).toBe('value'); expect(handle.state).toBe('completed'); }); - it('async: track by id, retrieve later', async () => { + it('resolves the value when a handle is tracked by id and awaited later', async () => { const registry = new Map(); const handle = svc.run(async () => { await new Promise((r) => setTimeout(r, 10)); @@ -223,7 +196,7 @@ describe('TaskService', () => { expect(result).toBe('async-result'); }); - it('sync→async: race against detach signal', async () => { + it('lets a detach signal win the race while the task keeps running', async () => { const detach = new Promise<'detach'>((r) => setTimeout(() => r('detach'), 5)); const handle = svc.run(async (signal) => { await new Promise((resolve) => { @@ -247,7 +220,7 @@ describe('TaskService', () => { handle.cancel(); }); - it('async wait: reattach to existing handle', async () => { + it('resolves a deferred handle settled from outside the awaiting turn', async () => { const handle = svc.defer(); // Simulate resolving from a different "turn" diff --git a/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts b/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts index a0f49a69d..52a99e05b 100644 --- a/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts +++ b/packages/agent-core-v2/test/toolExecutor/tool-executor.test.ts @@ -10,6 +10,7 @@ import { IAgentToolExecutorService, AgentToolExecutorService, parseToolCallArgum import { IAgentToolRegistryService, AgentToolRegistryService } from '#/agent/toolRegistry'; import { IAgentWireRecordService } from '#/agent/wireRecord'; import { IAgentWireService, WireService } from '#/wire'; +import { IEventBus } from '#/app/event/eventBus'; import { ITelemetryService } from '#/app/telemetry'; import { stubWireRecord } from '../contextMemory/stubs'; import { registerLogServices } from '../log/stubs'; @@ -41,16 +42,18 @@ beforeEach(() => { disposables.add(new WireService({ logScope: 'wire', logKey: 'tool-executor' })), ); reg.defineInstance(ITelemetryService, recordingTelemetry(telemetryEvents)); + reg.defineInstance(IEventBus, { + publish: (event: { type: string }) => { + if (event.type.startsWith('tool.')) { + protocolEvents.push(event as unknown as AgentEvent); + } + }, + subscribe: (..._args: unknown[]) => ({ dispose: () => {} }), + } as IEventBus); registerLogServices(reg); }, strict: true, }); - const wire = ix.get(IAgentWireService); - disposables.add( - wire.onEmission((e) => { - if (e.type === 'signal') protocolEvents.push(e.signal as unknown as AgentEvent); - }), - ); executor = ix.get(IAgentToolExecutorService); registry = ix.get(IAgentToolRegistryService); }); @@ -563,6 +566,29 @@ describe('AgentToolExecutorService', () => { }), }); }); + it('threads a declared delivery onto the yielded result for the agent layer to consume', async () => { + const message = { + role: 'user' as const, + content: [{ type: 'text' as const, text: 'injected' }], + toolCalls: [], + origin: { kind: 'skill_activation', skillName: 'commit', trigger: 'model-tool' }, + }; + const tool = new TestTool('skillish', { + result: { output: 'ack', delivery: { kind: 'steer', message } }, + }); + registry.register(tool); + + const results = await execute([toolCall('call_skillish', 'skillish', {})]); + + expect(results).toHaveLength(1); + expect(results[0]!.output).toBe('ack'); + // The executor only threads `delivery`; an L4 hook (AgentPromptService) is + // what consumes and strips it — that hook is not registered in this unit test. + expect(results[0]!.delivery).toMatchObject({ + kind: 'steer', + message: { content: [{ type: 'text', text: 'injected' }] }, + }); + }); }); describe('parseToolCallArguments', () => { diff --git a/packages/agent-core-v2/test/wireRecord/migration/utils.ts b/packages/agent-core-v2/test/wireRecord/migration/utils.ts index 9430c9360..adf1fdd8d 100644 --- a/packages/agent-core-v2/test/wireRecord/migration/utils.ts +++ b/packages/agent-core-v2/test/wireRecord/migration/utils.ts @@ -1,5 +1,4 @@ import { - applyWireMigrations, type WireMigration, type WireMigrationRecord, } from '#/agent/wireRecord/migration'; @@ -12,15 +11,6 @@ export function runMigration( return wireSnapshot(records.map((record) => migrateRecord(migration, record))); } -export function runMigrationRecords( - migration: WireMigration, - records: readonly WireMigrationRecord[], -) { - return wireSnapshot( - applyWireMigrations(records, [migration]).map((record) => updateMetadata(migration, record)), - ); -} - function migrateRecord( migration: WireMigration, record: WireMigrationRecord, diff --git a/packages/agent-core-v2/test/wireRecord/migration/v1.4.test.ts b/packages/agent-core-v2/test/wireRecord/migration/v1.4.test.ts index 72db77bd8..8668c9026 100644 --- a/packages/agent-core-v2/test/wireRecord/migration/v1.4.test.ts +++ b/packages/agent-core-v2/test/wireRecord/migration/v1.4.test.ts @@ -64,7 +64,7 @@ describe('1.3 to 1.4', () => { }, ]), ).toMatchInlineSnapshot(` - [wire] metadata { "protocol_version": "1.4", "created_at": "