From 67fbcdf1ba7dceeebb58875b3b7c81b4b30cf0de Mon Sep 17 00:00:00 2001 From: Haozhe Date: Wed, 19 Aug 2026 22:55:55 +0800 Subject: [PATCH] feat(agent-core-v2): guard Edit and Write against stale or unread files (#3096) * feat(agent-core-v2): guard Edit and Write against stale or unread files * feat(agent-core-v2): guard Edit and Write against stale or unread files --- .changeset/file-write-staleness-guard.md | 5 + .../agent-core-v2/docs/state-manifest.d.ts | 6 +- .../agent-core-v2/docs/wire-manifest.d.ts | 24 +- .../src/features/staleGuard/staleGuard.ts | 10 + .../features/staleGuard/staleGuardFeature.ts | 19 + .../src/features/staleGuard/staleGuardOps.ts | 41 ++ .../features/staleGuard/staleGuardService.ts | 146 +++++ packages/agent-core-v2/src/index.ts | 1 + .../features/staleGuard/staleGuard.test.ts | 532 ++++++++++++++++++ packages/agent-core-v2/test/index.test.ts | 2 + .../test/state/builtinReplayableKeys.ts | 2 + 11 files changed, 786 insertions(+), 2 deletions(-) create mode 100644 .changeset/file-write-staleness-guard.md create mode 100644 packages/agent-core-v2/src/features/staleGuard/staleGuard.ts create mode 100644 packages/agent-core-v2/src/features/staleGuard/staleGuardFeature.ts create mode 100644 packages/agent-core-v2/src/features/staleGuard/staleGuardOps.ts create mode 100644 packages/agent-core-v2/src/features/staleGuard/staleGuardService.ts create mode 100644 packages/agent-core-v2/test/features/staleGuard/staleGuard.test.ts diff --git a/.changeset/file-write-staleness-guard.md b/.changeset/file-write-staleness-guard.md new file mode 100644 index 000000000..6c0d0c3f2 --- /dev/null +++ b/.changeset/file-write-staleness-guard.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Edit and Write now require reading an existing file before modifying it, and reject the write when the file changed on disk since it was last read. diff --git a/packages/agent-core-v2/docs/state-manifest.d.ts b/packages/agent-core-v2/docs/state-manifest.d.ts index ea4fd6511..cde14a897 100644 --- a/packages/agent-core-v2/docs/state-manifest.d.ts +++ b/packages/agent-core-v2/docs/state-manifest.d.ts @@ -27,7 +27,7 @@ // references become '(circular)', and class instances collapse to a '(ClassName)' // marker — the wire shape of an entry is the JSON projection of the type here. // -// Index (App: 0 keys · Workspace: 6 keys · Session: 17 keys · Agent: 98 keys) +// Index (App: 0 keys · Workspace: 6 keys · Session: 17 keys · Agent: 99 keys) // App // Workspace // workspaceDirs.ephemeralDirs src/workspace/workspaceDirs/workspaceDirsService.ts @@ -125,6 +125,7 @@ // runtimeBinding src/agent/runtimeBinding/runtimeBindingOps.ts // shellCommand.tasks src/agent/shellCommand/shellCommandService.ts // skill src/agent/skill/skillOps.ts +// staleGuard src/features/staleGuard/staleGuardOps.ts // stepRetry.failedAttempts src/agent/stepRetry/stepRetryService.ts // stepRetry.lastFailedDriverId src/agent/stepRetry/stepRetryService.ts // swarm src/features/swarm/swarmOps.ts @@ -1574,6 +1575,9 @@ export interface AgentStateSnapshot { readonly id?: string; readonly revisionCount?: Readonly>; }; + // src/features/staleGuard/staleGuardOps.ts + // replayable · durable — folds: StaleGuardRecorded, StaleGuardCleared + 'staleGuard': /* StaleGuardModelState — packages/agent-core-v2/src/features/staleGuard/staleGuardOps.ts */ Map; // src/features/swarm/swarmOps.ts // replayable · durable — folds: SwarmModeEnter, SwarmModeExit 'swarm': 'task' | 'tool' | 'manual' | null; diff --git a/packages/agent-core-v2/docs/wire-manifest.d.ts b/packages/agent-core-v2/docs/wire-manifest.d.ts index e9616b0f7..518fe5433 100644 --- a/packages/agent-core-v2/docs/wire-manifest.d.ts +++ b/packages/agent-core-v2/docs/wire-manifest.d.ts @@ -24,7 +24,7 @@ // cross-reducers), blobs (the folding states whose blob codec offloads inline // media to blob storage), owner (the source file declaring the class). -// Index (53 record types) +// Index (55 record types) // config.update profile src/agent/profile/profileOps.ts // context.append_loop_event contextMemory, turn src/agent/contextMemory/contextEvents.ts // context.append_message contextMemory, goalForkNotice, plan, task.notificationDelivery, todo src/agent/contextMemory/contextEvents.ts @@ -57,6 +57,8 @@ // profile.bind profile, profile.activeTools src/agent/profile/profileOps.ts // prompt.accepted promptAdmission src/agent/prompt/promptOps.ts // runtime.set_binding runtimeBinding src/agent/runtimeBinding/runtimeBindingOps.ts +// staleGuard.cleared staleGuard src/features/staleGuard/staleGuardOps.ts +// staleGuard.recorded staleGuard src/features/staleGuard/staleGuardOps.ts // swarm_mode.enter swarm src/features/swarm/swarmOps.ts // swarm_mode.exit contextMemory, swarm src/features/swarm/swarmOps.ts // task.started task src/agent/task/taskOps.ts @@ -497,6 +499,24 @@ interface RuntimeSetBindingPayload { runtimeId: string; } +/** + * states: staleGuard + * owner: src/features/staleGuard/staleGuardOps.ts + */ +interface StaleGuardClearedPayload { + _name: 'staleGuard.cleared'; +} + +/** + * states: staleGuard + * owner: src/features/staleGuard/staleGuardOps.ts + */ +interface StaleGuardRecordedPayload { + _name: 'staleGuard.recorded'; + path: string; + mtimeMs: number; +} + /** * states: swarm * owner: src/features/swarm/swarmOps.ts @@ -792,6 +812,8 @@ interface WirePayloadMap { "profile.bind": ProfileBindPayload; "prompt.accepted": PromptAcceptedPayload; "runtime.set_binding": RuntimeSetBindingPayload; + "staleGuard.cleared": StaleGuardClearedPayload; + "staleGuard.recorded": StaleGuardRecordedPayload; "swarm_mode.enter": SwarmModeEnterPayload; "swarm_mode.exit": SwarmModeExitPayload; "task.started": TaskStartedPayload; diff --git a/packages/agent-core-v2/src/features/staleGuard/staleGuard.ts b/packages/agent-core-v2/src/features/staleGuard/staleGuard.ts new file mode 100644 index 000000000..83c05e900 --- /dev/null +++ b/packages/agent-core-v2/src/features/staleGuard/staleGuard.ts @@ -0,0 +1,10 @@ +import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; + +export interface IStaleGuardService { + readonly _serviceBrand: undefined; + + recordedMtimeMs(path: string): number | undefined; +} + +export const IStaleGuardService: ServiceIdentifier = + createDecorator('staleGuardService'); diff --git a/packages/agent-core-v2/src/features/staleGuard/staleGuardFeature.ts b/packages/agent-core-v2/src/features/staleGuard/staleGuardFeature.ts new file mode 100644 index 000000000..46f323e41 --- /dev/null +++ b/packages/agent-core-v2/src/features/staleGuard/staleGuardFeature.ts @@ -0,0 +1,19 @@ +import { ScopeActivation } from '#/_base/di/instantiation'; +import { Feature } from '#/features/feature'; +import { registerFeature } from '#/features/featureRegistry'; + +import { IStaleGuardService } from './staleGuard'; +import { StaleGuardService } from './staleGuardService'; + +export class StaleGuardFeature extends Feature { + static override readonly name = 'staleGuard'; + + constructor() { + super(); + this.contributeAgentService(IStaleGuardService, StaleGuardService, { + activation: ScopeActivation.OnScopeCreated, + }); + } +} + +registerFeature(StaleGuardFeature); diff --git a/packages/agent-core-v2/src/features/staleGuard/staleGuardOps.ts b/packages/agent-core-v2/src/features/staleGuard/staleGuardOps.ts new file mode 100644 index 000000000..3016deb53 --- /dev/null +++ b/packages/agent-core-v2/src/features/staleGuard/staleGuardOps.ts @@ -0,0 +1,41 @@ +/* oxlint-disable typescript-eslint/no-unsafe-declaration-merging, eslint-plugin-import/namespace -- Event2 class+payload-interface declaration merging is the sanctioned event-declaration idiom. */ +import { z } from 'zod'; + +import { Event2 } from '#/app/event/event2'; +import { defineState } from '#/state/state'; + +export type StaleGuardModelState = Map; + +const staleGuardRecordedSchema = z.object({ + path: z.string(), + mtimeMs: z.number(), +}); + +export class StaleGuardRecorded extends Event2> { + static override readonly type = 'staleGuard.recorded'; + static override readonly durable = true; + static override readonly schema = staleGuardRecordedSchema; +} +export interface StaleGuardRecorded extends z.infer {} + +const staleGuardClearedSchema = z.object({}); + +export class StaleGuardCleared extends Event2> { + static override readonly type = 'staleGuard.cleared'; + static override readonly durable = true; + static override readonly schema = staleGuardClearedSchema; +} +export interface StaleGuardCleared extends z.infer {} + +export const staleGuardKey = defineState( + 'staleGuard', + (): StaleGuardModelState => new Map(), +).replayable({ + schema: z.custom(), +}) + .on(StaleGuardRecorded, (s, e) => { + s.set(e.path, e.mtimeMs); + }) + .on(StaleGuardCleared, (s) => { + s.clear(); + }); diff --git a/packages/agent-core-v2/src/features/staleGuard/staleGuardService.ts b/packages/agent-core-v2/src/features/staleGuard/staleGuardService.ts new file mode 100644 index 000000000..3946e4c93 --- /dev/null +++ b/packages/agent-core-v2/src/features/staleGuard/staleGuardService.ts @@ -0,0 +1,146 @@ +import { Disposable } from '#/_base/di/lifecycle'; +import { IAgentStateService } from '#/agent/state/agentState'; +import { IAgentRuntimeService } from '#/agent/runtimeBinding/agentRuntime'; +import { denyToolExecution } from '#/agent/toolExecutor/beforeToolExecuteEvent'; +import { IAgentToolExecutorService } from '#/agent/toolExecutor/toolExecutor'; +import type { + BeforeToolExecuteEvent, + ToolDidExecuteContext, +} from '#/agent/toolExecutor/toolHooks'; +import type { ToolCall } from '#/kosong/contract/message'; +import type { HostFileStat } from '#/os/interface/hostFileSystem'; +import { IEventDispatcher } from '#/state/eventDispatcher'; +import type { ToolAccesses, ToolFileAccessOperation } from '#/tool/toolContract'; + +import { IStaleGuardService } from './staleGuard'; +import { StaleGuardCleared, StaleGuardRecorded, staleGuardKey } from './staleGuardOps'; + +const WRITE_OPERATIONS: readonly ToolFileAccessOperation[] = ['write', 'readwrite']; +const READ_OPERATIONS: readonly ToolFileAccessOperation[] = ['read']; + +function accessedFilePath( + accesses: ToolAccesses | undefined, + operations: readonly ToolFileAccessOperation[], +): string | undefined { + for (const access of accesses ?? []) { + if (access.kind === 'file' && operations.includes(access.operation)) return access.path; + } + return undefined; +} + +function stringArg(args: unknown, key: string): string | undefined { + if (typeof args !== 'object' || args === null) return undefined; + const value = (args as Record)[key]; + return typeof value === 'string' ? value : undefined; +} + +function callPathArg(call: ToolCall): string | undefined { + if (typeof call.arguments !== 'string') return undefined; + try { + return stringArg(JSON.parse(call.arguments), 'path'); + } catch { + return undefined; + } +} + +export class StaleGuardService extends Disposable implements IStaleGuardService { + declare readonly _serviceBrand: undefined; + + constructor( + @IAgentStateService private readonly states: IAgentStateService, + @IEventDispatcher private readonly dispatcher: IEventDispatcher, + @IAgentRuntimeService private readonly runtime: IAgentRuntimeService, + @IAgentToolExecutorService toolExecutor: IAgentToolExecutorService, + ) { + super(); + 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(); + }), + ); + this._register( + this.runtime.onDidChange(() => { + void this.dispatcher.dispatch(new StaleGuardCleared({})); + }), + ); + } + + recordedMtimeMs(path: string): number | undefined { + return this.states.get(staleGuardKey).get(path); + } + + private guardWrite(event: BeforeToolExecuteEvent): void { + const name = event.toolCall.name; + if (name !== 'Edit' && name !== 'Write') return; + const path = accessedFilePath(event.execution.accesses, WRITE_OPERATIONS); + if (path === undefined) return; + const displayPath = stringArg(event.args, 'path') ?? path; + if (coveredByEarlierRead(event, displayPath)) return; + event.waitUntil(async () => { + const error = await this.checkWritable(path, displayPath); + return error === undefined ? undefined : { veto: denyToolExecution(error) }; + }); + } + + private async observeExecution(ctx: ToolDidExecuteContext): Promise { + if (ctx.outcome !== 'executed' || ctx.result.isError === true) return; + const name = ctx.toolCall.name; + if (name === 'Read') { + const path = accessedFilePath(ctx.accesses, READ_OPERATIONS); + if (path !== undefined) await this.recordCurrentMtime(path); + return; + } + if (name === 'Edit' || name === 'Write') { + const path = accessedFilePath(ctx.accesses, WRITE_OPERATIONS); + if (path !== undefined) await this.recordCurrentMtime(path); + } + } + + private async checkWritable(path: string, displayPath: string): Promise { + const stat = await this.statFile(path); + if (stat === undefined || stat.mtimeMs === undefined) return undefined; + const recorded = this.recordedMtimeMs(path); + if (recorded === undefined) { + return ( + `"${displayPath}" has not been read by this agent yet. ` + + 'Read the file before writing to it.' + ); + } + if (recorded !== stat.mtimeMs) { + return ( + `"${displayPath}" has been modified on disk since this agent last read it. ` + + 'Read the file again before writing to it.' + ); + } + return undefined; + } + + private async recordCurrentMtime(path: string): Promise { + const stat = await this.statFile(path); + if (stat?.mtimeMs === undefined) return; + await this.dispatcher.dispatch(new StaleGuardRecorded({ path, mtimeMs: stat.mtimeMs })); + } + + private async statFile(path: string): Promise { + const lease = this.runtime.acquire(['fs']); + try { + const stat = await lease.runtime.fs!.stat(path); + return stat.isFile ? stat : undefined; + } catch { + return undefined; + } finally { + lease.dispose(); + } + } +} + +function coveredByEarlierRead(event: BeforeToolExecuteEvent, rawPath: string): boolean { + for (const call of event.toolCalls) { + if (call.id === event.toolCall.id) return false; + if (call.name === 'Read' && callPathArg(call) === rawPath) return true; + } + return false; +} diff --git a/packages/agent-core-v2/src/index.ts b/packages/agent-core-v2/src/index.ts index 1d3f9823a..b4320a000 100644 --- a/packages/agent-core-v2/src/index.ts +++ b/packages/agent-core-v2/src/index.ts @@ -353,6 +353,7 @@ import '#/agent/goal/goalDeadlineSchedulerService'; export * from '#/agent/goal/goal'; export * from '#/agent/goal/goalService'; export * from '#/agent/goal/types'; +import '#/features/staleGuard/staleGuardFeature'; export * from '#/features/tower/tower'; export * from '#/features/tower/towerService'; export * from '#/features/tower/towerRateLimit'; diff --git a/packages/agent-core-v2/test/features/staleGuard/staleGuard.test.ts b/packages/agent-core-v2/test/features/staleGuard/staleGuard.test.ts new file mode 100644 index 000000000..db060de53 --- /dev/null +++ b/packages/agent-core-v2/test/features/staleGuard/staleGuard.test.ts @@ -0,0 +1,532 @@ +import { mkdtemp, readFile, rm, utimes, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { SyncDescriptor } from '#/_base/di/descriptors'; +import { DisposableStore, toDisposable } from '#/_base/di/lifecycle'; +import { TestInstantiationService } from '#/_base/di/test'; +import { IAgentBlobService } from '#/agent/blob/agentBlobService'; +import { IAgentStateService } from '#/agent/state/agentState'; +import { AgentStateService } from '#/agent/state/agentStateService'; +import { IAgentRuntimeService } from '#/agent/runtimeBinding/agentRuntime'; +import { IAgentToolExecutorService } from '#/agent/toolExecutor/toolExecutor'; +import type { + BeforeExecuteDecision, + BeforeToolExecuteEvent, + ToolDidExecuteContext, +} from '#/agent/toolExecutor/toolHooks'; +import { IEventBus } from '#/app/event/eventBus'; +import { EventBusService } from '#/app/event/eventBusService'; +import type { ToolCall } from '#/kosong/contract/message'; +import { IStaleGuardService } from '#/features/staleGuard/staleGuard'; +import { StaleGuardService } from '#/features/staleGuard/staleGuardService'; +import { HostFileSystem } from '#/os/backends/node-local/hostFsService'; +import type { HostFileStat, IHostFileSystem } from '#/os/interface/hostFileSystem'; +import { IEventDispatcher } from '#/state/eventDispatcher'; +import { EventDispatcherService } from '#/state/eventDispatcherService'; +import { ToolAccesses, type ExecutableToolResult } from '#/tool/toolContract'; +import { IWireService } from '#/wire/wire'; +import type { WireRecord } from '#/wire/record'; + +import { createTestAgent } from '../../harness'; +import { stubWireJournal } from '../../wire/stubs'; + +const noopBlob: IAgentBlobService = { + _serviceBrand: undefined, + offloadParts: async (parts) => parts, + loadParts: async (parts) => parts, + isBlobRef: () => false, +}; + +interface CapturedHooks { + readonly before: ((event: BeforeToolExecuteEvent) => unknown)[]; + readonly did: ((ctx: ToolDidExecuteContext, next: () => Promise) => Promise)[]; +} + +function stubToolExecutor(captured: CapturedHooks): IAgentToolExecutorService { + return { + _serviceBrand: undefined, + onBeforeExecuteTool: (listener: (event: BeforeToolExecuteEvent) => unknown) => { + captured.before.push(listener); + return toDisposable(() => {}); + }, + hooks: { + onDidExecuteTool: { + register: ( + _name: string, + handler: (ctx: ToolDidExecuteContext, next: () => Promise) => Promise, + ) => { + captured.did.push(handler); + return toDisposable(() => {}); + }, + }, + }, + } as unknown as IAgentToolExecutorService; +} + +let activeFs: IHostFileSystem; +let fireRuntimeChange: () => void = () => {}; + +function stubRuntime(): IAgentRuntimeService { + return { + _serviceBrand: undefined, + onDidChange: (listener: () => void) => { + fireRuntimeChange = listener; + return toDisposable(() => {}); + }, + acquire: () => ({ + runtime: { fs: activeFs }, + track: (resource: unknown) => resource, + dispose: () => {}, + }), + } as unknown as IAgentRuntimeService; +} + +function stubFs(stat: Partial | Error): IHostFileSystem { + return { + _serviceBrand: undefined, + stat: async () => { + if (stat instanceof Error) throw stat; + return { isFile: true, isDirectory: false, size: 0, ...stat }; + }, + } as unknown as IHostFileSystem; +} + +function enoent(): Error { + return Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' }); +} + +function outputText(result: ExecutableToolResult | undefined): string { + const output = result?.output; + if (typeof output !== 'string') throw new TypeError('expected string output'); + return output; +} + +async function runBeforeExecute( + captured: CapturedHooks, + input: { name: string; args?: unknown; accesses?: ToolAccesses; batch?: ToolCall[] }, +): Promise { + const pending: (() => Promise)[] = []; + let veto: ExecutableToolResult | undefined; + const toolCall = { + type: 'function', + id: 'call_1', + name: input.name, + arguments: JSON.stringify(input.args ?? {}), + } as ToolCall; + const event = { + turnId: 0, + signal: new AbortController().signal, + toolCall, + toolCalls: input.batch ?? [toolCall], + args: input.args, + execution: { accesses: input.accesses }, + veto: (result: ExecutableToolResult) => { + veto = result; + }, + allow: () => {}, + pass: () => {}, + waitUntil: (factory: () => Promise) => { + pending.push(factory); + }, + } as unknown as BeforeToolExecuteEvent; + for (const listener of captured.before) await listener(event); + for (const factory of pending) { + const decision = await factory(); + if (decision?.veto !== undefined) veto = decision.veto; + } + return veto; +} + +async function runDidExecute( + captured: CapturedHooks, + input: { name: string; accesses?: ToolAccesses; isError?: boolean }, +): Promise { + 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 () => {}); +} + +describe('StaleGuardService', () => { + let disposables: DisposableStore; + let records: WireRecord[]; + let hooks: CapturedHooks; + let freshness: IStaleGuardService; + + function buildStack(journal: WireRecord[]): { + freshness: IStaleGuardService; + dispatcher: IEventDispatcher; + hooks: CapturedHooks; + } { + const captured: CapturedHooks = { before: [], did: [] }; + const ix = disposables.add(new TestInstantiationService()); + ix.set(IEventBus, new SyncDescriptor(EventBusService)); + ix.set(IAgentBlobService, noopBlob); + ix.set(IWireService, stubWireJournal(journal)); + ix.set(IAgentStateService, new AgentStateService()); + ix.set(IEventDispatcher, new SyncDescriptor(EventDispatcherService)); + ix.set(IAgentToolExecutorService, stubToolExecutor(captured)); + ix.set(IAgentRuntimeService, stubRuntime()); + ix.set(IStaleGuardService, new SyncDescriptor(StaleGuardService)); + return { + freshness: ix.get(IStaleGuardService), + dispatcher: ix.get(IEventDispatcher), + hooks: captured, + }; + } + + beforeEach(() => { + disposables = new DisposableStore(); + records = []; + activeFs = stubFs({}); + const stack = buildStack(records); + hooks = stack.hooks; + freshness = stack.freshness; + }); + + afterEach(() => { + disposables.dispose(); + }); + + 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') }); + + expect(freshness.recordedMtimeMs('/tmp/a.txt')).toBe(111); + expect(records).toEqual([ + { type: 'staleGuard.recorded', path: '/tmp/a.txt', mtimeMs: 111, time: expect.any(Number) }, + ]); + }); + + it('does not record when the read failed', async () => { + activeFs = stubFs({ mtimeMs: 111 }); + + await runDidExecute(hooks, { + name: 'Read', + accesses: ToolAccesses.readFile('/tmp/a.txt'), + isError: true, + }); + + expect(freshness.recordedMtimeMs('/tmp/a.txt')).toBeUndefined(); + expect(records).toEqual([]); + }); + + it('ignores tools without file semantics', async () => { + const veto = await runBeforeExecute(hooks, { name: 'Bash', args: { command: 'ls' } }); + expect(veto).toBeUndefined(); + + activeFs = stubFs({ mtimeMs: 111 }); + await runDidExecute(hooks, { name: 'Bash' }); + expect(records).toEqual([]); + }); + + it('vetoes editing an existing file the agent never read', async () => { + activeFs = stubFs({ mtimeMs: 5 }); + + 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?.isError).toBe(true); + expect(outputText(veto)).toContain('has not been read'); + expect(outputText(veto)).toContain('/tmp/a.txt'); + }); + + 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') }); + + const veto = await runBeforeExecute(hooks, { + name: 'Write', + args: { path: '/tmp/a.txt', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/a.txt'), + }); + + expect(veto).toBeUndefined(); + }); + + 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') }); + activeFs = stubFs({ mtimeMs: 222 }); + + 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?.isError).toBe(true); + expect(outputText(veto)).toContain('modified on disk'); + }); + + 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 = { + type: 'function', + id: 'call_0', + name: 'Read', + arguments: JSON.stringify({ path: '/tmp/a.txt' }), + }; + const writeCall: ToolCall = { + type: 'function', + id: 'call_1', + name: 'Write', + arguments: JSON.stringify({ path: '/tmp/a.txt', content: 'x' }), + }; + + const veto = await runBeforeExecute(hooks, { + name: 'Write', + args: { path: '/tmp/a.txt', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/a.txt'), + batch: [readCall, writeCall], + }); + + expect(veto).toBeUndefined(); + }); + + it('still vetoes when the earlier batch Read targets a different path', async () => { + activeFs = stubFs({ mtimeMs: 5 }); + const readCall: ToolCall = { + type: 'function', + id: 'call_0', + name: 'Read', + arguments: JSON.stringify({ path: '/tmp/other.txt' }), + }; + const writeCall: ToolCall = { + type: 'function', + id: 'call_1', + name: 'Write', + arguments: JSON.stringify({ path: '/tmp/a.txt', content: 'x' }), + }; + + const veto = await runBeforeExecute(hooks, { + name: 'Write', + args: { path: '/tmp/a.txt', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/a.txt'), + batch: [readCall, writeCall], + }); + + expect(veto?.isError).toBe(true); + expect(outputText(veto)).toContain('has not been read'); + }); + + it('clears recorded mtimes when the runtime changes', async () => { + activeFs = stubFs({ mtimeMs: 111 }); + await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile('/tmp/a.txt') }); + expect(freshness.recordedMtimeMs('/tmp/a.txt')).toBe(111); + + fireRuntimeChange(); + + expect(freshness.recordedMtimeMs('/tmp/a.txt')).toBeUndefined(); + expect(records).toContainEqual({ + type: 'staleGuard.cleared', + time: expect.any(Number), + }); + 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(outputText(veto)).toContain('has not been read'); + }); + + it('allows writing a file that does not exist yet', async () => { + activeFs = stubFs(enoent()); + + const veto = await runBeforeExecute(hooks, { + name: 'Write', + args: { path: '/tmp/new.txt', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/new.txt'), + }); + + expect(veto).toBeUndefined(); + }); + + it('skips the check when the runtime stat carries no mtimeMs', async () => { + activeFs = stubFs({}); + + 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(); + }); + + it('skips the check when the path is not a regular file', async () => { + activeFs = stubFs({ isFile: false, isDirectory: true, mtimeMs: 5 }); + + const veto = await runBeforeExecute(hooks, { + name: 'Write', + args: { path: '/tmp/dir', content: 'x' }, + accesses: ToolAccesses.writeFile('/tmp/dir'), + }); + + expect(veto).toBeUndefined(); + }); + + 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') }); + + activeFs = stubFs({ mtimeMs: 222 }); + await runDidExecute(hooks, { name: 'Edit', accesses: ToolAccesses.readWriteFile('/tmp/a.txt') }); + + expect(freshness.recordedMtimeMs('/tmp/a.txt')).toBe(222); + 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(); + }); + + 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') }); + + 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, { + 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'); + }); + + it('keeps records isolated between independent agent stacks', async () => { + activeFs = stubFs({ mtimeMs: 111 }); + await runDidExecute(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, { + 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'); + }); + + it('detects an external mtime change through the real filesystem', async () => { + const dir = await mkdtemp(join(tmpdir(), 'file-freshness-')); + const file = join(dir, 'a.txt'); + await writeFile(file, 'one', 'utf8'); + try { + activeFs = new HostFileSystem(); + await runDidExecute(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, { + name: 'Edit', + args: { path: file, old_string: 'one', new_string: 'two' }, + accesses: ToolAccesses.readWriteFile(file), + }); + expect(outputText(veto)).toContain('modified on disk'); + + await runDidExecute(hooks, { name: 'Read', accesses: ToolAccesses.readFile(file) }); + const allowed = await runBeforeExecute(hooks, { + name: 'Edit', + args: { path: file, old_string: 'one', new_string: 'two' }, + accesses: ToolAccesses.readWriteFile(file), + }); + expect(allowed).toBeUndefined(); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); + +describe('StaleGuardService in the agent test harness', () => { + it('is assembled by the feature seam with no manual registration', async () => { + const ctx = createTestAgent(); + try { + const svc = ctx.get(IStaleGuardService); + expect(svc).toBeDefined(); + expect(typeof svc.recordedMtimeMs).toBe('function'); + } finally { + await ctx.dispose(); + } + }); + + it('rejects an Edit with a stale-mtime error after an external modification', async () => { + const dir = await mkdtemp(join(tmpdir(), 'file-freshness-e2e-')); + const file = join(dir, 'a.txt'); + await writeFile(file, 'alpha beta', 'utf8'); + const ctx = createTestAgent(); + try { + await ctx.rpc.setPermission({ mode: 'yolo' }); + + const readCall: ToolCall = { + type: 'function', + id: 'call_read', + name: 'Read', + arguments: JSON.stringify({ path: file }), + }; + ctx.mockNextResponse({ type: 'text', text: 'Reading the file.' }, readCall); + ctx.mockNextResponse({ type: 'text', text: 'Read complete.' }); + await ctx.rpc.prompt({ input: [{ type: 'text', text: 'Read the file' }] }); + await ctx.untilTurnEnd(); + + const past = new Date(Date.now() - 60_000); + await utimes(file, past, past); + + const editCall: ToolCall = { + type: 'function', + id: 'call_edit', + name: 'Edit', + arguments: JSON.stringify({ path: file, old_string: 'beta', new_string: 'gamma' }), + }; + ctx.mockNextResponse({ type: 'text', text: 'Editing the file.' }, editCall); + ctx.mockNextResponse({ type: 'text', text: 'Done.' }); + await ctx.rpc.prompt({ input: [{ type: 'text', text: 'Edit the file' }] }); + await ctx.untilTurnEnd(); + + expect(toolResultText(ctx.llmCalls.at(-1)!.history)).toContain('modified on disk'); + expect(await readFile(file, 'utf8')).toBe('alpha beta'); + } finally { + await ctx.dispose(); + await rm(dir, { recursive: true, force: true }); + } + }); +}); + +function toolResultText(history: readonly { role: string; content: readonly unknown[] }[]): string { + return history + .filter((message) => message.role === 'tool') + .flatMap((message) => message.content) + .map((part) => { + if ( + part !== null && + typeof part === 'object' && + (part as { type?: unknown }).type === 'text' + ) { + const text = (part as { text?: unknown }).text; + return typeof text === 'string' ? text : ''; + } + return ''; + }) + .join('\n'); +} diff --git a/packages/agent-core-v2/test/index.test.ts b/packages/agent-core-v2/test/index.test.ts index c654d41ac..c164ed168 100644 --- a/packages/agent-core-v2/test/index.test.ts +++ b/packages/agent-core-v2/test/index.test.ts @@ -83,6 +83,8 @@ const V2_RECORD_TYPES: ReadonlySet = new Set([ 'task.started', 'task.terminated', 'task.waitDelivered', + 'staleGuard.recorded', + 'staleGuard.cleared', 'interaction.request', 'interaction.resolved', 'plan.revision', diff --git a/packages/agent-core-v2/test/state/builtinReplayableKeys.ts b/packages/agent-core-v2/test/state/builtinReplayableKeys.ts index 274858a40..e7ddae248 100644 --- a/packages/agent-core-v2/test/state/builtinReplayableKeys.ts +++ b/packages/agent-core-v2/test/state/builtinReplayableKeys.ts @@ -1,6 +1,7 @@ import type { ReplayableStateKey } from '#/state/state'; import { contextMemoryKey } from '#/agent/contextMemory/contextOps'; +import { staleGuardKey } from '#/features/staleGuard/staleGuardOps'; import { fullCompactionKey } from '#/agent/fullCompaction/compactionOps'; import { goalKey } from '#/agent/goal/goalOps'; import { goalForkNoticeKey } from '#/agent/goal/goalService'; @@ -32,6 +33,7 @@ import { todoKey } from '#/session/todo/todoOps'; export const BUILTIN_REPLAYABLE_STATE_KEYS: readonly ReplayableStateKey[] = [ contextMemoryKey, + staleGuardKey, fullCompactionKey, goalKey, goalForkNoticeKey,