diff --git a/packages/agent-core-v2/scripts/check-domain-layers.mjs b/packages/agent-core-v2/scripts/check-domain-layers.mjs index c8a9714b3..fa98f97d6 100644 --- a/packages/agent-core-v2/scripts/check-domain-layers.mjs +++ b/packages/agent-core-v2/scripts/check-domain-layers.mjs @@ -145,6 +145,7 @@ const DOMAIN_LAYER = new Map([ ['permissionGate', 3], ['toolApproval', 3], ['flag', 3], + ['multiServer', 3], ['toolExecutor', 3], ['toolResultTruncation', 3], ['toolRegistry', 3], diff --git a/packages/agent-core-v2/src/_base/di/instantiation.ts b/packages/agent-core-v2/src/_base/di/instantiation.ts index 5642c087b..c13d9091a 100644 --- a/packages/agent-core-v2/src/_base/di/instantiation.ts +++ b/packages/agent-core-v2/src/_base/di/instantiation.ts @@ -16,7 +16,7 @@ export namespace _util { export function getServiceDependencies( ctor: DI_TARGET_OBJ, // eslint-disable-next-line @typescript-eslint/no-explicit-any - ): { id: ServiceIdentifier; index: number }[] { + ): { id: ServiceIdentifier; index: number; optional: boolean }[] { return ctor[DI_DEPENDENCIES] || []; } @@ -25,7 +25,7 @@ export namespace _util { // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type [DI_TARGET]: Function; // eslint-disable-next-line @typescript-eslint/no-explicit-any - [DI_DEPENDENCIES]: { id: ServiceIdentifier; index: number }[]; + [DI_DEPENDENCIES]: { id: ServiceIdentifier; index: number; optional: boolean }[]; } } @@ -56,12 +56,13 @@ function storeServiceDependency( // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type target: Function, index: number, + optional: boolean, ): void { const t = target as _util.DI_TARGET_OBJ; if (t[_util.DI_TARGET] === target) { - t[_util.DI_DEPENDENCIES].push({ id, index }); + t[_util.DI_DEPENDENCIES].push({ id, index, optional }); } else { - t[_util.DI_DEPENDENCIES] = [{ id, index }]; + t[_util.DI_DEPENDENCIES] = [{ id, index, optional }]; t[_util.DI_TARGET] = target; } } @@ -83,7 +84,7 @@ export function createDecorator(name: string): ServiceIdentifier { '@IServiceName-decorator can only be used to decorate a parameter', ); } - storeServiceDependency(id, target, index); + storeServiceDependency(id, target, index, false); } as unknown as ServiceIdentifier; Object.defineProperty(id, 'toString', { @@ -99,6 +100,12 @@ export function createDecorator(name: string): ServiceIdentifier { return id; } +export function optional(id: ServiceIdentifier): ParameterDecorator { + return (target, _key, index): void => { + storeServiceDependency(id, target as Function, index, true); + }; +} + export function refineServiceDecorator( serviceIdentifier: ServiceIdentifier, ): ServiceIdentifier { diff --git a/packages/agent-core-v2/src/_base/di/instantiationService.ts b/packages/agent-core-v2/src/_base/di/instantiationService.ts index 52084a85b..625442b9d 100644 --- a/packages/agent-core-v2/src/_base/di/instantiationService.ts +++ b/packages/agent-core-v2/src/_base/di/instantiationService.ts @@ -255,8 +255,15 @@ export class InstantiationService implements IInstantiationService { const serviceDependencies = _util.getServiceDependencies(ctor).toSorted((a, b) => a.index - b.index); const serviceArgs: unknown[] = []; for (const dependency of serviceDependencies) { - const service = this._getOrCreateServiceInstance(dependency.id, _trace); - if (!service) { + if ( + dependency.optional && + this._getServiceInstanceOrDescriptor(dependency.id) === undefined + ) { + serviceArgs.push(undefined); + continue; + } + const service = this._getOrCreateServiceInstance(dependency.id, _trace); + if (!service) { this._throwIfStrict( `[createInstance] ${ctor.name} depends on UNKNOWN service ${String(dependency.id)}.`, false, @@ -349,6 +356,7 @@ export class InstantiationService implements IInstantiationService { for (const dependency of _util.getServiceDependencies(item.desc.ctor)) { const instanceOrDesc = this._getServiceInstanceOrDescriptor(dependency.id); if (!instanceOrDesc) { + if (dependency.optional) continue; this._throwIfStrict( `[createInstance] ${String(item.id)} depends on ${String(dependency.id)} which is NOT registered.`, true, diff --git a/packages/agent-core-v2/src/agent/fileFencing/fileFencingService.ts b/packages/agent-core-v2/src/agent/fileFencing/fileFencingService.ts index b65be3b67..f3ef5d6d0 100644 --- a/packages/agent-core-v2/src/agent/fileFencing/fileFencingService.ts +++ b/packages/agent-core-v2/src/agent/fileFencing/fileFencingService.ts @@ -4,7 +4,10 @@ * Registers the `writeFencing` participant on the `toolExecutor` * `onBeforeExecuteTool` / `onDidExecuteTool` hook slots, matching * `Read`/`Write`/`Edit` by tool name and letting every other tool pass - * through. The target path comes from the resolved execution's file accesses + * through. For Write/Edit the before hook injects an execution wrapper; the + * wrapper re-checks the ledger only after the scheduler grants the declared + * file access, so a queued write cannot rely on a stale preflight verdict. The + * target path comes from the resolved execution's file accesses * — the exact canonical path the tool itself computed — so the ledger and * the watcher key it identically. The before-hook records the target keyed * by `toolCall.id` (cleared in the did-hook and swept on turn change) and, @@ -113,7 +116,7 @@ export class AgentFileFencingService extends Disposable implements IAgentFileFen ) { super(); toolExecutor.hooks.onBeforeExecuteTool.register('writeFencing', async (ctx, next) => { - await this.onBefore(ctx); + this.onBefore(ctx); if (ctx.decision?.block === true) return; await next(); }); @@ -123,7 +126,7 @@ export class AgentFileFencingService extends Disposable implements IAgentFileFen }); } - private async onBefore(ctx: ToolBeforeExecuteContext): Promise { + private onBefore(ctx: ToolBeforeExecuteContext): void { if (!isFenced(ctx)) return; const path = targetPathOf(ctx); if (path === undefined) return; @@ -134,13 +137,22 @@ export class AgentFileFencingService extends Disposable implements IAgentFileFen } this.targets.set(ctx.toolCall.id, { toolName: ctx.toolCall.name, path }); if (!WRITE_TOOLS.has(ctx.toolCall.name)) return; - const verdict = await this.ledger.compare(path); - if (verdict === 'clean') return; - if (this.flags.enabled(MULTI_SERVER_FLAG_ID)) { - ctx.decision = { block: true, reason: blockReason(ctx.toolCall.name, path, verdict) }; - return; - } - this.staleMarks.set(ctx.toolCall.id, verdict); + const execute = ctx.decision?.execute ?? ctx.execution.execute; + ctx.decision = { + ...ctx.decision, + execute: async (executeCtx) => { + const verdict = await this.ledger.compare(path); + if (verdict !== 'clean') { + if (this.flags.enabled(MULTI_SERVER_FLAG_ID)) { + const reason = blockReason(ctx.toolCall.name, path, verdict); + ctx.decision = { ...ctx.decision, block: true, reason }; + return { output: reason, isError: true }; + } + this.staleMarks.set(ctx.toolCall.id, verdict); + } + return execute(executeCtx); + }, + }; } private async onDid(ctx: ToolDidExecuteContext): Promise { diff --git a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts index c0d3a6946..e0e4be739 100644 --- a/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts +++ b/packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts @@ -378,6 +378,8 @@ export class AgentToolExecutorService implements IAgentToolExecutorService { } const executionMetadata = decision?.executionMetadata; + const runnableExecution = + decision?.execute === undefined ? execution : { ...execution, execute: decision.execute }; await this.willExecuteEmitter.fireAsync( { @@ -393,9 +395,15 @@ export class AgentToolExecutorService implements IAgentToolExecutorService { return { task: { - accesses: execution.accesses ?? ToolAccesses.all(), + accesses: runnableExecution.accesses ?? ToolAccesses.all(), execute: async (taskSignal) => - this.runSingleExecution(call, execution, executionMetadata, options, taskSignal), + this.runSingleExecution( + call, + runnableExecution, + executionMetadata, + options, + taskSignal, + ), }, stopBatchAfterThis: execution.stopBatchAfterThis, }; diff --git a/packages/agent-core-v2/src/agent/toolExecutor/toolHooks.ts b/packages/agent-core-v2/src/agent/toolExecutor/toolHooks.ts index e4fa4e622..086635c5e 100644 --- a/packages/agent-core-v2/src/agent/toolExecutor/toolHooks.ts +++ b/packages/agent-core-v2/src/agent/toolExecutor/toolHooks.ts @@ -48,6 +48,7 @@ export interface ResolvedToolExecutionHookContext extends ToolExecutionHookConte export interface BeforeExecuteDecision { readonly veto?: ExecutableToolResult; readonly executionMetadata?: unknown; + readonly execute?: RunnableToolExecution['execute']; } export interface BeforeToolExecuteEvent extends ResolvedToolExecutionHookContext { diff --git a/packages/agent-core-v2/src/app/bootstrap/bootstrap.ts b/packages/agent-core-v2/src/app/bootstrap/bootstrap.ts index 9c54118cf..e4ce9db31 100644 --- a/packages/agent-core-v2/src/app/bootstrap/bootstrap.ts +++ b/packages/agent-core-v2/src/app/bootstrap/bootstrap.ts @@ -126,9 +126,7 @@ export function bootstrap(input: BootstrapInput = {}, extraSeeds: ScopeSeed = [] function storageSeed(options: IBootstrapOptions): ScopeSeed { const file = (): SyncDescriptor => new SyncDescriptor(FileStorageService, [options.homeDir, 0o700, 0o600], true); - return [ - [IFileSystemStorageService as ServiceIdentifier, file()], - ]; + return [[IFileSystemStorageService as ServiceIdentifier, file()]]; } function skillSeed(): ScopeSeed { diff --git a/packages/agent-core-v2/src/app/bootstrap/bootstrapService.ts b/packages/agent-core-v2/src/app/bootstrap/bootstrapService.ts index 381b00ddd..2334fc2a9 100644 --- a/packages/agent-core-v2/src/app/bootstrap/bootstrapService.ts +++ b/packages/agent-core-v2/src/app/bootstrap/bootstrapService.ts @@ -13,7 +13,7 @@ * Bound at App scope. */ -import { basename, join, relative } from 'pathe'; +import { join, relative } from 'pathe'; import { InstantiationType } from '#/_base/di/extensions'; import { LifecycleScope, registerScopedService } from '#/_base/di/scope'; @@ -58,7 +58,7 @@ export class BootstrapService implements IBootstrapService { this.storeDir = join(options.homeDir, 'store'); this.cacheDir = join(options.homeDir, 'cache'); this.logsDir = join(options.homeDir, 'logs'); - this.configKey = basename(options.configPath); + this.configKey = relative(options.homeDir, options.configPath); this.scopes = { config: '', sessions: relative(options.homeDir, this.sessionsDir), diff --git a/packages/agent-core-v2/src/app/config/configFileMutex.ts b/packages/agent-core-v2/src/app/config/configFileMutex.ts deleted file mode 100644 index 1e8a7f909..000000000 --- a/packages/agent-core-v2/src/app/config/configFileMutex.ts +++ /dev/null @@ -1,35 +0,0 @@ -/** - * `config` domain (L2) — cross-process lock target for the config.toml write path. - * - * Pure seam shared by `ConfigService.persist`: the lock file path derived from - * the resolved config file (`.lock` — `/config.toml.lock` - * in the default layout, so a custom `--config` path is protected at its real - * location) and the bounded-wait acquisition options for the lock-in - * read-modify-write critical section (design: `.tmp/refactor-watch-design-v2.md` - * §3.6). Owns no scoped state. - */ - -import type { - CrossProcessLockAcquireOptions, - CrossProcessLockWaitOptions, -} from '#/os/interface/crossProcessLock'; - -export interface ConfigFileMutexTarget { - readonly lockPath: string; - readonly options: CrossProcessLockAcquireOptions & { wait: CrossProcessLockWaitOptions }; -} - -export const CONFIG_FILE_LOCK_TIMEOUT_MS = 10_000; -export const CONFIG_FILE_LOCK_RETRY_INTERVAL_MS = 50; - -const DEFAULT_WAIT: CrossProcessLockWaitOptions = { - timeoutMs: CONFIG_FILE_LOCK_TIMEOUT_MS, - retryIntervalMs: CONFIG_FILE_LOCK_RETRY_INTERVAL_MS, -}; - -export function configFileMutexTarget( - configPath: string, - wait: CrossProcessLockWaitOptions = DEFAULT_WAIT, -): ConfigFileMutexTarget { - return { lockPath: `${configPath}.lock`, options: { wait } }; -} diff --git a/packages/agent-core-v2/src/app/config/configService.ts b/packages/agent-core-v2/src/app/config/configService.ts index dce9aef0d..1cd6b3809 100644 --- a/packages/agent-core-v2/src/app/config/configService.ts +++ b/packages/agent-core-v2/src/app/config/configService.ts @@ -18,10 +18,10 @@ * `onDidSectionChange`. Reads config paths and the environment overlay through * `bootstrap`, persists the TOML document through the `storage` TOML * atomic-document store (reloading when the document changes on disk), - * serializes every persist as a cross-process lock-in read-modify-write - * through `crossProcessLock` (re-reading the file inside the lock and applying - * only the touched section, so sections written by other processes survive — - * design: `.tmp/refactor-watch-design-v2.md` §3.6), and logs through `log`. + * serializes every persist through the atomic-document store's transactional + * update primitive (backed by a cross-process lock for file storage), re-reading + * inside the lock and applying only the touched section so writes from other + * processes survive, and logs through `log`. * Late section / overlay registration re-validates the already-loaded raw * value and re-runs overlays. Bound at App scope. */ @@ -36,9 +36,6 @@ import { IAtomicTomlDocumentStore, type IAtomicDocumentStore, } from '#/persistence/interface/atomicDocumentStore'; -import { ICrossProcessLockService } from '#/os/interface/crossProcessLock'; - -import { configFileMutexTarget } from './configFileMutex'; import { type AnyEnvBindings, type ConfigChangedEvent, @@ -249,7 +246,6 @@ export class ConfigService extends Disposable implements IConfigService { @IBootstrapService private readonly bootstrap: IBootstrapService, @ILogService private readonly log: ILogService, @IAtomicTomlDocumentStore private readonly documentStore: IAtomicDocumentStore, - @ICrossProcessLockService private readonly lock: ICrossProcessLockService, ) { super(); this.configKey = this.bootstrap.configKey; @@ -567,24 +563,17 @@ export class ConfigService extends Disposable implements IConfigService { } private async persist(domain: string): Promise { - const { lockPath, options } = configFileMutexTarget(this.bootstrap.configPath); - // Two exclusion layers: `enqueueStateTransition` serializes transitions - // inside this process; the cross-process lock makes the re-read → apply → - // atomic-write burst mutually exclusive with other processes sharing this - // config file. Lock-acquisition and write failures propagate to the set() - // caller as-is (no retry here). - await this.lock.withLock(lockPath, options, async () => { - const data = await this.documentStore.get(CONFIG_SCOPE, this.configKey); - const freshBase = data !== undefined && isPlainObject(data) ? cloneRecord(data) : {}; - applySectionToToml(freshBase, domain, this.raw[domain], this.registry); - await this.documentStore.set(CONFIG_SCOPE, this.configKey, freshBase); - this.rawSnake = freshBase; - // Re-derive the camelCase view too: the fresh base may carry sections - // written by other processes, and both views must match what we wrote - // (rawSnake equality is how the watch-triggered load() skips our own - // echo instead of double-reporting it). - this.raw = transformTomlData(freshBase, this.registry); - }); + const freshBase = await this.documentStore.update( + CONFIG_SCOPE, + this.configKey, + (data) => { + const next = data !== undefined && isPlainObject(data) ? cloneRecord(data) : {}; + applySectionToToml(next, domain, this.raw[domain], this.registry); + return next; + }, + ); + this.rawSnake = freshBase; + this.raw = transformTomlData(freshBase, this.registry); } private commitAbsorbed(previousRaw: ResolvedConfig): void { diff --git a/packages/agent-core-v2/src/app/multiServer/flag.ts b/packages/agent-core-v2/src/app/multiServer/flag.ts index b3bdc70f6..22c7c1e6a 100644 --- a/packages/agent-core-v2/src/app/multiServer/flag.ts +++ b/packages/agent-core-v2/src/app/multiServer/flag.ts @@ -1,5 +1,6 @@ /** - * `multi_server` experimental flag — gates the multi-server shared-homedir work. + * `multi_server` experimental flag — gates shared-home coordination among + * cooperating lease-aware server versions. * * When enabled, a kap-server instance registers itself under * `/server/instances/.json` instead of taking the legacy @@ -19,7 +20,7 @@ export const multiServerFlag: FlagDefinitionInput = { id: MULTI_SERVER_FLAG_ID, title: 'multi-server shared home', description: - 'Allow multiple kap-server instances to share one home directory by registering each instance under server/instances/ instead of taking a single homedir lock.', + 'Allow cooperating lease-aware kap-server instances to share one home directory by registering each instance under server/instances/ instead of taking a single homedir lock.', env: MULTI_SERVER_FLAG_ENV, default: false, surface: 'core', diff --git a/packages/agent-core-v2/src/app/sessionLifecycle/sessionLifecycle.ts b/packages/agent-core-v2/src/app/sessionLifecycle/sessionLifecycle.ts index c57bb8d21..192c468c3 100644 --- a/packages/agent-core-v2/src/app/sessionLifecycle/sessionLifecycle.ts +++ b/packages/agent-core-v2/src/app/sessionLifecycle/sessionLifecycle.ts @@ -6,6 +6,8 @@ * `ISessionLifecycleService` used to create sessions (`create`), look up the * live ones (`get` / `list`), close them (`close`), archive/restore them, * fork them (`fork`), and fork-then-tag them as direct children (`createChild`). Announces + * an ambiguous durability failure through explicit `forceAbort`, which records a + * dirty-abort marker before releasing the session lease. * lifecycle transitions through ordered hook slots plus * `onDidCreateSession` / `onDidCloseSession` / `onDidArchiveSession` / * `onDidForkSession`. App-scoped — a single @@ -91,6 +93,7 @@ export interface ISessionLifecycleService { list(): readonly ISessionScopeHandle[]; resume(sessionId: string): Promise; close(sessionId: string): Promise; + forceAbort(sessionId: string): Promise; archive(sessionId: string): Promise; restore(sessionId: string): Promise; fork(opts: ForkSessionOptions): Promise; diff --git a/packages/agent-core-v2/src/app/sessionLifecycle/sessionLifecycleService.ts b/packages/agent-core-v2/src/app/sessionLifecycle/sessionLifecycleService.ts index 9d0d70a31..d0d8f0335 100644 --- a/packages/agent-core-v2/src/app/sessionLifecycle/sessionLifecycleService.ts +++ b/packages/agent-core-v2/src/app/sessionLifecycle/sessionLifecycleService.ts @@ -36,15 +36,13 @@ * Every materialization (create/resume/fork-target) first takes the session's * cross-process write lease under `session-leases/` and registers its * `ISessionWriteAuthority` with the `writeAuthorityRegistry`, so the - * journal/state fencing gates have exactly one authority per live session - * (design: `.tmp/refactor-watch-design-v2.md` §3.4 — always on, no flag). - * Contended acquisitions are answered with `session.held_by_peer` carrying - * the structured ownership details; the multi_server flag gates only the - * address emission (`routable` phase) and the unregistered-writer freshness - * probe. Materialize failure arms unregister and release immediately, and - * close/archive finish the session tail (final journal flush) before - * unregistering the authority and releasing the lease. A lease lost under a - * live session (payload token mismatch) tears the whole session down. + * journal/state fencing gates have exactly one authority per live session. + * A preparing scope is private until metadata, MCP, caller-specific setup and + * lifecycle hooks finish. Close/archive move the entry through a draining + * phase, dispose producers, durably flush only that session's append-log tail, + * and release authority only after the barrier succeeds. A failed barrier + * keeps the lease registered for a safe retry; lease loss takes the explicit + * dirty-abort path without claiming a clean handoff. */ import { randomUUID } from 'node:crypto'; @@ -110,8 +108,6 @@ import { sessionLeaseSeed, SESSION_LEASE_HEARTBEAT_INTERVAL_MS, SESSION_LEASE_TTL_MS, - UNREGISTERED_WRITER_RECHECK_DELAY_MS, - UNREGISTERED_WRITER_WINDOW_MS, } from '#/session/sessionLease/sessionLease'; import { ISessionLeaseContactProvider } from '#/session/sessionLease/sessionLeaseContactProvider'; import { ISessionMetadata, type SessionMeta } from '#/session/sessionMetadata/sessionMetadata'; @@ -144,9 +140,24 @@ type MaterializeSessionOptions = Omit & { readonly workspaceId?: string; }; +type SessionEntryPhase = 'preparing' | 'active' | 'draining' | 'flush-failed'; +type SessionCloseKind = 'close' | 'archive'; + +interface SessionEntry { + phase: SessionEntryPhase; + readonly handle: ISessionScopeHandle; + readonly lease: SessionLease; + readonly registration: IDisposable; + readonly scope: string; + disposed: boolean; + closeKind?: SessionCloseKind; + closeStep: number; + closePromise?: Promise; +} + export class SessionLifecycleService extends Disposable implements ISessionLifecycleService { declare readonly _serviceBrand: undefined; - private readonly sessions = new Map(); + private readonly entries = new Map(); private readonly _onDidCreateSession = this._register(new Emitter()); readonly onDidCreateSession: Event = this._onDidCreateSession.event; private readonly _onDidCloseSession = this._register(new Emitter()); @@ -160,10 +171,6 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec 'onWillCloseSession', ]); private readonly resuming = new Map>(); - private readonly leases = new Map< - string, - { readonly lease: SessionLease; readonly registration: IDisposable } - >(); constructor( @IInstantiationService private readonly instantiation: IInstantiationService, @@ -192,7 +199,8 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec async create(opts: CreateSessionOptions): Promise { const sessionId = opts.sessionId ?? createSessionId(); - const handle = await this.materializeSession({ ...opts, sessionId }); + const entry = await this.materializeSession({ ...opts, sessionId }); + const handle = entry.handle; try { const main = opts.mainAgentBinding === undefined @@ -213,19 +221,19 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec opts.workDir, handle.accessor.get(ISessionContext).workspaceId, ); + this.activateSession(entry); + await this.announceCreated({ sessionId, handle, source: 'startup' }); + return handle; } catch (error) { const sessionDir = handle.accessor.get(ISessionContext).sessionDir; - this.sessions.delete(sessionId); await this.drainAgents(handle).catch(() => {}); - handle.dispose(); + this.rollbackSession(entry); await this.hostFs.remove(sessionDir).catch(() => {}); throw error; } - await this.announceCreated({ sessionId, handle, source: 'startup' }); - return handle; } - private async materializeSession(opts: MaterializeSessionOptions): Promise { + private async materializeSession(opts: MaterializeSessionOptions): Promise { const workspace = await this.workspaces.createOrTouch(opts.workDir); const workspaceId = opts.workspaceId ?? workspace.id; const sessionScope = this.bootstrap.sessionScope(workspaceId, opts.sessionId); @@ -248,10 +256,15 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec ); const additionalDirs = [...localWorkspaceDirs.additionalDirs, ...callerAdditionalDirs]; await this.hostEnv.ready; - await this.assertNoActiveUnregisteredWriter(sessionDir, opts.sessionId); - const lease = this.acquireSessionLease(opts.sessionId); - const registration = this.authorityRegistry.register(lease); - this.leases.set(opts.sessionId, { lease, registration }); + const lease = await this.acquireSessionLease(opts.sessionId); + let registration: IDisposable; + try { + registration = this.authorityRegistry.register(lease); + } catch (error) { + lease.release(); + throw error; + } + let entry: SessionEntry | undefined; try { const handle = createScopedChildHandle( this.instantiation, @@ -264,24 +277,31 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec if (additionalDirs.length > 0) { handle.accessor.get(ISessionWorkspaceContext).setAdditionalDirs(additionalDirs); } - try { - await handle.accessor.get(ISessionMetadata).ready; - await handle.accessor.get(ISessionToolPolicy).ready; - void handle.accessor.get(ISessionSkillCatalog).ready; - await handle.accessor.get(ISessionAgentProfileCatalog).ready; - await handle.accessor.get(ISessionMcpService).ensureMcpReady(opts.mcpServers); - // Force-instantiate the session-level eager services whose subscriptions - // must exist before the first agent / turn (external hooks, cron). - handle.accessor.get(ISessionExternalHooksService); - handle.accessor.get(ISessionCronService); - } catch (error) { - handle.dispose(); - throw error; - } - this.sessions.set(opts.sessionId, handle); - return handle; + entry = { + phase: 'preparing', + handle, + lease, + registration, + scope: sessionScope, + disposed: false, + closeStep: 0, + }; + this.entries.set(opts.sessionId, entry); + handle.accessor.get(ISessionExternalHooksService); + handle.accessor.get(ISessionCronService); + await handle.accessor.get(ISessionMetadata).ready; + await handle.accessor.get(ISessionToolPolicy).ready; + void handle.accessor.get(ISessionSkillCatalog).ready; + await handle.accessor.get(ISessionAgentProfileCatalog).ready; + await handle.accessor.get(ISessionMcpService).ensureMcpReady(opts.mcpServers); + return entry; } catch (error) { - this.teardownLease(opts.sessionId); + if (entry !== undefined) { + this.rollbackSession(entry); + } else { + registration.dispose(); + lease.release(); + } throw error; } } @@ -304,7 +324,7 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec sessionDir, workDir, }); - await this.appendLogStore.flush(); + await this.appendLogStore.flush(''); } private async announceCreated(event: SessionCreatedEvent): Promise { @@ -315,14 +335,14 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec } get(sessionId: string): ISessionScopeHandle | undefined { - if (this.resuming.has(sessionId)) return undefined; - return this.sessions.get(sessionId); + const entry = this.entries.get(sessionId); + return entry?.phase === 'active' ? entry.handle : undefined; } resume(sessionId: string): Promise { const inflight = this.resuming.get(sessionId); if (inflight !== undefined) return inflight; - const live = this.sessions.get(sessionId); + const live = this.get(sessionId); if (live !== undefined) return Promise.resolve(live); const promise = this.doResume(sessionId) .catch((error: unknown) => { @@ -338,7 +358,7 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec } private async doResume(sessionId: string): Promise { - const live = this.sessions.get(sessionId); + const live = this.get(sessionId); if (live !== undefined) return live; const summary = await this.index.get(sessionId); @@ -348,55 +368,73 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec const workDir = summary.cwd ?? workspace?.root; if (workDir === undefined) return undefined; - const handle = await this.materializeSession({ + const entry = await this.materializeSession({ sessionId, workDir, workspaceId: summary.workspaceId, }); - const agents = handle.accessor.get(IAgentLifecycleService); - if (agents.get(MAIN_AGENT_ID) === undefined) { - await agents.create({ agentId: MAIN_AGENT_ID }); + const handle = entry.handle; + try { + const agents = handle.accessor.get(IAgentLifecycleService); + if (agents.get(MAIN_AGENT_ID) === undefined) { + await agents.create({ agentId: MAIN_AGENT_ID }); + } + this.activateSession(entry); + await this.announceCreated({ sessionId, handle, source: 'resume' }); + return handle; + } catch (error) { + this.rollbackSession(entry); + throw error; } - await this.announceCreated({ sessionId, handle, source: 'resume' }); - return handle; } list(): readonly ISessionScopeHandle[] { const ready: ISessionScopeHandle[] = []; - for (const [id, handle] of this.sessions) { - if (!this.resuming.has(id)) ready.push(handle); + for (const entry of this.entries.values()) { + if (entry.phase === 'active') ready.push(entry.handle); } return ready; } async close(sessionId: string): Promise { - const handle = this.sessions.get(sessionId); - if (handle === undefined) return; - await this.announceWillClose({ sessionId, handle, reason: 'exit' }); - this.sessions.delete(sessionId); - await this.drainAgents(handle); - handle.dispose(); - await this.flushSessionTail(sessionId); - this.teardownLease(sessionId); - this._onDidCloseSession.fire({ sessionId }); + await this.closeSession(sessionId, 'close'); + } + + async forceAbort(sessionId: string): Promise { + const entry = this.entries.get(sessionId); + if (entry === undefined) return; + if (entry.phase !== 'flush-failed') { + throw new Error2( + ErrorCodes.SESSION_DURABILITY_FAILED, + `session ${sessionId} can only be force-aborted after a durability barrier failure`, + { details: { sessionId, phase: entry.phase } }, + ); + } + + entry.lease.assertWritable(); + await this.docs.update(entry.scope, 'state.json', (current) => { + if (current === undefined) { + throw new Error2( + ErrorCodes.SESSION_DURABILITY_FAILED, + `session ${sessionId} metadata is missing during force-abort`, + { details: { sessionId } }, + ); + } + return { + ...current, + custom: { + ...current.custom, + dirtyAbort: { reason: 'flush-failed', at: Date.now() }, + }, + }; + }); + this.telemetry.track2('session_dirty_abort', { session_id: sessionId, reason: 'flush-failed' }); + this.log.warn('force-aborting session after an ambiguous durability failure', { sessionId }); + this.dirtyAbortSession(entry); } async archive(sessionId: string): Promise { - const handle = this.sessions.get(sessionId); - if (handle === undefined) return; - const meta = handle.accessor.get(ISessionMetadata); - await meta.setArchived(true); - await this.drainAgents(handle); - this.event.publish({ - type: 'event.session.archived', - payload: { sessionId }, - }); - await this.announceWillClose({ sessionId, handle, reason: 'exit' }); - this.sessions.delete(sessionId); - handle.dispose(); - await this.flushSessionTail(sessionId); - this.teardownLease(sessionId); - this._onDidArchiveSession.fire({ sessionId }); + await this.closeSession(sessionId, 'archive'); } async restore(sessionId: string): Promise { @@ -417,10 +455,132 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec } } + private async closeSession(sessionId: string, kind: SessionCloseKind): Promise { + const entry = this.entries.get(sessionId); + if (entry === undefined) return; + if (entry.phase === 'preparing') { + this.rollbackSession(entry); + return; + } + if (entry.phase === 'active') { + entry.phase = 'draining'; + entry.closeKind = kind; + entry.closeStep = 0; + } + if (entry.closePromise !== undefined) { + await entry.closePromise; + return; + } + const closePromise = this.runSessionClose(sessionId, entry); + entry.closePromise = closePromise; + try { + await closePromise; + } finally { + if (this.entries.get(sessionId) === entry && entry.closePromise === closePromise) { + entry.closePromise = undefined; + } + } + } + + private async runSessionClose(sessionId: string, entry: SessionEntry): Promise { + const kind = entry.closeKind ?? 'close'; + const stepCount = kind === 'close' ? 3 : 5; + while (entry.closeStep < stepCount) { + if (kind === 'close') { + await this.runCloseStep(sessionId, entry); + } else { + await this.runArchiveStep(sessionId, entry); + } + entry.closeStep++; + } + try { + await this.flushSessionTail(sessionId, entry.scope); + } catch (error) { + entry.phase = 'flush-failed'; + throw error; + } + entry.registration.dispose(); + entry.lease.release(); + if (this.entries.get(sessionId) === entry) this.entries.delete(sessionId); + if (kind === 'archive') { + this._onDidArchiveSession.fire({ sessionId }); + } else { + this._onDidCloseSession.fire({ sessionId }); + } + } + + private async runCloseStep(sessionId: string, entry: SessionEntry): Promise { + switch (entry.closeStep) { + case 0: + await this.announceWillClose({ sessionId, handle: entry.handle, reason: 'exit' }); + return; + case 1: + await this.drainAgents(entry.handle); + return; + case 2: + this.disposeSessionHandle(entry); + return; + } + } + + private async runArchiveStep(sessionId: string, entry: SessionEntry): Promise { + switch (entry.closeStep) { + case 0: + await entry.handle.accessor.get(ISessionMetadata).setArchived(true); + return; + case 1: + await this.drainAgents(entry.handle); + return; + case 2: + this.event.publish({ + type: 'event.session.archived', + payload: { sessionId }, + }); + return; + case 3: + await this.announceWillClose({ sessionId, handle: entry.handle, reason: 'exit' }); + return; + case 4: + this.disposeSessionHandle(entry); + return; + } + } + + private activateSession(entry: SessionEntry): void { + if (this.entries.get(entry.handle.id) !== entry || entry.phase !== 'preparing') { + throw new Error2( + ErrorCodes.SESSION_LEASE_LOST, + `session ${entry.handle.id} was torn down before activation`, + { details: { sessionId: entry.handle.id } }, + ); + } + entry.lease.assertWritable(); + entry.phase = 'active'; + } + + private rollbackSession(entry: SessionEntry): void { + if (this.entries.get(entry.handle.id) === entry) this.entries.delete(entry.handle.id); + try { + this.disposeSessionHandle(entry); + } catch { + } + try { + entry.registration.dispose(); + } catch { + } + entry.lease.release(); + } + + private disposeSessionHandle(entry: SessionEntry): void { + if (entry.disposed) return; + entry.handle.dispose(); + entry.disposed = true; + } + async fork(opts: ForkSessionOptions): Promise { const sourceId = opts.sourceSessionId; - const sourceHandle = this.sessions.get(sourceId); + const sourceHandle = this.get(sourceId); const indexSummary = await this.index.get(sourceId); if (sourceHandle === undefined && indexSummary === undefined) { throw new Error2(ErrorCodes.SESSION_NOT_FOUND, `session ${sourceId} does not exist`); @@ -438,6 +598,7 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec // quiesce: the only requirement is a durable copy point, which // `copyAgentWire`'s flush provides. let targetId: string | undefined; + let targetEntry: SessionEntry | undefined; let target: ISessionScopeHandle | undefined; let targetSessionDir: string | undefined; try { @@ -452,26 +613,27 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec : await this.readMetaFromDisk(workspaceId, sourceId); targetId = opts.newSessionId ?? createSessionId(); - if (this.sessions.has(targetId) || (await this.index.get(targetId)) !== undefined) { + if (this.entries.has(targetId) || (await this.index.get(targetId)) !== undefined) { throw new Error2( ErrorCodes.SESSION_ALREADY_EXISTS, `Session "${targetId}" already exists`, ); } - targetSessionDir = this.bootstrap.sessionDir(workspaceId, targetId); - await this.copySessionFiles( - this.bootstrap.sessionDir(workspaceId, sourceId), - targetSessionDir, - ); - - target = await this.materializeSession({ + targetEntry = await this.materializeSession({ sessionId: targetId, workDir: workspace.root, }); + target = targetEntry.handle; const targetCtx = target.accessor.get(ISessionContext); + targetSessionDir = targetCtx.sessionDir; const targetMeta = target.accessor.get(ISessionMetadata); + await this.copySessionFiles( + this.bootstrap.sessionDir(workspaceId, sourceId), + targetCtx.sessionDir, + ); + const sourceAgents = sourceMeta?.agents ?? {}; const agentIds = Object.keys(sourceAgents); for (const agentId of agentIds) { @@ -507,6 +669,7 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec } await this.appendSessionIndexEntry(targetId, workspace.root, targetCtx.workspaceId); + this.activateSession(targetEntry); this._onDidForkSession.fire({ sourceSessionId: sourceId, sessionId: targetId, @@ -515,18 +678,7 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec await this.announceCreated({ sessionId: targetId, handle: target, source: 'fork' }); return target; } catch (error) { - if (targetId !== undefined) { - this.sessions.delete(targetId); - } - if (target !== undefined) { - try { - target.dispose(); - } catch { - } - } - if (targetId !== undefined) { - this.teardownLease(targetId); - } + if (targetEntry !== undefined) this.rollbackSession(targetEntry); if (targetSessionDir !== undefined) { await this.hostFs.remove(targetSessionDir).catch(() => {}); } @@ -552,7 +704,7 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec } private async resolveSourceTitle(sourceId: string): Promise { - const live = this.sessions.get(sourceId); + const live = this.get(sourceId); if (live !== undefined) { return (await live.accessor.get(ISessionMetadata).read()).title; } @@ -665,44 +817,23 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec } override dispose(): void { - for (const sessionId of this.leases.keys()) { - this.teardownLease(sessionId); - } + for (const entry of this.entries.values()) this.dirtyAbortSession(entry); super.dispose(); } private onLeaseLost(sessionId: string): void { this.log.error('session lease lost; tearing the session down', { sessionId }); - void this.close(sessionId).catch(() => {}); + const entry = this.entries.get(sessionId); + if (entry !== undefined) this.dirtyAbortSession(entry); } - private async assertNoActiveUnregisteredWriter( - sessionDir: string, - sessionId: string, - ): Promise { - if (!this.flags.enabled(MULTI_SERVER_FLAG_ID)) return; - const first = await this.hostFs.stat(sessionDir).catch(() => undefined); - if (first?.mtimeMs === undefined) return; - if (Date.now() - first.mtimeMs >= UNREGISTERED_WRITER_WINDOW_MS) return; - await sleep(UNREGISTERED_WRITER_RECHECK_DELAY_MS); - const second = await this.hostFs.stat(sessionDir).catch(() => undefined); - if (second?.mtimeMs === undefined) return; - if (second.mtimeMs > first.mtimeMs) { - throw new Error2( - ErrorCodes.SESSION_HELD_BY_PEER, - `session ${sessionId} directory is being written by an external writer; refusing to materialize it here`, - { details: { kind: 'unregistered-writer' } }, - ); - } - } - - private acquireSessionLease(sessionId: string): SessionLease { + private async acquireSessionLease(sessionId: string): Promise { const leasePath = sessionLeasePath(this.bootstrap.homeDir, sessionId); const contact = this.leaseContact.contact(); let lease: SessionLease | undefined; try { const prior = this.locks.inspect(leasePath); - const handle = this.locks.acquire(leasePath, { + const handle = await this.locks.acquire(leasePath, { heartbeat: { intervalMs: SESSION_LEASE_HEARTBEAT_INTERVAL_MS, ttlMs: SESSION_LEASE_TTL_MS, @@ -773,22 +904,28 @@ export class SessionLifecycleService extends Disposable implements ISessionLifec return { kind: 'held-by-peer', phase: 'creating', retry_after_ms: LEASE_CREATING_RETRY_AFTER_MS }; } - private async flushSessionTail(sessionId: string): Promise { + private async flushSessionTail(sessionId: string, scope: string): Promise { try { - await this.appendLogStore.flush(); + await this.appendLogStore.flush(scope); } catch (error) { this.log.warn('final journal flush failed while closing session', { sessionId, error: String(error), }); + throw error; } } - private teardownLease(sessionId: string): void { - const entry = this.leases.get(sessionId); - if (entry === undefined) return; - this.leases.delete(sessionId); - entry.registration.dispose(); + private dirtyAbortSession(entry: SessionEntry): void { + if (this.entries.get(entry.handle.id) === entry) this.entries.delete(entry.handle.id); + try { + this.disposeSessionHandle(entry); + } catch { + } + try { + entry.registration.dispose(); + } catch { + } entry.lease.release(); } @@ -824,13 +961,6 @@ function isMissingFileError(error: unknown): boolean { return code === 'ENOENT'; } -function sleep(ms: number): Promise { - return new Promise((resolvePromise) => { - const timer = setTimeout(resolvePromise, ms); - timer.unref?.(); - }); -} - function createSessionId(): string { return `session_${randomUUID()}`; } diff --git a/packages/agent-core-v2/src/app/telemetry/events.ts b/packages/agent-core-v2/src/app/telemetry/events.ts index 44fc04529..862038d56 100644 --- a/packages/agent-core-v2/src/app/telemetry/events.ts +++ b/packages/agent-core-v2/src/app/telemetry/events.ts @@ -453,6 +453,11 @@ export interface SessionLeaseHolderUnresponsiveEvent { session_id: string; } +export interface SessionDirtyAbortEvent { + session_id: string; + reason: 'flush-failed'; +} + export interface FirstLaunchEvent {} export interface ExitEvent { @@ -968,6 +973,14 @@ export const telemetryEventDefinitions = { comment: "A session's lease holder is alive but its heartbeat is past TTL (frozen).", properties: { session_id: 'Session whose holder is unresponsive' }, }), + session_dirty_abort: defineTelemetryEvent({ + owner: 'kimi-code', + comment: 'A session is force-aborted after its final durability barrier became ambiguous.', + properties: { + session_id: 'Session whose lease is being released after an ambiguous flush failure', + reason: 'Why the dirty abort was required', + }, + }), first_launch: defineTelemetryEvent({ owner: 'kimi-code', comment: 'The CLI runs for the first time on this device.', diff --git a/packages/agent-core-v2/src/app/workspace/fileWorkspacePersistence.ts b/packages/agent-core-v2/src/app/workspace/fileWorkspacePersistence.ts index 242769bbe..d41b6f63e 100644 --- a/packages/agent-core-v2/src/app/workspace/fileWorkspacePersistence.ts +++ b/packages/agent-core-v2/src/app/workspace/fileWorkspacePersistence.ts @@ -32,6 +32,10 @@ export class FileWorkspacePersistence implements IWorkspacePersistence { constructor(@IAtomicDocumentStore private readonly docs: IAtomicDocumentStore) {} + runExclusive(op: () => Promise): Promise { + return this.docs.runExclusive(WORKSPACE_CATALOG_SCOPE, WORKSPACE_CATALOG_KEY, op); + } + async load(): Promise { const file = await this.docs.get>( WORKSPACE_CATALOG_SCOPE, diff --git a/packages/agent-core-v2/src/app/workspace/workspacePersistence.ts b/packages/agent-core-v2/src/app/workspace/workspacePersistence.ts index 1567a62dd..975180b6d 100644 --- a/packages/agent-core-v2/src/app/workspace/workspacePersistence.ts +++ b/packages/agent-core-v2/src/app/workspace/workspacePersistence.ts @@ -21,9 +21,8 @@ * * `WorkspaceCatalog.raw` carries the opaque document the catalog was loaded * from; `save` re-applies the semantic view onto it so unknown top-level and - * entry fields written by other engine versions survive the round-trip — the - * read-modify-write contract of the shared file (design: - * `.tmp/refactor-watch-design-v2.md` §3.6). + * entry fields written by other engine versions survive the round-trip under + * the shared file's read-modify-write contract. */ import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; @@ -55,6 +54,7 @@ export interface WorkspaceCatalog { export interface IWorkspacePersistence { readonly _serviceBrand: undefined; + runExclusive(op: () => Promise): Promise; load(): Promise; save(catalog: WorkspaceCatalog): Promise; } diff --git a/packages/agent-core-v2/src/app/workspace/workspaceService.ts b/packages/agent-core-v2/src/app/workspace/workspaceService.ts index 873effd5d..55c00f982 100644 --- a/packages/agent-core-v2/src/app/workspace/workspaceService.ts +++ b/packages/agent-core-v2/src/app/workspace/workspaceService.ts @@ -10,12 +10,11 @@ * re-reads the file on every call), so a write-through cache would clobber * external additions and tombstones with stale state. * - * Two exclusion layers make each read-modify-write safe (design: - * `.tmp/refactor-watch-design-v2.md` §3.6): the in-process promise chain - * serializes ops (entered first, so cross-process waiting stays minimal), - * and an `ICrossProcessLockService` file lock (`workspaces.json.lock`, from - * `crossProcessLock`, resolved against the `bootstrap` home dir) makes the - * load → mutate → save burst atomic against other lock-aware v2 processes. + * Two exclusion layers make each read-modify-write safe: the in-process promise + * chain serializes ops (entered first, so cross-process waiting stays minimal), + * and the persistence store's transaction primitive locks the physical + * `workspaces.json` path, making the load → mutate → save burst atomic against + * other lock-aware v2 processes. * Unregistered writers (v1) stay best-effort: their lost updates are healed * by the session-index merge. Tombstone logic, format and key names are * unchanged, and unknown document fields round-trip verbatim via @@ -65,19 +64,13 @@ * as this directory's representative on the next `list()`. */ -import { basename, isAbsolute, join } from 'pathe'; +import { basename, isAbsolute } from 'pathe'; import { InstantiationType } from '#/_base/di/extensions'; import { LifecycleScope, registerScopedService } from '#/_base/di/scope'; import { encodeWorkDirKey, workspaceRootKey } from '#/_base/utils/workdir-slug'; import { ErrorCodes, Error2, unwrapErrorCause } from '#/errors'; -import { IBootstrapService } from '#/app/bootstrap/bootstrap'; import { IHostFileSystem } from '#/os/interface/hostFileSystem'; -import { - type CrossProcessLockAcquireOptions, - ICrossProcessLockService, - type CrossProcessLockWaitOptions, -} from '#/os/interface/crossProcessLock'; import { IFileSystemStorageService } from '#/persistence/interface/storage'; import { IWorkspaceService, type Workspace, type WorkspaceUpdate } from './workspace'; @@ -89,27 +82,18 @@ import { } from './workspaceAlias'; import { IWorkspacePersistence, type WorkspaceCatalog } from './workspacePersistence'; -const WORKSPACES_CATALOG_LOCK_OPTIONS: CrossProcessLockAcquireOptions & { - wait: CrossProcessLockWaitOptions; -} = { wait: { timeoutMs: 10_000 } }; - export class WorkspaceService implements IWorkspaceService { declare readonly _serviceBrand: undefined; /** Whether the once-per-process session-index sync already ran. */ private merged = false; private opQueue: Promise = Promise.resolve(); - private readonly catalogLockPath: string; constructor( @IWorkspacePersistence private readonly store: IWorkspacePersistence, @IFileSystemStorageService private readonly storage: IFileSystemStorageService, @IHostFileSystem private readonly hostFs: IHostFileSystem, - @IBootstrapService bootstrap: IBootstrapService, - @ICrossProcessLockService private readonly lock: ICrossProcessLockService, - ) { - this.catalogLockPath = join(bootstrap.homeDir, 'workspaces.json.lock'); - } + ) {} list(): Promise { return this.runExclusive(async () => { @@ -330,8 +314,7 @@ export class WorkspaceService implements IWorkspaceService { } private runExclusive(op: () => Promise): Promise { - const locked = (): Promise => - this.lock.withLock(this.catalogLockPath, WORKSPACES_CATALOG_LOCK_OPTIONS, op); + const locked = (): Promise => this.store.runExclusive(op); const next = this.opQueue.then(locked, locked); this.opQueue = next.then( () => {}, diff --git a/packages/agent-core-v2/src/os/backends/node-local/crossProcessLockService.ts b/packages/agent-core-v2/src/os/backends/node-local/crossProcessLockService.ts index 278afffd2..e0a6d3967 100644 --- a/packages/agent-core-v2/src/os/backends/node-local/crossProcessLockService.ts +++ b/packages/agent-core-v2/src/os/backends/node-local/crossProcessLockService.ts @@ -2,13 +2,11 @@ * `crossProcessLock` domain (L1) — `ICrossProcessLockService` implementation. * * Node-local backend for the cross-process exclusive file-lock protocol - * defined by `os/interface/crossProcessLock` (design: - * `.tmp/refactor-watch-design-v2.md` §3.3). Uses the synchronous `node:fs` - * API by design: every operation is a short burst (create / read / rename / - * beat) on low-frequency coordination paths — server lock, session lease, - * lock-in-RMW — and must never be called from turn/loop hot paths. Process - * probing goes through `createNodeProcessProbe`; every clock, pid, probe and - * token source is injectable for tests. + * defined by `os/interface/crossProcessLock`. Filesystem mutations are short + * synchronous bursts; contender settlement is asynchronous so a delayed + * process never forces the event loop to spin while another attempt exits. + * Process probing goes through `createNodeProcessProbe`; every clock, pid, + * probe and token source is injectable for tests. * * Protocol invariants implemented here: * @@ -20,16 +18,19 @@ * stale; a live identity-matching holder whose heartbeat is past ttl is * `holder-unresponsive` — reported, never seized. An identity that either * side cannot provide counts as matching (conservative). - * - Takeover is rename-isolated: the stale file is moved aside to - * `.stale.` before re-creating, and the freshly created - * payload is read back and confirmed against the new `lockId` — a creator - * frozen inside its create window cannot silently stomp the new lock. + * - Each attempt publishes a unique sidecar intent before inspecting the lock. + * A creator confirms its token, snapshots the foreign intents already + * present, and waits for that finite set to settle before returning. A + * delayed stale observer therefore either sees the new live generation and + * backs off, or steals it before settlement and causes the creator to fail + * rather than double-return; contenders arriving after the snapshot cannot + * starve the creator because they can only observe the new generation. * - Creation window: an empty/unparseable file younger than * `creationWindowMs` (default 5s) is `creating` (treated as held); past the * window it is stale. - * - Heartbeat is `write(position 0) + ftruncate + fsync` on the fd kept open - * from acquire — never tmp+rename, which would let a frozen old holder's - * next beat overwrite the lock that took it over. + * - The winning create fd stays open for every handle. Heartbeat and updates + * write only through that fd — never tmp+rename or path re-open — so a frozen + * old holder cannot overwrite a successor after losing the public path. * * Bound at App scope. */ @@ -40,13 +41,14 @@ import { ftruncateSync, mkdirSync, openSync, + readdirSync, readFileSync, renameSync, statSync, unlinkSync, writeSync, } from 'node:fs'; -import { dirname } from 'node:path'; +import { basename, dirname, join } from 'node:path'; import { ulid } from 'ulid'; @@ -71,6 +73,7 @@ import { createNodeProcessProbe } from './processProbe'; const DEFAULT_CREATION_WINDOW_MS = 5_000; const DEFAULT_WAIT_RETRY_INTERVAL_MS = 50; +const DEFAULT_SETTLE_RETRY_INTERVAL_MS = 10; const MAX_ACQUIRE_ATTEMPTS = 3; function readErrno(error: unknown): string | undefined { @@ -125,6 +128,19 @@ interface DiskLockPayload { [extra: string]: unknown; } +interface DiskIntentPayload { + intent_id?: string; + state?: 'active' | 'settled'; + pid?: number; + process_started_at?: string; +} + +interface RegisteredIntent { + readonly path: string; + readonly token: string; + readonly fd: number; +} + function renderPayloadJson(payload: CrossProcessLockPayload): string { const { lockId, instanceId, pid, processStartedAt, address, heartbeatAt, ...extras } = payload; const disk: DiskLockPayload = { @@ -239,6 +255,9 @@ class NodeCrossProcessLockHandle implements ICrossProcessLockHandle { if (readErrno(error) === 'ENOENT') throw lostError(this.lockPath, 'updating the payload'); throw toLockIoError(error, { path: this.lockPath, op: 'update' }); } + if (readPayloadFromPath(this.lockPath)?.lockId !== this.lockId) { + throw lostError(this.lockPath, 'confirming the updated payload'); + } } release(): void { @@ -267,10 +286,6 @@ class NodeCrossProcessLockHandle implements ICrossProcessLockHandle { this.writePayload(); } - sealPidOnly(): void { - this.closeFd(); - } - private tick(): void { if (this._released || this._fd < 0) return; try { @@ -308,14 +323,7 @@ class NodeCrossProcessLockHandle implements ICrossProcessLockHandle { fsyncSync(this._fd); return; } - const fd = openSync(this.lockPath, 'r+'); - try { - writeSync(fd, data, 0, data.length, 0); - ftruncateSync(fd, data.length); - fsyncSync(fd); - } finally { - closeSync(fd); - } + throw lostError(this.lockPath, 'writing the payload'); } private stopHeartbeat(): void { @@ -342,15 +350,19 @@ export class CrossProcessLockService implements ICrossProcessLockService { private readonly selfPid: number; private readonly probe: ProcessProbe; private readonly newLockId: () => string; + private readonly newAttemptId: () => string; private readonly instanceId: string; private readonly sleep: (ms: number) => Promise; + private readonly beforeStaleIsolation: (() => void | Promise) | undefined; constructor(deps: CrossProcessLockServiceDeps = {}) { this.now = deps.now ?? Date.now; this.selfPid = deps.selfPid ?? process.pid; this.probe = deps.probeProcess ?? createNodeProcessProbe(); this.newLockId = deps.newLockId ?? ulid; + this.newAttemptId = deps.newAttemptId ?? ulid; this.instanceId = deps.instanceId ?? ulid(); + this.beforeStaleIsolation = deps.beforeStaleIsolation; this.sleep = deps.sleep ?? ((ms) => @@ -360,10 +372,10 @@ export class CrossProcessLockService implements ICrossProcessLockService { })); } - acquire( + async acquire( lockPath: string, options: CrossProcessLockAcquireOptions = {}, - ): ICrossProcessLockHandle { + ): Promise { try { mkdirSync(dirname(lockPath), { recursive: true }); } catch (error) { @@ -371,32 +383,82 @@ export class CrossProcessLockService implements ICrossProcessLockService { } const creationWindowMs = options.creationWindowMs ?? DEFAULT_CREATION_WINDOW_MS; const observedTtlMs = options.heartbeat?.ttlMs ?? creationWindowMs; - let lastHolder: CrossProcessLockPayload | undefined; - for (let attempt = 0; attempt < MAX_ACQUIRE_ATTEMPTS; attempt++) { - let fd: number; - try { - fd = openSync(lockPath, 'wx', 0o600); - } catch (error) { - if (readErrno(error) !== 'EEXIST') { - throw toLockIoError(error, { path: lockPath, op: 'open' }); + const intent = this.registerIntent(lockPath); + let acquiredHandle: ICrossProcessLockHandle | undefined; + let primaryError: unknown; + let hasPrimaryError = false; + try { + let lastHolder: CrossProcessLockPayload | undefined; + for (let attempt = 0; attempt < MAX_ACQUIRE_ATTEMPTS; attempt++) { + let fd: number; + try { + fd = openSync(lockPath, 'wx', 0o600); + } catch (error) { + if (readErrno(error) !== 'EEXIST') { + throw toLockIoError(error, { path: lockPath, op: 'open' }); + } + const inspection = this.classify(lockPath, creationWindowMs); + lastHolder = inspection.payload; + switch (inspection.state) { + case 'free': + continue; + case 'creating': + throw heldError(lockPath, 'creating', undefined); + case 'held': + throw heldError( + lockPath, + this.reasonForHeld(inspection.payload, observedTtlMs), + inspection.payload, + ); + case 'stale': + if (!(await this.isolateStale(lockPath, inspection, creationWindowMs, intent.path))) { + continue; + } + continue; + } } - const inspection = this.classify(lockPath, creationWindowMs); - lastHolder = inspection.payload; - switch (inspection.state) { - case 'free': - continue; - case 'creating': - throw heldError(lockPath, 'creating', undefined); - case 'held': - throw heldError(lockPath, this.reasonForHeld(inspection.payload, observedTtlMs), inspection.payload); - case 'stale': - this.isolateStale(lockPath, inspection); - continue; + const handle = this.completeAcquire(lockPath, fd, options); + try { + await this.settleAcquire(handle, intent.path, creationWindowMs); + acquiredHandle = handle; + break; + } catch (error) { + handle.release(); + throw error; } } - return this.completeAcquire(lockPath, fd, options); + if (acquiredHandle === undefined) throw heldError(lockPath, 'held', lastHolder); + } catch (error) { + hasPrimaryError = true; + primaryError = error; } - throw heldError(lockPath, 'held', lastHolder); + let cleanupError: unknown; + let hasCleanupError = false; + try { + try { + this.markIntentSettled(intent); + } catch (error) { + hasCleanupError = true; + cleanupError = error; + } + try { + this.removeIntent(intent); + } catch (error) { + if (!hasCleanupError) { + hasCleanupError = true; + cleanupError = error; + } + } + } finally { + this.closeIntent(intent); + } + if (hasCleanupError && acquiredHandle !== undefined) acquiredHandle.release(); + if (hasPrimaryError) throw primaryError; + if (hasCleanupError) throw cleanupError; + if (acquiredHandle === undefined) { + throw new Error('cross-process lock acquisition finished without a result'); + } + return acquiredHandle; } async acquireWithWait( @@ -407,7 +469,7 @@ export class CrossProcessLockService implements ICrossProcessLockService { let lastError: CrossProcessLockError | undefined; for (;;) { try { - return this.acquire(lockPath, options); + return await this.acquire(lockPath, options); } catch (error) { if (!(error instanceof CrossProcessLockError) || error.code !== CrossProcessLockErrorCode.Held) { throw error; @@ -488,13 +550,23 @@ export class CrossProcessLockService implements ICrossProcessLockService { return 'held'; } - private isolateStale(lockPath: string, inspection: CrossProcessLockInspection): void { + private async isolateStale( + lockPath: string, + inspection: CrossProcessLockInspection, + creationWindowMs: number, + intentPath: string, + ): Promise { + const current = this.classify(lockPath, creationWindowMs); + if (current.state !== 'stale') return false; + if (inspection.payload?.lockId !== current.payload?.lockId) return false; + await this.beforeStaleIsolation?.(); const rawLockId = inspection.payload?.lockId; const staleLockId = rawLockId !== undefined && rawLockId !== '' ? rawLockId : 'unknown'; try { - renameSync(lockPath, `${lockPath}.stale.${staleLockId}`); + renameSync(lockPath, `${lockPath}.stale.${staleLockId}.${basename(intentPath)}`); + return true; } catch (error) { - if (readErrno(error) === 'ENOENT') return; + if (readErrno(error) === 'ENOENT') return false; throw toLockIoError(error, { path: lockPath, op: 'rename-stale' }); } } @@ -539,14 +611,173 @@ export class CrossProcessLockService implements ICrossProcessLockService { handle.release(); throw lostError(lockPath, 'confirming the newly created payload'); } - if (options.heartbeat !== undefined) { - handle.startHeartbeat(); - } else { - handle.sealPidOnly(); - } + if (options.heartbeat !== undefined) handle.startHeartbeat(); return handle; } + private registerIntent(lockPath: string): RegisteredIntent { + const token = this.newAttemptId(); + const intentPath = `${lockPath}.intent.${token}`; + const startedAt = this.safeProbe(this.selfPid).processStartedAt; + const payload: DiskIntentPayload = { intent_id: token, state: 'active', pid: this.selfPid }; + if (startedAt !== undefined) payload.process_started_at = startedAt; + let fd: number | undefined; + try { + fd = openSync(intentPath, 'wx+', 0o600); + this.writeIntent(fd, payload); + return { path: intentPath, token, fd }; + } catch (error) { + if (fd !== undefined) this.closeFd(fd); + try { + unlinkSync(intentPath); + } catch { + // best effort + } + throw toLockIoError(error, { path: intentPath, op: 'register-intent' }); + } + } + + private markIntentSettled(intent: RegisteredIntent): void { + const current = this.readIntent(intent.path); + if (current?.intent_id !== intent.token) return; + this.writeIntent(intent.fd, { ...current, intent_id: intent.token, state: 'settled' }); + } + + private removeIntent(intent: RegisteredIntent): void { + try { + const current = this.readIntent(intent.path); + if (current?.intent_id !== intent.token) return; + unlinkSync(intent.path); + } catch (error) { + if (readErrno(error) !== 'ENOENT') { + throw toLockIoError(error, { path: intent.path, op: 'remove-intent' }); + } + } + } + + private readIntent(intentPath: string): DiskIntentPayload | undefined { + let raw: string; + try { + raw = readFileSync(intentPath, 'utf8'); + } catch (error) { + if (readErrno(error) === 'ENOENT') return undefined; + throw toLockIoError(error, { path: intentPath, op: 'read-intent' }); + } + try { + const parsed: unknown = JSON.parse(raw); + return parsed !== null && typeof parsed === 'object' ? (parsed as DiskIntentPayload) : undefined; + } catch { + return undefined; + } + } + + private writeIntent(fd: number, payload: DiskIntentPayload): void { + const data = Buffer.from(JSON.stringify(payload), 'utf8'); + writeSync(fd, data, 0, data.length, 0); + ftruncateSync(fd, data.length); + fsyncSync(fd); + } + + private closeIntent(intent: RegisteredIntent): void { + this.closeFd(intent.fd); + } + + private closeFd(fd: number): void { + try { + closeSync(fd); + } catch { + // best effort + } + } + + private async settleAcquire( + handle: ICrossProcessLockHandle, + ownIntentPath: string, + timeoutMs: number, + ): Promise { + const startedAt = this.now(); + const contenders = this.snapshotForeignIntents(handle.lockPath, ownIntentPath); + for (;;) { + if (!handle.checkHeld()) { + throw lostError(handle.lockPath, 'settling concurrent contenders'); + } + if (!this.hasLiveIntent(contenders, timeoutMs)) return; + if (this.now() - startedAt >= timeoutMs) { + throw heldError(handle.lockPath, 'creating', readPayloadFromPath(handle.lockPath)); + } + await this.sleep(DEFAULT_SETTLE_RETRY_INTERVAL_MS); + } + } + + private snapshotForeignIntents( + lockPath: string, + ownIntentPath: string, + ): Set { + const dir = dirname(lockPath); + const prefix = `${basename(lockPath)}.intent.`; + const ownName = basename(ownIntentPath); + let names: string[]; + try { + names = readdirSync(dir); + } catch (error) { + throw toLockIoError(error, { path: dir, op: 'list-intents' }); + } + return new Set( + names + .filter((name) => name.startsWith(prefix) && name !== ownName) + .map((name) => join(dir, name)), + ); + } + + private hasLiveIntent(intentPaths: Set, creationWindowMs: number): boolean { + for (const path of intentPaths) { + let raw: string; + try { + raw = readFileSync(path, 'utf8'); + } catch (error) { + if (readErrno(error) === 'ENOENT') { + intentPaths.delete(path); + continue; + } + throw toLockIoError(error, { path, op: 'read-intent' }); + } + let payload: DiskIntentPayload | undefined; + try { + const parsed: unknown = JSON.parse(raw); + if (parsed !== null && typeof parsed === 'object') payload = parsed as DiskIntentPayload; + } catch { + // handled as a creation-window intent below + } + if (payload?.state === 'settled') { + this.removeIntent({ path, token: payload.intent_id ?? '', fd: -1 }); + intentPaths.delete(path); + continue; + } + const intentPid = payload?.pid; + if (!isProbingPid(intentPid ?? 0)) { + const mtimeMs = this.readMtimeMs(path); + if (mtimeMs !== undefined && this.now() - mtimeMs >= creationWindowMs) { + this.removeIntent({ path, token: payload?.intent_id ?? '', fd: -1 }); + intentPaths.delete(path); + continue; + } + return true; + } + const probed = this.safeProbe(intentPid as number); + const reused = + payload?.process_started_at !== undefined && + probed.processStartedAt !== undefined && + payload.process_started_at !== probed.processStartedAt; + if (!probed.alive || reused) { + this.removeIntent({ path, token: payload?.intent_id ?? '', fd: -1 }); + intentPaths.delete(path); + continue; + } + return true; + } + return false; + } + private safeProbe(pid: number): { alive: boolean; processStartedAt?: string } { try { return this.probe(pid); diff --git a/packages/agent-core-v2/src/os/interface/crossProcessLock.ts b/packages/agent-core-v2/src/os/interface/crossProcessLock.ts index da35aa1b8..598a62f20 100644 --- a/packages/agent-core-v2/src/os/interface/crossProcessLock.ts +++ b/packages/agent-core-v2/src/os/interface/crossProcessLock.ts @@ -2,7 +2,7 @@ * `crossProcessLock` domain (L1) — cross-process exclusive file-lock contract. * * Defines `ICrossProcessLockService`, the single lock protocol that replaces the - * repo's ad-hoc lockfiles (design: `.tmp/refactor-watch-design-v2.md` §3.3). + * repo's ad-hoc lockfiles. * One JSON lock file per resource, created with `O_EXCL`. Protocol invariants: * * - Token-guarded: every acquire generates a fresh `lockId` (ulid); release, @@ -13,16 +13,17 @@ * gone silent (`alive-unresponsive` — alert, never seize). Pid death, or a * pid whose identity no longer matches (pid reused by a new process), makes * the lock stale. - * - Takeover is rename-isolated: the stale file is renamed aside to - * `.stale.` before re-creating, then the new payload is read - * back and confirmed — a creator frozen inside its create window (SIGSTOP) - * cannot silently stomp the new lock when it resumes. + * - Every acquisition attempt registers a process-owned contender intent before + * inspecting the lock. A creator does not return until every older in-flight + * contender has either completed or disappeared. This closes the delayed + * stale-observer race where a contender could quarantine a newer generation + * after its creator had already returned. * - Creation window: an empty or unparseable file younger than the creation * window is "creating" (treated as held, no address yet); only past the * window may it be treated as stale. - * - Heartbeat, for modes that use it, is `pwrite + ftruncate + fsync` on the - * fd kept open from acquire — never tmp+rename, which would let a frozen old - * holder's next beat overwrite the lock that took it over. + * - The fd opened by the winning `O_EXCL` create stays open for the handle's + * lifetime. Heartbeat and payload updates write only through that fd, so a + * holder that lost the public path can never overwrite its successor. * * The on-disk JSON is flat and snake_case, matching operator-facing lock * conventions; the six known protocol keys map to the camelCase fields of @@ -70,7 +71,7 @@ export interface CrossProcessLockWaitOptions { } export interface CrossProcessLockAcquireOptions { - /** Heartbeat mode. Omit for pid-only locks (no fd kept, no beats). */ + /** Heartbeat mode. Omit for pid-only locks. */ readonly heartbeat?: CrossProcessLockHeartbeatOptions; /** Milliseconds an empty/unparseable lock file counts as "creating" rather than stale. Default 5000 for heartbeat-less modes. */ @@ -133,7 +134,7 @@ export interface ICrossProcessLockService { acquire( lockPath: string, options?: CrossProcessLockAcquireOptions, - ): ICrossProcessLockHandle; + ): Promise; /** Blocking acquisition for short critical sections (lock-in-RMW): retries while the lock is held/creating until `wait.timeoutMs` elapses. */ @@ -171,8 +172,10 @@ export interface CrossProcessLockServiceDeps { readonly selfPid?: number; readonly probeProcess?: ProcessProbe; readonly newLockId?: () => string; + readonly newAttemptId?: () => string; readonly instanceId?: string; readonly sleep?: (ms: number) => Promise; + readonly beforeStaleIsolation?: () => void | Promise; } export const OsLockErrors = { diff --git a/packages/agent-core-v2/src/persistence/backends/memory/inMemoryStorageService.ts b/packages/agent-core-v2/src/persistence/backends/memory/inMemoryStorageService.ts index cf5f716c0..f82948a76 100644 --- a/packages/agent-core-v2/src/persistence/backends/memory/inMemoryStorageService.ts +++ b/packages/agent-core-v2/src/persistence/backends/memory/inMemoryStorageService.ts @@ -38,6 +38,7 @@ export class InMemoryStorageService implements IFileSystemStorageService { private readonly scopes = new Map>(); private readonly watchers = new Map(); + private readonly operationQueues = new Map>(); async read(scope: string, key: string): Promise { return this.scopes.get(scope)?.get(key); @@ -131,6 +132,20 @@ export class InMemoryStorageService implements IFileSystemStorageService { }; } + runExclusive(scope: string, key: string, op: () => Promise): Promise { + const id = this.watchKey(scope, key); + const previous = this.operationQueues.get(id) ?? Promise.resolve(); + const result = previous.then(op); + const tail = result.then( + () => undefined, + () => undefined, + ); + this.operationQueues.set(id, tail); + return result.finally(() => { + if (this.operationQueues.get(id) === tail) this.operationQueues.delete(id); + }); + } + private notifyWatchers(scope: string, key: string): void { this.watchers.get(this.watchKey(scope, key))?.emitter.fire(); } diff --git a/packages/agent-core-v2/src/persistence/backends/node-fs/appendLogStore.ts b/packages/agent-core-v2/src/persistence/backends/node-fs/appendLogStore.ts index 4421f34a7..4ca54db20 100644 --- a/packages/agent-core-v2/src/persistence/backends/node-fs/appendLogStore.ts +++ b/packages/agent-core-v2/src/persistence/backends/node-fs/appendLogStore.ts @@ -9,9 +9,10 @@ * Serializes whole-log rewrites with live appends, preserves queued or * in-flight records across the atomic replacement, keeps ambiguous append and * rewrite failures sticky, keeps the shared flush pending until the - * post-rewrite drain is durable, waits every key before a global flush reports - * an error, and preserves per-key storage ordering while acquired buffers - * retire and hand off to replacement owners. Session-scoped writes (journal + * post-rewrite drain is durable, supports scoped durability barriers, waits + * every selected key before a flush reports an error, and preserves failed + * retired buffers so their in-memory tail remains observable while replacement + * owners wait for the prior generation. Session-scoped writes (journal * bytes under `sessions//`) are fenced: `drain` and `rewrite` * re-verify the session's registered `ISessionWriteAuthority` through * `IWriteAuthorityRegistry` immediately before bytes hit storage, a session @@ -135,14 +136,12 @@ export class AppendLogStore implements IAppendLogStore { await this.ownFlush(scope, key, state, rewrite); } - async flush(): Promise { + async flush(scopePrefix?: string): Promise { const inFlight: Promise[] = []; for (const [id, state] of this.logs) { const { scope, key } = fromLogId(id); + if (!matchesScope(scope, scopePrefix)) continue; inFlight.push(this.flushState(scope, key, state)); - // A retired buffer's fire-and-forget final flush must settle before a - // global flush reports, or the close path can return with the session - // tail still in flight. Its errors stay swallowed per the release path. if (state.retirement !== undefined) inFlight.push(state.retirement); } const settled = await Promise.allSettled(inFlight); @@ -208,16 +207,14 @@ export class AppendLogStore implements IAppendLogStore { state.refCount--; if (state.refCount > 0) return; state.retired = true; - state.retirement = this.settleRetiredState(scope, key, state).catch(() => undefined); + state.retirement = this.settleRetiredState(scope, key, state); + void state.retirement.catch((error) => state.onError?.(error)); } private async settleRetiredState(scope: string, key: string, state: LogState): Promise { - try { - await this.flushState(scope, key, state); - } finally { - const id = logId(scope, key); - if (this.logs.get(id) === state) this.logs.delete(id); - } + await this.flushState(scope, key, state); + const id = logId(scope, key); + if (this.logs.get(id) === state) this.logs.delete(id); } private ownFlush( @@ -302,6 +299,10 @@ function fromLogId(id: string): { scope: string; key: string } { return { scope: id.slice(0, index), key: id.slice(index + 1) }; } +function matchesScope(scope: string, scopePrefix: string | undefined): boolean { + return scopePrefix === undefined || scope === scopePrefix || scope.startsWith(`${scopePrefix}/`); +} + function encodeBatch(records: readonly unknown[]): Uint8Array { if (records.length === 0) return new Uint8Array(0); const content = records.map((record) => JSON.stringify(record) + '\n').join(''); diff --git a/packages/agent-core-v2/src/persistence/backends/node-fs/atomicDocumentStore.ts b/packages/agent-core-v2/src/persistence/backends/node-fs/atomicDocumentStore.ts index ca9532299..7388b94ac 100644 --- a/packages/agent-core-v2/src/persistence/backends/node-fs/atomicDocumentStore.ts +++ b/packages/agent-core-v2/src/persistence/backends/node-fs/atomicDocumentStore.ts @@ -48,6 +48,7 @@ export const tomlDocumentCodec: DocumentCodec = { class AtomicDocumentStoreBase implements IAtomicDocumentStore { declare readonly _serviceBrand: undefined; + private readonly operationQueues = new Map>(); constructor( private readonly storage: IFileSystemStorageService, @@ -75,6 +76,35 @@ class AtomicDocumentStoreBase implements IAtomicDocumentStore { await this.storage.write(scope, key, this.codec.encode(value), { atomic: true }); } + update( + scope: string, + key: string, + mutate: (current: T | undefined) => T | Promise, + ): Promise { + return this.runExclusive(scope, key, async () => { + const next = await mutate(await this.get(scope, key)); + await this.set(scope, key, next); + return next; + }); + } + + runExclusive(scope: string, key: string, op: () => Promise): Promise { + if (this.storage.runExclusive !== undefined) { + return this.storage.runExclusive(scope, key, op); + } + const id = `${scope}\0${key}`; + const previous = this.operationQueues.get(id) ?? Promise.resolve(); + const result = previous.then(op); + const tail = result.then( + () => undefined, + () => undefined, + ); + this.operationQueues.set(id, tail); + return result.finally(() => { + if (this.operationQueues.get(id) === tail) this.operationQueues.delete(id); + }); + } + async delete(scope: string, key: string): Promise { await this.storage.delete(scope, key); } diff --git a/packages/agent-core-v2/src/persistence/backends/node-fs/fileStorageService.ts b/packages/agent-core-v2/src/persistence/backends/node-fs/fileStorageService.ts index c1e1286d0..43bd55e6c 100644 --- a/packages/agent-core-v2/src/persistence/backends/node-fs/fileStorageService.ts +++ b/packages/agent-core-v2/src/persistence/backends/node-fs/fileStorageService.ts @@ -10,6 +10,8 @@ * fsync, so the replacement is both atomic and durable. * - `append` → `open('a')` + write + `fh.sync()` (when `durable`), plus a * one-time directory fsync per scope. + * - `runExclusive` → the shared cross-process lock protocol on + * `.lock`, bounding lock waits to 10 seconds. * - `watch` → chokidar on the parent directory, filtered to the exact key and * debounced, so it survives atomic-replace renames and observes a * file that does not exist yet at subscription time. @@ -27,9 +29,15 @@ import { FSWatcher } from 'chokidar'; import { dirname, join, normalize } from 'pathe'; import { DisposableStore, combinedDisposable, toDisposable, type IDisposable } from '#/_base/di/lifecycle'; +import { optional } from '#/_base/di/instantiation'; import { Emitter, type Event } from '#/_base/event'; import { onUnexpectedError } from '#/_base/errors/unexpectedError'; import { atomicWrite, syncDir } from '#/_base/utils/fs'; +import { + CrossProcessLockError, + CrossProcessLockErrorCode, + ICrossProcessLockService, +} from '#/os/interface/crossProcessLock'; import type { IFileSystemStorageService, @@ -37,9 +45,10 @@ import type { StorageReadRange, StorageWriteOptions, } from '#/persistence/interface/storage'; -import { toStorageIoError } from '#/persistence/interface/storage'; +import { StorageError, StorageErrors, toStorageIoError } from '#/persistence/interface/storage'; const WATCH_DEBOUNCE_MS = 150; +const STORAGE_LOCK_WAIT_TIMEOUT_MS = 10_000; function isEnoent(error: unknown): boolean { return (error as NodeJS.ErrnoException).code === 'ENOENT'; @@ -54,6 +63,7 @@ export class FileStorageService implements IFileSystemStorageService { private readonly baseDir: string, private readonly dirMode?: number, private readonly fileMode?: number, + @optional(ICrossProcessLockService) private readonly locks?: ICrossProcessLockService, ) {} async read(scope: string, key: string): Promise { @@ -220,6 +230,37 @@ export class FileStorageService implements IFileSystemStorageService { }; } + async runExclusive(scope: string, key: string, op: () => Promise): Promise { + const filePath = this.path(scope, key); + const lockPath = `${filePath}.lock`; + if (this.locks === undefined) { + throw new StorageError( + StorageErrors.codes.STORAGE_IO_FAILED, + 'file storage transaction requires a cross-process lock service', + { details: { path: filePath, op: 'lock' } }, + ); + } + try { + return await this.locks.withLock( + lockPath, + { wait: { timeoutMs: STORAGE_LOCK_WAIT_TIMEOUT_MS } }, + op, + ); + } catch (error) { + if (!(error instanceof CrossProcessLockError)) throw error; + if ( + error.code === CrossProcessLockErrorCode.Held || + error.code === CrossProcessLockErrorCode.WaitTimeout + ) { + throw new StorageError(StorageErrors.codes.STORAGE_LOCKED, 'storage transaction is locked', { + details: { path: filePath, op: 'lock' }, + cause: error, + }); + } + throw toStorageIoError(error, { path: lockPath, op: 'lock' }); + } + } + async flush(): Promise { } diff --git a/packages/agent-core-v2/src/persistence/interface/appendLogStore.ts b/packages/agent-core-v2/src/persistence/interface/appendLogStore.ts index c24183649..82352d20c 100644 --- a/packages/agent-core-v2/src/persistence/interface/appendLogStore.ts +++ b/packages/agent-core-v2/src/persistence/interface/appendLogStore.ts @@ -15,8 +15,10 @@ * later flush cannot duplicate data by guessing whether storage committed it. * A valid explicit `rewrite` is the recovery boundary: a successful atomic * replacement clears that failure before the preserved live tail drains. - * `flush` and `close` wait for every keyed buffer to settle before reporting - * the first failure in stable key insertion order. + * `flush(scopePrefix)` waits for the matching scope and descendants; omitting + * the prefix and `close` wait for every keyed buffer. Retired generations stay + * reachable after a failed final flush so later flushes report the same sticky + * failure instead of silently losing the in-memory tail. * * This file ships the interface, error class, and DI token only. * The concrete `AppendLogStore` implementation lives in @@ -52,7 +54,7 @@ export interface IAppendLogStore { append(scope: string, key: string, record: R, options?: AppendLogOptions): void; read(scope: string, key: string): AsyncIterable; rewrite(scope: string, key: string, records: readonly R[]): Promise; - flush(): Promise; + flush(scopePrefix?: string): Promise; close(): Promise; acquire(scope: string, key: string): IDisposable; } diff --git a/packages/agent-core-v2/src/persistence/interface/atomicDocumentStore.ts b/packages/agent-core-v2/src/persistence/interface/atomicDocumentStore.ts index 2d109d49b..2a413f13a 100644 --- a/packages/agent-core-v2/src/persistence/interface/atomicDocumentStore.ts +++ b/packages/agent-core-v2/src/persistence/interface/atomicDocumentStore.ts @@ -24,6 +24,12 @@ export interface IAtomicDocumentStore { get(scope: string, key: string): Promise; set(scope: string, key: string, value: T): Promise; + update( + scope: string, + key: string, + mutate: (current: T | undefined) => T | Promise, + ): Promise; + runExclusive(scope: string, key: string, op: () => Promise): Promise; delete(scope: string, key: string): Promise; list(scope: string, prefix?: string): Promise; watch(scope: string, key: string): Event; diff --git a/packages/agent-core-v2/src/persistence/interface/storage.ts b/packages/agent-core-v2/src/persistence/interface/storage.ts index 83c57ca17..5bbe5a849 100644 --- a/packages/agent-core-v2/src/persistence/interface/storage.ts +++ b/packages/agent-core-v2/src/persistence/interface/storage.ts @@ -7,6 +7,8 @@ * * - `write` — atomic whole-value replacement (the `Config` access pattern). * - `append` — ordered, durable byte extension (the `Record` access pattern). + * - `runExclusive` — keyed read-modify-write exclusion when the backend can + * coordinate independent processes. * * They are not interchangeable: building `append` on top of `write` is O(n) * per append, and building `write` on top of `append` yields awkward "read @@ -128,6 +130,7 @@ export interface IFileSystemStorageService { list(scope: string, prefix?: string): Promise; delete(scope: string, key: string): Promise; watch?(scope: string, key: string): Event; + runExclusive?(scope: string, key: string, op: () => Promise): Promise; flush(): Promise; close(): Promise; } diff --git a/packages/agent-core-v2/src/persistence/interface/writeAuthority.ts b/packages/agent-core-v2/src/persistence/interface/writeAuthority.ts index a54639995..f2ce8567d 100644 --- a/packages/agent-core-v2/src/persistence/interface/writeAuthority.ts +++ b/packages/agent-core-v2/src/persistence/interface/writeAuthority.ts @@ -2,9 +2,8 @@ * `persistence/interface` — session-scoped write fencing contract. * * Defines `ISessionWriteAuthority`, the per-session lease proof that a Store - * write must re-verify immediately before its bytes hit storage (design: - * `.tmp/refactor-watch-design-v2.md` §3.4.2/§3.4.5 — the pre-commit lease - * re-read is the hard gate and must fail closed), and the App-scoped + * write must re-verify immediately before its bytes hit storage (the + * pre-commit lease re-read is the hard gate and must fail closed), and the App-scoped * `IWriteAuthorityRegistry` the `AppendLogStore` resolves authorities through. * The registry never creates semantics of its own: it only maps `sessionId` * to the authority the session lifecycle registered, so a write for a diff --git a/packages/agent-core-v2/src/session/agentLifecycle/agentLifecycleService.ts b/packages/agent-core-v2/src/session/agentLifecycle/agentLifecycleService.ts index a1d7e978c..df61abfbd 100644 --- a/packages/agent-core-v2/src/session/agentLifecycle/agentLifecycleService.ts +++ b/packages/agent-core-v2/src/session/agentLifecycle/agentLifecycleService.ts @@ -242,9 +242,9 @@ export class AgentLifecycleService extends Disposable implements IAgentLifecycle // force-injections that used to pull it up, so it must be resolved here or // tool execution would run without policy adjudication. handle.accessor.get(IAgentPermissionGate); - // File fencing hooks Read/Write/Edit on the shared Session file ledger: - // resolve after externalHooks so permission/external hook participants - // register first on the executor's hook slots. + // File fencing wraps Read/Write/Edit execution on the shared Session file + // ledger. Resolve after externalHooks so permission and external preflight + // participants register first. handle.accessor.get(IAgentFileFencingService); handle.accessor.get(IAgentMcpService); // Agent plugin service: registers main-agent-only plugin session-start diff --git a/packages/agent-core-v2/src/session/errors.ts b/packages/agent-core-v2/src/session/errors.ts index d9fc6d255..e1b74998f 100644 --- a/packages/agent-core-v2/src/session/errors.ts +++ b/packages/agent-core-v2/src/session/errors.ts @@ -16,6 +16,7 @@ export const SessionErrors = { SESSION_INIT_FAILED: 'session.init_failed', SESSION_HELD_BY_PEER: 'session.held_by_peer', SESSION_LEASE_LOST: 'session.lease_lost', + SESSION_DURABILITY_FAILED: 'session.durability_failed', }, retryable: ['session.fork_active_turn'], } as const satisfies ErrorDomain; diff --git a/packages/agent-core-v2/src/session/sessionFileLedger/fileLedger.ts b/packages/agent-core-v2/src/session/sessionFileLedger/fileLedger.ts index 975b79a43..5674f7455 100644 --- a/packages/agent-core-v2/src/session/sessionFileLedger/fileLedger.ts +++ b/packages/agent-core-v2/src/session/sessionFileLedger/fileLedger.ts @@ -4,12 +4,11 @@ * Defines the `ISessionFileLedger` that remembers, per normalized absolute * path, the on-disk stat tuple (`ino`, `mtimeMs`, `size`, existence) this * session last successfully read or wrote, together with the - * `sessionFsWatch` tick captured at that moment. Before a Write/Edit, - * `compare` decides from the ledger entry plus live dirty signals whether - * the target was modified outside this session, punching a single stat only - * when a dirty signal is newer than the baseline (an unchanged tuple means - * the signal was the session's own write echo). Targets outside every - * watched root degrade to the same comparison without dirty signals. + * `sessionFsWatch` tick captured before that stat. Before every Write/Edit, + * `compare` stats the target again and compares the tuple directly; live dirty + * signals classify watcher echoes and truncated windows but are never the + * correctness gate. This avoids both debounce latency and delayed watcher + * delivery turning into a stale-write allowance. * * The verdict drives the write-path policy: `clean` lets the call through, * `stale` means the file diverged since the baseline (or a path-level dirty diff --git a/packages/agent-core-v2/src/session/sessionFileLedger/fileLedgerService.ts b/packages/agent-core-v2/src/session/sessionFileLedger/fileLedgerService.ts index f0b12137f..fc76f809b 100644 --- a/packages/agent-core-v2/src/session/sessionFileLedger/fileLedgerService.ts +++ b/packages/agent-core-v2/src/session/sessionFileLedger/fileLedgerService.ts @@ -2,12 +2,11 @@ * `sessionFileLedger` domain (L2) — `ISessionFileLedger` implementation. * * In-memory per-session ledger of on-disk stat tuples keyed by - * `normalizeFsWatchKey`. `recordBaseline` re-stats the target through - * `IHostFileSystem` and stores the tuple with the watch service's current - * tick; `compare` consults only already-folded `sessionFsWatch` dirty state - * (it never awaits watcher frames) and degrades gracefully: stat failures - * other than not-found yield `clean` rather than blocking the write path, - * and paths outside every watched root get a stat-only comparison. The + * `normalizeFsWatchKey`. `recordBaseline` captures the watch tick before it + * re-stats the target, so a concurrent event cannot be absorbed into an older + * tuple. `compare` always re-stats immediately before a write decision; + * watcher ticks classify the result but never replace the stat. An + * unverifiable stat fails closed as `stale`. The * service also guarantees `ISessionFsWatchService` watches the session roots * (`workDir` + `additionalDirs` from `ISessionWorkspaceContext`), adding an * unwatched containing root additively whenever a Write/Edit target falls @@ -52,9 +51,10 @@ export class SessionFileLedger implements ISessionFileLedger { async recordBaseline(path: string): Promise { this.ensureWatchedRootFor(path); + const tick = this.watch.currentTick; const tuple = await this.tryStat(path); if (tuple === undefined) return; - this.entries.set(normalizeFsWatchKey(path), { ...tuple, tick: this.watch.currentTick }); + this.entries.set(normalizeFsWatchKey(path), { ...tuple, tick }); } async compare(path: string): Promise { @@ -62,27 +62,18 @@ export class SessionFileLedger implements ISessionFileLedger { const key = normalizeFsWatchKey(path); const entry = this.entries.get(key); const root = this.containingWatchedRoot(key); - if (root === undefined) { - const current = await this.tryStat(path); - if (current === undefined) return 'clean'; - if (entry === undefined) return current.exists ? 'no-baseline' : 'clean'; - return fileStatTuplesEqual(entry, current) ? 'clean' : 'stale'; - } - if (entry === undefined) { - if ((this.watch.dirtyTickFor(path) ?? 0) > 0) return 'stale'; - const current = await this.tryStat(path); - if (current === undefined) return 'clean'; - return current.exists ? 'no-baseline' : 'clean'; - } + const current = await this.tryStat(path); + if (current === undefined) return 'stale'; const dirty = Math.max( this.watch.dirtyTickFor(path) ?? 0, - this.watch.rootDirtyTickFor(root) ?? 0, + root === undefined ? 0 : (this.watch.rootDirtyTickFor(root) ?? 0), ); - if (dirty <= entry.tick) return 'clean'; - const current = await this.tryStat(path); - if (current === undefined) return 'clean'; + if (entry === undefined) { + if (dirty > 0) return current.exists ? 'stale' : 'clean'; + return current.exists ? 'no-baseline' : 'clean'; + } if (fileStatTuplesEqual(entry, current)) { - this.entries.set(key, { ...entry, tick: dirty }); + if (dirty > entry.tick) this.entries.set(key, { ...current, tick: dirty }); return 'clean'; } return 'stale'; diff --git a/packages/agent-core-v2/src/session/sessionFs/fsWatch.ts b/packages/agent-core-v2/src/session/sessionFs/fsWatch.ts index ac3acdfe4..6b3682126 100644 --- a/packages/agent-core-v2/src/session/sessionFs/fsWatch.ts +++ b/packages/agent-core-v2/src/session/sessionFs/fsWatch.ts @@ -11,7 +11,7 @@ * Beyond the change feed, every confined change entry is folded into a * per-session dirty state for optimistic-concurrency consumers * (`sessionFileLedger`): a monotonic `currentTick` incremented per processed - * entry, per-path dirty ticks folded when a debounce window flushes, and + * entry, immediate per-path dirty ticks independent of debounce delivery, and * per-root dirty ticks for truncated windows whose exact paths were dropped. * Client subscriptions (`setWatchedPaths`, replace semantics) and * optimistic-concurrency watch anchors (`ensureWatchedRoots`, additive, diff --git a/packages/agent-core-v2/src/session/sessionFs/fsWatchService.ts b/packages/agent-core-v2/src/session/sessionFs/fsWatchService.ts index 65e11fc58..e0c882805 100644 --- a/packages/agent-core-v2/src/session/sessionFs/fsWatchService.ts +++ b/packages/agent-core-v2/src/session/sessionFs/fsWatchService.ts @@ -17,12 +17,11 @@ * into dirty state. The os workDir watcher runs while either set is * non-empty. * - * Dirty state: every confined change entry gets the next monotonic tick and - * is buffered with it; at flush the entries fold into - * `dirtyTicks[normalizedAbs]`, and a truncated window (exact paths dropped) - * conservatively folds `dirtyRootTicks` for every watched root at the - * window's last tick. Clearing a window without flushing still folds the - * buffered entries so in-flight dirty signals are never silently dropped. + * Dirty state is independent from event delivery: every confined raw change + * immediately advances `dirtyTicks[normalizedAbs]`, while protocol events are + * still buffered and debounced. A truncated window additionally advances + * `dirtyRootTicks` for every watched root. This keeps the optimistic + * concurrency ledger from inheriting the UI debounce window. * Key normalization is the shared `normalizeFsWatchKey` from the contract. */ @@ -247,11 +246,15 @@ export class SessionFsWatchService extends Disposable implements ISessionFsWatch private record(e: HostFsChange, rel: string | undefined): void { this.tick += 1; + this.dirtyTicks.set(normalizeFsWatchKey(e.path), this.tick); this.pending.push({ abs: e.path, rel, tick: this.tick, change: e.action, kind: e.kind }); this.rawCount += 1; if (this.pending.length > this.maxChangesPerWindow) { this.truncated = true; this.pending = []; + for (const root of this.watchedRoots) { + this.dirtyRootTicks.set(normalizeFsWatchKey(root), this.tick); + } } if (this.debounceTimer === undefined) { const timer = setTimeout(() => this.flush(), this.debounceMs); @@ -274,10 +277,6 @@ export class SessionFsWatchService extends Disposable implements ISessionFsWatch for (const root of this.watchedRoots) { this.dirtyRootTicks.set(normalizeFsWatchKey(root), this.tick); } - } else { - for (const entry of pending) { - this.dirtyTicks.set(normalizeFsWatchKey(entry.abs), entry.tick); - } } const changes = truncated @@ -304,8 +303,10 @@ export class SessionFsWatchService extends Disposable implements ISessionFsWatch clearTimeout(this.debounceTimer); this.debounceTimer = undefined; } - for (const entry of this.pending) { - this.dirtyTicks.set(normalizeFsWatchKey(entry.abs), entry.tick); + if (this.truncated) { + for (const root of this.watchedRoots) { + this.dirtyRootTicks.set(normalizeFsWatchKey(root), this.tick); + } } this.pending = []; this.rawCount = 0; diff --git a/packages/agent-core-v2/src/session/sessionLease/sessionLease.ts b/packages/agent-core-v2/src/session/sessionLease/sessionLease.ts index e5fd6baeb..d845e7d73 100644 --- a/packages/agent-core-v2/src/session/sessionLease/sessionLease.ts +++ b/packages/agent-core-v2/src/session/sessionLease/sessionLease.ts @@ -6,9 +6,8 @@ * and the `SessionLease` object that satisfies it: an App-owned wrapper * (`SessionLifecycleService` builds it; it is deliberately not a DI service) * around the cross-process lock handle at - * `/session-leases/.json`. `assertWritable` is the - * Quint-verified hard gate (design: `.tmp/refactor-watch-design-v2.md` - * §3.4.2/§3.4.5): it synchronously re-reads the on-disk lease payload and + * `/session-leases/.json`. `assertWritable` is the hard + * gate: it synchronously re-reads the on-disk lease payload and * compares the held `lockId` — a mismatch fails closed with * `session.lease_lost`, marks the lease lost, and fires the loss callback * exactly once so the owning session tears itself down. Release order is the @@ -33,8 +32,6 @@ export const SESSION_LEASE_HEARTBEAT_INTERVAL_MS = 2000; export const SESSION_LEASE_TTL_MS = 6000; export const LEASE_CREATING_RETRY_AFTER_MS = 1000; export const HOLDER_UNRESPONSIVE_RETRY_AFTER_MS = 2000; -export const UNREGISTERED_WRITER_WINDOW_MS = 5000; -export const UNREGISTERED_WRITER_RECHECK_DELAY_MS = 1000; /** `details` payload of `session.held_by_peer` errors; the zod twin lives in packages/protocol (`sessionOwnershipDetailsSchema`) and the shapes must diff --git a/packages/agent-core-v2/src/session/sessionLease/sessionLeaseContactProvider.ts b/packages/agent-core-v2/src/session/sessionLease/sessionLeaseContactProvider.ts index 1523cc79b..c9374ed9c 100644 --- a/packages/agent-core-v2/src/session/sessionLease/sessionLeaseContactProvider.ts +++ b/packages/agent-core-v2/src/session/sessionLease/sessionLeaseContactProvider.ts @@ -5,9 +5,8 @@ * `{type: 'address', address}` when the host runs a routable network service * (kap-server seeds its listening address), `{type: 'local'}` otherwise — * the value is recorded in the lease payload so a blocked peer can tell a - * routable holder from a local-only one (design: - * `.tmp/refactor-watch-design-v2.md` §3.4.1 — the discriminated - * `contact` union landing as the flat `address?` payload field). Read + * routable holder from a local-only one (the discriminated `contact` union + * lands as the flat `address?` payload field). Read * lazily at every lease acquisition through the `contact` thunk, so a host * whose address is only known after listen can seed a provider that closes * over it. Composition roots override the local-only default through diff --git a/packages/agent-core-v2/src/session/sessionMetadata/sessionMetadataService.ts b/packages/agent-core-v2/src/session/sessionMetadata/sessionMetadataService.ts index 4d234fe9d..d483ea9db 100644 --- a/packages/agent-core-v2/src/session/sessionMetadata/sessionMetadataService.ts +++ b/packages/agent-core-v2/src/session/sessionMetadata/sessionMetadataService.ts @@ -16,8 +16,8 @@ * never reorders session listings. Every durable write passes the * `sessionLease` hard gate first (`ISessionLeaseService.assertWritable`, * synchronously re-reading the lease payload), so an instance that lost the - * session lease fails closed instead of overwriting a live peer's state - * (design: `.tmp/refactor-watch-design-v2.md` §3.4.5). Bound at Session scope. + * session lease fails closed instead of overwriting a live peer's state. + * Bound at Session scope. * * Read-model mirroring (flag `persistence_minidb_readmodel`): after a metadata * update is persisted, the fresh summary is mirrored into the `IQueryStore` diff --git a/packages/agent-core-v2/test/_base/di/auto-inject.test.ts b/packages/agent-core-v2/test/_base/di/auto-inject.test.ts index c8f2eca18..f969257a6 100644 --- a/packages/agent-core-v2/test/_base/di/auto-inject.test.ts +++ b/packages/agent-core-v2/test/_base/di/auto-inject.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest'; import { SyncDescriptor } from '#/_base/di/descriptors'; import { CyclicDependencyError } from '#/_base/di/errors'; -import { IInstantiationService, createDecorator } from '#/_base/di/instantiation'; +import { IInstantiationService, createDecorator, optional } from '#/_base/di/instantiation'; import { InstantiationService } from '#/_base/di/instantiationService'; import { ServiceCollection } from '#/_base/di/serviceCollection'; @@ -66,6 +66,50 @@ describe('@IFoo auto-injection', () => { expect(bar.baz).toBeInstanceOf(Baz); }); + it('injects undefined for an unregistered optional service in strict mode', () => { + interface IDependency { + tag: 'dependency'; + } + const IDependency = createDecorator('p1.1-IDependency-optional-missing'); + class Consumer { + constructor(@optional(IDependency) readonly dependency?: IDependency) {} + } + const IConsumer = createDecorator('p1.1-IConsumer-optional-missing'); + const ix = new InstantiationService( + new ServiceCollection([IConsumer, new SyncDescriptor(Consumer)]), + true, + ); + + const consumer = ix.invokeFunction((accessor) => accessor.get(IConsumer)); + + expect(consumer.dependency).toBeUndefined(); + }); + + it('resolves a registered optional service normally', () => { + interface IDependency { + tag: 'dependency'; + } + const IDependency = createDecorator('p1.1-IDependency-optional-present'); + class Dependency implements IDependency { + tag = 'dependency' as const; + } + class Consumer { + constructor(@optional(IDependency) readonly dependency?: IDependency) {} + } + const IConsumer = createDecorator('p1.1-IConsumer-optional-present'); + const ix = new InstantiationService( + new ServiceCollection( + [IDependency, new SyncDescriptor(Dependency)], + [IConsumer, new SyncDescriptor(Consumer)], + ), + true, + ); + + const consumer = ix.invokeFunction((accessor) => accessor.get(IConsumer)); + + expect(consumer.dependency).toBeInstanceOf(Dependency); + }); + it('@IInstantiationService self-injection resolves to the OWNING container', () => { class Widget { constructor(public readonly label: string) {} diff --git a/packages/agent-core-v2/test/agent/fileFencing/fileFencing.test.ts b/packages/agent-core-v2/test/agent/fileFencing/fileFencing.test.ts index c0e061e14..eac937bf9 100644 --- a/packages/agent-core-v2/test/agent/fileFencing/fileFencing.test.ts +++ b/packages/agent-core-v2/test/agent/fileFencing/fileFencing.test.ts @@ -181,9 +181,20 @@ async function runBefore( ctx: ToolBeforeExecuteContext, ): Promise { await world.executor.hooks.onBeforeExecuteTool.run(ctx); + await runPrepared(ctx); return ctx; } +async function runPrepared(ctx: ToolBeforeExecuteContext): Promise { + if (ctx.decision?.execute === undefined) return; + await ctx.decision.execute({ + turnId: ctx.turnId, + toolCallId: ctx.toolCall.id, + trace: ctx.trace, + signal: ctx.signal, + }); +} + async function runDid( world: AgentWorld, ctx: ToolBeforeExecuteContext, @@ -268,6 +279,42 @@ describe('AgentFileFencingService', () => { expect(retry.decision?.block).not.toBe(true); }); + it('checks the file at execution time rather than during preflight', async () => { + multiServer = true; + const world = setup(); + const file = join(world.env.workDir, 'a.txt'); + writeFileSync(file, 'hello'); + await runOk(world, 'Read', file); + + const ctx = beforeCtx('Edit', file); + await world.executor.hooks.onBeforeExecuteTool.run(ctx); + writeFileSync(file, 'changed while queued'); + + await runPrepared(ctx); + expect(ctx.decision?.block).toBe(true); + expect(ctx.decision?.reason).toContain('changed on disk since'); + }); + + it('wraps an execution override installed by an earlier hook', async () => { + const world = setup(); + const file = join(world.env.workDir, 'a.txt'); + writeFileSync(file, 'hello'); + await runOk(world, 'Read', file); + let overrideCalls = 0; + const ctx = beforeCtx('Edit', file); + ctx.decision = { + execute: async () => { + overrideCalls++; + return { output: 'overridden' }; + }, + }; + + await world.executor.hooks.onBeforeExecuteTool.run(ctx); + await runPrepared(ctx); + + expect(overrideCalls).toBe(1); + }); + it('blocks Write over an existing file that was never read', async () => { multiServer = true; const world = setup(); @@ -329,7 +376,7 @@ describe('AgentFileFencingService', () => { const again = await runBefore(world, beforeCtx('Edit', file)); expect(again.decision?.block).not.toBe(true); - expect(world.env.statCalls()).toBe(2); + expect(world.env.statCalls()).toBe(3); }); it('resolves a truncated window by stat punch: unchanged passes, changed blocks', async () => { 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 6eedeaa02..2b35851b1 100644 --- a/packages/agent-core-v2/test/agent/toolExecutor/toolExecutor.test.ts +++ b/packages/agent-core-v2/test/agent/toolExecutor/toolExecutor.test.ts @@ -423,6 +423,27 @@ describe('AgentToolExecutorService', () => { expect(second.calls).toHaveLength(1); }); + it('an execution override runs instead of the original tool after scheduling', async () => { + const tool = new TestTool('echo'); + registry.register(tool); + executor.hooks.onBeforeExecuteTool.register('replace-execute', async (ctx) => { + ctx.decision = { + ...ctx.decision, + execute: async () => ({ output: 'changed while queued', isError: true }), + }; + }); + + const results = await execute([toolCall('call_echo', 'echo', { text: 'hi' })]); + + expect(results).toEqual([ + expect.objectContaining({ + output: 'changed while queued', + isError: true, + }), + ]); + expect(tool.calls).toEqual([]); + }); + it('skips later tool calls after an execution requests stopBatchAfterThis', async () => { const first = new TestTool('first', { stopBatchAfterThis: true }); const second = new TestTool('second'); diff --git a/packages/agent-core-v2/test/app/bootstrap/bootstrapService.test.ts b/packages/agent-core-v2/test/app/bootstrap/bootstrapService.test.ts index d3272107e..aa3e483f9 100644 --- a/packages/agent-core-v2/test/app/bootstrap/bootstrapService.test.ts +++ b/packages/agent-core-v2/test/app/bootstrap/bootstrapService.test.ts @@ -3,8 +3,12 @@ import { beforeEach, describe, expect, it } from 'vitest'; import { InstantiationType } from '#/_base/di/extensions'; import { LifecycleScope, _clearScopedRegistryForTests, registerScopedService } from '#/_base/di/scope'; import { createScopedTestHost } from '#/_base/di/test'; -import { IBootstrapService, bootstrapSeed, resolveBootstrapOptions } from '#/app/bootstrap/bootstrap'; -import { bootstrap } from '#/app/bootstrap/bootstrap'; +import { + IBootstrapService, + bootstrap, + bootstrapSeed, + resolveBootstrapOptions, +} from '#/app/bootstrap/bootstrap'; import { BootstrapService } from '#/app/bootstrap/bootstrapService'; import { FileStorageService } from '#/persistence/backends/node-fs/fileStorageService'; import { IFileSystemStorageService } from '#/persistence/interface/storage'; @@ -26,10 +30,23 @@ describe('BootstrapService (scoped)', () => { const svc = host.app.accessor.get(IBootstrapService); expect(svc.homeDir).toBe('/tmp/kimi-home'); expect(svc.configPath).toBe('/tmp/kimi-home/config.toml'); + expect(svc.configKey).toBe('config.toml'); expect(svc.sessionsDir).toBe('/tmp/kimi-home/sessions'); host.dispose(); }); + it('addresses a custom config path relative to the storage root', () => { + const host = createScopedTestHost( + bootstrapSeed({ + homeDir: '/tmp/kimi-home', + configPath: '/tmp/custom/config.toml', + }), + ); + const svc = host.app.accessor.get(IBootstrapService); + expect(svc.configKey).toBe('../custom/config.toml'); + host.dispose(); + }); + it('getEnv reads from the seeded env bag', () => { const host = createScopedTestHost(bootstrapSeed({ env: { FOO: 'bar' } })); const svc = host.app.accessor.get(IBootstrapService); diff --git a/packages/agent-core-v2/test/app/config/config.test.ts b/packages/agent-core-v2/test/app/config/config.test.ts index af0e2077d..5bf59b6cf 100644 --- a/packages/agent-core-v2/test/app/config/config.test.ts +++ b/packages/agent-core-v2/test/app/config/config.test.ts @@ -68,10 +68,8 @@ import { InMemoryStorageService } from '#/persistence/backends/memory/inMemorySt import { IFileSystemStorageService } from '#/persistence/interface/storage'; import { IAtomicTomlDocumentStore } from '#/persistence/interface/atomicDocumentStore'; import { TomlAtomicDocumentStore } from '#/persistence/backends/node-fs/atomicDocumentStore'; -import { ICrossProcessLockService } from '#/os/interface/crossProcessLock'; import { stubBootstrap } from '../bootstrap/stubs'; import { stubLog } from '../../_base/log/stubs'; -import { stubCrossProcessLock } from '../../os/stubs'; const TEST_OS_ENV = { osKind: 'Linux', @@ -384,7 +382,6 @@ describe('ConfigService env overlay (live)', () => { ix.stub(IFileSystemStorageService, new InMemoryStorageService()); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); - ix.stub(ICrossProcessLockService, stubCrossProcessLock()); ix.set(IConfigService, new SyncDescriptor(ConfigService)); const config = ix.get(IConfigService); await config.ready; @@ -407,7 +404,6 @@ describe('ConfigService env overlay (live)', () => { ix.stub(IFileSystemStorageService, new InMemoryStorageService()); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); - ix.stub(ICrossProcessLockService, stubCrossProcessLock()); ix.set(IConfigService, new SyncDescriptor(ConfigService)); const config = ix.get(IConfigService); await config.ready; @@ -429,7 +425,6 @@ describe('ConfigService env overlay (live)', () => { ix.stub(IFileSystemStorageService, new InMemoryStorageService()); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); - ix.stub(ICrossProcessLockService, stubCrossProcessLock()); ix.set(IConfigService, new SyncDescriptor(ConfigService)); const config = ix.get(IConfigService); await config.ready; @@ -451,7 +446,6 @@ describe('ConfigService env overlay (live)', () => { const ix = disposables.add(new TestInstantiationService()); ix.stub(ILogService, stubLog()); ix.stub(IBootstrapService, stubBootstrap('/tmp/kimi-cfg')); - ix.stub(ICrossProcessLockService, stubCrossProcessLock()); ix.stub(IFileSystemStorageService, new InMemoryStorageService()); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); @@ -521,7 +515,6 @@ describe('image config section', () => { ix.stub(IFileSystemStorageService, new InMemoryStorageService()); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); - ix.stub(ICrossProcessLockService, stubCrossProcessLock()); ix.set(IConfigService, new SyncDescriptor(ConfigService)); const config = ix.get(IConfigService); await config.ready; @@ -868,7 +861,6 @@ describe('task config section', () => { ix.stub(IFileSystemStorageService, new InMemoryStorageService()); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); - ix.stub(ICrossProcessLockService, stubCrossProcessLock()); ix.set(IConfigService, new SyncDescriptor(ConfigService)); const config = ix.get(IConfigService); await config.ready; @@ -903,7 +895,6 @@ describe('task config section', () => { ix.stub(IFileSystemStorageService, storage); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); - ix.stub(ICrossProcessLockService, stubCrossProcessLock()); ix.set(IConfigService, new SyncDescriptor(ConfigService)); const config = ix.get(IConfigService); await config.ready; @@ -1009,7 +1000,6 @@ describe('task config section', () => { ix.stub(IFileSystemStorageService, storage); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); - ix.stub(ICrossProcessLockService, stubCrossProcessLock()); ix.set(IConfigService, new SyncDescriptor(ConfigService)); const config = ix.get(IConfigService); await config.ready; @@ -1180,7 +1170,6 @@ describe('subagent config section', () => { ix.stub(IFileSystemStorageService, storage); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); - ix.stub(ICrossProcessLockService, stubCrossProcessLock()); ix.set(IConfigService, new SyncDescriptor(ConfigService)); const config = ix.get(IConfigService); await config.ready; diff --git a/packages/agent-core-v2/test/app/config/configFileMutex.test.ts b/packages/agent-core-v2/test/app/config/configFileMutex.test.ts index 799e7bbc8..72dca4298 100644 --- a/packages/agent-core-v2/test/app/config/configFileMutex.test.ts +++ b/packages/agent-core-v2/test/app/config/configFileMutex.test.ts @@ -1,20 +1,17 @@ /** - * Scenario: config.toml cross-process lock-in-RMW (design: - * .tmp/refactor-watch-design-v2.md §3.6). + * Scenario: config.toml atomic read-modify-write. * - * Two independent `ConfigService` instances share one home dir on the real - * filesystem; interleaved writes must merge without lost updates, the lock - * file must be released after each critical section, and a held lock must - * surface OS_LOCK_WAIT_TIMEOUT while leaving config.toml intact. Watch-based - * reloads ride real chokidar (150ms debounce), so assertions poll with real - * timers. Run with `pnpm --filter @moonshot-ai/agent-core-v2 exec vitest run - * test/app/config/configFileMutex.test.ts`. + * Two independent `ConfigService` instances share one storage root on the real + * filesystem. The atomic-document Store owns cross-process exclusion, so + * interleaved writes merge without lost updates and `ConfigService` never + * handles lock paths or lock services itself. Watch-based reloads ride real + * chokidar (150ms debounce), so assertions poll with real timers. */ import { existsSync, readFileSync, readdirSync } from 'node:fs'; import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { join, relative } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; @@ -28,13 +25,16 @@ import { ConfigRegistry, ConfigService } from '#/app/config/configService'; import { CrossProcessLockService } from '#/os/backends/node-local/crossProcessLockService'; import { CrossProcessLockErrorCode, - ICrossProcessLockService, + type ICrossProcessLockService, type ICrossProcessLockHandle, } from '#/os/interface/crossProcessLock'; import { TomlAtomicDocumentStore } from '#/persistence/backends/node-fs/atomicDocumentStore'; import { FileStorageService } from '#/persistence/backends/node-fs/fileStorageService'; import { IAtomicTomlDocumentStore } from '#/persistence/interface/atomicDocumentStore'; -import { IFileSystemStorageService } from '#/persistence/interface/storage'; +import { + IFileSystemStorageService, + StorageErrors, +} from '#/persistence/interface/storage'; import { stubLog } from '../../_base/log/stubs'; import { stubBootstrap } from '../bootstrap/stubs'; @@ -65,13 +65,22 @@ describe('ConfigService config.toml lock-in-RMW', () => { await rm(homeDir, { recursive: true, force: true }); }); - function createContainer(lock?: ICrossProcessLockService): IConfigService { + function createContainer( + lock: ICrossProcessLockService = new CrossProcessLockService(), + configPath = join(homeDir, 'config.toml'), + ): IConfigService { const ix = disposables.add(new TestInstantiationService()); ix.stub(ILogService, stubLog()); - ix.stub(IBootstrapService, stubBootstrap(homeDir, {})); - ix.stub(IFileSystemStorageService, new FileStorageService(homeDir)); + ix.stub(IBootstrapService, { + ...stubBootstrap(homeDir, {}), + configPath, + configKey: relative(homeDir, configPath), + }); + ix.stub( + IFileSystemStorageService, + new FileStorageService(homeDir, undefined, undefined, lock), + ); ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); - ix.stub(ICrossProcessLockService, lock ?? new CrossProcessLockService()); ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); ix.set(IConfigService, new SyncDescriptor(ConfigService)); return ix.get(IConfigService); @@ -134,7 +143,17 @@ describe('ConfigService config.toml lock-in-RMW', () => { expect(readdirSync(homeDir).filter((entry) => entry.includes('.stale.'))).toEqual([]); }); - it('fails set() with OS_LOCK_WAIT_TIMEOUT while another holder is stuck, leaving config.toml intact', async () => { + it('writes and locks a custom config path at its actual location', async () => { + const configPath = join(homeDir, 'nested', 'custom.toml'); + const config = createContainer(new CrossProcessLockService(), configPath); + await config.set('alphaSection', { one: 1 }); + + expect(readFileSync(configPath, 'utf8')).toContain('[alpha_section]'); + expect(existsSync(join(homeDir, 'custom.toml'))).toBe(false); + expect(existsSync(`${configPath}.lock`)).toBe(false); + }); + + it('fails set() with storage.locked while another holder is stuck, leaving config.toml intact', async () => { let nowValue = 1_000_000; let lockSeq = 0; const victim = new CrossProcessLockService({ @@ -159,10 +178,11 @@ describe('ConfigService config.toml lock-in-RMW', () => { newLockId: () => 'attacker-lock', }); const lockPath = join(homeDir, 'config.toml.lock'); - const handle: ICrossProcessLockHandle = attacker.acquire(lockPath); + const handle: ICrossProcessLockHandle = await attacker.acquire(lockPath); try { await expect(config.set('blockedSection', { no: true })).rejects.toMatchObject({ - code: CrossProcessLockErrorCode.WaitTimeout, + code: StorageErrors.codes.STORAGE_LOCKED, + cause: { code: CrossProcessLockErrorCode.WaitTimeout }, }); expect(readFileSync(join(homeDir, 'config.toml'), 'utf8')).toBe(before); } finally { diff --git a/packages/agent-core-v2/test/app/externalHooksRunner/integration.test.ts b/packages/agent-core-v2/test/app/externalHooksRunner/integration.test.ts index 43bcbcabb..5afb159cb 100644 --- a/packages/agent-core-v2/test/app/externalHooksRunner/integration.test.ts +++ b/packages/agent-core-v2/test/app/externalHooksRunner/integration.test.ts @@ -199,6 +199,7 @@ function stubSessionLifecycle(): ISessionLifecycleService { list: () => [], resume: async () => undefined, close: async () => {}, + forceAbort: async () => {}, archive: async () => {}, restore: async () => undefined, fork: async () => { diff --git a/packages/agent-core-v2/test/app/mcp/mcpConfigWatch.test.ts b/packages/agent-core-v2/test/app/mcp/mcpConfigWatch.test.ts index f12c6a97b..98acef57c 100644 --- a/packages/agent-core-v2/test/app/mcp/mcpConfigWatch.test.ts +++ b/packages/agent-core-v2/test/app/mcp/mcpConfigWatch.test.ts @@ -1,6 +1,5 @@ /** - * Scenario: App-scope watch of the user-level mcp.json (design: - * .tmp/refactor-watch-design-v2.md §3.6). + * Scenario: App-scope watch of the user-level mcp.json. * * Valid content fires `onDidChange` (and a fresh `loadMcpServers` observes * the new servers — v2 loads mcp.json per session creation and has no cache diff --git a/packages/agent-core-v2/test/app/sessionExport/sessionExport.test.ts b/packages/agent-core-v2/test/app/sessionExport/sessionExport.test.ts index 9c1230c6b..2ea25544f 100644 --- a/packages/agent-core-v2/test/app/sessionExport/sessionExport.test.ts +++ b/packages/agent-core-v2/test/app/sessionExport/sessionExport.test.ts @@ -863,6 +863,7 @@ function registerSessionExportServices( list: () => (options.lifecycleHandle === undefined ? [] : [options.lifecycleHandle]), resume: async () => options.lifecycleHandle, close: async () => {}, + forceAbort: async () => {}, archive: async () => {}, restore: async () => options.lifecycleHandle, fork: async () => { diff --git a/packages/agent-core-v2/test/app/sessionLifecycle/sessionLifecycle.test.ts b/packages/agent-core-v2/test/app/sessionLifecycle/sessionLifecycle.test.ts index a4e7fc3f9..4eae62759 100644 --- a/packages/agent-core-v2/test/app/sessionLifecycle/sessionLifecycle.test.ts +++ b/packages/agent-core-v2/test/app/sessionLifecycle/sessionLifecycle.test.ts @@ -1,6 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { randomUUID } from 'node:crypto'; import { mkdtemp, mkdir, readFile, rm, stat, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { isAbsolute, join, resolve } from 'node:path'; @@ -9,6 +8,7 @@ import { InstantiationType } from '#/_base/di/extensions'; import { Disposable } from '#/_base/di/lifecycle'; import { type IAgentScopeHandle, + type ISessionScopeHandle, LifecycleScope, _clearScopedRegistryForTests, registerScopedService, @@ -44,6 +44,7 @@ import { ISessionIndex, type SessionSummary } from '#/app/sessionIndex/sessionIn import { IAppendLogStore } from '#/persistence/interface/appendLogStore'; import { IAtomicDocumentStore } from '#/persistence/interface/atomicDocumentStore'; import { IProjectLocalConfigService } from '#/app/projectLocalConfig/projectLocalConfig'; +import { JsonAtomicDocumentStore } from '#/persistence/backends/node-fs/atomicDocumentStore'; import { IWriteAuthorityRegistry } from '#/persistence/interface/writeAuthority'; import { AppendLogStore } from '#/persistence/backends/node-fs/appendLogStore'; import { FileStorageService } from '#/persistence/backends/node-fs/fileStorageService'; @@ -60,7 +61,10 @@ import { } from '#/session/sessionLease/sessionLeaseContactProvider'; import { ITelemetryService } from '#/app/telemetry/telemetry'; import { Error2, ErrorCodes } from '#/errors'; -import { ICrossProcessLockService } from '#/os/interface/crossProcessLock'; +import { + CrossProcessLockErrorCode, + ICrossProcessLockService, +} from '#/os/interface/crossProcessLock'; import { CrossProcessLockService } from '#/os/backends/node-local/crossProcessLockService'; import { stubCrossProcessLock } from '../../os/stubs'; import { stubFlag } from '../flag/stubs'; @@ -294,6 +298,8 @@ function atomicDocumentStoreStub(): IAtomicDocumentStore { _serviceBrand: undefined, get: () => Promise.resolve(undefined), set: () => Promise.resolve(), + update: (_scope, _key, mutate) => Promise.resolve(mutate(undefined)), + runExclusive: (_scope, _key, op) => op(), delete: () => Promise.resolve(), list: () => Promise.resolve([]), watch: () => (_listener) => ({ dispose: () => {} }), @@ -413,8 +419,18 @@ async function waitFor(cond: () => boolean, timeoutMs = 5_000): Promise { } } -class NoopSessionExternalHooksService implements ISessionExternalHooksService { +let disposedSessionServices = 0; + +class NoopSessionExternalHooksService + extends Disposable + implements ISessionExternalHooksService +{ declare readonly _serviceBrand: undefined; + + override dispose(): void { + disposedSessionServices++; + super.dispose(); + } } let recordedSessionHookEvents: string[] = []; @@ -448,6 +464,7 @@ describe('SessionLifecycleService', () => { let tmpRoots: string[]; beforeEach(() => { + disposedSessionServices = 0; recordedSessionHookEvents = []; telemetryRecords = []; tmpRoots = []; @@ -856,11 +873,41 @@ describe('SessionLifecycleService', () => { it('fires onDidCreateSession with the new handle', async () => { const svc = build(); let captured: { readonly sessionId: string } | undefined; + let visibleDuringEvent: ISessionScopeHandle | undefined; svc.onDidCreateSession((e) => { captured = e; + visibleDuringEvent = svc.get(e.sessionId); }); const h = await svc.create({ sessionId: 's1', workDir: '/tmp/proj' }); expect(captured).toMatchObject({ sessionId: 's1', handle: h, source: 'startup' }); + expect(visibleDuringEvent).toBe(h); + expect(svc.list()).toContain(h); + }); + + it('makes a fork visible before fork and create events fire', async () => { + const svc = build([ + stubPair(IWorkspaceRegistry, { + ...workspaceRegistryStub(), + get: () => + Promise.resolve({ + id: 'wd_stub', + root: '/tmp/proj', + name: 'stub', + createdAt: 0, + lastOpenedAt: 0, + }), + }), + ]); + await svc.create({ sessionId: 'src', workDir: '/tmp/proj' }); + let visibleDuringFork: ISessionScopeHandle | undefined; + svc.onDidForkSession((event) => { + visibleDuringFork = svc.get(event.sessionId); + }); + + const target = await svc.fork({ sourceSessionId: 'src', newSessionId: 'dst' }); + + expect(visibleDuringFork).toBe(target); + expect(svc.list()).toContain(target); }); it('emits session_started with resumed: false and the bound session id on create', async () => { @@ -1333,18 +1380,26 @@ describe('SessionLifecycleService', () => { seeds: ReturnType[]; appendLog: AppendLogStore; registry: WriteAuthorityRegistryService; + storage: FileStorageService; + docs: JsonAtomicDocumentStore; } { const registry = new WriteAuthorityRegistryService(); - const appendLog = new AppendLogStore(new FileStorageService(root), registry); + const locks = new CrossProcessLockService(); + const storage = new FileStorageService(root, undefined, undefined, locks); + const appendLog = new AppendLogStore(storage, registry); + const docs = new JsonAtomicDocumentStore(storage); return { seeds: [ stubPair(IBootstrapService, tmpBootstrapStub(root)), - stubPair(ICrossProcessLockService, new CrossProcessLockService()), + stubPair(ICrossProcessLockService, locks), stubPair(IWriteAuthorityRegistry, registry), stubPair(IAppendLogStore, appendLog), + stubPair(IAtomicDocumentStore, docs), ], appendLog, registry, + storage, + docs, }; } @@ -1378,6 +1433,25 @@ describe('SessionLifecycleService', () => { }); }); + it('rolls back the private scope, authority, and lease when MCP initialization fails', async () => { + const root = await makeTmpRoot(); + const registry = new WriteAuthorityRegistryService(); + const failure = new Error('mcp failed'); + const svc = build([ + stubPair(IBootstrapService, tmpBootstrapStub(root)), + stubPair(ICrossProcessLockService, new CrossProcessLockService()), + stubPair(IWriteAuthorityRegistry, registry), + stubPair(ISessionMcpService, sessionMcpServiceStub(() => Promise.reject(failure))), + ]); + + await expect(svc.create({ sessionId: 's1', workDir: '/tmp/proj' })).rejects.toBe(failure); + expect(svc.get('s1')).toBeUndefined(); + expect(svc.list()).toEqual([]); + expect(registry.resolve('s1')).toBeUndefined(); + await expect(stat(leaseFile(root, 's1'))).rejects.toThrow(); + expect(disposedSessionServices).toBeGreaterThan(0); + }); + it('refuses to materialize a session live-held by another instance', async () => { const root = await makeTmpRoot(); const first = build(realInstanceSeeds(root)); @@ -1541,40 +1615,6 @@ describe('SessionLifecycleService', () => { expect(remaining.lock_id).toBe('peer-token'); }); - it('refuses to materialize a session directory an unregistered writer keeps writing', async () => { - const root = await makeTmpRoot(); - const sessionDir = join(root, 'sessions', 'wd_stub', 's1'); - await mkdir(sessionDir, { recursive: true }); - await writeFile(join(sessionDir, 'marker'), '1'); - const writer = setInterval(() => { - void writeFile(join(sessionDir, `tick-${randomUUID()}`), 'x').catch(() => {}); - }, 150); - writer.unref(); - try { - const svc = build( - realInstanceSeeds(root, [stubPair(IFlagService, stubFlag(true))]), - ); - const error = await createError(svc, 's1'); - expect(error.code).toBe(ErrorCodes.SESSION_HELD_BY_PEER); - expect(error.details).toEqual({ kind: 'unregistered-writer' }); - await expect(stat(leaseFile(root, 's1'))).rejects.toThrow(); - } finally { - clearInterval(writer); - } - }); - - it('materializes when the session directory stops being written between the two checks', async () => { - const root = await makeTmpRoot(); - const sessionDir = join(root, 'sessions', 'wd_stub', 's1'); - await mkdir(sessionDir, { recursive: true }); - await writeFile(join(sessionDir, 'marker'), '1'); - const svc = build( - realInstanceSeeds(root, [stubPair(IFlagService, stubFlag(true))]), - ); - const h = await svc.create({ sessionId: 's1', workDir: '/tmp/proj' }); - expect(h.id).toBe('s1'); - }); - it('fork acquires a distinct target lease; releasing the source does not fence the target', async () => { const root = await makeTmpRoot(); const { seeds, appendLog } = realAlsSeeds(root); @@ -1633,6 +1673,101 @@ describe('SessionLifecycleService', () => { expect(disk).toBe('{"tail":true}\n'); await expect(stat(leaseFile(root, 's1'))).rejects.toThrow(); }); + + it('keeps authority and lease when the session durability barrier fails', async () => { + const root = await makeTmpRoot(); + const { seeds, appendLog, registry, storage } = realAlsSeeds(root); + const svc = build(seeds); + await svc.create({ sessionId: 's1', workDir: '/tmp/proj' }); + const failure = new Error('durable append failed'); + const originalAppend = storage.append.bind(storage); + storage.append = async (...args) => { + if (args[0].startsWith('sessions/wd_stub/s1')) throw failure; + return originalAppend(...args); + }; + appendLog.append('sessions/wd_stub/s1/agents/main', 'wire.jsonl', { tail: true }); + + await expect(svc.close('s1')).rejects.toBe(failure); + expect(svc.get('s1')).toBeUndefined(); + expect(registry.resolve('s1')).toBeDefined(); + await expect(stat(leaseFile(root, 's1'))).resolves.toBeDefined(); + await expect( + new CrossProcessLockService().acquire(leaseFile(root, 's1')), + ).rejects.toMatchObject({ code: CrossProcessLockErrorCode.Held }); + await expect(svc.close('s1')).rejects.toBe(failure); + expect(registry.resolve('s1')).toBeDefined(); + }); + + it('requires an explicit dirty abort before releasing a failed durability lease', async () => { + const root = await makeTmpRoot(); + const { seeds, appendLog, registry, storage, docs } = realAlsSeeds(root); + const svc = build(seeds); + await svc.create({ sessionId: 's1', workDir: '/tmp/proj' }); + await docs.set('sessions/wd_stub/s1', 'state.json', { + id: 's1', + version: 2, + cwd: '/tmp/proj', + createdAt: 1, + updatedAt: 1, + archived: false, + agents: {}, + custom: {}, + }); + const failure = new Error('durable append failed'); + const originalAppend = storage.append.bind(storage); + storage.append = async (...args) => { + if (args[0].startsWith('sessions/wd_stub/s1')) throw failure; + return originalAppend(...args); + }; + appendLog.append('sessions/wd_stub/s1/agents/main', 'wire.jsonl', { tail: true }); + + await expect(svc.close('s1')).rejects.toBe(failure); + await svc.forceAbort('s1'); + + expect(registry.resolve('s1')).toBeUndefined(); + await expect(stat(leaseFile(root, 's1'))).rejects.toThrow(); + const successor = await new CrossProcessLockService().acquire(leaseFile(root, 's1')); + successor.release(); + expect(telemetryRecords).toContainEqual({ + event: 'session_dirty_abort', + properties: { session_id: 's1', reason: 'flush-failed', sessionId: 's1' }, + }); + }); + + it('closing one session does not wait for another session append', async () => { + const root = await makeTmpRoot(); + const { seeds, appendLog, storage } = realAlsSeeds(root); + const svc = build(seeds); + await svc.create({ sessionId: 's1', workDir: '/tmp/proj' }); + await svc.create({ sessionId: 's10', workDir: '/tmp/proj' }); + let markBlockedStarted!: () => void; + const blockedStarted = new Promise((resolvePromise) => { + markBlockedStarted = resolvePromise; + }); + let releaseBlocked!: () => void; + const blockedGate = new Promise((resolvePromise) => { + releaseBlocked = resolvePromise; + }); + const originalAppend = storage.append.bind(storage); + storage.append = async (...args) => { + if (args[0].startsWith('sessions/wd_stub/s10')) { + markBlockedStarted(); + await blockedGate; + } + return originalAppend(...args); + }; + appendLog.append('sessions/wd_stub/s10/agents/main', 'wire.jsonl', { n: 2 }); + appendLog.append('sessions/wd_stub/s1/agents/main', 'wire.jsonl', { n: 1 }); + await blockedStarted; + + await svc.close('s1'); + await expect(stat(leaseFile(root, 's1'))).rejects.toThrow(); + await expect(stat(leaseFile(root, 's10'))).resolves.toBeDefined(); + + releaseBlocked(); + await appendLog.flush('sessions/wd_stub/s10'); + await svc.close('s10'); + }); }); describe('defaultPlanMode bootstrap', () => { diff --git a/packages/agent-core-v2/test/app/workspace/workspaceService.test.ts b/packages/agent-core-v2/test/app/workspace/workspaceService.test.ts index 43e4d1ee8..c01ac0303 100644 --- a/packages/agent-core-v2/test/app/workspace/workspaceService.test.ts +++ b/packages/agent-core-v2/test/app/workspace/workspaceService.test.ts @@ -63,13 +63,14 @@ describe('WorkspaceService (file-backed)', () => { }); function build(hostFs: IHostFileSystem = new HostFileSystem()): IWorkspaceService { - const fileStorage = new FileStorageService(homeDir); + const locks = new CrossProcessLockService(); + const fileStorage = new FileStorageService(homeDir, undefined, undefined, locks); const host = createScopedTestHost([ stubPair(IFileSystemStorageService, fileStorage), stubPair(IAtomicDocumentStore, new JsonAtomicDocumentStore(fileStorage)), stubPair(IHostFileSystem, hostFs), stubPair(IBootstrapService, stubBootstrap(homeDir)), - stubPair(ICrossProcessLockService, new CrossProcessLockService()), + stubPair(ICrossProcessLockService, locks), ]); hosts.push(host); return host.app.accessor.get(IWorkspaceService); diff --git a/packages/agent-core-v2/test/os/backends/node-local/crossProcessLockService.test.ts b/packages/agent-core-v2/test/os/backends/node-local/crossProcessLockService.test.ts index 03e185476..41d577702 100644 --- a/packages/agent-core-v2/test/os/backends/node-local/crossProcessLockService.test.ts +++ b/packages/agent-core-v2/test/os/backends/node-local/crossProcessLockService.test.ts @@ -15,6 +15,7 @@ import { mkdtempSync, readFileSync, readdirSync, + renameSync, rmSync, utimesSync, writeFileSync, @@ -44,6 +45,7 @@ let tmpDir: string; let lockPath: string; let nowValue: number; let lockSeq: number; +let attemptSeq: number; const handles: ICrossProcessLockHandle[] = []; function track(handle: T): T { @@ -66,6 +68,8 @@ interface FakeServiceOptions { instanceId?: string; probe?: ProcessProbe; now?: () => number; + beforeStaleIsolation?: () => void | Promise; + sleep?: (ms: number) => Promise; } function makeService(options: FakeServiceOptions = {}): CrossProcessLockService { @@ -76,7 +80,9 @@ function makeService(options: FakeServiceOptions = {}): CrossProcessLockService probeProcess: options.probe ?? probeFor(new Map([[selfPid, 'self-start']])), now: options.now ?? (() => nowValue), newLockId: () => `lockid-${++lockSeq}`, - sleep: realSleep, + newAttemptId: () => `attempt-${++attemptSeq}`, + beforeStaleIsolation: options.beforeStaleIsolation, + sleep: options.sleep ?? realSleep, }); } @@ -111,6 +117,13 @@ function backdate(path: string, ageMs: number): void { utimesSync(path, t, t); } +function findStalePath(lockId: string): string { + const prefix = `lock.stale.${lockId}.`; + const name = readdirSync(tmpDir).find((entry) => entry.startsWith(prefix)); + expect(name).toBeDefined(); + return join(tmpDir, name!); +} + async function waitFor(cond: () => boolean, timeoutMs = 3_000): Promise { const start = Date.now(); for (;;) { @@ -125,6 +138,7 @@ beforeEach(() => { lockPath = join(tmpDir, 'lock'); nowValue = 1_000_000; lockSeq = 0; + attemptSeq = 0; }); afterEach(() => { @@ -133,10 +147,10 @@ afterEach(() => { }); describe('acquire / release', () => { - it('writes the snake_case payload with extras flat, and release removes the file', () => { + it('writes the snake_case payload with extras flat, and release removes the file', async () => { const svc = makeService(); const handle = track( - svc.acquire(lockPath, { + await svc.acquire(lockPath, { address: '127.0.0.1:58627', extraPayload: { port: 58627, role: 'primary' }, }), @@ -156,9 +170,9 @@ describe('acquire / release', () => { expect(existsSync(lockPath)).toBe(false); }); - it('omits optional keys when the platform cannot provide them', () => { + it('omits optional keys when the platform cannot provide them', async () => { const svc = makeService({ probe: () => ({ alive: true }) }); - const handle = track(svc.acquire(lockPath)); + const handle = track(await svc.acquire(lockPath)); expect(readDisk()).toEqual({ lock_id: 'lockid-1', instance_id: 'inst-self', @@ -166,19 +180,19 @@ describe('acquire / release', () => { }); }); - it('creates missing parent directories and release is idempotent', () => { + it('creates missing parent directories and release is idempotent', async () => { const nested = join(tmpDir, 'a', 'b', 'lock'); const svc = makeService(); - const handle = track(svc.acquire(nested)); + const handle = track(await svc.acquire(nested)); expect(existsSync(nested)).toBe(true); handle.release(); handle.release(); expect(existsSync(nested)).toBe(false); }); - it('release never unlinks a foreign lock', () => { + it('release never unlinks a foreign lock', async () => { const svc = makeService(); - const handle = track(svc.acquire(lockPath)); + const handle = track(await svc.acquire(lockPath)); handle.release(); writePayload({ lock_id: 'someone-else', instance_id: 'x', pid: OTHER_PID }); handle.release(); @@ -188,7 +202,7 @@ describe('acquire / release', () => { }); describe('held vs takeover', () => { - it('a live identity-matching holder blocks acquisition with OS_LOCK_HELD', () => { + it('a live identity-matching holder blocks acquisition with OS_LOCK_HELD', async () => { writePayload({ lock_id: 'old-id', instance_id: 'inst-other', @@ -198,13 +212,7 @@ describe('held vs takeover', () => { const svc = makeService({ probe: probeFor(liveWorld()) }); const before = readFileSync(lockPath, 'utf8'); - let caught: unknown; - try { - svc.acquire(lockPath); - } catch (error) { - caught = error; - } - expect(caught).toMatchObject({ + await expect(svc.acquire(lockPath)).rejects.toMatchObject({ code: CrossProcessLockErrorCode.Held, details: { reason: 'held' }, }); @@ -212,11 +220,11 @@ describe('held vs takeover', () => { expect(readdirSync(tmpDir)).toEqual(['lock']); }); - it('takes over a dead holder with rename isolation', () => { + it('takes over a dead holder with rename isolation', async () => { const live = liveWorld(); const probe = probeFor(live); const oldHandle = track( - makeService({ selfPid: OTHER_PID, instanceId: 'inst-other', probe }).acquire( + await makeService({ selfPid: OTHER_PID, instanceId: 'inst-other', probe }).acquire( lockPath, { extraPayload: { port: 1 } }, ), @@ -224,10 +232,10 @@ describe('held vs takeover', () => { const oldDisk = JSON.parse(readFileSync(lockPath, 'utf8')) as Record; live.delete(OTHER_PID); - const handle = track(makeService({ probe }).acquire(lockPath)); + const handle = track(await makeService({ probe }).acquire(lockPath)); - expect(existsSync(`${lockPath}.stale.lockid-1`)).toBe(true); - expect(JSON.parse(readFileSync(`${lockPath}.stale.lockid-1`, 'utf8'))).toEqual(oldDisk); + const stalePath = findStalePath('lockid-1'); + expect(JSON.parse(readFileSync(stalePath, 'utf8'))).toEqual(oldDisk); expect(readDisk().lock_id).toBe('lockid-2'); expect(handle.lockId).toBe('lockid-2'); @@ -237,7 +245,7 @@ describe('held vs takeover', () => { expect(readDisk().lock_id).toBe('lockid-2'); }); - it('treats a live pid with mismatched identity as stale (pid reused)', () => { + it('treats a live pid with mismatched identity as stale (pid reused)', async () => { writePayload({ lock_id: 'old-id', instance_id: 'inst-other', @@ -252,12 +260,12 @@ describe('held vs takeover', () => { state: 'stale', staleReason: 'pid-reused', }); - track(svc.acquire(lockPath)); - expect(existsSync(`${lockPath}.stale.old-id`)).toBe(true); + track(await svc.acquire(lockPath)); + expect(existsSync(findStalePath('old-id'))).toBe(true); expect(readDisk().lock_id).toBe('lockid-1'); }); - it('refuses takeover when the holder identity is unavailable (conservative held)', () => { + it('refuses takeover when the holder identity is unavailable (conservative held)', async () => { writePayload({ lock_id: 'old-id', instance_id: 'inst-other', @@ -269,14 +277,14 @@ describe('held vs takeover', () => { const svc = makeService({ probe: probeFor(live) }); expect(svc.inspect(lockPath)).toMatchObject({ state: 'held', unavailableReason: 'held' }); - expect(() => svc.acquire(lockPath)).toThrowError( - expect.objectContaining({ code: CrossProcessLockErrorCode.Held }), - ); + await expect(svc.acquire(lockPath)).rejects.toMatchObject({ + code: CrossProcessLockErrorCode.Held, + }); expect(readDisk().lock_id).toBe('old-id'); expect(readdirSync(tmpDir)).toEqual(['lock']); }); - it('takes over a legacy payload without lock_id, renamed aside as unknown', () => { + it('takes over a legacy payload without lock_id, renamed aside as unknown', async () => { writePayload({ pid: OTHER_PID, started_at: '1', port: 58627 }); const svc = makeService(); @@ -285,25 +293,113 @@ describe('held vs takeover', () => { staleReason: 'holder-dead', payload: { lockId: '', pid: OTHER_PID, port: 58627 }, }); - track(svc.acquire(lockPath)); - expect(existsSync(`${lockPath}.stale.unknown`)).toBe(true); + track(await svc.acquire(lockPath)); + expect(existsSync(findStalePath('unknown'))).toBe(true); expect(readDisk().lock_id).toBe('lockid-1'); }); + + it('never lets two delayed stale contenders both return ownership', async () => { + writePayload({ + lock_id: 'old-id', + instance_id: 'inst-dead', + pid: DEAD_PID, + }); + const probe = probeFor(liveWorld()); + let enterIsolation!: () => void; + const isolationEntered = new Promise((resolvePromise) => { + enterIsolation = resolvePromise; + }); + let resumeIsolation!: () => void; + const isolationPaused = new Promise((resolvePromise) => { + resumeIsolation = resolvePromise; + }); + let pauseCount = 0; + const contenderA = makeService({ + probe, + now: Date.now, + beforeStaleIsolation: async () => { + pauseCount += 1; + if (pauseCount !== 1) return; + enterIsolation(); + await isolationPaused; + }, + }); + const contenderB = makeService({ + selfPid: OTHER_PID, + instanceId: 'inst-b', + probe, + now: Date.now, + }); + + const acquireA = contenderA.acquire(lockPath, { creationWindowMs: 500 }); + await isolationEntered; + const acquireB = contenderB.acquire(lockPath, { creationWindowMs: 500 }); + await waitFor(() => readDisk().lock_id === 'lockid-1'); + resumeIsolation(); + + const outcomes = await Promise.allSettled([acquireA, acquireB]); + const fulfilled = outcomes.filter( + (outcome): outcome is PromiseFulfilledResult => + outcome.status === 'fulfilled', + ); + const rejected = outcomes.filter( + (outcome): outcome is PromiseRejectedResult => outcome.status === 'rejected', + ); + expect(fulfilled).toHaveLength(1); + expect(rejected).toHaveLength(1); + expect(rejected[0]?.reason).toMatchObject({ code: CrossProcessLockErrorCode.Lost }); + track(fulfilled[0]!.value); + expect(readDisk().lock_id).toBe(fulfilled[0]!.value.lockId); + expect(readdirSync(tmpDir).filter((entry) => entry.startsWith('lock.intent.'))).toEqual([]); + }); + + it('settles against the contender snapshot without waiting for later arrivals', async () => { + const olderIntent = `${lockPath}.intent.older`; + const laterIntent = `${lockPath}.intent.later`; + writeFileSync( + olderIntent, + JSON.stringify({ pid: OTHER_PID, process_started_at: 'other-start' }), + ); + let sleepCount = 0; + const svc = makeService({ + probe: probeFor(liveWorld()), + sleep: async () => { + sleepCount += 1; + writeFileSync( + laterIntent, + JSON.stringify({ pid: OTHER_PID, process_started_at: 'other-start' }), + ); + rmSync(olderIntent, { force: true }); + }, + }); + + const handle = track(await svc.acquire(lockPath, { creationWindowMs: 500 })); + + expect(sleepCount).toBe(1); + expect(handle.checkHeld()).toBe(true); + expect(existsSync(laterIntent)).toBe(true); + }); + + it('ignores a settled intent left behind after cleanup failure', async () => { + writeFileSync( + `${lockPath}.intent.orphan`, + JSON.stringify({ intent_id: 'orphan', state: 'settled', pid: OTHER_PID }), + ); + + const handle = track(await makeService({ probe: probeFor(liveWorld()) }).acquire(lockPath)); + + expect(handle.lockId).toBe('lockid-1'); + expect(handle.checkHeld()).toBe(true); + }); }); describe('creation window', () => { - it('an empty file inside the window is creating', () => { + it('an empty file inside the window is creating', async () => { writeFileSync(lockPath, ''); const svc = makeService({ now: () => Date.now() }); expect(svc.inspect(lockPath)).toMatchObject({ state: 'creating' }); - let caught: unknown; - try { - svc.acquire(lockPath); - } catch (error) { - caught = error; - } - expect(caught).toMatchObject({ + await expect(svc.acquire(lockPath)).rejects.toMatchObject({ code: CrossProcessLockErrorCode.Held, details: { reason: 'creating' }, }); @@ -311,7 +407,7 @@ describe('creation window', () => { expect(readdirSync(tmpDir)).toEqual(['lock']); }); - it('an empty file past the window is taken over as stale', () => { + it('an empty file past the window is taken over as stale', async () => { writeFileSync(lockPath, ''); backdate(lockPath, 10_000); const svc = makeService({ now: () => Date.now() }); @@ -320,12 +416,12 @@ describe('creation window', () => { state: 'stale', staleReason: 'creation-window-expired', }); - track(svc.acquire(lockPath)); - expect(existsSync(`${lockPath}.stale.unknown`)).toBe(true); + track(await svc.acquire(lockPath)); + expect(existsSync(findStalePath('unknown'))).toBe(true); expect(readDisk().lock_id).toBe('lockid-1'); }); - it('an unparseable file follows the same window', () => { + it('an unparseable file follows the same window', async () => { writeFileSync(lockPath, '{oops'); const svc = makeService({ now: () => Date.now() }); @@ -335,8 +431,8 @@ describe('creation window', () => { state: 'stale', staleReason: 'creation-window-expired', }); - track(svc.acquire(lockPath)); - expect(existsSync(`${lockPath}.stale.unknown`)).toBe(true); + track(await svc.acquire(lockPath)); + expect(existsSync(findStalePath('unknown'))).toBe(true); expect(readDisk().instance_id).toBe('inst-self'); }); }); @@ -345,7 +441,7 @@ describe('heartbeat mode', () => { it('beats heartbeat_at into the disk payload through the kept fd', async () => { const svc = makeService(); const handle = track( - svc.acquire(lockPath, { heartbeat: { intervalMs: 20, ttlMs: 60_000 } }), + await svc.acquire(lockPath, { heartbeat: { intervalMs: 20, ttlMs: 60_000 } }), ); expect(readDisk().heartbeat_at).toBe(1_000_000); @@ -360,7 +456,7 @@ describe('heartbeat mode', () => { const probe = probeFor(live); let lostCount = 0; const oldHandle = track( - makeService({ probe }).acquire(lockPath, { + await makeService({ probe }).acquire(lockPath, { heartbeat: { intervalMs: 20, ttlMs: 60_000 }, onLost: () => { lostCount += 1; @@ -370,7 +466,7 @@ describe('heartbeat mode', () => { live.delete(SELF_PID); track( - makeService({ selfPid: OTHER_PID, instanceId: 'inst-b', probe }).acquire(lockPath), + await makeService({ selfPid: OTHER_PID, instanceId: 'inst-b', probe }).acquire(lockPath), ); await waitFor(() => lostCount === 1); @@ -380,7 +476,7 @@ describe('heartbeat mode', () => { expect(readDisk().lock_id).toBe('lockid-2'); }); - it('a silent heartbeat with a live identity-matching pid is holder-unresponsive, never seized', () => { + it('a silent heartbeat with a live identity-matching pid is holder-unresponsive, never seized', async () => { writePayload({ lock_id: 'old-id', instance_id: 'inst-other', @@ -391,13 +487,9 @@ describe('heartbeat mode', () => { const svc = makeService({ probe: probeFor(liveWorld()) }); const before = readFileSync(lockPath, 'utf8'); - let caught: unknown; - try { - svc.acquire(lockPath, { heartbeat: { intervalMs: 100, ttlMs: 5_000 } }); - } catch (error) { - caught = error; - } - expect(caught).toMatchObject({ + await expect( + svc.acquire(lockPath, { heartbeat: { intervalMs: 100, ttlMs: 5_000 } }), + ).rejects.toMatchObject({ code: CrossProcessLockErrorCode.Held, details: { reason: 'holder-unresponsive' }, }); @@ -405,7 +497,7 @@ describe('heartbeat mode', () => { expect(readdirSync(tmpDir)).toEqual(['lock']); }); - it('a fresh heartbeat with a live holder is a plain held', () => { + it('a fresh heartbeat with a live holder is a plain held', async () => { writePayload({ lock_id: 'old-id', instance_id: 'inst-other', @@ -415,13 +507,9 @@ describe('heartbeat mode', () => { }); const svc = makeService({ probe: probeFor(liveWorld()) }); - let caught: unknown; - try { - svc.acquire(lockPath, { heartbeat: { intervalMs: 100, ttlMs: 5_000 } }); - } catch (error) { - caught = error; - } - expect(caught).toMatchObject({ + await expect( + svc.acquire(lockPath, { heartbeat: { intervalMs: 100, ttlMs: 5_000 } }), + ).rejects.toMatchObject({ code: CrossProcessLockErrorCode.Held, details: { reason: 'held' }, }); @@ -433,7 +521,7 @@ describe('acquireWithWait / withLock', () => { it('a waiting acquirer obtains the lock after the holder releases', async () => { const live = liveWorld(); const probe = probeFor(live); - const holder = track(makeService({ probe }).acquire(lockPath)); + const holder = track(await makeService({ probe }).acquire(lockPath)); const waiter = makeService({ selfPid: OTHER_PID, @@ -461,7 +549,7 @@ describe('acquireWithWait / withLock', () => { it('a waiting acquirer gives up with OS_LOCK_WAIT_TIMEOUT', async () => { const live = liveWorld(); const probe = probeFor(live); - track(makeService({ probe }).acquire(lockPath)); + track(await makeService({ probe }).acquire(lockPath)); const waiter = makeService({ selfPid: OTHER_PID, @@ -477,9 +565,9 @@ describe('acquireWithWait / withLock', () => { }); describe('update', () => { - it('rewrites extras and re-stamps the protocol keys', () => { + it('rewrites extras and re-stamps the protocol keys', async () => { const svc = makeService(); - const handle = track(svc.acquire(lockPath, { extraPayload: { port: 1 } })); + const handle = track(await svc.acquire(lockPath, { extraPayload: { port: 1 } })); handle.update((payload) => ({ ...payload, @@ -501,7 +589,7 @@ describe('update', () => { it('a later update keeps the extras the heartbeat rewrites', async () => { const svc = makeService(); const handle = track( - svc.acquire(lockPath, { + await svc.acquire(lockPath, { heartbeat: { intervalMs: 20, ttlMs: 60_000 }, extraPayload: { port: 1 }, }), @@ -519,15 +607,15 @@ describe('update', () => { handle.release(); }); - it('update after a takeover throws OS_LOCK_LOST', () => { + it('update after a takeover throws OS_LOCK_LOST', async () => { const live = liveWorld(); const probe = probeFor(live); const oldHandle = track( - makeService({ probe }).acquire(lockPath, { extraPayload: { port: 1 } }), + await makeService({ probe }).acquire(lockPath, { extraPayload: { port: 1 } }), ); live.delete(SELF_PID); track( - makeService({ selfPid: OTHER_PID, instanceId: 'inst-b', probe }).acquire(lockPath), + await makeService({ selfPid: OTHER_PID, instanceId: 'inst-b', probe }).acquire(lockPath), ); expect(() => { @@ -536,6 +624,28 @@ describe('update', () => { expect(readDisk().lock_id).toBe('lockid-2'); expect(readDisk().port).toBeUndefined(); }); + + it('update detects a takeover that happens after its initial token check', async () => { + const svc = makeService(); + const handle = track(await svc.acquire(lockPath, { extraPayload: { port: 1 } })); + const oldPath = `${lockPath}.old`; + + expect(() => { + handle.update((payload) => { + renameSync(lockPath, oldPath); + writeFileSync( + lockPath, + JSON.stringify({ + lock_id: 'foreign-lock', + instance_id: 'foreign-instance', + pid: OTHER_PID, + }), + ); + return { ...payload, port: 2 }; + }); + }).toThrowError(expect.objectContaining({ code: CrossProcessLockErrorCode.Lost })); + expect(readDisk().lock_id).toBe('foreign-lock'); + }); }); describe('inspect', () => { diff --git a/packages/agent-core-v2/test/os/stubs.ts b/packages/agent-core-v2/test/os/stubs.ts index 3d0cdb277..d7d7a053d 100644 --- a/packages/agent-core-v2/test/os/stubs.ts +++ b/packages/agent-core-v2/test/os/stubs.ts @@ -18,7 +18,7 @@ import { export function stubCrossProcessLock(): ICrossProcessLockService { const held = new Set(); - const acquire = (lockPath: string): ICrossProcessLockHandle => { + const acquireHandle = (lockPath: string): ICrossProcessLockHandle => { if (held.has(lockPath)) { throw new CrossProcessLockError( CrossProcessLockErrorCode.Held, @@ -42,14 +42,14 @@ export function stubCrossProcessLock(): ICrossProcessLockService { }; return { _serviceBrand: undefined, - acquire, - acquireWithWait: (lockPath) => Promise.resolve(acquire(lockPath)), + acquire: (lockPath) => Promise.resolve(acquireHandle(lockPath)), + acquireWithWait: (lockPath) => Promise.resolve(acquireHandle(lockPath)), withLock: async ( lockPath: string, _options: Parameters[1], fn: (handle: ICrossProcessLockHandle) => T | Promise, ): Promise => { - const handle = acquire(lockPath); + const handle = acquireHandle(lockPath); try { return await fn(handle); } finally { diff --git a/packages/agent-core-v2/test/persistence/backends/node-fs/appendLogStore.test.ts b/packages/agent-core-v2/test/persistence/backends/node-fs/appendLogStore.test.ts index ce4a20ed7..b4d2ffa6b 100644 --- a/packages/agent-core-v2/test/persistence/backends/node-fs/appendLogStore.test.ts +++ b/packages/agent-core-v2/test/persistence/backends/node-fs/appendLogStore.test.ts @@ -143,7 +143,7 @@ describe('AppendLogStore', () => { ); }); - it('reacquire after final release waits for retiring storage before fresh I/O', async () => { + it('keeps a failed retired generation reachable and blocks replacement I/O', async () => { const failure = new Error('retiring append failed'); let markAppendStarted!: () => void; const appendStarted = new Promise((resolve) => { @@ -153,32 +153,16 @@ describe('AppendLogStore', () => { const appendGate = new Promise((resolve) => { releaseAppend = resolve; }); - let markReplacementAppendStarted!: () => void; - const replacementAppendStarted = new Promise((resolve) => { - markReplacementAppendStarted = resolve; - }); - let releaseReplacementAppend!: () => void; - const replacementAppendGate = new Promise((resolve) => { - releaseReplacementAppend = resolve; - }); let reportFailure!: (error: unknown) => void; const reportedFailure = new Promise((resolve) => { reportFailure = resolve; }); let appendAttempts = 0; - let replacementStarted = false; - const originalAppend = storage.append.bind(storage); storage.append = async (...args) => { appendAttempts++; - if (appendAttempts === 1) { - markAppendStarted(); - await appendGate; - throw failure; - } - replacementStarted = true; - markReplacementAppendStarted(); - await replacementAppendGate; - return originalAppend(...args); + markAppendStarted(); + await appendGate; + throw failure; }; const retiringOwner = record.acquire(SCOPE, KEY); @@ -187,30 +171,48 @@ describe('AppendLogStore', () => { retiringOwner.dispose(); const replacementOwner = record.acquire(SCOPE, KEY); record.append(SCOPE, KEY, { n: 2 }); - const orderedFlush = record.flush(); - await Promise.resolve(); - await Promise.resolve(); - expect(replacementStarted).toBe(false); releaseAppend(); expect(await reportedFailure).toBe(failure); - await replacementAppendStarted; - - const currentFlush = record.flush(); - let flushSettled = false; - void currentFlush.then(() => { - flushSettled = true; - }); - await Promise.resolve(); - await Promise.resolve(); - expect(flushSettled).toBe(false); - - releaseReplacementAppend(); - await Promise.all([orderedFlush, currentFlush]); - expect(await collect(SCOPE, KEY)).toEqual([{ n: 2 }]); + await expect(record.flush(SCOPE)).rejects.toBe(failure); + await expect(record.flush(SCOPE)).rejects.toBe(failure); + expect(appendAttempts).toBe(1); + expect(await storage.read(SCOPE, KEY)).toBeUndefined(); replacementOwner.dispose(); }); + it('scoped flush does not wait for an unrelated scope', async () => { + const selectedScope = 'agents/s1'; + const blockedScope = 'agents/s10'; + let markBlockedStarted!: () => void; + const blockedStarted = new Promise((resolve) => { + markBlockedStarted = resolve; + }); + let releaseBlocked!: () => void; + const blockedGate = new Promise((resolve) => { + releaseBlocked = resolve; + }); + const originalAppend = storage.append.bind(storage); + storage.append = async (...args) => { + if (args[0] === blockedScope) { + markBlockedStarted(); + await blockedGate; + } + return originalAppend(...args); + }; + + record.append(blockedScope, KEY, { n: 2 }); + record.append(selectedScope, KEY, { n: 1 }); + await blockedStarted; + + await record.flush(selectedScope); + expect(new TextDecoder().decode(await storage.read(selectedScope, KEY))).toBe('{"n":1}\n'); + + releaseBlocked(); + await record.flush(blockedScope); + expect(new TextDecoder().decode(await storage.read(blockedScope, KEY))).toBe('{"n":2}\n'); + }); + it('keeps a sticky failure until every acquired owner releases it', async () => { const failure = new Error('shared append failed'); let reportFailure!: (error: unknown) => void; @@ -236,8 +238,8 @@ describe('AppendLogStore', () => { finalOwner.dispose(); const replacementOwner = record.acquire(SCOPE, KEY); record.append(SCOPE, KEY, { n: 2 }); - await record.flush(); - expect(await collect(SCOPE, KEY)).toEqual([{ n: 2 }]); + await expect(record.flush()).rejects.toBe(failure); + expect(appendAttempts).toBe(1); replacementOwner.dispose(); }); @@ -723,10 +725,10 @@ describe('AppendLogStore', () => { return { store: localIx.get(IAppendLogStore), storage }; } - function leaseFor(sessionId: string): SessionLease { + async function leaseFor(sessionId: string): Promise { return new SessionLease( sessionId, - locks.acquire(sessionLeasePath(tmpDir, sessionId)), + await locks.acquire(sessionLeasePath(tmpDir, sessionId)), () => {}, ); } @@ -751,7 +753,7 @@ describe('AppendLogStore', () => { }); it('flush rejects with session.lease_lost when the lease is gone and writes no bytes', async () => { - const lease = leaseFor('s1'); + const lease = await leaseFor('s1'); registry.register(lease); const { store, storage } = makeStore(); let appendAttempts = 0; @@ -775,7 +777,7 @@ describe('AppendLogStore', () => { }); it('rewrite is fenced by the same hard gate', async () => { - const lease = leaseFor('s1'); + const lease = await leaseFor('s1'); registry.register(lease); const { store, storage } = makeStore(); let writeAttempts = 0; @@ -813,7 +815,7 @@ describe('AppendLogStore', () => { }); it('writes pass while the registered lease is held', async () => { - const lease = leaseFor('s1'); + const lease = await leaseFor('s1'); registry.register(lease); const { store, storage } = makeStore(); store.append(SESSION_SCOPE, KEY, { n: 1 }); @@ -823,7 +825,7 @@ describe('AppendLogStore', () => { }); it('flush awaits the retired buffer final flush before returning', async () => { - const lease = leaseFor('s1'); + const lease = await leaseFor('s1'); registry.register(lease); const { store, storage } = makeStore(); const handle = store.acquire(SESSION_SCOPE, KEY); diff --git a/packages/agent-core-v2/test/persistence/backends/node-fs/atomicDocumentStore.test.ts b/packages/agent-core-v2/test/persistence/backends/node-fs/atomicDocumentStore.test.ts index ae427f496..3d6cc3689 100644 --- a/packages/agent-core-v2/test/persistence/backends/node-fs/atomicDocumentStore.test.ts +++ b/packages/agent-core-v2/test/persistence/backends/node-fs/atomicDocumentStore.test.ts @@ -45,6 +45,42 @@ describe('JsonAtomicDocumentStore', () => { expect(await config.get('session', 'state.json')).toEqual({ title: 'new', count: 2 }); }); + it('serializes concurrent read-modify-write updates for one key', async () => { + await config.set('session', 'state.json', { count: 0 }); + let releaseFirst!: () => void; + const firstMayFinish = new Promise((resolve) => { + releaseFirst = resolve; + }); + let firstEntered!: () => void; + const firstDidEnter = new Promise((resolve) => { + firstEntered = resolve; + }); + + const first = config.update('session', 'state.json', async (current) => { + firstEntered(); + await firstMayFinish; + return { count: (current?.count ?? 0) + 1 }; + }); + await firstDidEnter; + const second = config.update('session', 'state.json', (current) => ({ + count: (current?.count ?? 0) + 1, + })); + releaseFirst(); + + await expect(Promise.all([first, second])).resolves.toEqual([{ count: 1 }, { count: 2 }]); + expect(await config.get('session', 'state.json')).toEqual({ count: 2 }); + }); + + it('does not replace the document when an update callback fails', async () => { + await config.set('session', 'state.json', { count: 1 }); + await expect( + config.update('session', 'state.json', () => { + throw new Error('mutate failed'); + }), + ).rejects.toThrow('mutate failed'); + expect(await config.get('session', 'state.json')).toEqual({ count: 1 }); + }); + it('keys are independent', async () => { await config.set('session', 'a.json', { title: 'A' }); await config.set('session', 'b.json', { title: 'B' }); diff --git a/packages/agent-core-v2/test/session/agentLifecycle/agentLifecycle.test.ts b/packages/agent-core-v2/test/session/agentLifecycle/agentLifecycle.test.ts index 3eacfc9cd..2e4b1270a 100644 --- a/packages/agent-core-v2/test/session/agentLifecycle/agentLifecycle.test.ts +++ b/packages/agent-core-v2/test/session/agentLifecycle/agentLifecycle.test.ts @@ -210,6 +210,17 @@ describe('AgentLifecycleService', () => { set: async (scope: string, key: string, value: T): Promise => { atomicDocs.set(`${scope}/${key}`, value); }, + update: async ( + scope: string, + key: string, + mutate: (current: T | undefined) => T | Promise, + ): Promise => { + const next = await mutate(atomicDocs.get(`${scope}/${key}`) as T | undefined); + atomicDocs.set(`${scope}/${key}`, next); + return next; + }, + runExclusive: async (_scope: string, _key: string, op: () => Promise): Promise => + op(), delete: async (scope: string, key: string): Promise => { atomicDocs.delete(`${scope}/${key}`); }, diff --git a/packages/agent-core-v2/test/session/sessionFileLedger/fileLedger.test.ts b/packages/agent-core-v2/test/session/sessionFileLedger/fileLedger.test.ts index 31489b7fe..7cc1ac4ec 100644 --- a/packages/agent-core-v2/test/session/sessionFileLedger/fileLedger.test.ts +++ b/packages/agent-core-v2/test/session/sessionFileLedger/fileLedger.test.ts @@ -2,10 +2,9 @@ * `sessionFileLedger` domain (L2) — verifies the optimistic-concurrency * verdict matrix (clean / stale / no-baseline) against a real tmpdir, a real * `HostFileSystem` (stat-call counted) and a fake os watcher: baselines only - * refresh on success, dirty ticks come from the watch service's folded - * state, watcher echoes of the session's own writes punch a stat and - * re-baseline, truncated windows fall back to the per-root dirty tick, and - * out-of-root targets degrade to a stat-only comparison. + * refresh on success, every write decision performs a fresh stat, dirty ticks + * arrive before debounced event delivery, watcher echoes of the session's own + * writes re-baseline, and truncated windows retain conservative root ticks. */ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; @@ -171,6 +170,18 @@ describe('SessionFileLedger', () => { expect(await world.ledger.compare(file)).toBe('stale'); }); + it('detects an outside modification before the watch debounce flush', async () => { + const world = makeWorld(); + const file = join(world.workDir, 'a.txt'); + writeFileSync(file, 'hello'); + await world.ledger.recordBaseline(file); + + writeFileSync(file, 'hello world'); + world.fake.fire('a.txt', 'modified'); + + expect(await world.ledger.compare(file)).toBe('stale'); + }); + it('absorbs the watcher echo of the session own write and re-baselines the tick', async () => { const world = makeWorld(); const file = join(world.workDir, 'a.txt'); @@ -185,7 +196,7 @@ describe('SessionFileLedger', () => { expect(world.statCalls()).toBe(2); expect(await world.ledger.compare(file)).toBe('clean'); - expect(world.statCalls()).toBe(2); + expect(world.statCalls()).toBe(3); }); it('keeps a baselined file clean through an untouched truncated window', async () => { @@ -200,7 +211,7 @@ describe('SessionFileLedger', () => { expect(await world.ledger.compare(file)).toBe('clean'); expect(world.statCalls()).toBe(2); expect(await world.ledger.compare(file)).toBe('clean'); - expect(world.statCalls()).toBe(2); + expect(world.statCalls()).toBe(3); }); it('detects an outside modification through a truncated window via the stat punch', async () => { @@ -274,14 +285,14 @@ describe('SessionFileLedger', () => { expect(await world.ledger.compare(file)).toBe('stale'); }); - it('degrades to clean when stat fails for reasons other than not-found', async () => { + it('fails closed when stat fails for reasons other than not-found', async () => { const world = makeWorld(); const file = join(world.workDir, 'a.txt'); writeFileSync(file, 'hello'); world.poisonedPaths.add(file); await world.ledger.recordBaseline(file); - expect(await world.ledger.compare(file)).toBe('clean'); + expect(await world.ledger.compare(file)).toBe('stale'); world.poisonedPaths.clear(); await world.ledger.recordBaseline(file); @@ -289,7 +300,7 @@ describe('SessionFileLedger', () => { world.fake.fire('a.txt', 'modified'); vi.advanceTimersByTime(200); - expect(await world.ledger.compare(file)).toBe('clean'); + expect(await world.ledger.compare(file)).toBe('stale'); expect(world.statCalls()).toBeGreaterThan(0); }); }); diff --git a/packages/agent-core-v2/test/session/sessionFs/fsWatchService.test.ts b/packages/agent-core-v2/test/session/sessionFs/fsWatchService.test.ts index 48e4239fc..5af13a0f3 100644 --- a/packages/agent-core-v2/test/session/sessionFs/fsWatchService.test.ts +++ b/packages/agent-core-v2/test/session/sessionFs/fsWatchService.test.ts @@ -303,7 +303,7 @@ describe('SessionFsWatchService ensured roots and dirty ticks', () => { expect(events).toEqual([]); }); - it('increments the tick per confined change and folds per-path dirty ticks at flush', () => { + it('increments and exposes per-path dirty ticks before the debounce flush', () => { const { svc, watch } = makeSession(); svc.ensureWatchedRoots([WORK_DIR]); expect(svc.currentTick).toBe(0); @@ -311,7 +311,7 @@ describe('SessionFsWatchService ensured roots and dirty ticks', () => { const a = join(WORK_DIR, 'a.ts'); watch.fire('a.ts', 'created'); expect(svc.currentTick).toBe(1); - expect(svc.dirtyTickFor(a)).toBeUndefined(); + expect(svc.dirtyTickFor(a)).toBe(1); vi.advanceTimersByTime(200); expect(svc.dirtyTickFor(a)).toBe(1); @@ -327,7 +327,7 @@ describe('SessionFsWatchService ensured roots and dirty ticks', () => { for (let i = 0; i < 501; i++) watch.fire(`f${i}.ts`, 'created'); vi.advanceTimersByTime(200); - expect(svc.dirtyTickFor(join(WORK_DIR, 'f0.ts'))).toBeUndefined(); + expect(svc.dirtyTickFor(join(WORK_DIR, 'f0.ts'))).toBe(1); expect(svc.rootDirtyTickFor(WORK_DIR)).toBe(501); }); diff --git a/packages/agent-core-v2/test/session/sessionLease/sessionLease.test.ts b/packages/agent-core-v2/test/session/sessionLease/sessionLease.test.ts index 51472b7d7..fed97de1d 100644 --- a/packages/agent-core-v2/test/session/sessionLease/sessionLease.test.ts +++ b/packages/agent-core-v2/test/session/sessionLease/sessionLease.test.ts @@ -28,8 +28,6 @@ import { sessionLeasePath, SESSION_LEASE_HEARTBEAT_INTERVAL_MS, SESSION_LEASE_TTL_MS, - UNREGISTERED_WRITER_RECHECK_DELAY_MS, - UNREGISTERED_WRITER_WINDOW_MS, } from '#/session/sessionLease/sessionLease'; let tmpDir: string; @@ -46,8 +44,15 @@ afterEach(() => { rmSync(tmpDir, { recursive: true, force: true }); }); -function acquire(sessionId = 's1', onLost: (sessionId: string) => void = () => {}): SessionLease { - return new SessionLease(sessionId, locks.acquire(sessionLeasePath(tmpDir, sessionId)), onLost); +async function acquire( + sessionId = 's1', + onLost: (sessionId: string) => void = () => {}, +): Promise { + return new SessionLease( + sessionId, + await locks.acquire(sessionLeasePath(tmpDir, sessionId)), + onLost, + ); } function thrownError(fn: () => void): Error2 { @@ -66,17 +71,17 @@ function hostWith(seeds: Parameters[0] = []): Scope } describe('SessionLease', () => { - it('reports its identity through info and passes the hard gate while held', () => { - const lease = acquire(); + it('reports its identity through info and passes the hard gate while held', async () => { + const lease = await acquire(); expect(lease.checkHeld()).toBe(true); expect(lease.info).toEqual({ sessionId: 's1', lockId: lease.lockId }); expect(() => lease.assertWritable()).not.toThrow(); lease.release(); }); - it('fails closed with session.lease_lost once the payload no longer carries its token', () => { + it('fails closed with session.lease_lost once the payload no longer carries its token', async () => { const onLost = vi.fn(); - const lease = acquire('s1', onLost); + const lease = await acquire('s1', onLost); writeFileSync( sessionLeasePath(tmpDir, 's1'), JSON.stringify({ lock_id: 'peer-token', pid: process.pid }), @@ -93,8 +98,8 @@ describe('SessionLease', () => { expect(onLost).toHaveBeenCalledTimes(1); }); - it('release is idempotent, unlinks the owned file, and later assertions throw', () => { - const lease = acquire(); + it('release is idempotent, unlinks the owned file, and later assertions throw', async () => { + const lease = await acquire(); lease.release(); lease.release(); @@ -104,8 +109,8 @@ describe('SessionLease', () => { expect(thrownError(() => lease.assertWritable()).code).toBe(ErrorCodes.SESSION_LEASE_LOST); }); - it('release never unlinks a payload owned by a peer', () => { - const lease = acquire(); + it('release never unlinks a payload owned by a peer', async () => { + const lease = await acquire(); writeFileSync( sessionLeasePath(tmpDir, 's1'), JSON.stringify({ lock_id: 'peer-token', pid: process.pid }), @@ -122,8 +127,6 @@ describe('SessionLease', () => { expect(SESSION_LEASE_TTL_MS).toBe(6000); expect(LEASE_CREATING_RETRY_AFTER_MS).toBe(1000); expect(HOLDER_UNRESPONSIVE_RETRY_AFTER_MS).toBe(2000); - expect(UNREGISTERED_WRITER_WINDOW_MS).toBe(5000); - expect(UNREGISTERED_WRITER_RECHECK_DELAY_MS).toBe(1000); }); it('sessionLeasePath lives under /session-leases/', () => { diff --git a/packages/kap-server/src/services/sessionListWatch/sessionListWatchService.ts b/packages/kap-server/src/services/sessionListWatch/sessionListWatchService.ts index 1e9a0a8f3..3ebfd7839 100644 --- a/packages/kap-server/src/services/sessionListWatch/sessionListWatchService.ts +++ b/packages/kap-server/src/services/sessionListWatch/sessionListWatchService.ts @@ -1,6 +1,6 @@ /** * `SessionListWatchService` — the event plane of multi-instance session-list - * sync (design `.tmp/refactor-watch-design-v2.md` §3.8). + * sync. * * Several kap-server instances can share one home directory (the * `multi_server` experimental flag). The session list itself needs no diff --git a/packages/klient/src/contract/session/lifecycle.ts b/packages/klient/src/contract/session/lifecycle.ts index 355ceb401..064e16cfe 100644 --- a/packages/klient/src/contract/session/lifecycle.ts +++ b/packages/klient/src/contract/session/lifecycle.ts @@ -79,6 +79,7 @@ export const sessionLifecycleContract = { create: { input: z.tuple([createSessionOptionsSchema]), output: handleWireSchema }, resume: { input: z.tuple([z.string()]), output: maybe(handleWireSchema) }, close: { input: z.tuple([z.string()]), output: noResult }, + forceAbort: { input: z.tuple([z.string()]), output: noResult }, archive: { input: z.tuple([z.string()]), output: noResult }, restore: { input: z.tuple([z.string()]), output: maybe(handleWireSchema) }, fork: { input: z.tuple([forkSessionOptionsSchema]), output: handleWireSchema }, diff --git a/packages/klient/src/core/facade/session.ts b/packages/klient/src/core/facade/session.ts index cce957aa6..1edb7e2ed 100644 --- a/packages/klient/src/core/facade/session.ts +++ b/packages/klient/src/core/facade/session.ts @@ -67,6 +67,8 @@ export interface SessionFacade { setArchived(archived: boolean): Promise; status(): Promise; close(): Promise; + /** Explicitly release a session lease after an ambiguous close flush failure. */ + forceAbort(): Promise; archive(): Promise; /** Re-materialize a closed session; `false` when it no longer exists. */ restore(): Promise; @@ -128,6 +130,8 @@ export function createSessionFacade(call: ScopedCaller, sessionId: string): Sess return 'idle'; }, close: () => call({}, 'sessionLifecycleService', 'close', [sessionId]) as Promise, + forceAbort: () => + call({}, 'sessionLifecycleService', 'forceAbort', [sessionId]) as Promise, archive: () => call({}, 'sessionLifecycleService', 'archive', [sessionId]) as Promise, restore: async () => { const handle = (await call({}, 'sessionLifecycleService', 'restore', [ diff --git a/packages/minidb/src/index.ts b/packages/minidb/src/index.ts index a6ea10192..e90af77d3 100644 --- a/packages/minidb/src/index.ts +++ b/packages/minidb/src/index.ts @@ -226,6 +226,7 @@ export class MiniDb { private walTail: { dev: number; ino: number; size: number } | null = null; readOnly = false; private lock: LockFile | null = null; + private lockLossError: LockError | null = null; compactThresholdBytes = 64 * 1024 * 1024; autoCompact = true; @@ -286,7 +287,9 @@ export class MiniDb { db.readOnly = !!opts.readOnly; if (!db.readOnly) { - db.lock = new LockFile(path.join(db.dir, 'db.lock')); + db.lock = new LockFile(path.join(db.dir, 'db.lock'), { + onLost: () => db.markLockLost(), + }); const got = await db.lock.acquire(); if (!got) { if (opts.onLockFail === 'readonly') { @@ -1609,7 +1612,9 @@ export class MiniDb { * for a read-only instance. Exposed for lease-style holders such as the * cluster shard pool, which renew on a timer to prove liveness. */ async renewLock(): Promise { - await this.lock?.renew(); + if (this.lock === null) return; + await this.lock.renew(); + this.ensureWritable(); } /** Advanced/internal (read-replica owners such as the cluster shard pool): @@ -1664,5 +1669,11 @@ export class MiniDb { } private ensureWritable(): void { if (this.readOnly) throw new Error('MiniDb is open in read-only mode'); + if (this.lockLossError !== null) throw this.lockLossError; + } + + private markLockLost(): void { + if (this.lockLossError !== null) return; + this.lockLossError = new LockError(`database write lock was lost: ${this.dir}`); } } diff --git a/packages/minidb/src/lockfile.ts b/packages/minidb/src/lockfile.ts index f3521e430..e43437458 100644 --- a/packages/minidb/src/lockfile.ts +++ b/packages/minidb/src/lockfile.ts @@ -37,7 +37,6 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import { execFileSync } from 'node:child_process'; import { randomUUID } from 'node:crypto'; -import { renameReplace } from './rename-replace.js'; export class LockError extends Error { readonly code = 'ELOCKED'; @@ -59,6 +58,8 @@ export interface LockFileDeps { probeProcess?: (pid: number) => { alive: boolean; processStartedAt?: string }; /** Unique token of this acquire; compared on every guarded mutation. */ newLockId?: () => string; + /** Called exactly once when a held lock is replaced or disappears. */ + onLost?: () => void; } // Track held locks so we can release them on process exit as a safety net. @@ -165,6 +166,7 @@ export class LockFile { private readonly selfPid: number; private readonly probeProcess: NonNullable; private readonly newLockId: () => string; + private readonly onLost: (() => void) | undefined; constructor(path: string, deps: LockFileDeps = {}) { this.path = path; @@ -172,6 +174,7 @@ export class LockFile { this.selfPid = deps.selfPid ?? process.pid; this.probeProcess = deps.probeProcess ?? defaultProbeProcess; this.newLockId = deps.newLockId ?? randomUUID; + this.onLost = deps.onLost; } /** Try to acquire the lock exactly once. Returns true when this call created @@ -410,17 +413,38 @@ export class LockFile { } /** Refresh the lock timestamp (proves liveness to processes inspecting the - * lock file). No-op when the lock is not held. Uses write-tmp-then-rename - * so a crash mid-renew cannot leave a truncated, "stale-looking" lock file - * behind for a lock that is actually still owned. The payload keeps our - * token and start-time identity, so guards keep working after a renew. */ + * lock file). No-op when the lock is not held. The update is bound to the + * inode whose token was verified: replacing the public path after open can + * only make this holder lose ownership, never overwrite the replacement. */ async renew(): Promise { if (!this.held || this.lockId === undefined) return; - const tmp = `${this.path}.tmp-${process.pid}-${nextSidecarSeq()}`; - await fs.writeFile(tmp, this.payload(this.lockId)); - // Windows: replacing our own lock can still clash with a co-process's - // readFile/stat of it (EPERM) — the helper rides out such transients. - await renameReplace(tmp, this.path, { retries: 20 }); + const lockId = this.lockId; + let handle: fs.FileHandle; + try { + handle = await fs.open(this.path, 'r+'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + this.markLost(); + return; + } + throw error; + } + try { + const raw = await handle.readFile({ encoding: 'utf8' }); + if (tryParse(raw)?.lock_id !== lockId) { + this.markLost(); + return; + } + const data = Buffer.from(this.payload(lockId)); + await handle.write(data, 0, data.length, 0); + await handle.truncate(data.length); + await handle.sync(); + } finally { + await handle.close(); + } + if (tryParse((await this.readDiskText()) ?? '')?.lock_id !== lockId) { + this.markLost(); + } } /** Token-guarded release. Idempotent; a missing or foreign-owned file is @@ -428,6 +452,7 @@ export class LockFile { async release(): Promise { if (!this.held) return; this.held = false; + HELD.delete(this); const lockId = this.lockId; this.lockId = undefined; try { @@ -443,6 +468,7 @@ export class LockFile { releaseSync(): void { if (!this.held) return; this.held = false; + HELD.delete(this); const lockId = this.lockId; this.lockId = undefined; try { @@ -452,6 +478,14 @@ export class LockFile { /* same policy as release(): never unlink on uncertainty */ } } + + private markLost(): void { + if (!this.held) return; + this.held = false; + this.lockId = undefined; + HELD.delete(this); + this.onLost?.(); + } } function stringOrUndefined(v: unknown): string | undefined { diff --git a/packages/minidb/test/lock.test.ts b/packages/minidb/test/lock.test.ts index 5a81f6925..4a675fc34 100644 --- a/packages/minidb/test/lock.test.ts +++ b/packages/minidb/test/lock.test.ts @@ -167,6 +167,26 @@ test('release never unlinks a lock that was taken over meanwhile', async () => { await cleanup(dir); }); +test('renew never overwrites a foreign lock generation', async () => { + const dir = await tmpDir(); + const p = path.join(dir, 'db.lock'); + const old = path.join(dir, 'db.lock.old'); + const lock = new LockFile(p, { newLockId: () => 'a-token' }); + assert.equal(await lock.acquire(), true); + + await fs.rename(p, old); + await fs.writeFile( + p, + JSON.stringify({ pid: process.pid, ts: Date.now(), lock_id: 'b-token' }), + ); + await lock.renew(); + + assert.equal(lock.held, false); + const onDisk = JSON.parse(await fs.readFile(p, 'utf8')); + assert.equal(onDisk.lock_id, 'b-token'); + await cleanup(dir); +}); + test('a read-back token mismatch rejects the acquire', async () => { const dir = await tmpDir(); const p = path.join(dir, 'db.lock'); diff --git a/packages/minidb/test/review-fixes.test.ts b/packages/minidb/test/review-fixes.test.ts index 0b761bab3..10af7b600 100644 --- a/packages/minidb/test/review-fixes.test.ts +++ b/packages/minidb/test/review-fixes.test.ts @@ -84,6 +84,30 @@ test('expired keys are removed from secondary indexes', async () => { } }); +test('a writer that loses its lock cannot write into a successor generation', async () => { + const dir = await tmpDir(); + try { + const oldWriter = await MiniDb.open({ dir, valueCodec: 'string', autoCompact: false }); + await oldWriter.set('generation', 'old'); + + await fs.writeFile( + path.join(dir, 'db.lock'), + JSON.stringify({ pid: 0x7fffffff, ts: Date.now(), lock_id: 'successor-generation' }), + ); + await assert.rejects(oldWriter.renewLock(), /write lock was lost/); + + const newWriter = await MiniDb.open({ dir, valueCodec: 'string', autoCompact: false }); + await newWriter.set('generation', 'new'); + await assert.rejects(oldWriter.set('generation', 'old-again'), /write lock was lost/); + assert.equal(newWriter.get('generation'), 'new'); + + await newWriter.close(); + await oldWriter.close(); + } finally { + await fs.rm(dir, { recursive: true, force: true }); + } +}); + test('expired keys are removed from the full-text index', async () => { const dir = await tmpDir(); try {