diff --git a/.changeset/stale-guard-execute-recheck.md b/.changeset/stale-guard-execute-recheck.md new file mode 100644 index 0000000000..25f1da712d --- /dev/null +++ b/.changeset/stale-guard-execute-recheck.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Edit and Write now recheck the target file's on-disk state immediately before writing. diff --git a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutor.ts b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutor.ts index f4bd72b2ec..3a900a069d 100644 --- a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutor.ts +++ b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutor.ts @@ -5,6 +5,7 @@ import type { ToolResult } from '#/tool/toolContract'; import type { BeforeToolExecuteEvent, ToolDidExecuteContext, + ToolExecuteContext, WillExecuteToolEvent, } from '#/agent/toolExecutor/toolHooks'; import type { ToolCall } from '#/kosong/contract/message'; @@ -50,6 +51,7 @@ export interface IAgentToolExecutorService { readonly onWillExecuteTool: Event; readonly hooks: { + readonly onExecuteTool: OrderedHookSlot; readonly onDidExecuteTool: OrderedHookSlot; }; diff --git a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts index 45f4a74115..3c312908d8 100644 --- a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts +++ b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts @@ -15,6 +15,7 @@ import { import { parseToolCallArguments } from '#/tool/tool-args-parse'; import { PathSecurityError } from '#/tool/path-access'; import { isAbortError, isUserCancellation } from '#/_base/utils/abort'; +import { BugIndicatingError } from '#/errors'; import { IEventDispatcher } from '#/state/eventDispatcher'; import { ToolAccesses, @@ -29,7 +30,9 @@ import type { BeforeToolExecuteEvent, ResolvedToolExecutionHookContext, ToolDidExecuteContext, + ToolExecuteContext, ToolExecutionOutcome, + ToolExecutionRunResult, WillExecuteToolEvent, } from '#/agent/toolExecutor/toolHooks'; import { IAgentStateService } from '#/agent/state/agentState'; @@ -66,11 +69,6 @@ export interface ToolExecutionTask { readonly execute: (signal: AbortSignal) => Promise; } -export interface ToolExecutionRunResult { - readonly result: ToolResult; - readonly outcome: ToolExecutionOutcome; -} - interface TimedToolResult { readonly index: number; readonly result: ToolResult; @@ -115,6 +113,7 @@ export class AgentToolExecutorService implements IAgentToolExecutorService { readonly onWillExecuteTool: Event = this.willExecuteEmitter.event; readonly hooks = { + onExecuteTool: new OrderedHookSlot(), onDidExecuteTool: new OrderedHookSlot(), }; @@ -516,6 +515,34 @@ export class AgentToolExecutorService implements IAgentToolExecutorService { }; } + const ctx: ToolExecuteContext = { + turnId: options.turnId, + signal, + trace: options.trace, + toolCall: call.toolCall, + tool: call.tool, + args: call.args, + execution, + metadata, + }; + await this.hooks.onExecuteTool.run(ctx, async (c) => { + c.result = await this.executeResolvedTool(call, execution, metadata, options, signal); + }); + if (ctx.result === undefined) { + throw new BugIndicatingError( + `onExecuteTool hook chain for tool "${call.toolName}" completed without producing a result`, + ); + } + return ctx.result; + } + + private async executeResolvedTool( + call: RunnableToolCall, + execution: RunnableToolExecution, + metadata: unknown, + options: ToolExecutorExecuteOptions, + signal: AbortSignal, + ): Promise { let rawResult: ExecutableToolResult; try { const executePromise = execution.execute({ diff --git a/packages/agent-core-v2/src/agent/toolExecutor/toolHooks.ts b/packages/agent-core-v2/src/agent/toolExecutor/toolHooks.ts index c14472af0f..949fb2a4a8 100644 --- a/packages/agent-core-v2/src/agent/toolExecutor/toolHooks.ts +++ b/packages/agent-core-v2/src/agent/toolExecutor/toolHooks.ts @@ -7,6 +7,7 @@ import type { ExecutableToolResult, RunnableToolExecution, ToolAccesses, + ToolResult, } from '#/tool/toolContract'; export interface ToolExecutionHookContext { @@ -51,6 +52,23 @@ export type ToolExecutionOutcome = | 'synthetic' | 'skipped'; +export interface ToolExecutionRunResult { + readonly result: ToolResult; + readonly outcome: ToolExecutionOutcome; +} + +export interface ToolExecuteContext { + readonly turnId: number; + readonly signal: AbortSignal; + readonly trace?: LLMRequestTrace; + readonly toolCall: ToolCall; + readonly tool: ExecutableTool; + readonly args: unknown; + readonly execution: RunnableToolExecution; + readonly metadata: unknown; + result?: ToolExecutionRunResult; +} + export interface ToolDidExecuteContext extends ToolExecutionHookContext { readonly outcome: ToolExecutionOutcome; readonly accesses?: ToolAccesses; diff --git a/packages/agent-core-v2/src/features/staleGuard/staleGuardService.ts b/packages/agent-core-v2/src/features/staleGuard/staleGuardService.ts index 3946e4c93c..ecd5b3c164 100644 --- a/packages/agent-core-v2/src/features/staleGuard/staleGuardService.ts +++ b/packages/agent-core-v2/src/features/staleGuard/staleGuardService.ts @@ -5,7 +5,7 @@ import { denyToolExecution } from '#/agent/toolExecutor/beforeToolExecuteEvent'; import { IAgentToolExecutorService } from '#/agent/toolExecutor/toolExecutor'; import type { BeforeToolExecuteEvent, - ToolDidExecuteContext, + ToolExecuteContext, } from '#/agent/toolExecutor/toolHooks'; import type { ToolCall } from '#/kosong/contract/message'; import type { HostFileStat } from '#/os/interface/hostFileSystem'; @@ -56,9 +56,8 @@ export class StaleGuardService extends Disposable implements IStaleGuardService this.states.contributeState(staleGuardKey); this._register(toolExecutor.onBeforeExecuteTool((event) => this.guardWrite(event))); this._register( - toolExecutor.hooks.onDidExecuteTool.register('staleGuard', async (ctx, next) => { - await this.observeExecution(ctx); - await next(); + toolExecutor.hooks.onExecuteTool.register('staleGuard', async (ctx, next) => { + await this.executeWithGuard(ctx, next); }), ); this._register( @@ -85,18 +84,40 @@ export class StaleGuardService extends Disposable implements IStaleGuardService }); } - private async observeExecution(ctx: ToolDidExecuteContext): Promise { - if (ctx.outcome !== 'executed' || ctx.result.isError === true) return; + private async executeWithGuard( + ctx: ToolExecuteContext, + next: () => Promise, + ): Promise { const name = ctx.toolCall.name; - if (name === 'Read') { - const path = accessedFilePath(ctx.accesses, READ_OPERATIONS); - if (path !== undefined) await this.recordCurrentMtime(path); + if (name === 'Edit' || name === 'Write') { + const path = accessedFilePath(ctx.execution.accesses, WRITE_OPERATIONS); + if (path !== undefined) { + const displayPath = stringArg(ctx.args, 'path') ?? path; + const error = await this.checkWritable(path, displayPath); + if (error !== undefined) { + ctx.result = { result: denyToolExecution(error), outcome: 'vetoed' }; + return; + } + } + await next(); + await this.observeExecuted(ctx, WRITE_OPERATIONS); return; } - if (name === 'Edit' || name === 'Write') { - const path = accessedFilePath(ctx.accesses, WRITE_OPERATIONS); - if (path !== undefined) await this.recordCurrentMtime(path); + if (name === 'Read') { + await next(); + await this.observeExecuted(ctx, READ_OPERATIONS); + return; } + await next(); + } + + private async observeExecuted( + ctx: ToolExecuteContext, + operations: readonly ToolFileAccessOperation[], + ): Promise { + if (ctx.result?.outcome !== 'executed' || ctx.result.result.isError === true) return; + const path = accessedFilePath(ctx.execution.accesses, operations); + if (path !== undefined) await this.recordCurrentMtime(path); } private async checkWritable(path: string, displayPath: string): Promise { diff --git a/packages/agent-core-v2/test/agent/loop/stubs.ts b/packages/agent-core-v2/test/agent/loop/stubs.ts index 12dc196d6a..379aeac281 100644 --- a/packages/agent-core-v2/test/agent/loop/stubs.ts +++ b/packages/agent-core-v2/test/agent/loop/stubs.ts @@ -4,7 +4,7 @@ import type { IAgentLoopService, LoopErrorHandler, LoopErrorHandlerRegistrationO import type { StepRequest } from '#/agent/loop/stepRequest'; import { StepRequestQueue, type StepRequestBatch } from '#/agent/loop/stepRequestQueue'; import type { IAgentToolExecutorService } from '#/agent/toolExecutor/toolExecutor'; -import type { BeforeToolExecuteEvent, ToolDidExecuteContext, WillExecuteToolEvent } from '#/agent/toolExecutor/toolHooks'; +import type { BeforeToolExecuteEvent, ToolDidExecuteContext, ToolExecuteContext, WillExecuteToolEvent } from '#/agent/toolExecutor/toolHooks'; import { OrderedHookSlot } from '#/hooks'; import type { ContextMessage } from '#/agent/contextMemory/types'; import { createHooks } from '#/hooks'; @@ -92,4 +92,4 @@ export async function runWillBeginStepHooks( }); } export function stubWire(): IWireService { return { _serviceBrand: undefined, seal: async () => {}, appendRecord: () => {}, readJournal: async function* () {}, flush: async () => {} }; } -export function stubToolExecutor(): IAgentToolExecutorService { return { _serviceBrand: undefined, execute: async function* () {}, onBeforeExecuteTool: Event.None as Event, onWillExecuteTool: Event.None as Event, hooks: { onDidExecuteTool: new OrderedHookSlot() }, recordDupType: () => {}, registerToolCallGuard: () => ({ dispose() {} }), registerUnavailableToolDescriber: () => ({ dispose() {} }), registerMissingToolDescriber: () => ({ dispose() {} }) }; } +export function stubToolExecutor(): IAgentToolExecutorService { return { _serviceBrand: undefined, execute: async function* () {}, onBeforeExecuteTool: Event.None as Event, onWillExecuteTool: Event.None as Event, hooks: { onExecuteTool: new OrderedHookSlot(), onDidExecuteTool: new OrderedHookSlot() }, recordDupType: () => {}, registerToolCallGuard: () => ({ dispose() {} }), registerUnavailableToolDescriber: () => ({ dispose() {} }), registerMissingToolDescriber: () => ({ dispose() {} }) }; } diff --git a/packages/agent-core-v2/test/agent/toolExecutor/stubs.ts b/packages/agent-core-v2/test/agent/toolExecutor/stubs.ts index a721ee9439..53d7c6a01f 100644 --- a/packages/agent-core-v2/test/agent/toolExecutor/stubs.ts +++ b/packages/agent-core-v2/test/agent/toolExecutor/stubs.ts @@ -5,6 +5,7 @@ import type { BeforeExecuteDecision, ResolvedToolExecutionHookContext, ToolDidExecuteContext, + ToolExecuteContext, WillExecuteToolEvent, } from '#/agent/toolExecutor/toolHooks'; import { OrderedHookSlot } from '#/hooks'; @@ -25,12 +26,13 @@ export function stubToolExecutorEvents(): ToolExecutorEventStubs { const beforeEmitter = new BeforeToolExecuteEmitter(); const willEmitter = new AsyncEmitter(); const didExecuteSlot = new OrderedHookSlot(); + const executeSlot = new OrderedHookSlot(); const executor: IAgentToolExecutorService = { _serviceBrand: undefined, execute: async function* () {}, onBeforeExecuteTool: beforeEmitter.event, onWillExecuteTool: willEmitter.event, - hooks: { onDidExecuteTool: didExecuteSlot }, + hooks: { onExecuteTool: executeSlot, onDidExecuteTool: didExecuteSlot }, recordDupType: () => {}, registerToolCallGuard: () => ({ dispose() {} }), registerUnavailableToolDescriber: () => ({ dispose() {} }), diff --git a/packages/agent-core-v2/test/agent/toolExecutor/toolExecutor.test.ts b/packages/agent-core-v2/test/agent/toolExecutor/toolExecutor.test.ts index e1a448b18a..4f92505731 100644 --- a/packages/agent-core-v2/test/agent/toolExecutor/toolExecutor.test.ts +++ b/packages/agent-core-v2/test/agent/toolExecutor/toolExecutor.test.ts @@ -789,6 +789,71 @@ describe('AgentToolExecutorService', () => { }), }); }); + + it('runs onExecuteTool middleware inside the scheduled task around the execution', async () => { + const first = new ControlledTool('first', ToolAccesses.writeFile('/repo/a.ts')); + const second = new TestTool('second', { accesses: ToolAccesses.writeFile('/repo/a.ts') }); + registry.register(first); + registry.register(second); + const order: string[] = []; + executor.hooks.onExecuteTool.register('observe', async (ctx, next) => { + order.push(`before:${ctx.toolCall.id}`); + await next(); + order.push(`after:${ctx.toolCall.id}`); + }); + + const execution = execute([ + toolCall('call_first', 'first', {}), + toolCall('call_second', 'second', {}), + ]); + await first.started; + expect(order).toEqual(['before:call_first']); + const results = await execution; + + expect(order).toEqual([ + 'before:call_first', + 'after:call_first', + 'before:call_second', + 'after:call_second', + ]); + expect(results).toHaveLength(2); + }); + + it('an onExecuteTool middleware can veto the call without running the tool', async () => { + const tool = new TestTool('echo'); + registry.register(tool); + const outcomes = new Map(); + executor.hooks.onDidExecuteTool.register('capture-outcomes', async (ctx, next) => { + outcomes.set(ctx.toolCall.id, ctx.outcome); + await next(); + }); + executor.hooks.onExecuteTool.register('veto', (ctx) => { + ctx.result = { + result: { output: 'vetoed at run time', isError: true }, + outcome: 'vetoed', + }; + }); + + const results = await execute([toolCall('call_echo', 'echo', { text: 'hi' })]); + + expect(tool.calls).toEqual([]); + expect(results).toEqual([ + expect.objectContaining({ output: 'vetoed at run time', isError: true }), + ]); + expect(outcomes).toEqual(new Map([['call_echo', 'vetoed']])); + }); + + it('rejects the batch when the onExecuteTool chain completes without a result', async () => { + const tool = new TestTool('echo'); + registry.register(tool); + executor.hooks.onExecuteTool.register('swallow', async () => {}); + + await expect(execute([toolCall('call_echo', 'echo', { text: 'hi' })])).rejects.toThrow( + 'onExecuteTool hook chain for tool "echo" completed without producing a result', + ); + expect(tool.calls).toEqual([]); + }); + it('threads a declared delivery onto the yielded result for the agent layer to consume', async () => { const message = { role: 'user' as const, diff --git a/packages/agent-core-v2/test/features/staleGuard/staleGuard.test.ts b/packages/agent-core-v2/test/features/staleGuard/staleGuard.test.ts index db060de533..4d1ab1f142 100644 --- a/packages/agent-core-v2/test/features/staleGuard/staleGuard.test.ts +++ b/packages/agent-core-v2/test/features/staleGuard/staleGuard.test.ts @@ -15,10 +15,11 @@ import { IAgentToolExecutorService } from '#/agent/toolExecutor/toolExecutor'; import type { BeforeExecuteDecision, BeforeToolExecuteEvent, - ToolDidExecuteContext, + ToolExecuteContext, } from '#/agent/toolExecutor/toolHooks'; import { IEventBus } from '#/app/event/eventBus'; import { EventBusService } from '#/app/event/eventBusService'; +import { OrderedHookSlot } from '#/hooks'; import type { ToolCall } from '#/kosong/contract/message'; import { IStaleGuardService } from '#/features/staleGuard/staleGuard'; import { StaleGuardService } from '#/features/staleGuard/staleGuardService'; @@ -42,7 +43,7 @@ const noopBlob: IAgentBlobService = { interface CapturedHooks { readonly before: ((event: BeforeToolExecuteEvent) => unknown)[]; - readonly did: ((ctx: ToolDidExecuteContext, next: () => Promise) => Promise)[]; + readonly executeSlot: OrderedHookSlot; } function stubToolExecutor(captured: CapturedHooks): IAgentToolExecutorService { @@ -53,15 +54,7 @@ function stubToolExecutor(captured: CapturedHooks): IAgentToolExecutorService { return toDisposable(() => {}); }, hooks: { - onDidExecuteTool: { - register: ( - _name: string, - handler: (ctx: ToolDidExecuteContext, next: () => Promise) => Promise, - ) => { - captured.did.push(handler); - return toDisposable(() => {}); - }, - }, + onExecuteTool: captured.executeSlot, }, } as unknown as IAgentToolExecutorService; } @@ -140,21 +133,38 @@ async function runBeforeExecute( return veto; } -async function runDidExecute( +async function runExecute( captured: CapturedHooks, - input: { name: string; accesses?: ToolAccesses; isError?: boolean }, -): Promise { + input: { + name: string; + args?: unknown; + accesses?: ToolAccesses; + isError?: boolean; + during?: () => void | Promise; + }, +): Promise { + const toolCall = { + type: 'function', + id: 'call_1', + name: input.name, + arguments: JSON.stringify(input.args ?? {}), + } as ToolCall; const ctx = { turnId: 0, signal: new AbortController().signal, - toolCall: { id: 'call_1', name: input.name }, - toolCalls: [], - args: {}, - outcome: 'executed', - accesses: input.accesses, - result: input.isError === true ? { output: 'failed', isError: true } : { output: 'ok' }, - } as unknown as ToolDidExecuteContext; - for (const handler of captured.did) await handler(ctx, async () => {}); + toolCall, + args: input.args ?? {}, + execution: { accesses: input.accesses }, + metadata: undefined, + } as unknown as ToolExecuteContext; + await captured.executeSlot.run(ctx, async (c) => { + await input.during?.(); + c.result = { + result: input.isError === true ? { output: 'failed', isError: true } : { output: 'ok' }, + outcome: 'executed', + }; + }); + return ctx; } describe('StaleGuardService', () => { @@ -168,7 +178,7 @@ describe('StaleGuardService', () => { dispatcher: IEventDispatcher; hooks: CapturedHooks; } { - const captured: CapturedHooks = { before: [], did: [] }; + const captured: CapturedHooks = { before: [], executeSlot: new OrderedHookSlot() }; const ix = disposables.add(new TestInstantiationService()); ix.set(IEventBus, new SyncDescriptor(EventBusService)); ix.set(IAgentBlobService, noopBlob); @@ -201,7 +211,7 @@ describe('StaleGuardService', () => { it('records the mtime of a successfully read file into state and the wire journal', async () => { activeFs = stubFs({ mtimeMs: 111 }); - await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); expect(freshness.recordedMtimeMs('/tmp/a.txt')).toBe(111); expect(records).toEqual([ @@ -212,7 +222,7 @@ describe('StaleGuardService', () => { it('does not record when the read failed', async () => { activeFs = stubFs({ mtimeMs: 111 }); - await runDidExecute(hooks, { + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt'), isError: true, @@ -227,7 +237,8 @@ describe('StaleGuardService', () => { expect(veto).toBeUndefined(); activeFs = stubFs({ mtimeMs: 111 }); - await runDidExecute(hooks, { name: 'Bash' }); + const ctx = await runExecute(hooks, { name: 'Bash' }); + expect(ctx.result?.outcome).toBe('executed'); expect(records).toEqual([]); }); @@ -243,24 +254,38 @@ describe('StaleGuardService', () => { expect(veto?.isError).toBe(true); expect(outputText(veto)).toContain('has not been read'); expect(outputText(veto)).toContain('/tmp/a.txt'); + + const ctx = await runExecute(hooks, { + name: 'Edit', + args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, + accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), + }); + expect(ctx.result?.outcome).toBe('vetoed'); + expect(outputText(ctx.result?.result)).toContain('has not been read'); }); it('allows the write when the on-disk mtime matches the last read', async () => { activeFs = stubFs({ mtimeMs: 111 }); - await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); const veto = await runBeforeExecute(hooks, { name: 'Write', args: { path: '/tmp/a.txt', content: 'x' }, accesses: ToolAccesses.writeFile('/tmp/a.txt'), }); - expect(veto).toBeUndefined(); + + const ctx = await runExecute(hooks, { + name: 'Write', + args: { path: '/tmp/a.txt', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/a.txt'), + }); + expect(ctx.result?.outcome).toBe('executed'); }); it('vetoes the write when the file changed on disk since the last read', async () => { activeFs = stubFs({ mtimeMs: 111 }); - await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); activeFs = stubFs({ mtimeMs: 222 }); const veto = await runBeforeExecute(hooks, { @@ -273,6 +298,68 @@ describe('StaleGuardService', () => { expect(outputText(veto)).toContain('modified on disk'); }); + it('vetoes at execution time when the file changes after the prepare-time check passed', async () => { + activeFs = stubFs({ mtimeMs: 111 }); + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); + + const veto = await runBeforeExecute(hooks, { + name: 'Edit', + args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, + accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), + }); + expect(veto).toBeUndefined(); + + activeFs = stubFs({ mtimeMs: 222 }); + const ctx = await runExecute(hooks, { + name: 'Edit', + args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, + accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), + }); + + expect(ctx.result?.outcome).toBe('vetoed'); + expect(outputText(ctx.result?.result)).toContain('modified on disk'); + expect(freshness.recordedMtimeMs('/tmp/a.txt')).toBe(111); + expect( + records.filter((record) => record.type === 'staleGuard.recorded'), + ).toHaveLength(1); + }); + + it('passes the execution-time recheck for a same-batch Read followed by a Write', async () => { + activeFs = stubFs({ mtimeMs: 5 }); + await runExecute(hooks, { + name: 'Read', + args: { path: '/tmp/a.txt' }, + accesses: ToolAccesses.readFile('/tmp/a.txt'), + }); + + const ctx = await runExecute(hooks, { + name: 'Write', + args: { path: '/tmp/a.txt', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/a.txt'), + }); + + expect(ctx.result?.outcome).toBe('executed'); + }); + + it('vetoes at execution time when the earlier same-batch Read failed', async () => { + activeFs = stubFs({ mtimeMs: 5 }); + await runExecute(hooks, { + name: 'Read', + args: { path: '/tmp/a.txt' }, + accesses: ToolAccesses.readFile('/tmp/a.txt'), + isError: true, + }); + + const ctx = await runExecute(hooks, { + name: 'Write', + args: { path: '/tmp/a.txt', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/a.txt'), + }); + + expect(ctx.result?.outcome).toBe('vetoed'); + expect(outputText(ctx.result?.result)).toContain('has not been read'); + }); + it('allows a write covered by an earlier Read of the same path in the same batch', async () => { activeFs = stubFs({ mtimeMs: 5 }); const readCall: ToolCall = { @@ -326,7 +413,7 @@ describe('StaleGuardService', () => { it('clears recorded mtimes when the runtime changes', async () => { activeFs = stubFs({ mtimeMs: 111 }); - await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); expect(freshness.recordedMtimeMs('/tmp/a.txt')).toBe(111); fireRuntimeChange(); @@ -336,12 +423,13 @@ describe('StaleGuardService', () => { type: 'staleGuard.cleared', time: expect.any(Number), }); - const veto = await runBeforeExecute(hooks, { + const ctx = await runExecute(hooks, { name: 'Edit', args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), }); - expect(outputText(veto)).toContain('has not been read'); + expect(ctx.result?.outcome).toBe('vetoed'); + expect(outputText(ctx.result?.result)).toContain('has not been read'); }); it('allows writing a file that does not exist yet', async () => { @@ -352,8 +440,14 @@ describe('StaleGuardService', () => { args: { path: '/tmp/new.txt', content: 'x' }, accesses: ToolAccesses.writeFile('/tmp/new.txt'), }); - expect(veto).toBeUndefined(); + + const ctx = await runExecute(hooks, { + name: 'Write', + args: { path: '/tmp/new.txt', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/new.txt'), + }); + expect(ctx.result?.outcome).toBe('executed'); }); it('skips the check when the runtime stat carries no mtimeMs', async () => { @@ -364,8 +458,14 @@ describe('StaleGuardService', () => { args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), }); - expect(veto).toBeUndefined(); + + const ctx = await runExecute(hooks, { + name: 'Edit', + args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, + accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), + }); + expect(ctx.result?.outcome).toBe('executed'); }); it('skips the check when the path is not a regular file', async () => { @@ -376,56 +476,71 @@ describe('StaleGuardService', () => { args: { path: '/tmp/dir', content: 'x' }, accesses: ToolAccesses.writeFile('/tmp/dir'), }); - expect(veto).toBeUndefined(); + + const ctx = await runExecute(hooks, { + name: 'Write', + args: { path: '/tmp/dir', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/dir'), + }); + expect(ctx.result?.outcome).toBe('executed'); }); it('refreshes the record after a successful write so consecutive writes are not blocked', async () => { activeFs = stubFs({ mtimeMs: 111 }); - await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); - activeFs = stubFs({ mtimeMs: 222 }); - await runDidExecute(hooks, { name: 'Edit', accesses: ToolAccesses.readWriteFile('/tmp/a.txt') }); + const first = await runExecute(hooks, { + name: 'Edit', + args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, + accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), + during: () => { + activeFs = stubFs({ mtimeMs: 222 }); + }, + }); + expect(first.result?.outcome).toBe('executed'); expect(freshness.recordedMtimeMs('/tmp/a.txt')).toBe(222); - const veto = await runBeforeExecute(hooks, { + const second = await runExecute(hooks, { name: 'Edit', args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), }); - expect(veto).toBeUndefined(); + expect(second.result?.outcome).toBe('executed'); }); it('rebuilds recorded mtimes from the wire journal on restore', async () => { activeFs = stubFs({ mtimeMs: 111 }); - await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); const replayed = buildStack([...records]); await replayed.dispatcher.restore(); expect(replayed.freshness.recordedMtimeMs('/tmp/a.txt')).toBe(111); activeFs = stubFs({ mtimeMs: 999 }); - const veto = await runBeforeExecute(replayed.hooks, { + const ctx = await runExecute(replayed.hooks, { name: 'Edit', args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), }); - expect(outputText(veto)).toContain('modified on disk'); + expect(ctx.result?.outcome).toBe('vetoed'); + expect(outputText(ctx.result?.result)).toContain('modified on disk'); }); it('keeps records isolated between independent agent stacks', async () => { activeFs = stubFs({ mtimeMs: 111 }); - await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); const other = buildStack([]); expect(other.freshness.recordedMtimeMs('/tmp/a.txt')).toBeUndefined(); - const veto = await runBeforeExecute(other.hooks, { + const ctx = await runExecute(other.hooks, { name: 'Edit', args: { path: '/tmp/a.txt', old_string: 'a', new_string: 'b' }, accesses: ToolAccesses.readWriteFile('/tmp/a.txt'), }); - expect(outputText(veto)).toContain('has not been read'); + expect(ctx.result?.outcome).toBe('vetoed'); + expect(outputText(ctx.result?.result)).toContain('has not been read'); }); it('detects an external mtime change through the real filesystem', async () => { @@ -434,25 +549,26 @@ describe('StaleGuardService', () => { await writeFile(file, 'one', 'utf8'); try { activeFs = new HostFileSystem(); - await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile(file) }); + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile(file) }); const past = new Date(Date.now() - 60_000); await utimes(file, past, past); - const veto = await runBeforeExecute(hooks, { + const ctx = await runExecute(hooks, { name: 'Edit', args: { path: file, old_string: 'one', new_string: 'two' }, accesses: ToolAccesses.readWriteFile(file), }); - expect(outputText(veto)).toContain('modified on disk'); + expect(ctx.result?.outcome).toBe('vetoed'); + expect(outputText(ctx.result?.result)).toContain('modified on disk'); - await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile(file) }); - const allowed = await runBeforeExecute(hooks, { + await runExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile(file) }); + const allowed = await runExecute(hooks, { name: 'Edit', args: { path: file, old_string: 'one', new_string: 'two' }, accesses: ToolAccesses.readWriteFile(file), }); - expect(allowed).toBeUndefined(); + expect(allowed.result?.outcome).toBe('executed'); } finally { await rm(dir, { recursive: true, force: true }); }