From 2745c7b9d0ccc87266b90436fa89ac53dbde74d0 Mon Sep 17 00:00:00 2001 From: katereznykova Date: Fri, 13 Mar 2026 15:15:57 +0000 Subject: [PATCH 1/6] Add persistent file watch state Keep file watches usable for hibernating Durable Objects by separating live SSE delivery from retained watch state in the container. Add owner-scoped acknowledgement and idle expiry so background consumers can reconcile safely without sharing a global dirty bit. --- .changeset/persistent-watch-state.md | 6 + .../src/handlers/watch-handler.ts | 262 +++- .../sandbox-container/src/routes/setup.ts | 28 + .../src/services/watch-service.ts | 1159 ++++++++++++----- .../tests/handlers/watch-handler.test.ts | 163 ++- .../tests/services/watch-service.test.ts | 184 ++- packages/sandbox/src/clients/index.ts | 11 +- packages/sandbox/src/clients/watch-client.ts | 77 +- packages/sandbox/src/index.ts | 14 +- packages/sandbox/src/sandbox.ts | 56 +- packages/shared/src/index.ts | 7 + packages/shared/src/types.ts | 109 +- tests/e2e/file-watch-workflow.test.ts | 129 ++ tests/e2e/test-worker/index.ts | 53 + 14 files changed, 1841 insertions(+), 417 deletions(-) create mode 100644 .changeset/persistent-watch-state.md diff --git a/.changeset/persistent-watch-state.md b/.changeset/persistent-watch-state.md new file mode 100644 index 000000000..8ee0fe5c7 --- /dev/null +++ b/.changeset/persistent-watch-state.md @@ -0,0 +1,6 @@ +--- +'@cloudflare/sandbox': patch +--- + +Add persistent file watch state for hibernating Durable Object workflows. +Use `ensureWatch()`, `getWatchState()`, `ackWatchState()`, and `stopWatch()` to keep a watch alive without holding an SSE stream open. diff --git a/packages/sandbox-container/src/handlers/watch-handler.ts b/packages/sandbox-container/src/handlers/watch-handler.ts index db8355c19..9a09dd6d2 100644 --- a/packages/sandbox-container/src/handlers/watch-handler.ts +++ b/packages/sandbox-container/src/handlers/watch-handler.ts @@ -1,5 +1,13 @@ import { posix as pathPosix } from 'node:path'; -import type { Logger, WatchRequest } from '@repo/shared'; +import type { + Logger, + WatchAckRequest, + WatchAckResult, + WatchEnsureResult, + WatchRequest, + WatchStateResult, + WatchStopResult +} from '@repo/shared'; import { ErrorCode } from '@repo/shared/errors'; import { CONFIG } from '../config'; import type { RequestContext } from '../core/types'; @@ -9,7 +17,7 @@ import { BaseHandler } from './base-handler'; const WORKSPACE_ROOT = CONFIG.DEFAULT_CWD; /** - * Handler for file watch operations + * Handler for file watch operations. */ export class WatchHandler extends BaseHandler { constructor( @@ -22,28 +30,214 @@ export class WatchHandler extends BaseHandler { async handle(request: Request, context: RequestContext): Promise { const pathname = new URL(request.url).pathname; - if (pathname === '/api/watch') { + if (pathname === '/api/watch' && request.method === 'POST') { return this.handleWatch(request, context); } + if (pathname === '/api/watch/ensure' && request.method === 'POST') { + return this.handleEnsureWatch(request, context); + } + + if (pathname.startsWith('/api/watch/')) { + const segments = pathname.split('/'); + const watchId = segments[3]; + const action = segments[4]; + + if (!watchId) { + return this.createErrorResponse( + { + message: 'Watch ID is required', + code: ErrorCode.VALIDATION_FAILED + }, + context + ); + } + + if (!action && request.method === 'GET') { + return this.handleGetWatchState(context, watchId); + } + + if (!action && request.method === 'DELETE') { + return this.handleStopWatch(request, context, watchId); + } + + if (action === 'ack' && request.method === 'POST') { + return this.handleAckWatchState(request, context, watchId); + } + } + return this.createErrorResponse( { message: 'Invalid watch endpoint', code: ErrorCode.VALIDATION_FAILED, - details: { pathname } + details: { pathname, method: request.method } }, context ); } - /** - * Start watching a directory. - * Returns an SSE stream of file change events. - */ private async handleWatch( request: Request, context: RequestContext ): Promise { + const normalizedRequest = await this.parseAndNormalizeWatchRequest( + request, + context + ); + if (normalizedRequest instanceof Response) { + return normalizedRequest; + } + + const result = await this.watchService.watchDirectory( + normalizedRequest.path, + normalizedRequest + ); + + if (!result.success) { + return this.createErrorResponse(result.error, context); + } + + return new Response(result.data, { + headers: { + 'Content-Type': 'text/event-stream', + 'Cache-Control': 'no-cache', + Connection: 'keep-alive', + ...context.corsHeaders + } + }); + } + + private async handleEnsureWatch( + request: Request, + context: RequestContext + ): Promise { + const normalizedRequest = await this.parseAndNormalizeWatchRequest( + request, + context + ); + if (normalizedRequest instanceof Response) { + return normalizedRequest; + } + + const result = await this.watchService.ensureWatch( + normalizedRequest.path, + normalizedRequest + ); + + if (!result.success) { + return this.createErrorResponse(result.error, context); + } + + const response: WatchEnsureResult = { + success: true, + watch: result.data, + timestamp: new Date().toISOString() + }; + + return this.createTypedResponse(response, context); + } + + private async handleGetWatchState( + context: RequestContext, + watchId: string + ): Promise { + const result = await this.watchService.getWatchState(watchId); + if (!result.success) { + return this.createErrorResponse(result.error, context); + } + + const response: WatchStateResult = { + success: true, + watch: result.data, + timestamp: new Date().toISOString() + }; + + return this.createTypedResponse(response, context); + } + + private async handleAckWatchState( + request: Request, + context: RequestContext, + watchId: string + ): Promise { + let body: WatchAckRequest; + try { + body = await this.parseRequestBody(request); + } catch (error) { + return this.createErrorResponse( + { + message: + error instanceof Error ? error.message : 'Invalid request body', + code: ErrorCode.VALIDATION_FAILED + }, + context + ); + } + + if (!Number.isInteger(body.cursor) || body.cursor < 0) { + return this.createErrorResponse( + { + message: 'cursor must be a non-negative integer', + code: ErrorCode.VALIDATION_FAILED, + details: { cursor: body.cursor } + }, + context + ); + } + + const ownerIdError = this.validateOwnerId(body.ownerId); + if (ownerIdError) { + return this.createErrorResponse(ownerIdError, context); + } + + const result = await this.watchService.ackWatchState( + watchId, + body.cursor, + body.ownerId + ); + if (!result.success) { + return this.createErrorResponse(result.error, context); + } + + const response: WatchAckResult = { + success: true, + acknowledged: result.data.acknowledged, + watch: result.data.watch, + timestamp: new Date().toISOString() + }; + + return this.createTypedResponse(response, context); + } + + private async handleStopWatch( + request: Request, + context: RequestContext, + watchId: string + ): Promise { + const ownerId = this.extractQueryParam(request, 'ownerId') ?? undefined; + const ownerIdError = this.validateOwnerId(ownerId); + if (ownerIdError) { + return this.createErrorResponse(ownerIdError, context); + } + + const result = await this.watchService.stopWatch(watchId, ownerId); + if (!result.success) { + return this.createErrorResponse(result.error, context); + } + + const response: WatchStopResult = { + success: true, + watchId, + timestamp: new Date().toISOString() + }; + + return this.createTypedResponse(response, context); + } + + private async parseAndNormalizeWatchRequest( + request: Request, + context: RequestContext + ): Promise { let body: WatchRequest; try { body = await this.parseRequestBody(request); @@ -68,23 +262,10 @@ export class WatchHandler extends BaseHandler { return this.createErrorResponse(pathResult.error, context); } - const result = await this.watchService.watchDirectory(pathResult.path, { + return { ...body, path: pathResult.path - }); - - if (result.success) { - return new Response(result.data, { - headers: { - 'Content-Type': 'text/event-stream', - 'Cache-Control': 'no-cache', - Connection: 'keep-alive', - ...context.corsHeaders - } - }); - } - - return this.createErrorResponse(result.error, context); + }; } private validateWatchBody(body: WatchRequest): { @@ -160,6 +341,39 @@ export class WatchHandler extends BaseHandler { }; } + const ownerIdError = this.validateOwnerId(body.ownerId); + if (ownerIdError) { + return ownerIdError; + } + + return null; + } + + private validateOwnerId(ownerId: unknown): { + message: string; + code: string; + details?: Record; + } | null { + if (ownerId === undefined) { + return null; + } + + if (typeof ownerId !== 'string' || ownerId.trim() === '') { + return { + message: 'ownerId must be a non-empty string when provided', + code: ErrorCode.VALIDATION_FAILED, + details: { ownerId } + }; + } + + if (ownerId.includes('\0')) { + return { + message: 'ownerId contains invalid null bytes', + code: ErrorCode.VALIDATION_FAILED, + details: { ownerId } + }; + } + return null; } @@ -177,8 +391,6 @@ export class WatchHandler extends BaseHandler { return null; } - // Supported syntax is intentionally narrow: *, **, ? and path separators. - // Character classes and brace expansion are rejected so behavior is explicit. const unsupportedTokens = /[[\]{}]/; for (const pattern of patterns) { diff --git a/packages/sandbox-container/src/routes/setup.ts b/packages/sandbox-container/src/routes/setup.ts index 391db093c..a9c9a44fe 100644 --- a/packages/sandbox-container/src/routes/setup.ts +++ b/packages/sandbox-container/src/routes/setup.ts @@ -444,6 +444,34 @@ export function setupRoutes(router: Router, container: Container): void { middleware: [container.get('loggingMiddleware')] }); + router.register({ + method: 'POST', + path: '/api/watch/ensure', + handler: async (req, ctx) => container.get('watchHandler').handle(req, ctx), + middleware: [container.get('loggingMiddleware')] + }); + + router.register({ + method: 'GET', + path: '/api/watch/{id}', + handler: async (req, ctx) => container.get('watchHandler').handle(req, ctx), + middleware: [container.get('loggingMiddleware')] + }); + + router.register({ + method: 'POST', + path: '/api/watch/{id}/ack', + handler: async (req, ctx) => container.get('watchHandler').handle(req, ctx), + middleware: [container.get('loggingMiddleware')] + }); + + router.register({ + method: 'DELETE', + path: '/api/watch/{id}', + handler: async (req, ctx) => container.get('watchHandler').handle(req, ctx), + middleware: [container.get('loggingMiddleware')] + }); + // Miscellaneous routes router.register({ method: 'GET', diff --git a/packages/sandbox-container/src/services/watch-service.ts b/packages/sandbox-container/src/services/watch-service.ts index f6d2bc277..75d7ab0a0 100644 --- a/packages/sandbox-container/src/services/watch-service.ts +++ b/packages/sandbox-container/src/services/watch-service.ts @@ -2,62 +2,269 @@ import type { FileWatchEventType, FileWatchSSEEvent, Logger, - WatchRequest + WatchRequest, + WatchState } from '@repo/shared'; import { ErrorCode } from '@repo/shared/errors'; import type { Subprocess } from 'bun'; import type { ServiceResult } from '../core/types'; import { serviceError, serviceSuccess } from '../core/types'; +interface Deferred { + promise: Promise; + resolve(value: T | PromiseLike): void; + reject(reason?: unknown): void; +} + +interface WatchSubscriber { + id: string; + controller: ReadableStreamDefaultController; + encoder: TextEncoder; + pendingEvents: Map; + droppedEvents: number; + flushInterval: ReturnType; + watchingSent: boolean; + closed: boolean; +} + interface ActiveWatch { id: string; + key: string; path: string; + recursive: boolean; + include?: string[]; + exclude?: string[]; process: Subprocess; startedAt: Date; + state: WatchState; + persistent: boolean; + subscribers: Map; + ready: Deferred; + readyState: 'pending' | 'resolved' | 'rejected'; + expiryTimer: ReturnType | null; stopPromise?: Promise; } +type TerminalWatchEvent = Extract< + FileWatchSSEEvent, + { type: 'error' | 'stopped' } +>; + const WATCH_SETUP_TIMEOUT_MS = 10000; const EVENT_COALESCE_WINDOW_MS = 75; const MAX_PENDING_EVENTS = 1000; +const PERSISTENT_WATCH_IDLE_TTL_MS = 10 * 60 * 1000; const STOP_TIMEOUT_MS = 5000; /** - * Service for watching filesystem changes using inotifywait + * Service for watching filesystem changes using inotifywait. */ export class WatchService { private activeWatches: Map = new Map(); + private watchIdsByKey: Map = new Map(); private watchCounter = 0; + private subscriberCounter = 0; constructor(private logger: Logger) {} /** - * Start watching a directory for changes - * Returns a ReadableStream of SSE events - * - * @param path - Absolute path to watch - * @param options - Watch options + * Start watching a directory and subscribe to live events. */ async watchDirectory( path: string, options: WatchRequest = { path } ): Promise>> { - const watchId = `watch-${++this.watchCounter}-${Date.now()}`; - const watchLogger = this.logger.child({ watchId, path }); + const watchResult = this.getOrCreateWatch(path, { + ...options, + ownerId: undefined + }); + if (!watchResult.success) { + return watchResult; + } + + const stream = this.createSubscriberStream(watchResult.data); + return serviceSuccess(stream); + } + + /** + * Ensure a persistent watch exists and wait until it is ready. + */ + async ensureWatch( + path: string, + options: WatchRequest = { path } + ): Promise> { + const watchResult = this.getOrCreateWatch(path, options); + if (!watchResult.success) { + return watchResult; + } + + const watch = watchResult.data; + const ownershipError = this.claimPersistentWatch(watch, options.ownerId); + if (ownershipError) { + return serviceError(ownershipError); + } - // Clean up any existing watches on this path before starting a new one. - // Snapshot matching IDs first to avoid mutating the map during iteration. - const staleWatchIds = Array.from(this.activeWatches.entries()) - .filter(([, w]) => w.path === path) - .map(([id]) => id); - if (staleWatchIds.length > 0) { - watchLogger.debug('Cleaning up existing watches on path', { - staleWatchIds + watch.persistent = true; + this.refreshPersistentWatchLease(watch); + + try { + await watch.ready.promise; + return serviceSuccess(this.snapshotWatchState(watch)); + } catch (error) { + return serviceError({ + message: + error instanceof Error + ? error.message + : 'Failed to establish persistent watch', + code: ErrorCode.WATCH_START_ERROR, + details: { path } + }); + } + } + + /** + * Return the current state for a persistent or active watch. + */ + async getWatchState(watchId: string): Promise> { + const watch = this.activeWatches.get(watchId); + if (!watch) { + return serviceError({ + message: `Watch not found: ${watchId}`, + code: ErrorCode.WATCH_NOT_FOUND, + details: { watchId } }); - await Promise.all(staleWatchIds.map((id) => this.stopWatch(id))); } - // Verify path exists + try { + await watch.ready.promise; + this.refreshPersistentWatchLease(watch); + return serviceSuccess(this.snapshotWatchState(watch)); + } catch (error) { + return serviceError({ + message: + error instanceof Error ? error.message : 'Watch failed to establish', + code: ErrorCode.WATCH_START_ERROR, + details: { watchId } + }); + } + } + + /** + * Acknowledge the current watch cursor. + */ + async ackWatchState( + watchId: string, + cursor: number, + ownerId?: string + ): Promise> { + const watch = this.activeWatches.get(watchId); + if (!watch) { + return serviceError({ + message: `Watch not found: ${watchId}`, + code: ErrorCode.WATCH_NOT_FOUND, + details: { watchId } + }); + } + + try { + await watch.ready.promise; + } catch (error) { + return serviceError({ + message: + error instanceof Error ? error.message : 'Watch failed to establish', + code: ErrorCode.WATCH_START_ERROR, + details: { watchId } + }); + } + + const ownershipError = this.verifyPersistentWatchOwner( + watch, + ownerId, + 'acknowledge' + ); + if (ownershipError) { + return serviceError(ownershipError); + } + + const acknowledged = cursor === watch.state.cursor; + if (acknowledged) { + watch.state.dirty = false; + watch.state.overflowed = false; + } + + this.refreshPersistentWatchLease(watch); + + return serviceSuccess({ + acknowledged, + watch: this.snapshotWatchState(watch) + }); + } + + /** + * Stop a specific watch. + */ + async stopWatch( + watchId: string, + ownerId?: string + ): Promise> { + const watch = this.activeWatches.get(watchId); + if (!watch) { + return serviceError({ + message: `Watch not found: ${watchId}`, + code: ErrorCode.WATCH_NOT_FOUND, + details: { watchId } + }); + } + + const ownershipError = this.verifyPersistentWatchOwner( + watch, + ownerId, + 'stop' + ); + if (ownershipError) { + return serviceError(ownershipError); + } + + await this.stopWatchInternal(watchId, { + type: 'stopped', + reason: 'Watch stopped' + }); + + return serviceSuccess(undefined); + } + + /** + * Stop all active watches. + */ + async stopAllWatches(): Promise { + const watchIds = Array.from(this.activeWatches.keys()); + await Promise.all(watchIds.map((id) => this.stopWatchInternal(id))); + return watchIds.length; + } + + /** + * Get list of active watches. + */ + getActiveWatches(): WatchState[] { + return Array.from(this.activeWatches.values()).map((watch) => + this.snapshotWatchState(watch) + ); + } + + private getOrCreateWatch( + path: string, + options: WatchRequest + ): ServiceResult { + const key = this.createWatchKey(path, options); + const existingWatchId = this.watchIdsByKey.get(key); + if (existingWatchId) { + const existing = this.activeWatches.get(existingWatchId); + if (existing) { + return serviceSuccess(existing); + } + this.watchIdsByKey.delete(key); + } + const pathCheck = Bun.spawnSync(['test', '-e', path]); if (pathCheck.exitCode !== 0) { return serviceError({ @@ -67,8 +274,9 @@ export class WatchService { }); } - // Build inotifywait command + const watchId = `watch-${++this.watchCounter}-${Date.now()}`; const args = this.buildInotifyArgs(path, options); + const watchLogger = this.logger.child({ watchId, path }); watchLogger.debug('Starting inotifywait', { args }); try { @@ -77,105 +285,598 @@ export class WatchService { stderr: 'pipe' }); - // Store active watch - this.activeWatches.set(watchId, { + const watch: ActiveWatch = { id: watchId, + key, path, + recursive: options.recursive !== false, + include: options.include, + exclude: options.include ? undefined : options.exclude, process: proc, - startedAt: new Date() - }); + startedAt: new Date(), + state: { + watchId, + path, + recursive: options.recursive !== false, + include: options.include, + exclude: options.include ? undefined : options.exclude, + ownerId: options.ownerId, + cursor: 0, + dirty: false, + overflowed: false, + lastEventAt: null, + expiresAt: null, + subscriberCount: 0, + startedAt: new Date().toISOString() + }, + persistent: false, + subscribers: new Map(), + ready: createDeferred(), + readyState: 'pending', + expiryTimer: null + }; - // Create SSE stream from inotifywait output - const stream = this.createWatchStream(watchId, path, proc, watchLogger); + this.activeWatches.set(watchId, watch); + this.watchIdsByKey.set(key, watchId); + this.runWatchLoop(watch, watchLogger); - return serviceSuccess(stream); + return serviceSuccess(watch); } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); watchLogger.error('Failed to start inotifywait', err); return serviceError({ - message: `Failed to start file watcher: ${error instanceof Error ? error.message : 'Unknown error'}`, + message: `Failed to start file watcher: ${err.message}`, code: ErrorCode.WATCH_START_ERROR, details: { path } }); } } - /** - * Stop all active watches - */ - async stopAllWatches(): Promise { - const watchIds = Array.from(this.activeWatches.keys()); - await Promise.all(watchIds.map((id) => this.stopWatch(id))); - return watchIds.length; + private createWatchKey(path: string, options: WatchRequest): string { + return JSON.stringify({ + path, + recursive: options.recursive !== false, + include: options.include ?? null, + exclude: options.include ? null : (options.exclude ?? null) + }); } - /** - * Get list of active watches - */ - getActiveWatches(): Array<{ id: string; path: string; startedAt: Date }> { - return Array.from(this.activeWatches.values()).map((w) => ({ - id: w.id, - path: w.path, - startedAt: w.startedAt - })); + private snapshotWatchState(watch: ActiveWatch): WatchState { + return { + ...watch.state, + include: watch.include, + exclude: watch.exclude, + subscriberCount: watch.subscribers.size, + startedAt: watch.startedAt.toISOString() + }; } - /** - * Stop a specific watch by ID. Idempotent via stored stopPromise. - * Sends SIGTERM, waits for exit with timeout, escalates to SIGKILL if needed. - */ - private stopWatch(watchId: string): Promise { + private claimPersistentWatch( + watch: ActiveWatch, + ownerId?: string + ): { + message: string; + code: string; + details?: Record; + } | null { + if (!watch.state.ownerId) { + if (ownerId) { + watch.state.ownerId = ownerId; + } + return null; + } + + if (!ownerId) { + return { + message: + 'Persistent watch is already owned by another consumer. Provide the same ownerId to reuse it.', + code: ErrorCode.RESOURCE_BUSY, + details: { + watchId: watch.id, + ownerId: watch.state.ownerId + } + }; + } + + if (watch.state.ownerId !== ownerId) { + return { + message: `Persistent watch is owned by '${watch.state.ownerId}' and cannot be claimed by '${ownerId}'.`, + code: ErrorCode.RESOURCE_BUSY, + details: { + watchId: watch.id, + ownerId: watch.state.ownerId, + requestedOwnerId: ownerId + } + }; + } + + return null; + } + + private verifyPersistentWatchOwner( + watch: ActiveWatch, + ownerId: string | undefined, + action: 'acknowledge' | 'stop' + ): { + message: string; + code: string; + details?: Record; + } | null { + if (!watch.state.ownerId) { + return null; + } + + if (!ownerId) { + return { + message: `Persistent watch requires ownerId to ${action}.`, + code: ErrorCode.RESOURCE_BUSY, + details: { + watchId: watch.id, + ownerId: watch.state.ownerId, + action + } + }; + } + + if (watch.state.ownerId !== ownerId) { + return { + message: `Persistent watch is owned by '${watch.state.ownerId}' and cannot be ${action}d by '${ownerId}'.`, + code: ErrorCode.RESOURCE_BUSY, + details: { + watchId: watch.id, + ownerId: watch.state.ownerId, + requestedOwnerId: ownerId, + action + } + }; + } + + return null; + } + + private refreshPersistentWatchLease(watch: ActiveWatch): void { + if (!watch.persistent) { + watch.state.expiresAt = null; + this.clearPersistentWatchExpiry(watch); + return; + } + + this.clearPersistentWatchExpiry(watch); + + if (watch.subscribers.size > 0) { + watch.state.expiresAt = null; + return; + } + + const expiresAt = new Date(Date.now() + PERSISTENT_WATCH_IDLE_TTL_MS); + watch.state.expiresAt = expiresAt.toISOString(); + watch.expiryTimer = setTimeout(() => { + void this.stopWatchInternal(watch.id, { + type: 'stopped', + reason: 'Persistent watch expired after idle period' + }); + }, PERSISTENT_WATCH_IDLE_TTL_MS); + } + + private clearPersistentWatchExpiry(watch: ActiveWatch): void { + if (watch.expiryTimer) { + clearTimeout(watch.expiryTimer); + watch.expiryTimer = null; + } + } + + private createSubscriberStream( + watch: ActiveWatch + ): ReadableStream { + const self = this; + const encoder = new TextEncoder(); + let subscriberId: string | undefined; + + return new ReadableStream({ + async start(controller) { + subscriberId = self.addSubscriber(watch, controller, encoder); + + try { + await watch.ready.promise; + } catch (error) { + self.closeSubscriber( + watch, + subscriberId, + errorEvent( + error instanceof Error + ? error.message + : 'Watch failed to establish' + ) + ); + return; + } + + const subscriber = subscriberId + ? watch.subscribers.get(subscriberId) + : undefined; + if (!subscriber || subscriber.closed) { + return; + } + + subscriber.watchingSent = true; + try { + controller.enqueue( + encoder.encode( + `data: ${JSON.stringify({ + type: 'watching', + path: watch.path, + watchId: watch.id + } satisfies FileWatchSSEEvent)}\n\n` + ) + ); + } catch { + await self.removeSubscriber(watch, subscriber.id); + return; + } + + self.flushSubscriberEvents(watch, subscriber); + }, + + cancel() { + if (subscriberId) { + return self.removeSubscriber(watch, subscriberId); + } + return Promise.resolve(); + } + }); + } + + private addSubscriber( + watch: ActiveWatch, + controller: ReadableStreamDefaultController, + encoder: TextEncoder + ): string { + const subscriberId = `subscriber-${++this.subscriberCounter}`; + const subscriber: WatchSubscriber = { + id: subscriberId, + controller, + encoder, + pendingEvents: new Map(), + droppedEvents: 0, + flushInterval: setInterval(() => { + this.flushSubscriberEvents(watch, subscriber); + }, EVENT_COALESCE_WINDOW_MS), + watchingSent: false, + closed: false + }; + + watch.subscribers.set(subscriberId, subscriber); + watch.state.subscriberCount = watch.subscribers.size; + this.refreshPersistentWatchLease(watch); + return subscriberId; + } + + private async removeSubscriber( + watch: ActiveWatch, + subscriberId: string + ): Promise { + this.closeSubscriber(watch, subscriberId); + await this.maybeStopWatchWhenUnused(watch); + } + + private async maybeStopWatchWhenUnused(watch: ActiveWatch): Promise { + if (!watch.persistent && watch.subscribers.size === 0) { + await this.stopWatchInternal(watch.id, { + type: 'stopped', + reason: 'Watch stopped after last subscriber disconnected' + }); + return; + } + + this.refreshPersistentWatchLease(watch); + } + + private closeSubscriber( + watch: ActiveWatch, + subscriberId: string, + terminalEvent?: TerminalWatchEvent + ): void { + const subscriber = watch.subscribers.get(subscriberId); + if (!subscriber || subscriber.closed) { + return; + } + + subscriber.closed = true; + clearInterval(subscriber.flushInterval); + watch.subscribers.delete(subscriberId); + watch.state.subscriberCount = watch.subscribers.size; + + try { + const shouldSendTerminalEvent = + terminalEvent !== undefined && + (subscriber.watchingSent || terminalEvent.type === 'error'); + if (shouldSendTerminalEvent) { + subscriber.controller.enqueue( + subscriber.encoder.encode( + `data: ${JSON.stringify(terminalEvent)}\n\n` + ) + ); + } + } catch { + // Stream already closed. + } + + try { + subscriber.controller.close(); + } catch { + // Stream already closed. + } + } + + private enqueueSubscriberEvent( + watch: ActiveWatch, + subscriber: WatchSubscriber, + event: FileWatchSSEEvent + ): void { + if (subscriber.closed) { + return; + } + + const key = + event.type === 'event' + ? `${event.eventType}|${event.path}|${event.isDirectory}` + : `${event.type}|${Date.now()}`; + + if ( + !subscriber.pendingEvents.has(key) && + subscriber.pendingEvents.size >= MAX_PENDING_EVENTS + ) { + subscriber.droppedEvents++; + watch.state.overflowed = true; + + if ( + subscriber.droppedEvents === 1 || + subscriber.droppedEvents % 100 === 0 + ) { + this.logger.warn('Dropping watch events due to backpressure', { + watchId: watch.id, + subscriberId: subscriber.id, + droppedEvents: subscriber.droppedEvents, + pendingCount: subscriber.pendingEvents.size + }); + } + return; + } + + subscriber.pendingEvents.set(key, event); + } + + private flushSubscriberEvents( + watch: ActiveWatch, + subscriber: WatchSubscriber + ): void { + if (subscriber.closed || !subscriber.watchingSent) { + return; + } + + try { + for (const event of subscriber.pendingEvents.values()) { + subscriber.controller.enqueue( + subscriber.encoder.encode(`data: ${JSON.stringify(event)}\n\n`) + ); + } + subscriber.pendingEvents.clear(); + } catch { + subscriber.closed = true; + clearInterval(subscriber.flushInterval); + watch.subscribers.delete(subscriber.id); + watch.state.subscriberCount = watch.subscribers.size; + void this.maybeStopWatchWhenUnused(watch); + } + } + + private broadcastEvent(watch: ActiveWatch, event: FileWatchSSEEvent): void { + for (const subscriber of watch.subscribers.values()) { + this.enqueueSubscriberEvent(watch, subscriber, event); + } + } + + private broadcastTerminalEvent( + watch: ActiveWatch, + terminalEvent: TerminalWatchEvent + ): void { + for (const subscriberId of Array.from(watch.subscribers.keys())) { + this.closeSubscriber(watch, subscriberId, terminalEvent); + } + } + + private async stopWatchInternal( + watchId: string, + terminalEvent?: TerminalWatchEvent + ): Promise { const watch = this.activeWatches.get(watchId); - if (!watch) return Promise.resolve(); + if (!watch) { + return; + } - // Return existing stop promise if already stopping - if (watch.stopPromise) return watch.stopPromise; + if (watch.stopPromise) { + return watch.stopPromise; + } const cleanup = async () => { + const resolvedTerminalEvent: TerminalWatchEvent = terminalEvent ?? { + type: 'stopped', + reason: 'Watch process ended' + }; + + if (watch.readyState === 'pending') { + const terminalMessage = + resolvedTerminalEvent.type === 'error' + ? resolvedTerminalEvent.error + : resolvedTerminalEvent.reason; + this.rejectWatchReady(watch, new Error(terminalMessage)); + } + + this.broadcastTerminalEvent(watch, resolvedTerminalEvent); + try { watch.process.kill(); } catch { - // Process may have already exited + // Process may have already exited. } - // Wait for graceful exit with timeout, escalate to SIGKILL if needed - let timeoutHandle: ReturnType; + let timeoutHandle: ReturnType | undefined; const exitedCleanly = await Promise.race([ watch.process.exited.then(() => true as const), new Promise((resolve) => { timeoutHandle = setTimeout(() => resolve(false), STOP_TIMEOUT_MS); }) ]); - clearTimeout(timeoutHandle!); + + if (timeoutHandle) { + clearTimeout(timeoutHandle); + } if (!exitedCleanly) { try { - watch.process.kill(9); // SIGKILL + watch.process.kill(9); } catch { - // Already dead + // Process may have already exited. } } + this.clearPersistentWatchExpiry(watch); + this.activeWatches.delete(watchId); + this.watchIdsByKey.delete(watch.key); }; watch.stopPromise = cleanup(); return watch.stopPromise; } + private runWatchLoop(watch: ActiveWatch, logger: Logger): void { + const stdout = watch.process.stdout; + const stderr = watch.process.stderr; + + if (!stdout || typeof stdout === 'number') { + const error = new Error('Failed to capture process output'); + this.rejectWatchReady(watch, error); + void this.stopWatchInternal(watch.id, errorEvent(error.message)); + return; + } + + void (async () => { + try { + if (stderr && typeof stderr !== 'number') { + const monitor = await this.waitForWatchesEstablished(stderr, logger); + this.continueStderrMonitoring( + monitor.reader, + monitor.decoder, + monitor.buffer, + watch, + logger + ); + } + + this.resolveWatchReady(watch); + + const reader = stdout.getReader(); + const decoder = new TextDecoder(); + let buffer = ''; + + while (true) { + const { done, value } = await reader.read(); + if (done) { + break; + } + + buffer += decoder.decode(value, { stream: true }); + const lines = buffer.split('\n'); + buffer = lines.pop() || ''; + + for (const line of lines) { + if (!line.trim()) { + continue; + } + + const parsed = this.parseInotifyEvent(line); + if (!parsed) { + continue; + } + + const timestamp = new Date().toISOString(); + const nextCursor = watch.state.cursor + 1; + const event: Extract = { + type: 'event', + eventId: `${watch.id}:${nextCursor}`, + eventType: parsed.eventType, + path: parsed.path, + isDirectory: parsed.isDirectory, + timestamp + }; + + watch.state.cursor = nextCursor; + watch.state.dirty = true; + watch.state.lastEventAt = timestamp; + this.broadcastEvent(watch, event); + } + } + + if (buffer.trim()) { + const parsed = this.parseInotifyEvent(buffer); + if (parsed) { + const timestamp = new Date().toISOString(); + const nextCursor = watch.state.cursor + 1; + const event: Extract = { + type: 'event', + eventId: `${watch.id}:${nextCursor}`, + eventType: parsed.eventType, + path: parsed.path, + isDirectory: parsed.isDirectory, + timestamp + }; + + watch.state.cursor = nextCursor; + watch.state.dirty = true; + watch.state.lastEventAt = timestamp; + this.broadcastEvent(watch, event); + } + } + + await this.stopWatchInternal(watch.id, { + type: 'stopped', + reason: 'Watch process ended' + }); + } catch (error) { + const err = error instanceof Error ? error : new Error(String(error)); + logger.error('Error reading watch output', err); + this.rejectWatchReady(watch, err); + await this.stopWatchInternal(watch.id, errorEvent(err.message)); + } + })(); + } + + private resolveWatchReady(watch: ActiveWatch): void { + if (watch.readyState !== 'pending') { + return; + } + + watch.readyState = 'resolved'; + watch.ready.resolve(); + } + + private rejectWatchReady(watch: ActiveWatch, error: Error): void { + if (watch.readyState !== 'pending') { + return; + } + + watch.readyState = 'rejected'; + watch.ready.reject(error); + } + private buildInotifyArgs(path: string, options: WatchRequest): string[] { - const args: string[] = [ - '-m', // Monitor mode (continuous) - '--format', - '%e|%w%f' // event|path (ISDIR is part of event flags) - ]; + const args: string[] = ['-m', '--format', '%e|%w%f']; - // Recursive watching if (options.recursive !== false) { args.push('-r'); } - // Event types const events: FileWatchEventType[] = options.events || [ 'create', 'modify', @@ -190,9 +891,6 @@ export class WatchService { args.push('-e', inotifyEvents.join(',')); } - // inotifywait does not allow --include and --exclude together. - // Include filters take precedence; when include is set, exclusion is handled - // implicitly by only matching included paths. const includeRegex = this.buildCombinedPathRegex(options.include); if (includeRegex) { args.push('--include', includeRegex); @@ -204,7 +902,6 @@ export class WatchService { } } - // Add path last args.push(path); return args; @@ -233,8 +930,6 @@ export class WatchService { } private globToPathRegex(pattern: string): string { - // Supported glob syntax is intentionally limited to *, ** and ?. - // Handler validation rejects unsupported tokens such as [] and {}. return pattern .replace(/[.+^${}()|[\]\\]/g, '\\$&') .replace(/\*\*/g, '::double_star::') @@ -248,25 +943,24 @@ export class WatchService { path: string; isDirectory: boolean; } | null { - // Format: EVENT|/path/to/file|EVENT_FLAGS - // The third part (%:e) contains colon-separated flags like CREATE:ISDIR const parts = line.trim().split('|'); - if (parts.length < 2) return null; + if (parts.length < 2) { + return null; + } const [rawEvent, filePath, flagsPart] = parts; - // Check if ISDIR appears in either the event or the flags const isDirectory = rawEvent.includes('ISDIR') || (flagsPart?.includes('ISDIR') ?? false); - // Map inotify event back to our type const eventType = this.parseEventType(rawEvent); - if (!eventType) return null; + if (!eventType) { + return null; + } return { eventType, path: filePath, isDirectory }; } private parseEventType(rawEvent: string): FileWatchEventType | null { - // inotify can emit multiple events like "CREATE,ISDIR" const events = rawEvent.split(','); const primary = events[0].toLowerCase(); @@ -277,235 +971,24 @@ export class WatchService { moved_from: 'move_from', moved_to: 'move_to', attrib: 'attrib', - // Handle close_write as modify (common for editors) close_write: 'modify' }; return mapping[primary] || null; } - private createWatchStream( - watchId: string, - path: string, - proc: Subprocess, - logger: Logger - ): ReadableStream { - const encoder = new TextEncoder(); - const self = this; - const stdout = proc.stdout; - const stderr = proc.stderr; - - if (!stdout || typeof stdout === 'number') { - // Return a stream that immediately errors - return new ReadableStream({ - start(controller) { - const errorEvent: FileWatchSSEEvent = { - type: 'error', - error: 'Failed to capture process output' - }; - controller.enqueue( - encoder.encode(`data: ${JSON.stringify(errorEvent)}\n\n`) - ); - controller.close(); - } - }); - } - - return new ReadableStream({ - async start(controller) { - // Wait for inotifywait to establish watches before sending watching event. - // If setup emits an error or times out, fail fast and stop this watch. - if (stderr && typeof stderr !== 'number') { - try { - await self.waitForWatchesEstablished( - stderr, - controller, - encoder, - logger - ); - } catch { - await self.stopWatch(watchId); - controller.close(); - return; - } - } - - // Send watching event only after watches are established - const watchingEvent: FileWatchSSEEvent = { - type: 'watching', - path, - watchId - }; - controller.enqueue( - encoder.encode(`data: ${JSON.stringify(watchingEvent)}\n\n`) - ); - - // Read stdout line by line - const reader = stdout.getReader(); - const decoder = new TextDecoder(); - let buffer = ''; - - const pendingEvents = new Map(); - let droppedEvents = 0; - const enqueueEvent = (event: FileWatchSSEEvent) => { - const key = - event.type === 'event' - ? `${event.eventType}|${event.path}|${event.isDirectory}` - : `${event.type}|${Date.now()}`; - - if ( - !pendingEvents.has(key) && - pendingEvents.size >= MAX_PENDING_EVENTS - ) { - droppedEvents++; - if (droppedEvents === 1 || droppedEvents % 100 === 0) { - logger.warn('Dropping watch events due to backpressure', { - watchId, - droppedEvents, - pendingCount: pendingEvents.size - }); - } - return; - } - - pendingEvents.set(key, event); - }; - - const flushPendingEvents = () => { - for (const event of pendingEvents.values()) { - controller.enqueue( - encoder.encode(`data: ${JSON.stringify(event)}\n\n`) - ); - } - pendingEvents.clear(); - }; - - const flushInterval = setInterval( - flushPendingEvents, - EVENT_COALESCE_WINDOW_MS - ); - - const processLine = (line: string) => { - const parsed = self.parseInotifyEvent(line); - if (!parsed) return; - - const event: FileWatchSSEEvent = { - type: 'event', - eventType: parsed.eventType, - path: parsed.path, - isDirectory: parsed.isDirectory, - timestamp: new Date().toISOString() - }; - enqueueEvent(event); - }; - - try { - logger.debug('Starting to read inotifywait stdout'); - while (true) { - const { done, value } = await reader.read(); - if (done) { - logger.debug('inotifywait stdout stream ended'); - break; - } - - const chunk = decoder.decode(value, { stream: true }); - logger.debug('Received chunk from inotifywait', { - chunkLength: chunk.length, - chunk: chunk.substring(0, 200) - }); - buffer += chunk; - const lines = buffer.split('\n'); - buffer = lines.pop() || ''; - - for (const line of lines) { - if (line.trim()) { - logger.debug('Processing inotifywait line', { line }); - processLine(line); - } - } - } - - // Process any remaining buffer - if (buffer.trim()) { - processLine(buffer); - } - - clearInterval(flushInterval); - flushPendingEvents(); - - // Send stopped event - const reason = - droppedEvents > 0 - ? `Watch process ended. Dropped ${droppedEvents} events due to backpressure.` - : 'Watch process ended'; - - const stoppedEvent: FileWatchSSEEvent = { - type: 'stopped', - reason - }; - controller.enqueue( - encoder.encode(`data: ${JSON.stringify(stoppedEvent)}\n\n`) - ); - controller.close(); - } catch (error) { - clearInterval(flushInterval); - const err = error instanceof Error ? error : new Error(String(error)); - logger.error('Error reading watch output', err); - const errorEvent: FileWatchSSEEvent = { - type: 'error', - error: error instanceof Error ? error.message : 'Unknown error' - }; - controller.enqueue( - encoder.encode(`data: ${JSON.stringify(errorEvent)}\n\n`) - ); - controller.close(); - } finally { - await self.stopWatch(watchId); - } - }, - - cancel() { - // Fire-and-forget: stopWatch handles kill + await exit + map cleanup. - // Returning the promise lets the stream machinery await it if needed. - return self.stopWatch(watchId); - } - }); - } - - /** - * Wait for inotifywait to output "Watches established" on stderr. - * This ensures the watch is ready to detect file changes before we signal readiness to clients. - * After watches are established, continues monitoring stderr for errors in background. - */ private async waitForWatchesEstablished( stderr: ReadableStream, - controller: ReadableStreamDefaultController, - encoder: TextEncoder, logger: Logger - ): Promise { + ): Promise<{ + reader: { read(): Promise<{ done: boolean; value?: Uint8Array }> }; + decoder: TextDecoder; + buffer: string; + }> { const reader = stderr.getReader(); const decoder = new TextDecoder(); let buffer = ''; - const emitSetupError = (message: string) => { - const errorEvent: FileWatchSSEEvent = { - type: 'error', - error: message - }; - try { - controller.enqueue( - encoder.encode(`data: ${JSON.stringify(errorEvent)}\n\n`) - ); - } catch (enqueueError) { - logger.debug('Could not enqueue setup error event', { - error: - enqueueError instanceof Error - ? enqueueError.message - : String(enqueueError) - }); - } - }; - const readLoop = async (): Promise<'established'> => { while (true) { const { done, value } = await reader.read(); @@ -562,18 +1045,8 @@ export class WatchService { throw new Error(timeoutMessage); } - this.continueStderrMonitoring( - reader, - decoder, - buffer, - controller, - encoder, - logger - ); + return { reader, decoder, buffer }; } catch (error) { - const message = - error instanceof Error ? error.message : 'Failed to establish watch'; - emitSetupError(message); await reader.cancel().catch(() => {}); throw error; } finally { @@ -583,24 +1056,21 @@ export class WatchService { } } - /** - * Continue monitoring stderr for errors after watches are established. - * Runs in background without blocking. - */ private continueStderrMonitoring( reader: { read(): Promise<{ done: boolean; value?: Uint8Array }> }, decoder: TextDecoder, initialBuffer: string, - controller: ReadableStreamDefaultController, - encoder: TextEncoder, + watch: ActiveWatch, logger: Logger ): void { - (async () => { + void (async () => { let buffer = initialBuffer; try { while (true) { const { done, value } = await reader.read(); - if (done) break; + if (done) { + break; + } buffer += decoder.decode(value, { stream: true }); const lines = buffer.split('\n'); @@ -608,42 +1078,23 @@ export class WatchService { for (const line of lines) { const trimmed = line.trim(); - if (trimmed) { - // Skip info messages - if ( - trimmed.includes('Watches established') || - trimmed.includes('Setting up watches') - ) { - logger.debug('inotifywait info', { message: trimmed }); - continue; - } - - logger.warn('inotifywait stderr', { message: trimmed }); - const errorEvent: FileWatchSSEEvent = { - type: 'error', - error: trimmed - }; - try { - controller.enqueue( - encoder.encode(`data: ${JSON.stringify(errorEvent)}\n\n`) - ); - } catch (enqueueError) { - logger.debug( - 'Could not enqueue stderr error event, stream likely closed', - { - error: - enqueueError instanceof Error - ? enqueueError.message - : String(enqueueError) - } - ); - break; - } + if (!trimmed) { + continue; } + + if ( + trimmed.includes('Watches established') || + trimmed.includes('Setting up watches') + ) { + logger.debug('inotifywait info', { message: trimmed }); + continue; + } + + logger.warn('inotifywait stderr', { message: trimmed }); + this.broadcastEvent(watch, errorEvent(trimmed)); } } } catch (error) { - // Stream closed or other error - expected when process terminates logger.debug('stderr monitoring ended', { error: error instanceof Error ? error.message : 'Unknown' }); @@ -651,3 +1102,21 @@ export class WatchService { })(); } } + +function createDeferred(): Deferred { + let resolve: Deferred['resolve'] = () => {}; + let reject: Deferred['reject'] = () => {}; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + + return { promise, resolve, reject }; +} + +function errorEvent(message: string): TerminalWatchEvent { + return { + type: 'error', + error: message + }; +} diff --git a/packages/sandbox-container/tests/handlers/watch-handler.test.ts b/packages/sandbox-container/tests/handlers/watch-handler.test.ts index ccf0b0623..b841cd533 100644 --- a/packages/sandbox-container/tests/handlers/watch-handler.test.ts +++ b/packages/sandbox-container/tests/handlers/watch-handler.test.ts @@ -1,19 +1,47 @@ +import type { WatchState } from '@repo/shared'; import { createNoOpLogger } from '@repo/shared'; import { describe, expect, it, vi } from 'vitest'; import { WatchHandler } from '../../src/handlers/watch-handler'; import type { WatchService } from '../../src/services/watch-service'; +function sampleWatchState(overrides: Partial = {}): WatchState { + return { + watchId: 'watch-1', + path: '/workspace/test', + recursive: true, + include: undefined, + exclude: ['.git'], + ownerId: undefined, + cursor: 0, + dirty: false, + overflowed: false, + lastEventAt: null, + expiresAt: null, + subscriberCount: 0, + startedAt: new Date().toISOString(), + ...overrides + }; +} + function createMockWatchService(): WatchService { return { - watchDirectory: vi.fn() + watchDirectory: vi.fn(), + ensureWatch: vi.fn(), + getWatchState: vi.fn(), + ackWatchState: vi.fn(), + stopWatch: vi.fn() } as unknown as WatchService; } -function makeRequest(body: Record): Request { - return new Request('http://localhost:3000/api/watch', { - method: 'POST', +function makeRequest( + path: string, + method: string, + body?: Record +): Request { + return new Request(`http://localhost:3000${path}`, { + method, headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body) + body: body ? JSON.stringify(body) : undefined }); } @@ -25,7 +53,7 @@ const defaultContext = { }; describe('WatchHandler', () => { - describe('include/exclude validation', () => { + describe('stream request validation', () => { it('should reject requests with both include and exclude', async () => { const handler = new WatchHandler( createMockWatchService(), @@ -33,7 +61,7 @@ describe('WatchHandler', () => { ); const response = await handler.handle( - makeRequest({ + makeRequest('/api/watch', 'POST', { path: '/workspace/test', include: ['*.ts'], exclude: ['node_modules'] @@ -61,58 +89,143 @@ describe('WatchHandler', () => { const handler = new WatchHandler(watchService, createNoOpLogger()); const response = await handler.handle( - makeRequest({ path: '/workspace/test', include: ['*.ts'] }), + makeRequest('/api/watch', 'POST', { + path: '/workspace/test', + include: ['*.ts'] + }), defaultContext ); expect(response.status).toBe(200); }); + }); - it('should allow exclude without include', async () => { + describe('persistent watch routes', () => { + it('should ensure a persistent watch', async () => { const watchService = createMockWatchService(); - const mockStream = new ReadableStream(); - ( - watchService.watchDirectory as ReturnType - ).mockResolvedValue({ + const watch = sampleWatchState({ ownerId: 'owner-1' }); + (watchService.ensureWatch as ReturnType).mockResolvedValue({ success: true, - data: mockStream + data: watch }); const handler = new WatchHandler(watchService, createNoOpLogger()); + const response = await handler.handle( + makeRequest('/api/watch/ensure', 'POST', { path: '/workspace/test' }), + defaultContext + ); + + expect(response.status).toBe(200); + const body = (await response.json()) as { watch: WatchState }; + expect(body.watch.watchId).toBe(watch.watchId); + expect(body.watch.ownerId).toBe('owner-1'); + }); + + it('should reject invalid ownerId on ensure', async () => { + const handler = new WatchHandler( + createMockWatchService(), + createNoOpLogger() + ); const response = await handler.handle( - makeRequest({ + makeRequest('/api/watch/ensure', 'POST', { path: '/workspace/test', - exclude: ['node_modules'] + ownerId: '' + }), + defaultContext + ); + + expect(response.status).toBe(400); + }); + + it('should reject invalid ack cursor', async () => { + const handler = new WatchHandler( + createMockWatchService(), + createNoOpLogger() + ); + + const response = await handler.handle( + makeRequest('/api/watch/watch-1/ack', 'POST', { cursor: -1 }), + defaultContext + ); + + expect(response.status).toBe(400); + const body = (await response.json()) as { message: string }; + expect(body.message).toContain('cursor must be a non-negative integer'); + }); + + it('should acknowledge a watch cursor', async () => { + const watchService = createMockWatchService(); + const watch = sampleWatchState({ + cursor: 3, + dirty: false, + ownerId: 'owner-1' + }); + ( + watchService.ackWatchState as ReturnType + ).mockResolvedValue({ + success: true, + data: { + acknowledged: true, + watch + } + }); + + const handler = new WatchHandler(watchService, createNoOpLogger()); + const response = await handler.handle( + makeRequest('/api/watch/watch-1/ack', 'POST', { + cursor: 3, + ownerId: 'owner-1' }), defaultContext ); expect(response.status).toBe(200); + const body = (await response.json()) as { + acknowledged: boolean; + watch: WatchState; + }; + expect(body.acknowledged).toBe(true); + expect(body.watch.cursor).toBe(3); }); - it('should allow empty include with non-empty exclude', async () => { + it('should fetch watch state', async () => { const watchService = createMockWatchService(); - const mockStream = new ReadableStream(); + const watch = sampleWatchState({ cursor: 5, dirty: true }); ( - watchService.watchDirectory as ReturnType + watchService.getWatchState as ReturnType ).mockResolvedValue({ success: true, - data: mockStream + data: watch }); const handler = new WatchHandler(watchService, createNoOpLogger()); + const response = await handler.handle( + makeRequest('/api/watch/watch-1', 'GET'), + defaultContext + ); + expect(response.status).toBe(200); + const body = (await response.json()) as { watch: WatchState }; + expect(body.watch.cursor).toBe(5); + expect(body.watch.dirty).toBe(true); + }); + + it('should stop a watch', async () => { + const watchService = createMockWatchService(); + (watchService.stopWatch as ReturnType).mockResolvedValue({ + success: true + }); + + const handler = new WatchHandler(watchService, createNoOpLogger()); const response = await handler.handle( - makeRequest({ - path: '/workspace/test', - include: [], - exclude: ['node_modules'] - }), + makeRequest('/api/watch/watch-1?ownerId=owner-1', 'DELETE'), defaultContext ); expect(response.status).toBe(200); + const body = (await response.json()) as { watchId: string }; + expect(body.watchId).toBe('watch-1'); }); }); }); diff --git a/packages/sandbox-container/tests/services/watch-service.test.ts b/packages/sandbox-container/tests/services/watch-service.test.ts index e82e8a783..58be7393f 100644 --- a/packages/sandbox-container/tests/services/watch-service.test.ts +++ b/packages/sandbox-container/tests/services/watch-service.test.ts @@ -1,16 +1,19 @@ import { beforeEach, describe, expect, it, vi } from 'bun:test'; -import type { WatchRequest } from '@repo/shared'; +import type { WatchRequest, WatchState } from '@repo/shared'; import { createNoOpLogger } from '@repo/shared'; import { ErrorCode } from '@repo/shared/errors'; import { WatchService } from '@sandbox-container/services/watch-service'; const mockLogger = createNoOpLogger(); -/** - * Type-safe accessor for testing private WatchService methods. - * Uses module augmentation to expose private methods for testing only. - */ +interface FakeWatchProcess { + exited: Promise; + kill: ReturnType; +} + interface WatchServiceTestAccessor { + activeWatches: Map; + watchIdsByKey: Map; parseInotifyEvent(line: string): { eventType: string; path: string; @@ -19,12 +22,70 @@ interface WatchServiceTestAccessor { buildInotifyArgs(path: string, options: WatchRequest): string[]; } +function makeWatchState(overrides: Partial = {}): WatchState { + return { + watchId: 'watch-1', + path: '/workspace/test', + recursive: true, + include: undefined, + exclude: ['.git'], + ownerId: undefined, + cursor: 0, + dirty: false, + overflowed: false, + lastEventAt: null, + expiresAt: null, + subscriberCount: 0, + startedAt: new Date().toISOString(), + ...overrides + }; +} + +function makeFakeProcess(): FakeWatchProcess { + return { + exited: Promise.resolve(0), + kill: vi.fn() + }; +} + +function makeActiveWatch(overrides: Partial> = {}) { + const state = makeWatchState( + (overrides.state as Partial | undefined) ?? {} + ); + const process = + (overrides.process as FakeWatchProcess | undefined) ?? makeFakeProcess(); + + return { + id: state.watchId, + key: 'watch-key', + path: state.path, + recursive: state.recursive, + include: state.include, + exclude: state.exclude, + process, + startedAt: new Date(state.startedAt), + state, + persistent: true, + subscribers: new Map(), + ready: { + promise: Promise.resolve(), + resolve: () => {}, + reject: () => {} + }, + readyState: 'resolved', + expiryTimer: null, + ...overrides + }; +} + describe('WatchService', () => { let watchService: WatchService; + let accessor: WatchServiceTestAccessor; beforeEach(() => { vi.clearAllMocks(); watchService = new WatchService(mockLogger); + accessor = watchService as unknown as WatchServiceTestAccessor; }); describe('getActiveWatches', () => { @@ -41,6 +102,113 @@ describe('WatchService', () => { }); }); + describe('watch state management', () => { + it('should return not found for unknown watch state', async () => { + const result = await watchService.getWatchState('missing-watch'); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.code).toBe(ErrorCode.WATCH_NOT_FOUND); + } + }); + + it('should clear dirty state only when ack cursor matches', async () => { + const watch = makeActiveWatch({ + state: { cursor: 7, dirty: true, overflowed: true, ownerId: 'owner-1' } + }); + accessor.activeWatches.set('watch-1', watch); + accessor.watchIdsByKey.set('watch-key', 'watch-1'); + + const result = await watchService.ackWatchState('watch-1', 7, 'owner-1'); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.acknowledged).toBe(true); + expect(result.data.watch.dirty).toBe(false); + expect(result.data.watch.overflowed).toBe(false); + } + }); + + it('should keep dirty state when ack cursor is stale', async () => { + const watch = makeActiveWatch({ + state: { cursor: 8, dirty: true, overflowed: true } + }); + accessor.activeWatches.set('watch-1', watch); + accessor.watchIdsByKey.set('watch-key', 'watch-1'); + + const result = await watchService.ackWatchState('watch-1', 7); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.acknowledged).toBe(false); + expect(result.data.watch.dirty).toBe(true); + expect(result.data.watch.overflowed).toBe(true); + } + }); + + it('should reject ack from a different owner', async () => { + const watch = makeActiveWatch({ + state: { cursor: 8, dirty: true, ownerId: 'owner-1' } + }); + accessor.activeWatches.set('watch-1', watch); + accessor.watchIdsByKey.set('watch-key', 'watch-1'); + + const result = await watchService.ackWatchState('watch-1', 8, 'other'); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.code).toBe(ErrorCode.RESOURCE_BUSY); + } + }); + + it('should require ownerId when stopping an owned watch', async () => { + const process = makeFakeProcess(); + const watch = makeActiveWatch({ + process, + state: { ownerId: 'owner-1' } + }); + accessor.activeWatches.set('watch-1', watch); + accessor.watchIdsByKey.set('watch-key', 'watch-1'); + + const result = await watchService.stopWatch('watch-1'); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.code).toBe(ErrorCode.RESOURCE_BUSY); + } + expect(process.kill).not.toHaveBeenCalled(); + }); + + it('should refresh persistent watch expiry on read', async () => { + const watch = makeActiveWatch({ + state: { expiresAt: null, ownerId: 'owner-1' } + }); + accessor.activeWatches.set('watch-1', watch); + accessor.watchIdsByKey.set('watch-key', 'watch-1'); + + const result = await watchService.getWatchState('watch-1'); + + expect(result.success).toBe(true); + if (result.success) { + expect(typeof result.data.expiresAt).toBe('string'); + } + }); + + it('should stop an active watch and clean up indexes', async () => { + const process = makeFakeProcess(); + const watch = makeActiveWatch({ process, state: { ownerId: 'owner-1' } }); + accessor.activeWatches.set('watch-1', watch); + accessor.watchIdsByKey.set('watch-key', 'watch-1'); + + const result = await watchService.stopWatch('watch-1', 'owner-1'); + + expect(result.success).toBe(true); + expect(process.kill).toHaveBeenCalled(); + expect(accessor.activeWatches.has('watch-1')).toBe(false); + expect(accessor.watchIdsByKey.has('watch-key')).toBe(false); + }); + }); + describe('watchDirectory', () => { it('should return error for non-existent path', async () => { const result = await watchService.watchDirectory( @@ -55,7 +223,6 @@ describe('WatchService', () => { }); describe('parseInotifyEvent', () => { - // Access private method for testing via type assertion const testParseEvent = (service: WatchService, line: string) => { return (service as unknown as WatchServiceTestAccessor).parseInotifyEvent( line @@ -84,7 +251,6 @@ describe('WatchService', () => { }); it('should parse CREATE,ISDIR with colon-separated flags from %:e format', () => { - // This is the actual output format from inotifywait with --format '%e|%w%f|%:e' const result = testParseEvent( watchService, 'CREATE,ISDIR|/app/newdir|CREATE:ISDIR' @@ -157,7 +323,6 @@ describe('WatchService', () => { }); describe('buildInotifyArgs', () => { - // Access private method for testing via type assertion const testBuildArgs = ( service: WatchService, path: string, @@ -192,15 +357,12 @@ describe('WatchService', () => { it('should include default excludes as combined regex pattern', () => { const args = testBuildArgs(watchService, '/app', { path: '/app' }); expect(args).toContain('--exclude'); - // inotifywait only supports a single --exclude, so patterns are combined with OR const excludeIndex = args.indexOf('--exclude'); expect(excludeIndex).toBeGreaterThan(-1); const excludePattern = args[excludeIndex + 1]; - // Verify combined regex format: (^|/)pattern(/|$)|(^|/)pattern(/|$)|... expect(excludePattern).toContain('(^|/)\\.git(/|$)'); expect(excludePattern).toContain('(^|/)node_modules(/|$)'); expect(excludePattern).toContain('(^|/)\\.DS_Store(/|$)'); - // Patterns are joined with | expect(excludePattern.split('|').length).toBeGreaterThanOrEqual(3); }); diff --git a/packages/sandbox/src/clients/index.ts b/packages/sandbox/src/clients/index.ts index 727778a1f..338a5afe3 100644 --- a/packages/sandbox/src/clients/index.ts +++ b/packages/sandbox/src/clients/index.ts @@ -41,7 +41,16 @@ export { // Client types and interfaces // ============================================================================= -export type { WatchRequest } from '@repo/shared'; +export type { + WatchAckRequest, + WatchAckResult, + WatchEnsureResult, + WatchRequest, + WatchState, + WatchStateResult, + WatchStopOptions, + WatchStopResult +} from '@repo/shared'; // Command client types export type { ExecuteRequest, ExecuteResponse } from './command-client'; // Desktop client types diff --git a/packages/sandbox/src/clients/watch-client.ts b/packages/sandbox/src/clients/watch-client.ts index 42bd0e94e..b76bf963c 100644 --- a/packages/sandbox/src/clients/watch-client.ts +++ b/packages/sandbox/src/clients/watch-client.ts @@ -2,7 +2,13 @@ import { type FileWatchSSEEvent, parseSSEFrames, type SSEPartialEvent, - type WatchRequest + type WatchAckRequest, + type WatchAckResult, + type WatchEnsureResult, + type WatchRequest, + type WatchStateResult, + type WatchStopOptions, + type WatchStopResult } from '@repo/shared'; import { BaseHttpClient } from './base-client'; @@ -14,6 +20,75 @@ import { BaseHttpClient } from './base-client'; * Users should use `sandbox.watch()` instead. */ export class WatchClient extends BaseHttpClient { + async ensureWatch(request: WatchRequest): Promise { + try { + const response = await this.post( + '/api/watch/ensure', + request + ); + + this.logSuccess('Persistent watch ensured', request.path); + return response; + } catch (error) { + this.logError('ensureWatch', error); + throw error; + } + } + + async getWatchState(watchId: string): Promise { + try { + const response = await this.get( + `/api/watch/${watchId}` + ); + + this.logSuccess('Watch state retrieved', watchId); + return response; + } catch (error) { + this.logError('getWatchState', error); + throw error; + } + } + + async ackWatchState( + watchId: string, + request: WatchAckRequest + ): Promise { + try { + const response = await this.post( + `/api/watch/${watchId}/ack`, + request + ); + + this.logSuccess('Watch state acknowledged', watchId); + return response; + } catch (error) { + this.logError('ackWatchState', error); + throw error; + } + } + + async stopWatch( + watchId: string, + options: WatchStopOptions = {} + ): Promise { + try { + const searchParams = new URLSearchParams(); + if (options.ownerId) { + searchParams.set('ownerId', options.ownerId); + } + const suffix = searchParams.size > 0 ? `?${searchParams.toString()}` : ''; + const response = await this.delete( + `/api/watch/${watchId}${suffix}` + ); + + this.logSuccess('Watch stopped', watchId); + return response; + } catch (error) { + this.logError('stopWatch', error); + throw error; + } + } + /** * Start watching a directory for changes. * The returned promise resolves only after the watcher is established diff --git a/packages/sandbox/src/index.ts b/packages/sandbox/src/index.ts index e6722fe78..9952c4df8 100644 --- a/packages/sandbox/src/index.ts +++ b/packages/sandbox/src/index.ts @@ -10,7 +10,8 @@ export { PortClient, ProcessClient, SandboxClient, - UtilityClient + UtilityClient, + WatchClient } from './clients'; export { getSandbox, Sandbox } from './sandbox'; @@ -33,7 +34,6 @@ export type { FileChunk, FileMetadata, FileStreamEvent, - // File watch types FileWatchSSEEvent, GitCheckoutResult, ISandbox, @@ -53,7 +53,15 @@ export type { StreamOptions, WaitForLogResult, WaitForPortOptions, - WatchOptions + // File watch types + WatchAckRequest, + WatchAckResult, + WatchEnsureResult, + WatchOptions, + WatchState, + WatchStateResult, + WatchStopOptions, + WatchStopResult } from '@repo/shared'; // Export type guards for runtime validation export { isExecResult, isProcess, isProcessStatus } from '@repo/shared'; diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 769bfb656..a1037199e 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -29,7 +29,11 @@ import type { WaitForExitResult, WaitForLogResult, WaitForPortOptions, - WatchOptions + WatchAckRequest, + WatchAckResult, + WatchEnsureResult, + WatchOptions, + WatchStopOptions } from '@repo/shared'; import { createLogger, @@ -39,7 +43,9 @@ import { partitionEnvVars, type SessionDeleteResult, shellEscape, - TraceContext + TraceContext, + type WatchStateResult, + type WatchStopResult } from '@repo/shared'; import { AwsClient } from 'aws4fetch'; import { type Desktop, type ExecuteResponse, SandboxClient } from './clients'; @@ -2618,7 +2624,7 @@ export class Sandbox extends Container implements ISandbox { */ async watch( path: string, - options: WatchOptions = {} + options: Omit = {} ): Promise> { const sessionId = options.sessionId ?? (await this.ensureDefaultSession()); return this.client.watch.watch({ @@ -2630,6 +2636,38 @@ export class Sandbox extends Container implements ISandbox { }); } + async ensureWatch( + path: string, + options: WatchOptions = {} + ): Promise { + const sessionId = options.sessionId ?? (await this.ensureDefaultSession()); + return this.client.watch.ensureWatch({ + path, + recursive: options.recursive, + include: options.include, + exclude: options.exclude, + sessionId + }); + } + + async getWatchState(watchId: string): Promise { + return this.client.watch.getWatchState(watchId); + } + + async ackWatchState( + watchId: string, + request: WatchAckRequest + ): Promise { + return this.client.watch.ackWatchState(watchId, request); + } + + async stopWatch( + watchId: string, + options: WatchStopOptions = {} + ): Promise { + return this.client.watch.stopWatch(watchId, options); + } + /** * Expose a port and get a preview URL for accessing services running in the sandbox * @@ -3029,7 +3067,17 @@ export class Sandbox extends Container implements ISandbox { readFile: (path, options) => this.readFile(path, { ...options, sessionId }), readFileStream: (path) => this.readFileStream(path, { sessionId }), - watch: (path, options) => this.watch(path, { ...options, sessionId }), + watch: ( + path, + options: Omit = {} + ) => this.watch(path, { ...options, sessionId }), + ensureWatch: (path: string, options?: Omit) => + this.ensureWatch(path, { ...options, sessionId }), + getWatchState: (watchId: string) => this.getWatchState(watchId), + ackWatchState: (watchId: string, request: WatchAckRequest) => + this.ackWatchState(watchId, request), + stopWatch: (watchId: string, options?: WatchStopOptions) => + this.stopWatch(watchId, options), mkdir: (path, options) => this.mkdir(path, { ...options, sessionId }), deleteFile: (path) => this.deleteFile(path, sessionId), renameFile: (oldPath, newPath) => diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index cf4442889..d57e06532 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -168,8 +168,15 @@ export type { WaitForLogResult, WaitForPortOptions, // File watch types + WatchAckRequest, + WatchAckResult, + WatchEnsureResult, WatchOptions, WatchRequest, + WatchState, + WatchStateResult, + WatchStopOptions, + WatchStopResult, WriteFileResult } from './types.js'; export { diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 622d4fea1..601c6fb71 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -723,6 +723,15 @@ export interface WatchOptions { * If omitted, the default session is used. */ sessionId?: string; + + /** + * Stable consumer identity for persistent watches created with `ensureWatch()`. + * + * Provide this for background agents or Durable Objects so `ackWatchState()` + * and `stopWatch()` can reject conflicting consumers instead of sharing one + * global dirty bit. + */ + ownerId?: string; } // Internal types for SSE protocol (not user-facing) @@ -748,6 +757,7 @@ export interface WatchRequest { include?: string[]; exclude?: string[]; sessionId?: string; + ownerId?: string; } /** @@ -761,6 +771,7 @@ export type FileWatchSSEEvent = } | { type: 'event'; + eventId: string; eventType: FileWatchEventType; path: string; isDirectory: boolean; @@ -775,6 +786,77 @@ export type FileWatchSSEEvent = reason: string; }; +/** + * Persistent watch state retained while the container stays alive. + */ +export interface WatchState { + watchId: string; + path: string; + recursive: boolean; + include?: string[]; + exclude?: string[]; + ownerId?: string; + cursor: number; + dirty: boolean; + overflowed: boolean; + lastEventAt: string | null; + expiresAt: string | null; + subscriberCount: number; + startedAt: string; +} + +/** + * Request body for acknowledging processed watch state. + */ +export interface WatchAckRequest { + cursor: number; + ownerId?: string; +} + +/** + * Options for stopping a persistent watch. + */ +export interface WatchStopOptions { + ownerId?: string; +} + +/** + * Result returned when ensuring a persistent watch exists. + */ +export interface WatchEnsureResult { + success: boolean; + watch: WatchState; + timestamp: string; +} + +/** + * Result returned when retrieving persistent watch state. + */ +export interface WatchStateResult { + success: boolean; + watch: WatchState; + timestamp: string; +} + +/** + * Result returned when acknowledging a watch cursor. + */ +export interface WatchAckResult { + success: boolean; + acknowledged: boolean; + watch: WatchState; + timestamp: string; +} + +/** + * Result returned when stopping a persistent watch. + */ +export interface WatchStopResult { + success: boolean; + watchId: string; + timestamp: string; +} + // Process management result types export interface ProcessStartResult { success: boolean; @@ -968,8 +1050,21 @@ export interface ExecutionSession { readFileStream(path: string): Promise>; watch( path: string, - options?: Omit + options?: Omit ): Promise>; + ensureWatch( + path: string, + options?: Omit + ): Promise; + getWatchState(watchId: string): Promise; + ackWatchState( + watchId: string, + request: WatchAckRequest + ): Promise; + stopWatch( + watchId: string, + options?: WatchStopOptions + ): Promise; mkdir(path: string, options?: { recursive?: boolean }): Promise; deleteFile(path: string): Promise; renameFile(oldPath: string, newPath: string): Promise; @@ -1225,8 +1320,18 @@ export interface ISandbox { readFileStream(path: string): Promise>; watch( path: string, - options?: WatchOptions + options?: Omit ): Promise>; + ensureWatch(path: string, options?: WatchOptions): Promise; + getWatchState(watchId: string): Promise; + ackWatchState( + watchId: string, + request: WatchAckRequest + ): Promise; + stopWatch( + watchId: string, + options?: WatchStopOptions + ): Promise; mkdir(path: string, options?: { recursive?: boolean }): Promise; deleteFile(path: string): Promise; renameFile(oldPath: string, newPath: string): Promise; diff --git a/tests/e2e/file-watch-workflow.test.ts b/tests/e2e/file-watch-workflow.test.ts index ba85c560e..e6f8c3e51 100644 --- a/tests/e2e/file-watch-workflow.test.ts +++ b/tests/e2e/file-watch-workflow.test.ts @@ -26,6 +26,26 @@ import { type TestSandbox } from './helpers/global-sandbox'; +interface PersistentWatchState { + watchId: string; + path: string; + ownerId?: string; + cursor: number; + dirty: boolean; + overflowed: boolean; + lastEventAt: string | null; + expiresAt: string | null; +} + +interface WatchStateResponse { + success: boolean; + watch: PersistentWatchState; +} + +interface WatchAckResponse extends WatchStateResponse { + acknowledged: boolean; +} + describe('File Watch Workflow', () => { let sandbox: TestSandbox | null = null; let workerUrl: string; @@ -148,6 +168,76 @@ describe('File Watch Workflow', () => { throw new Error(`${context} failed with ${response.status}: ${body}`); } + async function ensureWatch( + path: string, + options: { + recursive?: boolean; + include?: string[]; + exclude?: string[]; + ownerId?: string; + } = {} + ): Promise { + const response = await fetch(`${workerUrl}/api/watch/ensure`, { + method: 'POST', + headers, + body: JSON.stringify({ path, ...options }) + }); + await expectOk(response, `ensureWatch(${path})`); + const body = (await response.json()) as WatchStateResponse; + return body.watch; + } + + async function getWatchState(watchId: string): Promise { + const response = await fetch(`${workerUrl}/api/watch/${watchId}`, { + method: 'GET', + headers + }); + await expectOk(response, `getWatchState(${watchId})`); + const body = (await response.json()) as WatchStateResponse; + return body.watch; + } + + async function ackWatchState( + watchId: string, + cursor: number, + ownerId?: string + ): Promise { + const response = await fetch(`${workerUrl}/api/watch/${watchId}/ack`, { + method: 'POST', + headers, + body: JSON.stringify({ cursor, ownerId }) + }); + await expectOk(response, `ackWatchState(${watchId})`); + return (await response.json()) as WatchAckResponse; + } + + async function stopWatch(watchId: string, ownerId?: string): Promise { + const suffix = ownerId ? `?ownerId=${encodeURIComponent(ownerId)}` : ''; + const response = await fetch(`${workerUrl}/api/watch/${watchId}${suffix}`, { + method: 'DELETE', + headers + }); + await expectOk(response, `stopWatch(${watchId})`); + } + + async function waitForWatchState( + watchId: string, + predicate: (state: PersistentWatchState) => boolean, + timeoutMs = 8000 + ): Promise { + const startedAt = Date.now(); + + while (Date.now() - startedAt < timeoutMs) { + const state = await getWatchState(watchId); + if (predicate(state)) { + return state; + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } + + throw new Error(`Timed out waiting for watch state ${watchId}`); + } + /** * Helper to create a file via the API. */ @@ -335,6 +425,45 @@ describe('File Watch Workflow', () => { await secondResponse.body?.cancel(); }, 30000); + test('should retain dirty state across reconnect gaps', async () => { + testDir = sandbox!.uniquePath('watch-persistent-state'); + await createDir(testDir); + + const ownerId = `owner-${Date.now()}`; + const ensuredWatch = await ensureWatch(testDir, { ownerId }); + expect(ensuredWatch.dirty).toBe(false); + expect(ensuredWatch.cursor).toBe(0); + expect(ensuredWatch.ownerId).toBe(ownerId); + expect(ensuredWatch.expiresAt).toBeTruthy(); + + await createFile(`${testDir}/background.txt`, 'hello'); + + const dirtyState = await waitForWatchState( + ensuredWatch.watchId, + (state) => state.dirty && state.cursor > 0 + ); + + expect(dirtyState.lastEventAt).toBeTruthy(); + + const ackResult = await ackWatchState( + ensuredWatch.watchId, + dirtyState.cursor, + ownerId + ); + expect(ackResult.acknowledged).toBe(true); + expect(ackResult.watch.dirty).toBe(false); + + await createFile(`${testDir}/second.txt`, 'hello again'); + + const secondDirtyState = await waitForWatchState( + ensuredWatch.watchId, + (state) => state.dirty && state.cursor > dirtyState.cursor + ); + expect(secondDirtyState.cursor).toBeGreaterThan(dirtyState.cursor); + + await stopWatch(ensuredWatch.watchId, ownerId); + }, 30000); + test('should return error for non-existent path', async () => { const response = await fetch(`${workerUrl}/api/watch`, { method: 'POST', diff --git a/tests/e2e/test-worker/index.ts b/tests/e2e/test-worker/index.ts index 5599b04fd..59f53a1ad 100644 --- a/tests/e2e/test-worker/index.ts +++ b/tests/e2e/test-worker/index.ts @@ -1026,6 +1026,59 @@ console.log('Terminal server on port ' + port); }); } + if (url.pathname === '/api/watch/ensure' && request.method === 'POST') { + const result = await sandbox.ensureWatch(body.path, { + recursive: body.recursive, + include: body.include, + exclude: body.exclude, + ownerId: body.ownerId, + sessionId: sessionId ?? undefined + }); + + return new Response(JSON.stringify(result), { + headers: { 'Content-Type': 'application/json' } + }); + } + + if ( + url.pathname.startsWith('/api/watch/') && + request.method === 'GET' && + !url.pathname.endsWith('/ack') + ) { + const watchId = url.pathname.split('/')[3]; + const result = await sandbox.getWatchState(watchId); + + return new Response(JSON.stringify(result), { + headers: { 'Content-Type': 'application/json' } + }); + } + + if (url.pathname.endsWith('/ack') && request.method === 'POST') { + const watchId = url.pathname.split('/')[3]; + const result = await sandbox.ackWatchState(watchId, { + cursor: body.cursor, + ownerId: body.ownerId + }); + + return new Response(JSON.stringify(result), { + headers: { 'Content-Type': 'application/json' } + }); + } + + if ( + url.pathname.startsWith('/api/watch/') && + request.method === 'DELETE' + ) { + const watchId = url.pathname.split('/')[3]; + const result = await sandbox.stopWatch(watchId, { + ownerId: url.searchParams.get('ownerId') ?? undefined + }); + + return new Response(JSON.stringify(result), { + headers: { 'Content-Type': 'application/json' } + }); + } + // Cleanup endpoint - destroys the sandbox container // This is used by E2E tests to explicitly clean up after each test if (url.pathname === '/cleanup' && request.method === 'POST') { From b970fd53947ffb5a7fb23bd54af41e9ac3b75ffc Mon Sep 17 00:00:00 2001 From: katereznykova Date: Fri, 13 Mar 2026 15:35:36 +0000 Subject: [PATCH 2/6] Forward persistent watch owner IDs Pass ownerId through Sandbox.ensureWatch so persistent watches keep their ownership metadata and the reconnect workflow can validate the same consumer across ack and stop operations. --- packages/sandbox/src/sandbox.ts | 1 + packages/sandbox/tests/sandbox.test.ts | 33 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index a1037199e..86e81de28 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -2646,6 +2646,7 @@ export class Sandbox extends Container implements ISandbox { recursive: options.recursive, include: options.include, exclude: options.exclude, + ownerId: options.ownerId, sessionId }); } diff --git a/packages/sandbox/tests/sandbox.test.ts b/packages/sandbox/tests/sandbox.test.ts index a1519e0c2..6f8750666 100644 --- a/packages/sandbox/tests/sandbox.test.ts +++ b/packages/sandbox/tests/sandbox.test.ts @@ -143,6 +143,23 @@ describe('Sandbox - Automatic Session Management', () => { path: '/test.txt', timestamp: new Date().toISOString() } as any); + + vi.spyOn(sandbox.client.watch, 'ensureWatch').mockResolvedValue({ + success: true, + watch: { + watchId: 'watch-1', + path: '/workspace/test', + recursive: true, + cursor: 0, + dirty: false, + overflowed: false, + lastEventAt: null, + expiresAt: null, + subscriberCount: 0, + startedAt: new Date().toISOString() + }, + timestamp: new Date().toISOString() + } as any); }); afterEach(() => { @@ -195,6 +212,22 @@ describe('Sandbox - Automatic Session Management', () => { ); }); + it('should forward ownerId when ensuring a watch', async () => { + await sandbox.ensureWatch('/workspace/test', { + ownerId: 'owner-1', + recursive: false + }); + + expect(sandbox.client.watch.ensureWatch).toHaveBeenCalledWith({ + path: '/workspace/test', + recursive: false, + include: undefined, + exclude: undefined, + ownerId: 'owner-1', + sessionId: expect.stringMatching(/^sandbox-/) + }); + }); + it('should reuse default session across multiple operations', async () => { await sandbox.exec('echo test1'); await sandbox.writeFile('/test.txt', 'content'); From b4a884c2d95051f6e653a8e47436b6fe6115f2b2 Mon Sep 17 00:00:00 2001 From: katereznykova Date: Fri, 13 Mar 2026 16:39:26 +0000 Subject: [PATCH 3/6] Refine persistent watch leases Replace the initial ownership-flavoured watch API with a cleaner checkpoint and lease model for background consumers. Use `changed`, `checkpointWatch()`, and returned lease tokens for the public flow, while `resumeToken` keeps `ensureWatch()` retryable without exposing another consumer's lease. --- .changeset/persistent-watch-state.md | 2 +- .../src/handlers/watch-handler.ts | 80 ++++--- .../sandbox-container/src/routes/setup.ts | 2 +- .../src/services/watch-service.ts | 222 +++++++++++------- .../tests/handlers/watch-handler.test.ts | 72 ++++-- .../tests/services/watch-service.test.ts | 183 +++++++++++++-- packages/sandbox/src/clients/index.ts | 5 +- packages/sandbox/src/clients/watch-client.ts | 22 +- packages/sandbox/src/index.ts | 7 +- packages/sandbox/src/sandbox.ts | 35 +-- packages/sandbox/tests/sandbox.test.ts | 9 +- packages/shared/src/index.ts | 7 +- packages/shared/src/types.ts | 54 +++-- tests/e2e/file-watch-workflow.test.ts | 89 +++---- tests/e2e/test-worker/index.ts | 12 +- 15 files changed, 522 insertions(+), 279 deletions(-) diff --git a/.changeset/persistent-watch-state.md b/.changeset/persistent-watch-state.md index 8ee0fe5c7..a778215e2 100644 --- a/.changeset/persistent-watch-state.md +++ b/.changeset/persistent-watch-state.md @@ -3,4 +3,4 @@ --- Add persistent file watch state for hibernating Durable Object workflows. -Use `ensureWatch()`, `getWatchState()`, `ackWatchState()`, and `stopWatch()` to keep a watch alive without holding an SSE stream open. +Use `ensureWatch()`, `getWatchState()`, `checkpointWatch()`, and `stopWatch()` to keep a watch alive without holding an SSE stream open. diff --git a/packages/sandbox-container/src/handlers/watch-handler.ts b/packages/sandbox-container/src/handlers/watch-handler.ts index 9a09dd6d2..b31dcd7bc 100644 --- a/packages/sandbox-container/src/handlers/watch-handler.ts +++ b/packages/sandbox-container/src/handlers/watch-handler.ts @@ -1,8 +1,8 @@ import { posix as pathPosix } from 'node:path'; import type { Logger, - WatchAckRequest, - WatchAckResult, + WatchCheckpointRequest, + WatchCheckpointResult, WatchEnsureResult, WatchRequest, WatchStateResult, @@ -61,8 +61,8 @@ export class WatchHandler extends BaseHandler { return this.handleStopWatch(request, context, watchId); } - if (action === 'ack' && request.method === 'POST') { - return this.handleAckWatchState(request, context, watchId); + if (action === 'checkpoint' && request.method === 'POST') { + return this.handleCheckpointWatch(request, context, watchId); } } @@ -130,7 +130,8 @@ export class WatchHandler extends BaseHandler { const response: WatchEnsureResult = { success: true, - watch: result.data, + watch: result.data.watch, + leaseToken: result.data.leaseToken, timestamp: new Date().toISOString() }; @@ -155,14 +156,14 @@ export class WatchHandler extends BaseHandler { return this.createTypedResponse(response, context); } - private async handleAckWatchState( + private async handleCheckpointWatch( request: Request, context: RequestContext, watchId: string ): Promise { - let body: WatchAckRequest; + let body: WatchCheckpointRequest; try { - body = await this.parseRequestBody(request); + body = await this.parseRequestBody(request); } catch (error) { return this.createErrorResponse( { @@ -185,23 +186,33 @@ export class WatchHandler extends BaseHandler { ); } - const ownerIdError = this.validateOwnerId(body.ownerId); - if (ownerIdError) { - return this.createErrorResponse(ownerIdError, context); + if (body.leaseToken === undefined) { + return this.createErrorResponse( + { + message: 'leaseToken is required', + code: ErrorCode.VALIDATION_FAILED + }, + context + ); } - const result = await this.watchService.ackWatchState( + const leaseTokenError = this.validateToken('leaseToken', body.leaseToken); + if (leaseTokenError) { + return this.createErrorResponse(leaseTokenError, context); + } + + const result = await this.watchService.checkpointWatch( watchId, body.cursor, - body.ownerId + body.leaseToken ); if (!result.success) { return this.createErrorResponse(result.error, context); } - const response: WatchAckResult = { + const response: WatchCheckpointResult = { success: true, - acknowledged: result.data.acknowledged, + checkpointed: result.data.checkpointed, watch: result.data.watch, timestamp: new Date().toISOString() }; @@ -214,13 +225,14 @@ export class WatchHandler extends BaseHandler { context: RequestContext, watchId: string ): Promise { - const ownerId = this.extractQueryParam(request, 'ownerId') ?? undefined; - const ownerIdError = this.validateOwnerId(ownerId); - if (ownerIdError) { - return this.createErrorResponse(ownerIdError, context); + const leaseToken = + this.extractQueryParam(request, 'leaseToken') ?? undefined; + const leaseTokenError = this.validateToken('leaseToken', leaseToken); + if (leaseTokenError) { + return this.createErrorResponse(leaseTokenError, context); } - const result = await this.watchService.stopWatch(watchId, ownerId); + const result = await this.watchService.stopWatch(watchId, leaseToken); if (!result.success) { return this.createErrorResponse(result.error, context); } @@ -341,36 +353,42 @@ export class WatchHandler extends BaseHandler { }; } - const ownerIdError = this.validateOwnerId(body.ownerId); - if (ownerIdError) { - return ownerIdError; + const resumeTokenError = this.validateToken( + 'resumeToken', + body.resumeToken + ); + if (resumeTokenError) { + return resumeTokenError; } return null; } - private validateOwnerId(ownerId: unknown): { + private validateToken( + tokenName: 'leaseToken' | 'resumeToken', + token: unknown + ): { message: string; code: string; details?: Record; } | null { - if (ownerId === undefined) { + if (token === undefined) { return null; } - if (typeof ownerId !== 'string' || ownerId.trim() === '') { + if (typeof token !== 'string' || token.trim() === '') { return { - message: 'ownerId must be a non-empty string when provided', + message: `${tokenName} must be a non-empty string when provided`, code: ErrorCode.VALIDATION_FAILED, - details: { ownerId } + details: { [tokenName]: token } }; } - if (ownerId.includes('\0')) { + if (token.includes('\0')) { return { - message: 'ownerId contains invalid null bytes', + message: `${tokenName} contains invalid null bytes`, code: ErrorCode.VALIDATION_FAILED, - details: { ownerId } + details: { [tokenName]: token } }; } diff --git a/packages/sandbox-container/src/routes/setup.ts b/packages/sandbox-container/src/routes/setup.ts index a9c9a44fe..00557b1b6 100644 --- a/packages/sandbox-container/src/routes/setup.ts +++ b/packages/sandbox-container/src/routes/setup.ts @@ -460,7 +460,7 @@ export function setupRoutes(router: Router, container: Container): void { router.register({ method: 'POST', - path: '/api/watch/{id}/ack', + path: '/api/watch/{id}/checkpoint', handler: async (req, ctx) => container.get('watchHandler').handle(req, ctx), middleware: [container.get('loggingMiddleware')] }); diff --git a/packages/sandbox-container/src/services/watch-service.ts b/packages/sandbox-container/src/services/watch-service.ts index 75d7ab0a0..21b854e7f 100644 --- a/packages/sandbox-container/src/services/watch-service.ts +++ b/packages/sandbox-container/src/services/watch-service.ts @@ -36,6 +36,8 @@ interface ActiveWatch { exclude?: string[]; process: Subprocess; startedAt: Date; + leaseToken: string | null; + resumeToken: string | null; state: WatchState; persistent: boolean; subscribers: Map; @@ -74,10 +76,7 @@ export class WatchService { path: string, options: WatchRequest = { path } ): Promise>> { - const watchResult = this.getOrCreateWatch(path, { - ...options, - ownerId: undefined - }); + const watchResult = this.getOrCreateWatch(path, options); if (!watchResult.success) { return watchResult; } @@ -92,16 +91,16 @@ export class WatchService { async ensureWatch( path: string, options: WatchRequest = { path } - ): Promise> { + ): Promise> { const watchResult = this.getOrCreateWatch(path, options); if (!watchResult.success) { return watchResult; } const watch = watchResult.data; - const ownershipError = this.claimPersistentWatch(watch, options.ownerId); - if (ownershipError) { - return serviceError(ownershipError); + const leaseResult = this.claimPersistentWatch(watch, options.resumeToken); + if (!leaseResult.success) { + return serviceError(leaseResult.error); } watch.persistent = true; @@ -109,7 +108,10 @@ export class WatchService { try { await watch.ready.promise; - return serviceSuccess(this.snapshotWatchState(watch)); + return serviceSuccess({ + watch: this.snapshotWatchState(watch), + leaseToken: leaseResult.leaseToken + }); } catch (error) { return serviceError({ message: @@ -152,11 +154,11 @@ export class WatchService { /** * Acknowledge the current watch cursor. */ - async ackWatchState( + async checkpointWatch( watchId: string, cursor: number, - ownerId?: string - ): Promise> { + leaseToken: string + ): Promise> { const watch = this.activeWatches.get(watchId); if (!watch) { return serviceError({ @@ -177,25 +179,25 @@ export class WatchService { }); } - const ownershipError = this.verifyPersistentWatchOwner( + const leaseError = this.verifyPersistentWatchLease( watch, - ownerId, - 'acknowledge' + leaseToken, + 'checkpoint' ); - if (ownershipError) { - return serviceError(ownershipError); + if (leaseError) { + return serviceError(leaseError); } - const acknowledged = cursor === watch.state.cursor; - if (acknowledged) { - watch.state.dirty = false; + const checkpointed = cursor === watch.state.cursor; + if (checkpointed) { + watch.state.changed = false; watch.state.overflowed = false; } this.refreshPersistentWatchLease(watch); return serviceSuccess({ - acknowledged, + checkpointed, watch: this.snapshotWatchState(watch) }); } @@ -205,7 +207,7 @@ export class WatchService { */ async stopWatch( watchId: string, - ownerId?: string + leaseToken?: string ): Promise> { const watch = this.activeWatches.get(watchId); if (!watch) { @@ -216,13 +218,13 @@ export class WatchService { }); } - const ownershipError = this.verifyPersistentWatchOwner( + const leaseError = this.verifyPersistentWatchLease( watch, - ownerId, + leaseToken, 'stop' ); - if (ownershipError) { - return serviceError(ownershipError); + if (leaseError) { + return serviceError(leaseError); } await this.stopWatchInternal(watchId, { @@ -255,6 +257,10 @@ export class WatchService { path: string, options: WatchRequest ): ServiceResult { + const include = this.normalizePatterns(options.include); + const exclude = include + ? undefined + : this.normalizePatterns(options.exclude); const key = this.createWatchKey(path, options); const existingWatchId = this.watchIdsByKey.get(key); if (existingWatchId) { @@ -290,19 +296,20 @@ export class WatchService { key, path, recursive: options.recursive !== false, - include: options.include, - exclude: options.include ? undefined : options.exclude, + include, + exclude, process: proc, startedAt: new Date(), + leaseToken: null, + resumeToken: null, state: { watchId, path, recursive: options.recursive !== false, - include: options.include, - exclude: options.include ? undefined : options.exclude, - ownerId: options.ownerId, + include, + exclude, cursor: 0, - dirty: false, + changed: false, overflowed: false, lastEventAt: null, expiresAt: null, @@ -333,11 +340,16 @@ export class WatchService { } private createWatchKey(path: string, options: WatchRequest): string { + const include = this.normalizePatterns(options.include); + const exclude = include + ? null + : (this.normalizePatterns(options.exclude) ?? null); return JSON.stringify({ path, recursive: options.recursive !== false, - include: options.include ?? null, - exclude: options.include ? null : (options.exclude ?? null) + include: include ?? null, + exclude, + events: this.normalizeEvents(options.events) }); } @@ -353,79 +365,82 @@ export class WatchService { private claimPersistentWatch( watch: ActiveWatch, - ownerId?: string - ): { - message: string; - code: string; - details?: Record; - } | null { - if (!watch.state.ownerId) { - if (ownerId) { - watch.state.ownerId = ownerId; - } - return null; + resumeToken?: string + ): + | { success: true; leaseToken: string } + | { + success: false; + error: { + message: string; + code: string; + details?: Record; + }; + } { + if (!watch.leaseToken) { + const nextLeaseToken = crypto.randomUUID(); + watch.leaseToken = nextLeaseToken; + watch.resumeToken = resumeToken ?? null; + return { success: true, leaseToken: nextLeaseToken }; } - if (!ownerId) { - return { - message: - 'Persistent watch is already owned by another consumer. Provide the same ownerId to reuse it.', - code: ErrorCode.RESOURCE_BUSY, - details: { - watchId: watch.id, - ownerId: watch.state.ownerId - } - }; - } - - if (watch.state.ownerId !== ownerId) { + if ( + !watch.resumeToken || + !resumeToken || + watch.resumeToken !== resumeToken + ) { return { - message: `Persistent watch is owned by '${watch.state.ownerId}' and cannot be claimed by '${ownerId}'.`, - code: ErrorCode.RESOURCE_BUSY, - details: { - watchId: watch.id, - ownerId: watch.state.ownerId, - requestedOwnerId: ownerId + success: false, + error: { + message: + 'A persistent watch already exists for this path. Reuse it with the same resumeToken or wait for it to expire.', + code: ErrorCode.RESOURCE_BUSY, + details: { + watchId: watch.id + } } }; } - return null; + return { success: true, leaseToken: watch.leaseToken }; } - private verifyPersistentWatchOwner( + private verifyPersistentWatchLease( watch: ActiveWatch, - ownerId: string | undefined, - action: 'acknowledge' | 'stop' + leaseToken: string | undefined, + action: 'checkpoint' | 'stop' ): { message: string; code: string; details?: Record; } | null { - if (!watch.state.ownerId) { - return null; + if (!watch.persistent || !watch.leaseToken) { + return { + message: `Only persistent watches can ${action}.`, + code: ErrorCode.RESOURCE_BUSY, + details: { + watchId: watch.id, + action + } + }; } - if (!ownerId) { + if (!leaseToken) { return { - message: `Persistent watch requires ownerId to ${action}.`, + message: `Persistent watch requires a lease token to ${action}.`, code: ErrorCode.RESOURCE_BUSY, details: { watchId: watch.id, - ownerId: watch.state.ownerId, action } }; } - if (watch.state.ownerId !== ownerId) { + if (watch.leaseToken !== leaseToken) { return { - message: `Persistent watch is owned by '${watch.state.ownerId}' and cannot be ${action}d by '${ownerId}'.`, + message: `Persistent watch lease token does not allow this ${action}.`, code: ErrorCode.RESOURCE_BUSY, details: { watchId: watch.id, - ownerId: watch.state.ownerId, - requestedOwnerId: ownerId, action } }; @@ -812,7 +827,7 @@ export class WatchService { }; watch.state.cursor = nextCursor; - watch.state.dirty = true; + watch.state.changed = true; watch.state.lastEventAt = timestamp; this.broadcastEvent(watch, event); } @@ -833,7 +848,7 @@ export class WatchService { }; watch.state.cursor = nextCursor; - watch.state.dirty = true; + watch.state.changed = true; watch.state.lastEventAt = timestamp; this.broadcastEvent(watch, event); } @@ -877,13 +892,7 @@ export class WatchService { args.push('-r'); } - const events: FileWatchEventType[] = options.events || [ - 'create', - 'modify', - 'delete', - 'move_from', - 'move_to' - ]; + const events = this.normalizeEvents(options.events); const inotifyEvents = events .map((e) => this.mapEventType(e)) .filter((e): e is string => e !== undefined); @@ -891,11 +900,17 @@ export class WatchService { args.push('-e', inotifyEvents.join(',')); } - const includeRegex = this.buildCombinedPathRegex(options.include); + const includeRegex = this.buildCombinedPathRegex( + this.normalizePatterns(options.include) + ); if (includeRegex) { args.push('--include', includeRegex); } else { - const excludes = options.exclude || ['.git', 'node_modules', '.DS_Store']; + const excludes = this.normalizePatterns(options.exclude) ?? [ + '.git', + 'node_modules', + '.DS_Store' + ]; const excludeRegex = this.buildCombinedPathRegex(excludes); if (excludeRegex) { args.push('--exclude', excludeRegex); @@ -919,6 +934,37 @@ export class WatchService { return mapping[type]; } + private normalizePatterns(patterns?: string[]): string[] | undefined { + if (!patterns || patterns.length === 0) { + return undefined; + } + + return Array.from(new Set(patterns)).sort(); + } + + private normalizeEvents(events?: FileWatchEventType[]): FileWatchEventType[] { + const defaultEvents: FileWatchEventType[] = [ + 'create', + 'modify', + 'delete', + 'move_from', + 'move_to' + ]; + + if (!events || events.length === 0) { + return defaultEvents; + } + + const orderedEvents = defaultEvents.filter((eventType) => + events.includes(eventType) + ); + const additionalEvents = events.filter( + (eventType) => !orderedEvents.includes(eventType) + ); + + return [...orderedEvents, ...additionalEvents]; + } + private buildCombinedPathRegex(patterns?: string[]): string | undefined { if (!patterns || patterns.length === 0) { return undefined; diff --git a/packages/sandbox-container/tests/handlers/watch-handler.test.ts b/packages/sandbox-container/tests/handlers/watch-handler.test.ts index b841cd533..e58378cc9 100644 --- a/packages/sandbox-container/tests/handlers/watch-handler.test.ts +++ b/packages/sandbox-container/tests/handlers/watch-handler.test.ts @@ -11,9 +11,8 @@ function sampleWatchState(overrides: Partial = {}): WatchState { recursive: true, include: undefined, exclude: ['.git'], - ownerId: undefined, cursor: 0, - dirty: false, + changed: false, overflowed: false, lastEventAt: null, expiresAt: null, @@ -28,7 +27,7 @@ function createMockWatchService(): WatchService { watchDirectory: vi.fn(), ensureWatch: vi.fn(), getWatchState: vi.fn(), - ackWatchState: vi.fn(), + checkpointWatch: vi.fn(), stopWatch: vi.fn() } as unknown as WatchService; } @@ -103,25 +102,34 @@ describe('WatchHandler', () => { describe('persistent watch routes', () => { it('should ensure a persistent watch', async () => { const watchService = createMockWatchService(); - const watch = sampleWatchState({ ownerId: 'owner-1' }); + const watch = sampleWatchState(); (watchService.ensureWatch as ReturnType).mockResolvedValue({ success: true, - data: watch + data: { + watch, + leaseToken: 'lease-1' + } }); const handler = new WatchHandler(watchService, createNoOpLogger()); const response = await handler.handle( - makeRequest('/api/watch/ensure', 'POST', { path: '/workspace/test' }), + makeRequest('/api/watch/ensure', 'POST', { + path: '/workspace/test', + resumeToken: 'resume-1' + }), defaultContext ); expect(response.status).toBe(200); - const body = (await response.json()) as { watch: WatchState }; + const body = (await response.json()) as { + watch: WatchState; + leaseToken: string; + }; expect(body.watch.watchId).toBe(watch.watchId); - expect(body.watch.ownerId).toBe('owner-1'); + expect(body.leaseToken).toBe('lease-1'); }); - it('should reject invalid ownerId on ensure', async () => { + it('should reject invalid resumeToken on ensure', async () => { const handler = new WatchHandler( createMockWatchService(), createNoOpLogger() @@ -130,7 +138,7 @@ describe('WatchHandler', () => { const response = await handler.handle( makeRequest('/api/watch/ensure', 'POST', { path: '/workspace/test', - ownerId: '' + resumeToken: '' }), defaultContext ); @@ -138,14 +146,17 @@ describe('WatchHandler', () => { expect(response.status).toBe(400); }); - it('should reject invalid ack cursor', async () => { + it('should reject invalid checkpoint cursor', async () => { const handler = new WatchHandler( createMockWatchService(), createNoOpLogger() ); const response = await handler.handle( - makeRequest('/api/watch/watch-1/ack', 'POST', { cursor: -1 }), + makeRequest('/api/watch/watch-1/checkpoint', 'POST', { + cursor: -1, + leaseToken: 'lease-1' + }), defaultContext ); @@ -154,44 +165,57 @@ describe('WatchHandler', () => { expect(body.message).toContain('cursor must be a non-negative integer'); }); - it('should acknowledge a watch cursor', async () => { + it('should require leaseToken for checkpoint requests', async () => { + const handler = new WatchHandler( + createMockWatchService(), + createNoOpLogger() + ); + + const response = await handler.handle( + makeRequest('/api/watch/watch-1/checkpoint', 'POST', { cursor: 3 }), + defaultContext + ); + + expect(response.status).toBe(400); + }); + + it('should checkpoint a watch cursor', async () => { const watchService = createMockWatchService(); const watch = sampleWatchState({ cursor: 3, - dirty: false, - ownerId: 'owner-1' + changed: false }); ( - watchService.ackWatchState as ReturnType + watchService.checkpointWatch as ReturnType ).mockResolvedValue({ success: true, data: { - acknowledged: true, + checkpointed: true, watch } }); const handler = new WatchHandler(watchService, createNoOpLogger()); const response = await handler.handle( - makeRequest('/api/watch/watch-1/ack', 'POST', { + makeRequest('/api/watch/watch-1/checkpoint', 'POST', { cursor: 3, - ownerId: 'owner-1' + leaseToken: 'lease-1' }), defaultContext ); expect(response.status).toBe(200); const body = (await response.json()) as { - acknowledged: boolean; + checkpointed: boolean; watch: WatchState; }; - expect(body.acknowledged).toBe(true); + expect(body.checkpointed).toBe(true); expect(body.watch.cursor).toBe(3); }); it('should fetch watch state', async () => { const watchService = createMockWatchService(); - const watch = sampleWatchState({ cursor: 5, dirty: true }); + const watch = sampleWatchState({ cursor: 5, changed: true }); ( watchService.getWatchState as ReturnType ).mockResolvedValue({ @@ -208,7 +232,7 @@ describe('WatchHandler', () => { expect(response.status).toBe(200); const body = (await response.json()) as { watch: WatchState }; expect(body.watch.cursor).toBe(5); - expect(body.watch.dirty).toBe(true); + expect(body.watch.changed).toBe(true); }); it('should stop a watch', async () => { @@ -219,7 +243,7 @@ describe('WatchHandler', () => { const handler = new WatchHandler(watchService, createNoOpLogger()); const response = await handler.handle( - makeRequest('/api/watch/watch-1?ownerId=owner-1', 'DELETE'), + makeRequest('/api/watch/watch-1?leaseToken=lease-1', 'DELETE'), defaultContext ); diff --git a/packages/sandbox-container/tests/services/watch-service.test.ts b/packages/sandbox-container/tests/services/watch-service.test.ts index 58be7393f..737a2b874 100644 --- a/packages/sandbox-container/tests/services/watch-service.test.ts +++ b/packages/sandbox-container/tests/services/watch-service.test.ts @@ -14,6 +14,7 @@ interface FakeWatchProcess { interface WatchServiceTestAccessor { activeWatches: Map; watchIdsByKey: Map; + createWatchKey(path: string, options: WatchRequest): string; parseInotifyEvent(line: string): { eventType: string; path: string; @@ -29,9 +30,8 @@ function makeWatchState(overrides: Partial = {}): WatchState { recursive: true, include: undefined, exclude: ['.git'], - ownerId: undefined, cursor: 0, - dirty: false, + changed: false, overflowed: false, lastEventAt: null, expiresAt: null, @@ -64,6 +64,8 @@ function makeActiveWatch(overrides: Partial> = {}) { exclude: state.exclude, process, startedAt: new Date(state.startedAt), + leaseToken: + (overrides.leaseToken as string | null | undefined) ?? 'lease-1', state, persistent: true, subscribers: new Map(), @@ -112,48 +114,61 @@ describe('WatchService', () => { } }); - it('should clear dirty state only when ack cursor matches', async () => { + it('should clear changed state only when checkpoint cursor matches', async () => { const watch = makeActiveWatch({ - state: { cursor: 7, dirty: true, overflowed: true, ownerId: 'owner-1' } + state: { cursor: 7, changed: true, overflowed: true } }); accessor.activeWatches.set('watch-1', watch); accessor.watchIdsByKey.set('watch-key', 'watch-1'); - const result = await watchService.ackWatchState('watch-1', 7, 'owner-1'); + const result = await watchService.checkpointWatch( + 'watch-1', + 7, + 'lease-1' + ); expect(result.success).toBe(true); if (result.success) { - expect(result.data.acknowledged).toBe(true); - expect(result.data.watch.dirty).toBe(false); + expect(result.data.checkpointed).toBe(true); + expect(result.data.watch.changed).toBe(false); expect(result.data.watch.overflowed).toBe(false); } }); - it('should keep dirty state when ack cursor is stale', async () => { + it('should keep changed state when checkpoint cursor is stale', async () => { const watch = makeActiveWatch({ - state: { cursor: 8, dirty: true, overflowed: true } + state: { cursor: 8, changed: true, overflowed: true } }); accessor.activeWatches.set('watch-1', watch); accessor.watchIdsByKey.set('watch-key', 'watch-1'); - const result = await watchService.ackWatchState('watch-1', 7); + const result = await watchService.checkpointWatch( + 'watch-1', + 7, + 'lease-1' + ); expect(result.success).toBe(true); if (result.success) { - expect(result.data.acknowledged).toBe(false); - expect(result.data.watch.dirty).toBe(true); + expect(result.data.checkpointed).toBe(false); + expect(result.data.watch.changed).toBe(true); expect(result.data.watch.overflowed).toBe(true); } }); - it('should reject ack from a different owner', async () => { + it('should reject checkpoint from a different lease token', async () => { const watch = makeActiveWatch({ - state: { cursor: 8, dirty: true, ownerId: 'owner-1' } + state: { cursor: 8, changed: true }, + leaseToken: 'lease-1' }); accessor.activeWatches.set('watch-1', watch); accessor.watchIdsByKey.set('watch-key', 'watch-1'); - const result = await watchService.ackWatchState('watch-1', 8, 'other'); + const result = await watchService.checkpointWatch( + 'watch-1', + 8, + 'lease-2' + ); expect(result.success).toBe(false); if (!result.success) { @@ -161,11 +176,30 @@ describe('WatchService', () => { } }); - it('should require ownerId when stopping an owned watch', async () => { + it('should reject checkpoint for non-persistent watches', async () => { + const watch = makeActiveWatch({ + persistent: false, + leaseToken: null + }); + accessor.activeWatches.set('watch-1', watch); + accessor.watchIdsByKey.set('watch-key', 'watch-1'); + + const result = await watchService.checkpointWatch( + 'watch-1', + 8, + 'lease-1' + ); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.code).toBe(ErrorCode.RESOURCE_BUSY); + } + }); + + it('should require leaseToken when stopping a leased watch', async () => { const process = makeFakeProcess(); const watch = makeActiveWatch({ - process, - state: { ownerId: 'owner-1' } + process }); accessor.activeWatches.set('watch-1', watch); accessor.watchIdsByKey.set('watch-key', 'watch-1'); @@ -180,9 +214,7 @@ describe('WatchService', () => { }); it('should refresh persistent watch expiry on read', async () => { - const watch = makeActiveWatch({ - state: { expiresAt: null, ownerId: 'owner-1' } - }); + const watch = makeActiveWatch({ state: { expiresAt: null } }); accessor.activeWatches.set('watch-1', watch); accessor.watchIdsByKey.set('watch-key', 'watch-1'); @@ -194,13 +226,56 @@ describe('WatchService', () => { } }); + it('should return the same lease token when reusing a persistent watch', async () => { + const watch = makeActiveWatch({ + leaseToken: 'lease-1', + resumeToken: 'resume-1' + }); + accessor.activeWatches.set('watch-1', watch); + accessor.watchIdsByKey.set( + accessor.createWatchKey('/workspace/test', { path: '/workspace/test' }), + 'watch-1' + ); + + const result = await watchService.ensureWatch('/workspace/test', { + path: '/workspace/test', + resumeToken: 'resume-1' + }); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.leaseToken).toBe('lease-1'); + } + }); + + it('should reject reusing a persistent watch without the same resume token', async () => { + const watch = makeActiveWatch({ + leaseToken: 'lease-1', + resumeToken: 'resume-1' + }); + accessor.activeWatches.set('watch-1', watch); + accessor.watchIdsByKey.set( + accessor.createWatchKey('/workspace/test', { path: '/workspace/test' }), + 'watch-1' + ); + + const result = await watchService.ensureWatch('/workspace/test', { + path: '/workspace/test' + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.code).toBe(ErrorCode.RESOURCE_BUSY); + } + }); + it('should stop an active watch and clean up indexes', async () => { const process = makeFakeProcess(); - const watch = makeActiveWatch({ process, state: { ownerId: 'owner-1' } }); + const watch = makeActiveWatch({ process }); accessor.activeWatches.set('watch-1', watch); accessor.watchIdsByKey.set('watch-key', 'watch-1'); - const result = await watchService.stopWatch('watch-1', 'owner-1'); + const result = await watchService.stopWatch('watch-1', 'lease-1'); expect(result.success).toBe(true); expect(process.kill).toHaveBeenCalled(); @@ -323,6 +398,17 @@ describe('WatchService', () => { }); describe('buildInotifyArgs', () => { + const testCreateWatchKey = ( + service: WatchService, + path: string, + options: WatchRequest + ) => { + return (service as unknown as WatchServiceTestAccessor).createWatchKey( + path, + options + ); + }; + const testBuildArgs = ( service: WatchService, path: string, @@ -391,6 +477,57 @@ describe('WatchService', () => { expect(includePattern).toContain('(^|/)src/.*(/|$)'); }); + it('should treat empty include arrays like no include filter', () => { + const args = testBuildArgs(watchService, '/app', { + path: '/app', + include: [], + exclude: ['tmp'] + }); + expect(args).not.toContain('--include'); + expect(args).toContain('--exclude'); + const excludeIndex = args.indexOf('--exclude'); + expect(args[excludeIndex + 1]).toContain('(^|/)tmp(/|$)'); + }); + + it('should include event filters in the watch key', () => { + const createOnlyKey = testCreateWatchKey(watchService, '/app', { + path: '/app', + events: ['create'] + }); + const modifyOnlyKey = testCreateWatchKey(watchService, '/app', { + path: '/app', + events: ['modify'] + }); + + expect(createOnlyKey).not.toBe(modifyOnlyKey); + }); + + it('should normalize event order in the watch key', () => { + const firstKey = testCreateWatchKey(watchService, '/app', { + path: '/app', + events: ['create', 'modify'] + }); + const secondKey = testCreateWatchKey(watchService, '/app', { + path: '/app', + events: ['modify', 'create'] + }); + + expect(firstKey).toBe(secondKey); + }); + + it('should normalize include pattern order in the watch key', () => { + const firstKey = testCreateWatchKey(watchService, '/app', { + path: '/app', + include: ['*.ts', 'src/**'] + }); + const secondKey = testCreateWatchKey(watchService, '/app', { + path: '/app', + include: ['src/**', '*.ts'] + }); + + expect(firstKey).toBe(secondKey); + }); + it('should add path as last argument', () => { const args = testBuildArgs(watchService, '/app', { path: '/app' }); expect(args[args.length - 1]).toBe('/app'); diff --git a/packages/sandbox/src/clients/index.ts b/packages/sandbox/src/clients/index.ts index 338a5afe3..94d2ce8c7 100644 --- a/packages/sandbox/src/clients/index.ts +++ b/packages/sandbox/src/clients/index.ts @@ -42,8 +42,9 @@ export { // ============================================================================= export type { - WatchAckRequest, - WatchAckResult, + PersistentWatchOptions, + WatchCheckpointRequest, + WatchCheckpointResult, WatchEnsureResult, WatchRequest, WatchState, diff --git a/packages/sandbox/src/clients/watch-client.ts b/packages/sandbox/src/clients/watch-client.ts index b76bf963c..afebef6c0 100644 --- a/packages/sandbox/src/clients/watch-client.ts +++ b/packages/sandbox/src/clients/watch-client.ts @@ -2,8 +2,8 @@ import { type FileWatchSSEEvent, parseSSEFrames, type SSEPartialEvent, - type WatchAckRequest, - type WatchAckResult, + type WatchCheckpointRequest, + type WatchCheckpointResult, type WatchEnsureResult, type WatchRequest, type WatchStateResult, @@ -49,20 +49,20 @@ export class WatchClient extends BaseHttpClient { } } - async ackWatchState( + async checkpointWatch( watchId: string, - request: WatchAckRequest - ): Promise { + request: WatchCheckpointRequest + ): Promise { try { - const response = await this.post( - `/api/watch/${watchId}/ack`, + const response = await this.post( + `/api/watch/${watchId}/checkpoint`, request ); - this.logSuccess('Watch state acknowledged', watchId); + this.logSuccess('Watch checkpoint recorded', watchId); return response; } catch (error) { - this.logError('ackWatchState', error); + this.logError('checkpointWatch', error); throw error; } } @@ -73,8 +73,8 @@ export class WatchClient extends BaseHttpClient { ): Promise { try { const searchParams = new URLSearchParams(); - if (options.ownerId) { - searchParams.set('ownerId', options.ownerId); + if (options.leaseToken) { + searchParams.set('leaseToken', options.leaseToken); } const suffix = searchParams.size > 0 ? `?${searchParams.toString()}` : ''; const response = await this.delete( diff --git a/packages/sandbox/src/index.ts b/packages/sandbox/src/index.ts index 9952c4df8..8920b5c24 100644 --- a/packages/sandbox/src/index.ts +++ b/packages/sandbox/src/index.ts @@ -41,6 +41,8 @@ export type { LocalMountBucketOptions, LogEvent, MountBucketOptions, + // File watch types + PersistentWatchOptions, Process, ProcessOptions, ProcessStatus, @@ -53,9 +55,8 @@ export type { StreamOptions, WaitForLogResult, WaitForPortOptions, - // File watch types - WatchAckRequest, - WatchAckResult, + WatchCheckpointRequest, + WatchCheckpointResult, WatchEnsureResult, WatchOptions, WatchState, diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 86e81de28..be29d4a22 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -15,6 +15,7 @@ import type { LocalMountBucketOptions, LogEvent, MountBucketOptions, + PersistentWatchOptions, PortWatchEvent, Process, ProcessOptions, @@ -29,8 +30,8 @@ import type { WaitForExitResult, WaitForLogResult, WaitForPortOptions, - WatchAckRequest, - WatchAckResult, + WatchCheckpointRequest, + WatchCheckpointResult, WatchEnsureResult, WatchOptions, WatchStopOptions @@ -2624,7 +2625,7 @@ export class Sandbox extends Container implements ISandbox { */ async watch( path: string, - options: Omit = {} + options: WatchOptions = {} ): Promise> { const sessionId = options.sessionId ?? (await this.ensureDefaultSession()); return this.client.watch.watch({ @@ -2638,7 +2639,7 @@ export class Sandbox extends Container implements ISandbox { async ensureWatch( path: string, - options: WatchOptions = {} + options: PersistentWatchOptions = {} ): Promise { const sessionId = options.sessionId ?? (await this.ensureDefaultSession()); return this.client.watch.ensureWatch({ @@ -2646,7 +2647,7 @@ export class Sandbox extends Container implements ISandbox { recursive: options.recursive, include: options.include, exclude: options.exclude, - ownerId: options.ownerId, + resumeToken: options.resumeToken, sessionId }); } @@ -2655,11 +2656,11 @@ export class Sandbox extends Container implements ISandbox { return this.client.watch.getWatchState(watchId); } - async ackWatchState( + async checkpointWatch( watchId: string, - request: WatchAckRequest - ): Promise { - return this.client.watch.ackWatchState(watchId, request); + request: WatchCheckpointRequest + ): Promise { + return this.client.watch.checkpointWatch(watchId, request); } async stopWatch( @@ -3068,15 +3069,15 @@ export class Sandbox extends Container implements ISandbox { readFile: (path, options) => this.readFile(path, { ...options, sessionId }), readFileStream: (path) => this.readFileStream(path, { sessionId }), - watch: ( - path, - options: Omit = {} - ) => this.watch(path, { ...options, sessionId }), - ensureWatch: (path: string, options?: Omit) => - this.ensureWatch(path, { ...options, sessionId }), + watch: (path, options: Omit = {}) => + this.watch(path, { ...options, sessionId }), + ensureWatch: ( + path: string, + options?: Omit + ) => this.ensureWatch(path, { ...options, sessionId }), getWatchState: (watchId: string) => this.getWatchState(watchId), - ackWatchState: (watchId: string, request: WatchAckRequest) => - this.ackWatchState(watchId, request), + checkpointWatch: (watchId: string, request: WatchCheckpointRequest) => + this.checkpointWatch(watchId, request), stopWatch: (watchId: string, options?: WatchStopOptions) => this.stopWatch(watchId, options), mkdir: (path, options) => this.mkdir(path, { ...options, sessionId }), diff --git a/packages/sandbox/tests/sandbox.test.ts b/packages/sandbox/tests/sandbox.test.ts index 6f8750666..e0226d57d 100644 --- a/packages/sandbox/tests/sandbox.test.ts +++ b/packages/sandbox/tests/sandbox.test.ts @@ -151,13 +151,14 @@ describe('Sandbox - Automatic Session Management', () => { path: '/workspace/test', recursive: true, cursor: 0, - dirty: false, + changed: false, overflowed: false, lastEventAt: null, expiresAt: null, subscriberCount: 0, startedAt: new Date().toISOString() }, + leaseToken: 'lease-1', timestamp: new Date().toISOString() } as any); }); @@ -212,9 +213,9 @@ describe('Sandbox - Automatic Session Management', () => { ); }); - it('should forward ownerId when ensuring a watch', async () => { + it('should forward ensureWatch options to the watch client', async () => { await sandbox.ensureWatch('/workspace/test', { - ownerId: 'owner-1', + resumeToken: 'resume-1', recursive: false }); @@ -223,7 +224,7 @@ describe('Sandbox - Automatic Session Management', () => { recursive: false, include: undefined, exclude: undefined, - ownerId: 'owner-1', + resumeToken: 'resume-1', sessionId: expect.stringMatching(/^sandbox-/) }); }); diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index d57e06532..c9f05404c 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -132,6 +132,8 @@ export type { MkdirResult, MountBucketOptions, MoveFileResult, + // File watch types + PersistentWatchOptions, PortCheckRequest, PortCheckResponse, PortCloseResult, @@ -167,9 +169,8 @@ export type { WaitForExitResult, WaitForLogResult, WaitForPortOptions, - // File watch types - WatchAckRequest, - WatchAckResult, + WatchCheckpointRequest, + WatchCheckpointResult, WatchEnsureResult, WatchOptions, WatchRequest, diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 601c6fb71..949d06575 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -723,15 +723,20 @@ export interface WatchOptions { * If omitted, the default session is used. */ sessionId?: string; +} +/** + * Options for creating a persistent watch. + */ +export interface PersistentWatchOptions extends WatchOptions { /** - * Stable consumer identity for persistent watches created with `ensureWatch()`. + * Stable token used to safely reconnect to the same persistent watch. * - * Provide this for background agents or Durable Objects so `ackWatchState()` - * and `stopWatch()` can reject conflicting consumers instead of sharing one - * global dirty bit. + * If provided, repeated `ensureWatch()` calls with the same path and + * `resumeToken` will return the existing lease instead of creating a + * conflicting watch. */ - ownerId?: string; + resumeToken?: string; } // Internal types for SSE protocol (not user-facing) @@ -757,7 +762,7 @@ export interface WatchRequest { include?: string[]; exclude?: string[]; sessionId?: string; - ownerId?: string; + resumeToken?: string; } /** @@ -795,9 +800,8 @@ export interface WatchState { recursive: boolean; include?: string[]; exclude?: string[]; - ownerId?: string; cursor: number; - dirty: boolean; + changed: boolean; overflowed: boolean; lastEventAt: string | null; expiresAt: string | null; @@ -808,16 +812,16 @@ export interface WatchState { /** * Request body for acknowledging processed watch state. */ -export interface WatchAckRequest { +export interface WatchCheckpointRequest { cursor: number; - ownerId?: string; + leaseToken: string; } /** * Options for stopping a persistent watch. */ export interface WatchStopOptions { - ownerId?: string; + leaseToken?: string; } /** @@ -826,6 +830,7 @@ export interface WatchStopOptions { export interface WatchEnsureResult { success: boolean; watch: WatchState; + leaseToken: string; timestamp: string; } @@ -841,9 +846,9 @@ export interface WatchStateResult { /** * Result returned when acknowledging a watch cursor. */ -export interface WatchAckResult { +export interface WatchCheckpointResult { success: boolean; - acknowledged: boolean; + checkpointed: boolean; watch: WatchState; timestamp: string; } @@ -1050,17 +1055,17 @@ export interface ExecutionSession { readFileStream(path: string): Promise>; watch( path: string, - options?: Omit + options?: Omit ): Promise>; ensureWatch( path: string, - options?: Omit + options?: Omit ): Promise; getWatchState(watchId: string): Promise; - ackWatchState( + checkpointWatch( watchId: string, - request: WatchAckRequest - ): Promise; + request: WatchCheckpointRequest + ): Promise; stopWatch( watchId: string, options?: WatchStopOptions @@ -1320,14 +1325,17 @@ export interface ISandbox { readFileStream(path: string): Promise>; watch( path: string, - options?: Omit + options?: WatchOptions ): Promise>; - ensureWatch(path: string, options?: WatchOptions): Promise; + ensureWatch( + path: string, + options?: PersistentWatchOptions + ): Promise; getWatchState(watchId: string): Promise; - ackWatchState( + checkpointWatch( watchId: string, - request: WatchAckRequest - ): Promise; + request: WatchCheckpointRequest + ): Promise; stopWatch( watchId: string, options?: WatchStopOptions diff --git a/tests/e2e/file-watch-workflow.test.ts b/tests/e2e/file-watch-workflow.test.ts index e6f8c3e51..942233a3f 100644 --- a/tests/e2e/file-watch-workflow.test.ts +++ b/tests/e2e/file-watch-workflow.test.ts @@ -29,9 +29,8 @@ import { interface PersistentWatchState { watchId: string; path: string; - ownerId?: string; cursor: number; - dirty: boolean; + changed: boolean; overflowed: boolean; lastEventAt: string | null; expiresAt: string | null; @@ -42,8 +41,12 @@ interface WatchStateResponse { watch: PersistentWatchState; } -interface WatchAckResponse extends WatchStateResponse { - acknowledged: boolean; +interface WatchEnsureResponse extends WatchStateResponse { + leaseToken: string; +} + +interface WatchCheckpointResponse extends WatchStateResponse { + checkpointed: boolean; } describe('File Watch Workflow', () => { @@ -174,17 +177,16 @@ describe('File Watch Workflow', () => { recursive?: boolean; include?: string[]; exclude?: string[]; - ownerId?: string; + resumeToken?: string; } = {} - ): Promise { + ): Promise { const response = await fetch(`${workerUrl}/api/watch/ensure`, { method: 'POST', headers, body: JSON.stringify({ path, ...options }) }); await expectOk(response, `ensureWatch(${path})`); - const body = (await response.json()) as WatchStateResponse; - return body.watch; + return (await response.json()) as WatchEnsureResponse; } async function getWatchState(watchId: string): Promise { @@ -197,22 +199,25 @@ describe('File Watch Workflow', () => { return body.watch; } - async function ackWatchState( + async function checkpointWatch( watchId: string, cursor: number, - ownerId?: string - ): Promise { - const response = await fetch(`${workerUrl}/api/watch/${watchId}/ack`, { - method: 'POST', - headers, - body: JSON.stringify({ cursor, ownerId }) - }); - await expectOk(response, `ackWatchState(${watchId})`); - return (await response.json()) as WatchAckResponse; + leaseToken: string + ): Promise { + const response = await fetch( + `${workerUrl}/api/watch/${watchId}/checkpoint`, + { + method: 'POST', + headers, + body: JSON.stringify({ cursor, leaseToken }) + } + ); + await expectOk(response, `checkpointWatch(${watchId})`); + return (await response.json()) as WatchCheckpointResponse; } - async function stopWatch(watchId: string, ownerId?: string): Promise { - const suffix = ownerId ? `?ownerId=${encodeURIComponent(ownerId)}` : ''; + async function stopWatch(watchId: string, leaseToken: string): Promise { + const suffix = `?leaseToken=${encodeURIComponent(leaseToken)}`; const response = await fetch(`${workerUrl}/api/watch/${watchId}${suffix}`, { method: 'DELETE', headers @@ -425,43 +430,43 @@ describe('File Watch Workflow', () => { await secondResponse.body?.cancel(); }, 30000); - test('should retain dirty state across reconnect gaps', async () => { + test('should retain changed state across reconnect gaps', async () => { testDir = sandbox!.uniquePath('watch-persistent-state'); await createDir(testDir); - const ownerId = `owner-${Date.now()}`; - const ensuredWatch = await ensureWatch(testDir, { ownerId }); - expect(ensuredWatch.dirty).toBe(false); - expect(ensuredWatch.cursor).toBe(0); - expect(ensuredWatch.ownerId).toBe(ownerId); - expect(ensuredWatch.expiresAt).toBeTruthy(); + const resumeToken = `resume-${Date.now()}`; + const ensuredWatch = await ensureWatch(testDir, { resumeToken }); + expect(ensuredWatch.watch.changed).toBe(false); + expect(ensuredWatch.watch.cursor).toBe(0); + expect(ensuredWatch.leaseToken).toBeTruthy(); + expect(ensuredWatch.watch.expiresAt).toBeTruthy(); await createFile(`${testDir}/background.txt`, 'hello'); - const dirtyState = await waitForWatchState( - ensuredWatch.watchId, - (state) => state.dirty && state.cursor > 0 + const changedState = await waitForWatchState( + ensuredWatch.watch.watchId, + (state) => state.changed && state.cursor > 0 ); - expect(dirtyState.lastEventAt).toBeTruthy(); + expect(changedState.lastEventAt).toBeTruthy(); - const ackResult = await ackWatchState( - ensuredWatch.watchId, - dirtyState.cursor, - ownerId + const checkpointResult = await checkpointWatch( + ensuredWatch.watch.watchId, + changedState.cursor, + ensuredWatch.leaseToken ); - expect(ackResult.acknowledged).toBe(true); - expect(ackResult.watch.dirty).toBe(false); + expect(checkpointResult.checkpointed).toBe(true); + expect(checkpointResult.watch.changed).toBe(false); await createFile(`${testDir}/second.txt`, 'hello again'); - const secondDirtyState = await waitForWatchState( - ensuredWatch.watchId, - (state) => state.dirty && state.cursor > dirtyState.cursor + const secondChangedState = await waitForWatchState( + ensuredWatch.watch.watchId, + (state) => state.changed && state.cursor > changedState.cursor ); - expect(secondDirtyState.cursor).toBeGreaterThan(dirtyState.cursor); + expect(secondChangedState.cursor).toBeGreaterThan(changedState.cursor); - await stopWatch(ensuredWatch.watchId, ownerId); + await stopWatch(ensuredWatch.watch.watchId, ensuredWatch.leaseToken); }, 30000); test('should return error for non-existent path', async () => { diff --git a/tests/e2e/test-worker/index.ts b/tests/e2e/test-worker/index.ts index 59f53a1ad..6c621189f 100644 --- a/tests/e2e/test-worker/index.ts +++ b/tests/e2e/test-worker/index.ts @@ -1031,7 +1031,7 @@ console.log('Terminal server on port ' + port); recursive: body.recursive, include: body.include, exclude: body.exclude, - ownerId: body.ownerId, + resumeToken: body.resumeToken, sessionId: sessionId ?? undefined }); @@ -1043,7 +1043,7 @@ console.log('Terminal server on port ' + port); if ( url.pathname.startsWith('/api/watch/') && request.method === 'GET' && - !url.pathname.endsWith('/ack') + !url.pathname.endsWith('/checkpoint') ) { const watchId = url.pathname.split('/')[3]; const result = await sandbox.getWatchState(watchId); @@ -1053,11 +1053,11 @@ console.log('Terminal server on port ' + port); }); } - if (url.pathname.endsWith('/ack') && request.method === 'POST') { + if (url.pathname.endsWith('/checkpoint') && request.method === 'POST') { const watchId = url.pathname.split('/')[3]; - const result = await sandbox.ackWatchState(watchId, { + const result = await sandbox.checkpointWatch(watchId, { cursor: body.cursor, - ownerId: body.ownerId + leaseToken: body.leaseToken }); return new Response(JSON.stringify(result), { @@ -1071,7 +1071,7 @@ console.log('Terminal server on port ' + port); ) { const watchId = url.pathname.split('/')[3]; const result = await sandbox.stopWatch(watchId, { - ownerId: url.searchParams.get('ownerId') ?? undefined + leaseToken: url.searchParams.get('leaseToken') ?? undefined }); return new Response(JSON.stringify(result), { From aa31d8c175bd356f2a770f8c99d43e16a75e11b1 Mon Sep 17 00:00:00 2001 From: katereznykova Date: Fri, 13 Mar 2026 17:25:23 +0000 Subject: [PATCH 4/6] Polish persistent watch compatibility Clarify stopWatch token validation, remove redundant key normalization work, and normalize legacy watch responses so clients still see `changed` while older paths return `dirty`. --- .../src/handlers/watch-handler.ts | 8 +- .../src/services/watch-service.ts | 32 +++-- .../tests/services/watch-service.test.ts | 37 +++++- packages/sandbox/src/clients/watch-client.ts | 29 ++++- packages/sandbox/tests/watch-client.test.ts | 112 ++++++++++++++++++ packages/shared/src/types.ts | 3 + 6 files changed, 203 insertions(+), 18 deletions(-) create mode 100644 packages/sandbox/tests/watch-client.test.ts diff --git a/packages/sandbox-container/src/handlers/watch-handler.ts b/packages/sandbox-container/src/handlers/watch-handler.ts index b31dcd7bc..c5ec50231 100644 --- a/packages/sandbox-container/src/handlers/watch-handler.ts +++ b/packages/sandbox-container/src/handlers/watch-handler.ts @@ -227,9 +227,11 @@ export class WatchHandler extends BaseHandler { ): Promise { const leaseToken = this.extractQueryParam(request, 'leaseToken') ?? undefined; - const leaseTokenError = this.validateToken('leaseToken', leaseToken); - if (leaseTokenError) { - return this.createErrorResponse(leaseTokenError, context); + if (leaseToken !== undefined) { + const leaseTokenError = this.validateToken('leaseToken', leaseToken); + if (leaseTokenError) { + return this.createErrorResponse(leaseTokenError, context); + } } const result = await this.watchService.stopWatch(watchId, leaseToken); diff --git a/packages/sandbox-container/src/services/watch-service.ts b/packages/sandbox-container/src/services/watch-service.ts index 21b854e7f..f2ada8ef8 100644 --- a/packages/sandbox-container/src/services/watch-service.ts +++ b/packages/sandbox-container/src/services/watch-service.ts @@ -261,7 +261,13 @@ export class WatchService { const exclude = include ? undefined : this.normalizePatterns(options.exclude); - const key = this.createWatchKey(path, options); + const events = this.normalizeEvents(options.events); + const key = this.createWatchKey(path, { + recursive: options.recursive !== false, + include, + exclude, + events + }); const existingWatchId = this.watchIdsByKey.get(key); if (existingWatchId) { const existing = this.activeWatches.get(existingWatchId); @@ -339,15 +345,25 @@ export class WatchService { } } - private createWatchKey(path: string, options: WatchRequest): string { - const include = this.normalizePatterns(options.include); - const exclude = include - ? null - : (this.normalizePatterns(options.exclude) ?? null); + private createWatchKey( + path: string, + options: { + recursive: boolean; + include?: string[]; + exclude?: string[]; + events: FileWatchEventType[]; + } + ): string { + const include = options.include + ? Array.from(new Set(options.include)).sort() + : null; + const exclude = options.exclude + ? Array.from(new Set(options.exclude)).sort() + : null; return JSON.stringify({ path, - recursive: options.recursive !== false, - include: include ?? null, + recursive: options.recursive, + include, exclude, events: this.normalizeEvents(options.events) }); diff --git a/packages/sandbox-container/tests/services/watch-service.test.ts b/packages/sandbox-container/tests/services/watch-service.test.ts index 737a2b874..d134793bd 100644 --- a/packages/sandbox-container/tests/services/watch-service.test.ts +++ b/packages/sandbox-container/tests/services/watch-service.test.ts @@ -14,7 +14,15 @@ interface FakeWatchProcess { interface WatchServiceTestAccessor { activeWatches: Map; watchIdsByKey: Map; - createWatchKey(path: string, options: WatchRequest): string; + createWatchKey( + path: string, + options: { + recursive: boolean; + include?: string[]; + exclude?: string[]; + events: string[]; + } + ): string; parseInotifyEvent(line: string): { eventType: string; path: string; @@ -233,7 +241,12 @@ describe('WatchService', () => { }); accessor.activeWatches.set('watch-1', watch); accessor.watchIdsByKey.set( - accessor.createWatchKey('/workspace/test', { path: '/workspace/test' }), + accessor.createWatchKey('/workspace/test', { + recursive: true, + include: undefined, + exclude: undefined, + events: ['create', 'modify', 'delete', 'move_from', 'move_to'] + }), 'watch-1' ); @@ -255,7 +268,12 @@ describe('WatchService', () => { }); accessor.activeWatches.set('watch-1', watch); accessor.watchIdsByKey.set( - accessor.createWatchKey('/workspace/test', { path: '/workspace/test' }), + accessor.createWatchKey('/workspace/test', { + recursive: true, + include: undefined, + exclude: undefined, + events: ['create', 'modify', 'delete', 'move_from', 'move_to'] + }), 'watch-1' ); @@ -405,7 +423,18 @@ describe('WatchService', () => { ) => { return (service as unknown as WatchServiceTestAccessor).createWatchKey( path, - options + { + recursive: options.recursive !== false, + include: options.include, + exclude: options.exclude, + events: options.events ?? [ + 'create', + 'modify', + 'delete', + 'move_from', + 'move_to' + ] + } ); }; diff --git a/packages/sandbox/src/clients/watch-client.ts b/packages/sandbox/src/clients/watch-client.ts index afebef6c0..37e990572 100644 --- a/packages/sandbox/src/clients/watch-client.ts +++ b/packages/sandbox/src/clients/watch-client.ts @@ -6,6 +6,7 @@ import { type WatchCheckpointResult, type WatchEnsureResult, type WatchRequest, + type WatchState, type WatchStateResult, type WatchStopOptions, type WatchStopResult @@ -28,7 +29,10 @@ export class WatchClient extends BaseHttpClient { ); this.logSuccess('Persistent watch ensured', request.path); - return response; + return { + ...response, + watch: normalizeWatchState(response.watch) + }; } catch (error) { this.logError('ensureWatch', error); throw error; @@ -42,7 +46,10 @@ export class WatchClient extends BaseHttpClient { ); this.logSuccess('Watch state retrieved', watchId); - return response; + return { + ...response, + watch: normalizeWatchState(response.watch) + }; } catch (error) { this.logError('getWatchState', error); throw error; @@ -60,7 +67,10 @@ export class WatchClient extends BaseHttpClient { ); this.logSuccess('Watch checkpoint recorded', watchId); - return response; + return { + ...response, + watch: normalizeWatchState(response.watch) + }; } catch (error) { this.logError('checkpointWatch', error); throw error; @@ -202,3 +212,16 @@ export class WatchClient extends BaseHttpClient { }); } } + +function normalizeWatchState(watch: WatchState): WatchState { + const legacyWatch = watch as WatchState & { dirty?: boolean }; + + if (legacyWatch.changed !== undefined) { + return watch; + } + + return { + ...watch, + changed: legacyWatch.dirty ?? false + }; +} diff --git a/packages/sandbox/tests/watch-client.test.ts b/packages/sandbox/tests/watch-client.test.ts new file mode 100644 index 000000000..d60da19f7 --- /dev/null +++ b/packages/sandbox/tests/watch-client.test.ts @@ -0,0 +1,112 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { WatchClient } from '../src/clients/watch-client'; + +describe('WatchClient', () => { + let client: WatchClient; + let mockFetch: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + + mockFetch = vi.fn(); + global.fetch = mockFetch as unknown as typeof fetch; + + client = new WatchClient({ + baseUrl: 'http://test.com', + port: 3000 + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should normalize legacy dirty state from ensureWatch', async () => { + mockFetch.mockResolvedValue( + new Response( + JSON.stringify({ + success: true, + watch: { + watchId: 'watch-1', + path: '/workspace/test', + recursive: true, + cursor: 0, + dirty: false, + overflowed: false, + lastEventAt: null, + expiresAt: null, + subscriberCount: 0, + startedAt: '2023-01-01T00:00:00Z' + }, + leaseToken: 'lease-1', + timestamp: '2023-01-01T00:00:00Z' + }), + { status: 200 } + ) + ); + + const result = await client.ensureWatch({ path: '/workspace/test' }); + + expect(result.watch.changed).toBe(false); + }); + + it('should normalize legacy dirty state from getWatchState', async () => { + mockFetch.mockResolvedValue( + new Response( + JSON.stringify({ + success: true, + watch: { + watchId: 'watch-1', + path: '/workspace/test', + recursive: true, + cursor: 2, + dirty: true, + overflowed: false, + lastEventAt: null, + expiresAt: null, + subscriberCount: 0, + startedAt: '2023-01-01T00:00:00Z' + }, + timestamp: '2023-01-01T00:00:00Z' + }), + { status: 200 } + ) + ); + + const result = await client.getWatchState('watch-1'); + + expect(result.watch.changed).toBe(true); + }); + + it('should normalize legacy dirty state from checkpointWatch', async () => { + mockFetch.mockResolvedValue( + new Response( + JSON.stringify({ + success: true, + checkpointed: true, + watch: { + watchId: 'watch-1', + path: '/workspace/test', + recursive: true, + cursor: 2, + dirty: false, + overflowed: false, + lastEventAt: null, + expiresAt: null, + subscriberCount: 0, + startedAt: '2023-01-01T00:00:00Z' + }, + timestamp: '2023-01-01T00:00:00Z' + }), + { status: 200 } + ) + ); + + const result = await client.checkpointWatch('watch-1', { + cursor: 2, + leaseToken: 'lease-1' + }); + + expect(result.watch.changed).toBe(false); + }); +}); diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 949d06575..586361f34 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -821,6 +821,9 @@ export interface WatchCheckpointRequest { * Options for stopping a persistent watch. */ export interface WatchStopOptions { + /** + * Required when stopping a persistent watch obtained from `ensureWatch()`. + */ leaseToken?: string; } From 47d0d3019fb853cef9f989252c3a4b174525f987 Mon Sep 17 00:00:00 2001 From: katereznykova Date: Tue, 17 Mar 2026 14:38:05 +0000 Subject: [PATCH 5/6] Refocus file watches on change checks Background consumers only need to know whether a path changed while disconnected. Replace the lease-based persistent watch API with checkChanges() so callers store one version token and choose whether to skip work, sync incrementally, or rescan. --- .changeset/persistent-watch-state.md | 5 +- .../src/handlers/watch-handler.ts | 211 ++----- .../sandbox-container/src/routes/setup.ts | 23 +- .../src/services/watch-service.ts | 563 +++++++----------- .../tests/handlers/watch-handler.test.ts | 216 +++---- .../tests/services/watch-service.test.ts | 351 +---------- packages/sandbox/src/clients/index.ts | 13 +- packages/sandbox/src/clients/watch-client.ts | 104 +--- packages/sandbox/src/index.ts | 17 +- packages/sandbox/src/sandbox.ts | 317 ++++++++-- packages/sandbox/tests/sandbox.test.ts | 27 +- packages/sandbox/tests/watch-client.test.ts | 95 +-- packages/shared/src/index.ts | 13 +- packages/shared/src/types.ts | 142 ++--- tests/e2e/file-watch-workflow.test.ts | 201 ++----- tests/e2e/test-worker/index.ts | 46 +- 16 files changed, 760 insertions(+), 1584 deletions(-) diff --git a/.changeset/persistent-watch-state.md b/.changeset/persistent-watch-state.md index a778215e2..391cda4aa 100644 --- a/.changeset/persistent-watch-state.md +++ b/.changeset/persistent-watch-state.md @@ -2,5 +2,6 @@ '@cloudflare/sandbox': patch --- -Add persistent file watch state for hibernating Durable Object workflows. -Use `ensureWatch()`, `getWatchState()`, `checkpointWatch()`, and `stopWatch()` to keep a watch alive without holding an SSE stream open. +Add `checkChanges()` for apps that disconnect and reconnect later but still need to know whether files changed in the meantime. + +Use the returned `version` in a later call to learn whether a path is unchanged, changed, or needs a full resync. Retained change state lasts for the current container lifetime only. diff --git a/packages/sandbox-container/src/handlers/watch-handler.ts b/packages/sandbox-container/src/handlers/watch-handler.ts index c5ec50231..40e2f7826 100644 --- a/packages/sandbox-container/src/handlers/watch-handler.ts +++ b/packages/sandbox-container/src/handlers/watch-handler.ts @@ -1,13 +1,5 @@ import { posix as pathPosix } from 'node:path'; -import type { - Logger, - WatchCheckpointRequest, - WatchCheckpointResult, - WatchEnsureResult, - WatchRequest, - WatchStateResult, - WatchStopResult -} from '@repo/shared'; +import type { CheckChangesRequest, Logger, WatchRequest } from '@repo/shared'; import { ErrorCode } from '@repo/shared/errors'; import { CONFIG } from '../config'; import type { RequestContext } from '../core/types'; @@ -34,36 +26,8 @@ export class WatchHandler extends BaseHandler { return this.handleWatch(request, context); } - if (pathname === '/api/watch/ensure' && request.method === 'POST') { - return this.handleEnsureWatch(request, context); - } - - if (pathname.startsWith('/api/watch/')) { - const segments = pathname.split('/'); - const watchId = segments[3]; - const action = segments[4]; - - if (!watchId) { - return this.createErrorResponse( - { - message: 'Watch ID is required', - code: ErrorCode.VALIDATION_FAILED - }, - context - ); - } - - if (!action && request.method === 'GET') { - return this.handleGetWatchState(context, watchId); - } - - if (!action && request.method === 'DELETE') { - return this.handleStopWatch(request, context, watchId); - } - - if (action === 'checkpoint' && request.method === 'POST') { - return this.handleCheckpointWatch(request, context, watchId); - } + if (pathname === '/api/watch/check' && request.method === 'POST') { + return this.handleCheckChanges(request, context); } return this.createErrorResponse( @@ -76,6 +40,10 @@ export class WatchHandler extends BaseHandler { ); } + /** + * Start watching a directory. + * Returns an SSE stream of file change events. + */ private async handleWatch( request: Request, context: RequestContext @@ -107,11 +75,14 @@ export class WatchHandler extends BaseHandler { }); } - private async handleEnsureWatch( + /** + * Check whether a path changed since a previously returned version. + */ + private async handleCheckChanges( request: Request, context: RequestContext ): Promise { - const normalizedRequest = await this.parseAndNormalizeWatchRequest( + const normalizedRequest = await this.parseAndNormalizeCheckRequest( request, context ); @@ -119,51 +90,24 @@ export class WatchHandler extends BaseHandler { return normalizedRequest; } - const result = await this.watchService.ensureWatch( + const result = await this.watchService.checkChanges( normalizedRequest.path, normalizedRequest ); - - if (!result.success) { - return this.createErrorResponse(result.error, context); - } - - const response: WatchEnsureResult = { - success: true, - watch: result.data.watch, - leaseToken: result.data.leaseToken, - timestamp: new Date().toISOString() - }; - - return this.createTypedResponse(response, context); - } - - private async handleGetWatchState( - context: RequestContext, - watchId: string - ): Promise { - const result = await this.watchService.getWatchState(watchId); if (!result.success) { return this.createErrorResponse(result.error, context); } - const response: WatchStateResult = { - success: true, - watch: result.data, - timestamp: new Date().toISOString() - }; - - return this.createTypedResponse(response, context); + return this.createTypedResponse(result.data, context); } - private async handleCheckpointWatch( + private async parseAndNormalizeWatchRequest( request: Request, - context: RequestContext, - watchId: string - ): Promise { - let body: WatchCheckpointRequest; + context: RequestContext + ): Promise { + let body: WatchRequest; try { - body = await this.parseRequestBody(request); + body = await this.parseRequestBody(request); } catch (error) { return this.createErrorResponse( { @@ -175,86 +119,29 @@ export class WatchHandler extends BaseHandler { ); } - if (!Number.isInteger(body.cursor) || body.cursor < 0) { - return this.createErrorResponse( - { - message: 'cursor must be a non-negative integer', - code: ErrorCode.VALIDATION_FAILED, - details: { cursor: body.cursor } - }, - context - ); - } - - if (body.leaseToken === undefined) { - return this.createErrorResponse( - { - message: 'leaseToken is required', - code: ErrorCode.VALIDATION_FAILED - }, - context - ); - } - - const leaseTokenError = this.validateToken('leaseToken', body.leaseToken); - if (leaseTokenError) { - return this.createErrorResponse(leaseTokenError, context); - } - - const result = await this.watchService.checkpointWatch( - watchId, - body.cursor, - body.leaseToken - ); - if (!result.success) { - return this.createErrorResponse(result.error, context); - } - - const response: WatchCheckpointResult = { - success: true, - checkpointed: result.data.checkpointed, - watch: result.data.watch, - timestamp: new Date().toISOString() - }; - - return this.createTypedResponse(response, context); - } - - private async handleStopWatch( - request: Request, - context: RequestContext, - watchId: string - ): Promise { - const leaseToken = - this.extractQueryParam(request, 'leaseToken') ?? undefined; - if (leaseToken !== undefined) { - const leaseTokenError = this.validateToken('leaseToken', leaseToken); - if (leaseTokenError) { - return this.createErrorResponse(leaseTokenError, context); - } + const validationError = this.validateWatchBody(body); + if (validationError) { + return this.createErrorResponse(validationError, context); } - const result = await this.watchService.stopWatch(watchId, leaseToken); - if (!result.success) { - return this.createErrorResponse(result.error, context); + const pathResult = this.normalizeWatchPath(body.path); + if (!pathResult.success) { + return this.createErrorResponse(pathResult.error, context); } - const response: WatchStopResult = { - success: true, - watchId, - timestamp: new Date().toISOString() + return { + ...body, + path: pathResult.path }; - - return this.createTypedResponse(response, context); } - private async parseAndNormalizeWatchRequest( + private async parseAndNormalizeCheckRequest( request: Request, context: RequestContext - ): Promise { - let body: WatchRequest; + ): Promise { + let body: CheckChangesRequest; try { - body = await this.parseRequestBody(request); + body = await this.parseRequestBody(request); } catch (error) { return this.createErrorResponse( { @@ -266,7 +153,7 @@ export class WatchHandler extends BaseHandler { ); } - const validationError = this.validateWatchBody(body); + const validationError = this.validateCheckChangesBody(body); if (validationError) { return this.createErrorResponse(validationError, context); } @@ -355,42 +242,36 @@ export class WatchHandler extends BaseHandler { }; } - const resumeTokenError = this.validateToken( - 'resumeToken', - body.resumeToken - ); - if (resumeTokenError) { - return resumeTokenError; - } - return null; } - private validateToken( - tokenName: 'leaseToken' | 'resumeToken', - token: unknown - ): { + private validateCheckChangesBody(body: CheckChangesRequest): { message: string; code: string; details?: Record; } | null { - if (token === undefined) { + const watchValidationError = this.validateWatchBody(body); + if (watchValidationError) { + return watchValidationError; + } + + if (body.since === undefined) { return null; } - if (typeof token !== 'string' || token.trim() === '') { + if (typeof body.since !== 'string' || body.since.trim() === '') { return { - message: `${tokenName} must be a non-empty string when provided`, + message: 'since must be a non-empty string when provided', code: ErrorCode.VALIDATION_FAILED, - details: { [tokenName]: token } + details: { since: body.since } }; } - if (token.includes('\0')) { + if (body.since.includes('\0')) { return { - message: `${tokenName} contains invalid null bytes`, + message: 'since contains invalid null bytes', code: ErrorCode.VALIDATION_FAILED, - details: { [tokenName]: token } + details: { since: body.since } }; } diff --git a/packages/sandbox-container/src/routes/setup.ts b/packages/sandbox-container/src/routes/setup.ts index 00557b1b6..c8bfc5c6d 100644 --- a/packages/sandbox-container/src/routes/setup.ts +++ b/packages/sandbox-container/src/routes/setup.ts @@ -446,28 +446,7 @@ export function setupRoutes(router: Router, container: Container): void { router.register({ method: 'POST', - path: '/api/watch/ensure', - handler: async (req, ctx) => container.get('watchHandler').handle(req, ctx), - middleware: [container.get('loggingMiddleware')] - }); - - router.register({ - method: 'GET', - path: '/api/watch/{id}', - handler: async (req, ctx) => container.get('watchHandler').handle(req, ctx), - middleware: [container.get('loggingMiddleware')] - }); - - router.register({ - method: 'POST', - path: '/api/watch/{id}/checkpoint', - handler: async (req, ctx) => container.get('watchHandler').handle(req, ctx), - middleware: [container.get('loggingMiddleware')] - }); - - router.register({ - method: 'DELETE', - path: '/api/watch/{id}', + path: '/api/watch/check', handler: async (req, ctx) => container.get('watchHandler').handle(req, ctx), middleware: [container.get('loggingMiddleware')] }); diff --git a/packages/sandbox-container/src/services/watch-service.ts b/packages/sandbox-container/src/services/watch-service.ts index f2ada8ef8..ceb9c834c 100644 --- a/packages/sandbox-container/src/services/watch-service.ts +++ b/packages/sandbox-container/src/services/watch-service.ts @@ -1,9 +1,10 @@ import type { + CheckChangesRequest, + CheckChangesResult, FileWatchEventType, FileWatchSSEEvent, Logger, - WatchRequest, - WatchState + WatchRequest } from '@repo/shared'; import { ErrorCode } from '@repo/shared/errors'; import type { Subprocess } from 'bun'; @@ -16,11 +17,17 @@ interface Deferred { reject(reason?: unknown): void; } +type LiveWatchEvent = Extract; +type TerminalWatchEvent = Extract< + FileWatchSSEEvent, + { type: 'error' | 'stopped' } +>; + interface WatchSubscriber { id: string; controller: ReadableStreamDefaultController; encoder: TextEncoder; - pendingEvents: Map; + pendingEvents: Map; droppedEvents: number; flushInterval: ReturnType; watchingSent: boolean; @@ -36,10 +43,10 @@ interface ActiveWatch { exclude?: string[]; process: Subprocess; startedAt: Date; - leaseToken: string | null; - resumeToken: string | null; - state: WatchState; - persistent: boolean; + retained: boolean; + cursor: number; + lastEventAt: string | null; + expiresAt: string | null; subscribers: Map; ready: Deferred; readyState: 'pending' | 'resolved' | 'rejected'; @@ -47,16 +54,19 @@ interface ActiveWatch { stopPromise?: Promise; } -type TerminalWatchEvent = Extract< - FileWatchSSEEvent, - { type: 'error' | 'stopped' } ->; - const WATCH_SETUP_TIMEOUT_MS = 10000; const EVENT_COALESCE_WINDOW_MS = 75; const MAX_PENDING_EVENTS = 1000; -const PERSISTENT_WATCH_IDLE_TTL_MS = 10 * 60 * 1000; +const CHANGE_STATE_IDLE_TTL_MS = 10 * 60 * 1000; const STOP_TIMEOUT_MS = 5000; +const DEFAULT_EXCLUDE_PATTERNS = ['.git', 'node_modules', '.DS_Store']; +const DEFAULT_WATCH_EVENTS: FileWatchEventType[] = [ + 'create', + 'modify', + 'delete', + 'move_from', + 'move_to' +]; /** * Service for watching filesystem changes using inotifywait. @@ -78,163 +88,43 @@ export class WatchService { ): Promise>> { const watchResult = this.getOrCreateWatch(path, options); if (!watchResult.success) { - return watchResult; + return serviceError(watchResult.error); } - const stream = this.createSubscriberStream(watchResult.data); - return serviceSuccess(stream); + return serviceSuccess(this.createSubscriberStream(watchResult.data.watch)); } /** - * Ensure a persistent watch exists and wait until it is ready. + * Check whether a path changed since a previously returned version. */ - async ensureWatch( + async checkChanges( path: string, - options: WatchRequest = { path } - ): Promise> { + options: CheckChangesRequest = { path } + ): Promise> { const watchResult = this.getOrCreateWatch(path, options); if (!watchResult.success) { - return watchResult; + return serviceError(watchResult.error); } - const watch = watchResult.data; - const leaseResult = this.claimPersistentWatch(watch, options.resumeToken); - if (!leaseResult.success) { - return serviceError(leaseResult.error); - } - - watch.persistent = true; - this.refreshPersistentWatchLease(watch); + const { watch, created } = watchResult.data; + watch.retained = true; try { await watch.ready.promise; - return serviceSuccess({ - watch: this.snapshotWatchState(watch), - leaseToken: leaseResult.leaseToken - }); + this.refreshRetainedWatchExpiry(watch); + return this.buildCheckChangesResult(watch, options.since, created); } catch (error) { return serviceError({ message: error instanceof Error ? error.message - : 'Failed to establish persistent watch', + : 'Failed to establish retained change state', code: ErrorCode.WATCH_START_ERROR, details: { path } }); } } - /** - * Return the current state for a persistent or active watch. - */ - async getWatchState(watchId: string): Promise> { - const watch = this.activeWatches.get(watchId); - if (!watch) { - return serviceError({ - message: `Watch not found: ${watchId}`, - code: ErrorCode.WATCH_NOT_FOUND, - details: { watchId } - }); - } - - try { - await watch.ready.promise; - this.refreshPersistentWatchLease(watch); - return serviceSuccess(this.snapshotWatchState(watch)); - } catch (error) { - return serviceError({ - message: - error instanceof Error ? error.message : 'Watch failed to establish', - code: ErrorCode.WATCH_START_ERROR, - details: { watchId } - }); - } - } - - /** - * Acknowledge the current watch cursor. - */ - async checkpointWatch( - watchId: string, - cursor: number, - leaseToken: string - ): Promise> { - const watch = this.activeWatches.get(watchId); - if (!watch) { - return serviceError({ - message: `Watch not found: ${watchId}`, - code: ErrorCode.WATCH_NOT_FOUND, - details: { watchId } - }); - } - - try { - await watch.ready.promise; - } catch (error) { - return serviceError({ - message: - error instanceof Error ? error.message : 'Watch failed to establish', - code: ErrorCode.WATCH_START_ERROR, - details: { watchId } - }); - } - - const leaseError = this.verifyPersistentWatchLease( - watch, - leaseToken, - 'checkpoint' - ); - if (leaseError) { - return serviceError(leaseError); - } - - const checkpointed = cursor === watch.state.cursor; - if (checkpointed) { - watch.state.changed = false; - watch.state.overflowed = false; - } - - this.refreshPersistentWatchLease(watch); - - return serviceSuccess({ - checkpointed, - watch: this.snapshotWatchState(watch) - }); - } - - /** - * Stop a specific watch. - */ - async stopWatch( - watchId: string, - leaseToken?: string - ): Promise> { - const watch = this.activeWatches.get(watchId); - if (!watch) { - return serviceError({ - message: `Watch not found: ${watchId}`, - code: ErrorCode.WATCH_NOT_FOUND, - details: { watchId } - }); - } - - const leaseError = this.verifyPersistentWatchLease( - watch, - leaseToken, - 'stop' - ); - if (leaseError) { - return serviceError(leaseError); - } - - await this.stopWatchInternal(watchId, { - type: 'stopped', - reason: 'Watch stopped' - }); - - return serviceSuccess(undefined); - } - /** * Stop all active watches. */ @@ -247,33 +137,32 @@ export class WatchService { /** * Get list of active watches. */ - getActiveWatches(): WatchState[] { - return Array.from(this.activeWatches.values()).map((watch) => - this.snapshotWatchState(watch) - ); + getActiveWatches(): Array<{ id: string; path: string; startedAt: Date }> { + return Array.from(this.activeWatches.values()).map((watch) => ({ + id: watch.id, + path: watch.path, + startedAt: watch.startedAt + })); } private getOrCreateWatch( path: string, options: WatchRequest - ): ServiceResult { - const include = this.normalizePatterns(options.include); - const exclude = include - ? undefined - : this.normalizePatterns(options.exclude); - const events = this.normalizeEvents(options.events); - const key = this.createWatchKey(path, { - recursive: options.recursive !== false, - include, - exclude, - events - }); + ): ServiceResult<{ watch: ActiveWatch; created: boolean }> { + const normalized = this.normalizeWatchOptions(options); + const key = this.createWatchKey(path, normalized); const existingWatchId = this.watchIdsByKey.get(key); + if (existingWatchId) { const existing = this.activeWatches.get(existingWatchId); - if (existing) { - return serviceSuccess(existing); + if ( + existing && + !existing.stopPromise && + existing.readyState !== 'rejected' + ) { + return serviceSuccess({ watch: existing, created: false }); } + this.activeWatches.delete(existingWatchId); this.watchIdsByKey.delete(key); } @@ -301,28 +190,15 @@ export class WatchService { id: watchId, key, path, - recursive: options.recursive !== false, - include, - exclude, + recursive: normalized.recursive, + include: normalized.include, + exclude: normalized.exclude, process: proc, startedAt: new Date(), - leaseToken: null, - resumeToken: null, - state: { - watchId, - path, - recursive: options.recursive !== false, - include, - exclude, - cursor: 0, - changed: false, - overflowed: false, - lastEventAt: null, - expiresAt: null, - subscriberCount: 0, - startedAt: new Date().toISOString() - }, - persistent: false, + retained: false, + cursor: 0, + lastEventAt: null, + expiresAt: null, subscribers: new Map(), ready: createDeferred(), readyState: 'pending', @@ -333,7 +209,7 @@ export class WatchService { this.watchIdsByKey.set(key, watchId); this.runWatchLoop(watch, watchLogger); - return serviceSuccess(watch); + return serviceSuccess({ watch, created: true }); } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); watchLogger.error('Failed to start inotifywait', err); @@ -345,6 +221,25 @@ export class WatchService { } } + private normalizeWatchOptions(options: WatchRequest): { + recursive: boolean; + include?: string[]; + exclude?: string[]; + events: FileWatchEventType[]; + } { + const include = this.normalizePatterns(options.include); + const exclude = include + ? undefined + : (this.normalizePatterns(options.exclude) ?? DEFAULT_EXCLUDE_PATTERNS); + + return { + recursive: options.recursive !== false, + include, + exclude, + events: this.normalizeEvents(options.events) + }; + } + private createWatchKey( path: string, options: { @@ -354,142 +249,121 @@ export class WatchService { events: FileWatchEventType[]; } ): string { - const include = options.include - ? Array.from(new Set(options.include)).sort() - : null; - const exclude = options.exclude - ? Array.from(new Set(options.exclude)).sort() - : null; return JSON.stringify({ path, recursive: options.recursive, - include, - exclude, - events: this.normalizeEvents(options.events) + include: options.include ?? null, + exclude: options.exclude ?? null, + events: options.events }); } - private snapshotWatchState(watch: ActiveWatch): WatchState { - return { - ...watch.state, - include: watch.include, - exclude: watch.exclude, - subscriberCount: watch.subscribers.size, - startedAt: watch.startedAt.toISOString() - }; - } - - private claimPersistentWatch( + private buildCheckChangesResult( watch: ActiveWatch, - resumeToken?: string - ): - | { success: true; leaseToken: string } - | { - success: false; - error: { - message: string; - code: string; - details?: Record; - }; - } { - if (!watch.leaseToken) { - const nextLeaseToken = crypto.randomUUID(); - watch.leaseToken = nextLeaseToken; - watch.resumeToken = resumeToken ?? null; - return { success: true, leaseToken: nextLeaseToken }; + since: string | undefined, + created: boolean + ): ServiceResult { + const version = this.buildVersionToken(watch); + const timestamp = new Date().toISOString(); + + if (since === undefined) { + return serviceSuccess({ + success: true, + status: 'unchanged', + version, + timestamp + }); } - if ( - !watch.resumeToken || - !resumeToken || - watch.resumeToken !== resumeToken - ) { - return { - success: false, - error: { - message: - 'A persistent watch already exists for this path. Reuse it with the same resumeToken or wait for it to expire.', - code: ErrorCode.RESOURCE_BUSY, - details: { - watchId: watch.id - } - } - }; + const parsedVersion = this.parseVersionToken(since); + if (!parsedVersion) { + return serviceError({ + message: 'since must be a version returned by checkChanges()', + code: ErrorCode.VALIDATION_FAILED, + details: { since } + }); } - return { success: true, leaseToken: watch.leaseToken }; - } + if (parsedVersion.watchId !== watch.id) { + return serviceSuccess({ + success: true, + status: 'resync', + reason: created ? 'expired' : 'restarted', + version, + timestamp + }); + } - private verifyPersistentWatchLease( - watch: ActiveWatch, - leaseToken: string | undefined, - action: 'checkpoint' | 'stop' - ): { - message: string; - code: string; - details?: Record; - } | null { - if (!watch.persistent || !watch.leaseToken) { - return { - message: `Only persistent watches can ${action}.`, - code: ErrorCode.RESOURCE_BUSY, + if (parsedVersion.cursor > watch.cursor) { + return serviceError({ + message: 'since refers to a newer version than the current watch state', + code: ErrorCode.VALIDATION_FAILED, details: { - watchId: watch.id, - action + since, + currentVersion: version } - }; + }); } - if (!leaseToken) { - return { - message: `Persistent watch requires a lease token to ${action}.`, - code: ErrorCode.RESOURCE_BUSY, - details: { - watchId: watch.id, - action - } - }; + return serviceSuccess({ + success: true, + status: parsedVersion.cursor === watch.cursor ? 'unchanged' : 'changed', + version, + timestamp + }); + } + + private buildVersionToken(watch: ActiveWatch): string { + return `${watch.id}:${watch.cursor}`; + } + + private parseVersionToken( + version: string + ): { watchId: string; cursor: number } | null { + const separatorIndex = version.lastIndexOf(':'); + if (separatorIndex <= 0 || separatorIndex === version.length - 1) { + return null; } - if (watch.leaseToken !== leaseToken) { - return { - message: `Persistent watch lease token does not allow this ${action}.`, - code: ErrorCode.RESOURCE_BUSY, - details: { - watchId: watch.id, - action - } - }; + const watchId = version.slice(0, separatorIndex); + const cursorText = version.slice(separatorIndex + 1); + if (!/^\d+$/.test(cursorText)) { + return null; } - return null; + const cursor = Number(cursorText); + if (!Number.isSafeInteger(cursor) || cursor < 0) { + return null; + } + + return { watchId, cursor }; } - private refreshPersistentWatchLease(watch: ActiveWatch): void { - if (!watch.persistent) { - watch.state.expiresAt = null; - this.clearPersistentWatchExpiry(watch); + private refreshRetainedWatchExpiry(watch: ActiveWatch): void { + if (!watch.retained) { + watch.expiresAt = null; + this.clearRetainedWatchExpiry(watch); return; } - this.clearPersistentWatchExpiry(watch); + this.clearRetainedWatchExpiry(watch); if (watch.subscribers.size > 0) { - watch.state.expiresAt = null; + watch.expiresAt = null; return; } - const expiresAt = new Date(Date.now() + PERSISTENT_WATCH_IDLE_TTL_MS); - watch.state.expiresAt = expiresAt.toISOString(); + const expiresAt = new Date(Date.now() + CHANGE_STATE_IDLE_TTL_MS); + watch.expiresAt = expiresAt.toISOString(); watch.expiryTimer = setTimeout(() => { void this.stopWatchInternal(watch.id, { type: 'stopped', - reason: 'Persistent watch expired after idle period' + reason: 'Retained change state expired after idle period' }); - }, PERSISTENT_WATCH_IDLE_TTL_MS); + }, CHANGE_STATE_IDLE_TTL_MS); } - private clearPersistentWatchExpiry(watch: ActiveWatch): void { + private clearRetainedWatchExpiry(watch: ActiveWatch): void { if (watch.expiryTimer) { clearTimeout(watch.expiryTimer); watch.expiryTimer = null; @@ -577,8 +451,7 @@ export class WatchService { }; watch.subscribers.set(subscriberId, subscriber); - watch.state.subscriberCount = watch.subscribers.size; - this.refreshPersistentWatchLease(watch); + this.refreshRetainedWatchExpiry(watch); return subscriberId; } @@ -591,7 +464,7 @@ export class WatchService { } private async maybeStopWatchWhenUnused(watch: ActiveWatch): Promise { - if (!watch.persistent && watch.subscribers.size === 0) { + if (!watch.retained && watch.subscribers.size === 0) { await this.stopWatchInternal(watch.id, { type: 'stopped', reason: 'Watch stopped after last subscriber disconnected' @@ -599,7 +472,7 @@ export class WatchService { return; } - this.refreshPersistentWatchLease(watch); + this.refreshRetainedWatchExpiry(watch); } private closeSubscriber( @@ -615,7 +488,6 @@ export class WatchService { subscriber.closed = true; clearInterval(subscriber.flushInterval); watch.subscribers.delete(subscriberId); - watch.state.subscriberCount = watch.subscribers.size; try { const shouldSendTerminalEvent = @@ -642,23 +514,19 @@ export class WatchService { private enqueueSubscriberEvent( watch: ActiveWatch, subscriber: WatchSubscriber, - event: FileWatchSSEEvent + event: LiveWatchEvent ): void { if (subscriber.closed) { return; } - const key = - event.type === 'event' - ? `${event.eventType}|${event.path}|${event.isDirectory}` - : `${event.type}|${Date.now()}`; + const key = `${event.eventType}|${event.path}|${event.isDirectory}`; if ( !subscriber.pendingEvents.has(key) && subscriber.pendingEvents.size >= MAX_PENDING_EVENTS ) { subscriber.droppedEvents++; - watch.state.overflowed = true; if ( subscriber.droppedEvents === 1 || @@ -696,12 +564,11 @@ export class WatchService { subscriber.closed = true; clearInterval(subscriber.flushInterval); watch.subscribers.delete(subscriber.id); - watch.state.subscriberCount = watch.subscribers.size; void this.maybeStopWatchWhenUnused(watch); } } - private broadcastEvent(watch: ActiveWatch, event: FileWatchSSEEvent): void { + private broadcastEvent(watch: ActiveWatch, event: LiveWatchEvent): void { for (const subscriber of watch.subscribers.values()) { this.enqueueSubscriberEvent(watch, subscriber, event); } @@ -735,6 +602,10 @@ export class WatchService { reason: 'Watch process ended' }; + this.activeWatches.delete(watchId); + this.watchIdsByKey.delete(watch.key); + this.clearRetainedWatchExpiry(watch); + if (watch.readyState === 'pending') { const terminalMessage = resolvedTerminalEvent.type === 'error' @@ -770,11 +641,6 @@ export class WatchService { // Process may have already exited. } } - - this.clearPersistentWatchExpiry(watch); - - this.activeWatches.delete(watchId); - this.watchIdsByKey.delete(watch.key); }; watch.stopPromise = cleanup(); @@ -822,52 +688,12 @@ export class WatchService { buffer = lines.pop() || ''; for (const line of lines) { - if (!line.trim()) { - continue; - } - - const parsed = this.parseInotifyEvent(line); - if (!parsed) { - continue; - } - - const timestamp = new Date().toISOString(); - const nextCursor = watch.state.cursor + 1; - const event: Extract = { - type: 'event', - eventId: `${watch.id}:${nextCursor}`, - eventType: parsed.eventType, - path: parsed.path, - isDirectory: parsed.isDirectory, - timestamp - }; - - watch.state.cursor = nextCursor; - watch.state.changed = true; - watch.state.lastEventAt = timestamp; - this.broadcastEvent(watch, event); + this.handleWatchLine(watch, line); } } if (buffer.trim()) { - const parsed = this.parseInotifyEvent(buffer); - if (parsed) { - const timestamp = new Date().toISOString(); - const nextCursor = watch.state.cursor + 1; - const event: Extract = { - type: 'event', - eventId: `${watch.id}:${nextCursor}`, - eventType: parsed.eventType, - path: parsed.path, - isDirectory: parsed.isDirectory, - timestamp - }; - - watch.state.cursor = nextCursor; - watch.state.changed = true; - watch.state.lastEventAt = timestamp; - this.broadcastEvent(watch, event); - } + this.handleWatchLine(watch, buffer); } await this.stopWatchInternal(watch.id, { @@ -883,6 +709,29 @@ export class WatchService { })(); } + private handleWatchLine(watch: ActiveWatch, line: string): void { + if (!line.trim()) { + return; + } + + const parsed = this.parseInotifyEvent(line); + if (!parsed) { + return; + } + + const timestamp = new Date().toISOString(); + watch.cursor += 1; + watch.lastEventAt = timestamp; + + this.broadcastEvent(watch, { + type: 'event', + eventType: parsed.eventType, + path: parsed.path, + isDirectory: parsed.isDirectory, + timestamp + }); + } + private resolveWatchReady(watch: ActiveWatch): void { if (watch.readyState !== 'pending') { return; @@ -910,8 +759,8 @@ export class WatchService { const events = this.normalizeEvents(options.events); const inotifyEvents = events - .map((e) => this.mapEventType(e)) - .filter((e): e is string => e !== undefined); + .map((eventType) => this.mapEventType(eventType)) + .filter((eventType): eventType is string => eventType !== undefined); if (inotifyEvents.length > 0) { args.push('-e', inotifyEvents.join(',')); } @@ -922,11 +771,8 @@ export class WatchService { if (includeRegex) { args.push('--include', includeRegex); } else { - const excludes = this.normalizePatterns(options.exclude) ?? [ - '.git', - 'node_modules', - '.DS_Store' - ]; + const excludes = + this.normalizePatterns(options.exclude) ?? DEFAULT_EXCLUDE_PATTERNS; const excludeRegex = this.buildCombinedPathRegex(excludes); if (excludeRegex) { args.push('--exclude', excludeRegex); @@ -959,19 +805,11 @@ export class WatchService { } private normalizeEvents(events?: FileWatchEventType[]): FileWatchEventType[] { - const defaultEvents: FileWatchEventType[] = [ - 'create', - 'modify', - 'delete', - 'move_from', - 'move_to' - ]; - if (!events || events.length === 0) { - return defaultEvents; + return DEFAULT_WATCH_EVENTS; } - const orderedEvents = defaultEvents.filter((eventType) => + const orderedEvents = DEFAULT_WATCH_EVENTS.filter((eventType) => events.includes(eventType) ); const additionalEvents = events.filter( @@ -1153,7 +991,8 @@ export class WatchService { } logger.warn('inotifywait stderr', { message: trimmed }); - this.broadcastEvent(watch, errorEvent(trimmed)); + await this.stopWatchInternal(watch.id, errorEvent(trimmed)); + return; } } } catch (error) { diff --git a/packages/sandbox-container/tests/handlers/watch-handler.test.ts b/packages/sandbox-container/tests/handlers/watch-handler.test.ts index e58378cc9..9f0890312 100644 --- a/packages/sandbox-container/tests/handlers/watch-handler.test.ts +++ b/packages/sandbox-container/tests/handlers/watch-handler.test.ts @@ -1,46 +1,20 @@ -import type { WatchState } from '@repo/shared'; import { createNoOpLogger } from '@repo/shared'; import { describe, expect, it, vi } from 'vitest'; import { WatchHandler } from '../../src/handlers/watch-handler'; import type { WatchService } from '../../src/services/watch-service'; -function sampleWatchState(overrides: Partial = {}): WatchState { - return { - watchId: 'watch-1', - path: '/workspace/test', - recursive: true, - include: undefined, - exclude: ['.git'], - cursor: 0, - changed: false, - overflowed: false, - lastEventAt: null, - expiresAt: null, - subscriberCount: 0, - startedAt: new Date().toISOString(), - ...overrides - }; -} - function createMockWatchService(): WatchService { return { watchDirectory: vi.fn(), - ensureWatch: vi.fn(), - getWatchState: vi.fn(), - checkpointWatch: vi.fn(), - stopWatch: vi.fn() + checkChanges: vi.fn() } as unknown as WatchService; } -function makeRequest( - path: string, - method: string, - body?: Record -): Request { - return new Request(`http://localhost:3000${path}`, { - method, +function makeRequest(body: Record): Request { + return new Request('http://localhost:3000/api/watch', { + method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: body ? JSON.stringify(body) : undefined + body: JSON.stringify(body) }); } @@ -52,7 +26,7 @@ const defaultContext = { }; describe('WatchHandler', () => { - describe('stream request validation', () => { + describe('include/exclude validation', () => { it('should reject requests with both include and exclude', async () => { const handler = new WatchHandler( createMockWatchService(), @@ -60,7 +34,7 @@ describe('WatchHandler', () => { ); const response = await handler.handle( - makeRequest('/api/watch', 'POST', { + makeRequest({ path: '/workspace/test', include: ['*.ts'], exclude: ['node_modules'] @@ -88,168 +62,120 @@ describe('WatchHandler', () => { const handler = new WatchHandler(watchService, createNoOpLogger()); const response = await handler.handle( - makeRequest('/api/watch', 'POST', { - path: '/workspace/test', - include: ['*.ts'] - }), + makeRequest({ path: '/workspace/test', include: ['*.ts'] }), defaultContext ); expect(response.status).toBe(200); }); - }); - describe('persistent watch routes', () => { - it('should ensure a persistent watch', async () => { + it('should allow exclude without include', async () => { const watchService = createMockWatchService(); - const watch = sampleWatchState(); - (watchService.ensureWatch as ReturnType).mockResolvedValue({ + const mockStream = new ReadableStream(); + ( + watchService.watchDirectory as ReturnType + ).mockResolvedValue({ success: true, - data: { - watch, - leaseToken: 'lease-1' - } + data: mockStream }); const handler = new WatchHandler(watchService, createNoOpLogger()); - const response = await handler.handle( - makeRequest('/api/watch/ensure', 'POST', { - path: '/workspace/test', - resumeToken: 'resume-1' - }), - defaultContext - ); - - expect(response.status).toBe(200); - const body = (await response.json()) as { - watch: WatchState; - leaseToken: string; - }; - expect(body.watch.watchId).toBe(watch.watchId); - expect(body.leaseToken).toBe('lease-1'); - }); - - it('should reject invalid resumeToken on ensure', async () => { - const handler = new WatchHandler( - createMockWatchService(), - createNoOpLogger() - ); const response = await handler.handle( - makeRequest('/api/watch/ensure', 'POST', { + makeRequest({ path: '/workspace/test', - resumeToken: '' - }), - defaultContext - ); - - expect(response.status).toBe(400); - }); - - it('should reject invalid checkpoint cursor', async () => { - const handler = new WatchHandler( - createMockWatchService(), - createNoOpLogger() - ); - - const response = await handler.handle( - makeRequest('/api/watch/watch-1/checkpoint', 'POST', { - cursor: -1, - leaseToken: 'lease-1' + exclude: ['node_modules'] }), defaultContext ); - expect(response.status).toBe(400); - const body = (await response.json()) as { message: string }; - expect(body.message).toContain('cursor must be a non-negative integer'); - }); - - it('should require leaseToken for checkpoint requests', async () => { - const handler = new WatchHandler( - createMockWatchService(), - createNoOpLogger() - ); - - const response = await handler.handle( - makeRequest('/api/watch/watch-1/checkpoint', 'POST', { cursor: 3 }), - defaultContext - ); - - expect(response.status).toBe(400); + expect(response.status).toBe(200); }); - it('should checkpoint a watch cursor', async () => { + it('should allow empty include with non-empty exclude', async () => { const watchService = createMockWatchService(); - const watch = sampleWatchState({ - cursor: 3, - changed: false - }); + const mockStream = new ReadableStream(); ( - watchService.checkpointWatch as ReturnType + watchService.watchDirectory as ReturnType ).mockResolvedValue({ success: true, - data: { - checkpointed: true, - watch - } + data: mockStream }); const handler = new WatchHandler(watchService, createNoOpLogger()); + const response = await handler.handle( - makeRequest('/api/watch/watch-1/checkpoint', 'POST', { - cursor: 3, - leaseToken: 'lease-1' + makeRequest({ + path: '/workspace/test', + include: [], + exclude: ['node_modules'] }), defaultContext ); expect(response.status).toBe(200); - const body = (await response.json()) as { - checkpointed: boolean; - watch: WatchState; - }; - expect(body.checkpointed).toBe(true); - expect(body.watch.cursor).toBe(3); }); + }); - it('should fetch watch state', async () => { + describe('checkChanges', () => { + it('should call watchService.checkChanges for valid requests', async () => { const watchService = createMockWatchService(); - const watch = sampleWatchState({ cursor: 5, changed: true }); - ( - watchService.getWatchState as ReturnType - ).mockResolvedValue({ - success: true, - data: watch - }); + (watchService.checkChanges as ReturnType).mockResolvedValue( + { + success: true, + data: { + success: true, + status: 'changed', + version: 'watch-1:1', + timestamp: '2026-03-17T00:00:00.000Z' + } + } + ); const handler = new WatchHandler(watchService, createNoOpLogger()); const response = await handler.handle( - makeRequest('/api/watch/watch-1', 'GET'), + new Request('http://localhost:3000/api/watch/check', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + path: 'relative/path', + since: 'watch-1:0' + }) + }), defaultContext ); expect(response.status).toBe(200); - const body = (await response.json()) as { watch: WatchState }; - expect(body.watch.cursor).toBe(5); - expect(body.watch.changed).toBe(true); + expect(watchService.checkChanges).toHaveBeenCalledWith( + '/workspace/relative/path', + { + path: '/workspace/relative/path', + since: 'watch-1:0' + } + ); }); - it('should stop a watch', async () => { - const watchService = createMockWatchService(); - (watchService.stopWatch as ReturnType).mockResolvedValue({ - success: true - }); + it('should reject empty since values', async () => { + const handler = new WatchHandler( + createMockWatchService(), + createNoOpLogger() + ); - const handler = new WatchHandler(watchService, createNoOpLogger()); const response = await handler.handle( - makeRequest('/api/watch/watch-1?leaseToken=lease-1', 'DELETE'), + new Request('http://localhost:3000/api/watch/check', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + path: '/workspace/test', + since: '' + }) + }), defaultContext ); - expect(response.status).toBe(200); - const body = (await response.json()) as { watchId: string }; - expect(body.watchId).toBe('watch-1'); + expect(response.status).toBe(400); + const body = (await response.json()) as { message: string }; + expect(body.message).toContain('since must be a non-empty string'); }); }); }); diff --git a/packages/sandbox-container/tests/services/watch-service.test.ts b/packages/sandbox-container/tests/services/watch-service.test.ts index d134793bd..c4fe84ced 100644 --- a/packages/sandbox-container/tests/services/watch-service.test.ts +++ b/packages/sandbox-container/tests/services/watch-service.test.ts @@ -1,28 +1,16 @@ import { beforeEach, describe, expect, it, vi } from 'bun:test'; -import type { WatchRequest, WatchState } from '@repo/shared'; +import type { WatchRequest } from '@repo/shared'; import { createNoOpLogger } from '@repo/shared'; import { ErrorCode } from '@repo/shared/errors'; import { WatchService } from '@sandbox-container/services/watch-service'; const mockLogger = createNoOpLogger(); -interface FakeWatchProcess { - exited: Promise; - kill: ReturnType; -} - +/** + * Type-safe accessor for testing private WatchService methods. + * Uses module augmentation to expose private methods for testing only. + */ interface WatchServiceTestAccessor { - activeWatches: Map; - watchIdsByKey: Map; - createWatchKey( - path: string, - options: { - recursive: boolean; - include?: string[]; - exclude?: string[]; - events: string[]; - } - ): string; parseInotifyEvent(line: string): { eventType: string; path: string; @@ -31,71 +19,12 @@ interface WatchServiceTestAccessor { buildInotifyArgs(path: string, options: WatchRequest): string[]; } -function makeWatchState(overrides: Partial = {}): WatchState { - return { - watchId: 'watch-1', - path: '/workspace/test', - recursive: true, - include: undefined, - exclude: ['.git'], - cursor: 0, - changed: false, - overflowed: false, - lastEventAt: null, - expiresAt: null, - subscriberCount: 0, - startedAt: new Date().toISOString(), - ...overrides - }; -} - -function makeFakeProcess(): FakeWatchProcess { - return { - exited: Promise.resolve(0), - kill: vi.fn() - }; -} - -function makeActiveWatch(overrides: Partial> = {}) { - const state = makeWatchState( - (overrides.state as Partial | undefined) ?? {} - ); - const process = - (overrides.process as FakeWatchProcess | undefined) ?? makeFakeProcess(); - - return { - id: state.watchId, - key: 'watch-key', - path: state.path, - recursive: state.recursive, - include: state.include, - exclude: state.exclude, - process, - startedAt: new Date(state.startedAt), - leaseToken: - (overrides.leaseToken as string | null | undefined) ?? 'lease-1', - state, - persistent: true, - subscribers: new Map(), - ready: { - promise: Promise.resolve(), - resolve: () => {}, - reject: () => {} - }, - readyState: 'resolved', - expiryTimer: null, - ...overrides - }; -} - describe('WatchService', () => { let watchService: WatchService; - let accessor: WatchServiceTestAccessor; beforeEach(() => { vi.clearAllMocks(); watchService = new WatchService(mockLogger); - accessor = watchService as unknown as WatchServiceTestAccessor; }); describe('getActiveWatches', () => { @@ -112,199 +41,22 @@ describe('WatchService', () => { }); }); - describe('watch state management', () => { - it('should return not found for unknown watch state', async () => { - const result = await watchService.getWatchState('missing-watch'); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error.code).toBe(ErrorCode.WATCH_NOT_FOUND); - } - }); - - it('should clear changed state only when checkpoint cursor matches', async () => { - const watch = makeActiveWatch({ - state: { cursor: 7, changed: true, overflowed: true } - }); - accessor.activeWatches.set('watch-1', watch); - accessor.watchIdsByKey.set('watch-key', 'watch-1'); - - const result = await watchService.checkpointWatch( - 'watch-1', - 7, - 'lease-1' - ); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.data.checkpointed).toBe(true); - expect(result.data.watch.changed).toBe(false); - expect(result.data.watch.overflowed).toBe(false); - } - }); - - it('should keep changed state when checkpoint cursor is stale', async () => { - const watch = makeActiveWatch({ - state: { cursor: 8, changed: true, overflowed: true } - }); - accessor.activeWatches.set('watch-1', watch); - accessor.watchIdsByKey.set('watch-key', 'watch-1'); - - const result = await watchService.checkpointWatch( - 'watch-1', - 7, - 'lease-1' - ); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.data.checkpointed).toBe(false); - expect(result.data.watch.changed).toBe(true); - expect(result.data.watch.overflowed).toBe(true); - } - }); - - it('should reject checkpoint from a different lease token', async () => { - const watch = makeActiveWatch({ - state: { cursor: 8, changed: true }, - leaseToken: 'lease-1' - }); - accessor.activeWatches.set('watch-1', watch); - accessor.watchIdsByKey.set('watch-key', 'watch-1'); - - const result = await watchService.checkpointWatch( - 'watch-1', - 8, - 'lease-2' - ); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error.code).toBe(ErrorCode.RESOURCE_BUSY); - } - }); - - it('should reject checkpoint for non-persistent watches', async () => { - const watch = makeActiveWatch({ - persistent: false, - leaseToken: null - }); - accessor.activeWatches.set('watch-1', watch); - accessor.watchIdsByKey.set('watch-key', 'watch-1'); - - const result = await watchService.checkpointWatch( - 'watch-1', - 8, - 'lease-1' - ); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error.code).toBe(ErrorCode.RESOURCE_BUSY); - } - }); - - it('should require leaseToken when stopping a leased watch', async () => { - const process = makeFakeProcess(); - const watch = makeActiveWatch({ - process - }); - accessor.activeWatches.set('watch-1', watch); - accessor.watchIdsByKey.set('watch-key', 'watch-1'); - - const result = await watchService.stopWatch('watch-1'); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error.code).toBe(ErrorCode.RESOURCE_BUSY); - } - expect(process.kill).not.toHaveBeenCalled(); - }); - - it('should refresh persistent watch expiry on read', async () => { - const watch = makeActiveWatch({ state: { expiresAt: null } }); - accessor.activeWatches.set('watch-1', watch); - accessor.watchIdsByKey.set('watch-key', 'watch-1'); - - const result = await watchService.getWatchState('watch-1'); - - expect(result.success).toBe(true); - if (result.success) { - expect(typeof result.data.expiresAt).toBe('string'); - } - }); - - it('should return the same lease token when reusing a persistent watch', async () => { - const watch = makeActiveWatch({ - leaseToken: 'lease-1', - resumeToken: 'resume-1' - }); - accessor.activeWatches.set('watch-1', watch); - accessor.watchIdsByKey.set( - accessor.createWatchKey('/workspace/test', { - recursive: true, - include: undefined, - exclude: undefined, - events: ['create', 'modify', 'delete', 'move_from', 'move_to'] - }), - 'watch-1' - ); - - const result = await watchService.ensureWatch('/workspace/test', { - path: '/workspace/test', - resumeToken: 'resume-1' - }); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.data.leaseToken).toBe('lease-1'); - } - }); - - it('should reject reusing a persistent watch without the same resume token', async () => { - const watch = makeActiveWatch({ - leaseToken: 'lease-1', - resumeToken: 'resume-1' - }); - accessor.activeWatches.set('watch-1', watch); - accessor.watchIdsByKey.set( - accessor.createWatchKey('/workspace/test', { - recursive: true, - include: undefined, - exclude: undefined, - events: ['create', 'modify', 'delete', 'move_from', 'move_to'] - }), - 'watch-1' + describe('watchDirectory', () => { + it('should return error for non-existent path', async () => { + const result = await watchService.watchDirectory( + '/non/existent/path/12345' ); - const result = await watchService.ensureWatch('/workspace/test', { - path: '/workspace/test' - }); - expect(result.success).toBe(false); if (!result.success) { - expect(result.error.code).toBe(ErrorCode.RESOURCE_BUSY); + expect(result.error.code).toBe(ErrorCode.FILE_NOT_FOUND); } }); - - it('should stop an active watch and clean up indexes', async () => { - const process = makeFakeProcess(); - const watch = makeActiveWatch({ process }); - accessor.activeWatches.set('watch-1', watch); - accessor.watchIdsByKey.set('watch-key', 'watch-1'); - - const result = await watchService.stopWatch('watch-1', 'lease-1'); - - expect(result.success).toBe(true); - expect(process.kill).toHaveBeenCalled(); - expect(accessor.activeWatches.has('watch-1')).toBe(false); - expect(accessor.watchIdsByKey.has('watch-key')).toBe(false); - }); }); - describe('watchDirectory', () => { + describe('checkChanges', () => { it('should return error for non-existent path', async () => { - const result = await watchService.watchDirectory( + const result = await watchService.checkChanges( '/non/existent/path/12345' ); @@ -316,6 +68,7 @@ describe('WatchService', () => { }); describe('parseInotifyEvent', () => { + // Access private method for testing via type assertion const testParseEvent = (service: WatchService, line: string) => { return (service as unknown as WatchServiceTestAccessor).parseInotifyEvent( line @@ -344,6 +97,7 @@ describe('WatchService', () => { }); it('should parse CREATE,ISDIR with colon-separated flags from %:e format', () => { + // This is the actual output format from inotifywait with --format '%e|%w%f|%:e' const result = testParseEvent( watchService, 'CREATE,ISDIR|/app/newdir|CREATE:ISDIR' @@ -416,28 +170,7 @@ describe('WatchService', () => { }); describe('buildInotifyArgs', () => { - const testCreateWatchKey = ( - service: WatchService, - path: string, - options: WatchRequest - ) => { - return (service as unknown as WatchServiceTestAccessor).createWatchKey( - path, - { - recursive: options.recursive !== false, - include: options.include, - exclude: options.exclude, - events: options.events ?? [ - 'create', - 'modify', - 'delete', - 'move_from', - 'move_to' - ] - } - ); - }; - + // Access private method for testing via type assertion const testBuildArgs = ( service: WatchService, path: string, @@ -472,12 +205,15 @@ describe('WatchService', () => { it('should include default excludes as combined regex pattern', () => { const args = testBuildArgs(watchService, '/app', { path: '/app' }); expect(args).toContain('--exclude'); + // inotifywait only supports a single --exclude, so patterns are combined with OR const excludeIndex = args.indexOf('--exclude'); expect(excludeIndex).toBeGreaterThan(-1); const excludePattern = args[excludeIndex + 1]; + // Verify combined regex format: (^|/)pattern(/|$)|(^|/)pattern(/|$)|... expect(excludePattern).toContain('(^|/)\\.git(/|$)'); expect(excludePattern).toContain('(^|/)node_modules(/|$)'); expect(excludePattern).toContain('(^|/)\\.DS_Store(/|$)'); + // Patterns are joined with | expect(excludePattern.split('|').length).toBeGreaterThanOrEqual(3); }); @@ -506,57 +242,6 @@ describe('WatchService', () => { expect(includePattern).toContain('(^|/)src/.*(/|$)'); }); - it('should treat empty include arrays like no include filter', () => { - const args = testBuildArgs(watchService, '/app', { - path: '/app', - include: [], - exclude: ['tmp'] - }); - expect(args).not.toContain('--include'); - expect(args).toContain('--exclude'); - const excludeIndex = args.indexOf('--exclude'); - expect(args[excludeIndex + 1]).toContain('(^|/)tmp(/|$)'); - }); - - it('should include event filters in the watch key', () => { - const createOnlyKey = testCreateWatchKey(watchService, '/app', { - path: '/app', - events: ['create'] - }); - const modifyOnlyKey = testCreateWatchKey(watchService, '/app', { - path: '/app', - events: ['modify'] - }); - - expect(createOnlyKey).not.toBe(modifyOnlyKey); - }); - - it('should normalize event order in the watch key', () => { - const firstKey = testCreateWatchKey(watchService, '/app', { - path: '/app', - events: ['create', 'modify'] - }); - const secondKey = testCreateWatchKey(watchService, '/app', { - path: '/app', - events: ['modify', 'create'] - }); - - expect(firstKey).toBe(secondKey); - }); - - it('should normalize include pattern order in the watch key', () => { - const firstKey = testCreateWatchKey(watchService, '/app', { - path: '/app', - include: ['*.ts', 'src/**'] - }); - const secondKey = testCreateWatchKey(watchService, '/app', { - path: '/app', - include: ['src/**', '*.ts'] - }); - - expect(firstKey).toBe(secondKey); - }); - it('should add path as last argument', () => { const args = testBuildArgs(watchService, '/app', { path: '/app' }); expect(args[args.length - 1]).toBe('/app'); diff --git a/packages/sandbox/src/clients/index.ts b/packages/sandbox/src/clients/index.ts index 94d2ce8c7..68766537f 100644 --- a/packages/sandbox/src/clients/index.ts +++ b/packages/sandbox/src/clients/index.ts @@ -42,15 +42,10 @@ export { // ============================================================================= export type { - PersistentWatchOptions, - WatchCheckpointRequest, - WatchCheckpointResult, - WatchEnsureResult, - WatchRequest, - WatchState, - WatchStateResult, - WatchStopOptions, - WatchStopResult + CheckChangesOptions, + CheckChangesRequest, + CheckChangesResult, + WatchRequest } from '@repo/shared'; // Command client types export type { ExecuteRequest, ExecuteResponse } from './command-client'; diff --git a/packages/sandbox/src/clients/watch-client.ts b/packages/sandbox/src/clients/watch-client.ts index 37e990572..3f3e1b37b 100644 --- a/packages/sandbox/src/clients/watch-client.ts +++ b/packages/sandbox/src/clients/watch-client.ts @@ -1,15 +1,10 @@ import { + type CheckChangesRequest, + type CheckChangesResult, type FileWatchSSEEvent, parseSSEFrames, type SSEPartialEvent, - type WatchCheckpointRequest, - type WatchCheckpointResult, - type WatchEnsureResult, - type WatchRequest, - type WatchState, - type WatchStateResult, - type WatchStopOptions, - type WatchStopResult + type WatchRequest } from '@repo/shared'; import { BaseHttpClient } from './base-client'; @@ -18,83 +13,25 @@ import { BaseHttpClient } from './base-client'; * Uses inotify under the hood for native filesystem event notifications * * @internal This client is used internally by the SDK. - * Users should use `sandbox.watch()` instead. + * Users should use `sandbox.watch()` or `sandbox.checkChanges()` instead. */ export class WatchClient extends BaseHttpClient { - async ensureWatch(request: WatchRequest): Promise { - try { - const response = await this.post( - '/api/watch/ensure', - request - ); - - this.logSuccess('Persistent watch ensured', request.path); - return { - ...response, - watch: normalizeWatchState(response.watch) - }; - } catch (error) { - this.logError('ensureWatch', error); - throw error; - } - } - - async getWatchState(watchId: string): Promise { - try { - const response = await this.get( - `/api/watch/${watchId}` - ); - - this.logSuccess('Watch state retrieved', watchId); - return { - ...response, - watch: normalizeWatchState(response.watch) - }; - } catch (error) { - this.logError('getWatchState', error); - throw error; - } - } - - async checkpointWatch( - watchId: string, - request: WatchCheckpointRequest - ): Promise { + /** + * Check whether a path changed since a previously returned version. + */ + async checkChanges( + request: CheckChangesRequest + ): Promise { try { - const response = await this.post( - `/api/watch/${watchId}/checkpoint`, + const response = await this.post( + '/api/watch/check', request ); - this.logSuccess('Watch checkpoint recorded', watchId); - return { - ...response, - watch: normalizeWatchState(response.watch) - }; - } catch (error) { - this.logError('checkpointWatch', error); - throw error; - } - } - - async stopWatch( - watchId: string, - options: WatchStopOptions = {} - ): Promise { - try { - const searchParams = new URLSearchParams(); - if (options.leaseToken) { - searchParams.set('leaseToken', options.leaseToken); - } - const suffix = searchParams.size > 0 ? `?${searchParams.toString()}` : ''; - const response = await this.delete( - `/api/watch/${watchId}${suffix}` - ); - - this.logSuccess('Watch stopped', watchId); + this.logSuccess('Checked retained file changes', request.path); return response; } catch (error) { - this.logError('stopWatch', error); + this.logError('checkChanges', error); throw error; } } @@ -212,16 +149,3 @@ export class WatchClient extends BaseHttpClient { }); } } - -function normalizeWatchState(watch: WatchState): WatchState { - const legacyWatch = watch as WatchState & { dirty?: boolean }; - - if (legacyWatch.changed !== undefined) { - return watch; - } - - return { - ...watch, - changed: legacyWatch.dirty ?? false - }; -} diff --git a/packages/sandbox/src/index.ts b/packages/sandbox/src/index.ts index 8920b5c24..3f4bb8903 100644 --- a/packages/sandbox/src/index.ts +++ b/packages/sandbox/src/index.ts @@ -10,8 +10,7 @@ export { PortClient, ProcessClient, SandboxClient, - UtilityClient, - WatchClient + UtilityClient } from './clients'; export { getSandbox, Sandbox } from './sandbox'; @@ -23,6 +22,8 @@ export type { BaseExecOptions, BucketCredentials, BucketProvider, + CheckChangesOptions, + CheckChangesResult, CodeContext, CreateContextOptions, DirectoryBackup, @@ -34,6 +35,7 @@ export type { FileChunk, FileMetadata, FileStreamEvent, + // File watch types FileWatchSSEEvent, GitCheckoutResult, ISandbox, @@ -41,8 +43,6 @@ export type { LocalMountBucketOptions, LogEvent, MountBucketOptions, - // File watch types - PersistentWatchOptions, Process, ProcessOptions, ProcessStatus, @@ -55,14 +55,7 @@ export type { StreamOptions, WaitForLogResult, WaitForPortOptions, - WatchCheckpointRequest, - WatchCheckpointResult, - WatchEnsureResult, - WatchOptions, - WatchState, - WatchStateResult, - WatchStopOptions, - WatchStopResult + WatchOptions } from '@repo/shared'; // Export type guards for runtime validation export { isExecResult, isProcess, isProcessStatus } from '@repo/shared'; diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index be29d4a22..0c8da431f 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -3,6 +3,8 @@ import type { BackupOptions, BucketCredentials, BucketProvider, + CheckChangesOptions, + CheckChangesResult, CodeContext, CreateContextOptions, DirectoryBackup, @@ -15,7 +17,6 @@ import type { LocalMountBucketOptions, LogEvent, MountBucketOptions, - PersistentWatchOptions, PortWatchEvent, Process, ProcessOptions, @@ -30,11 +31,7 @@ import type { WaitForExitResult, WaitForLogResult, WaitForPortOptions, - WatchCheckpointRequest, - WatchCheckpointResult, - WatchEnsureResult, - WatchOptions, - WatchStopOptions + WatchOptions } from '@repo/shared'; import { createLogger, @@ -44,9 +41,7 @@ import { partitionEnvVars, type SessionDeleteResult, shellEscape, - TraceContext, - type WatchStateResult, - type WatchStopResult + TraceContext } from '@repo/shared'; import { AwsClient } from 'aws4fetch'; import { type Desktop, type ExecuteResponse, SandboxClient } from './clients'; @@ -88,6 +83,193 @@ import type { } from './storage-mount/types'; import { SDK_VERSION } from './version'; +type SandboxConfiguration = { + sandboxName?: { + name: string; + normalizeId?: boolean; + }; + baseUrl?: string; + sleepAfter?: string | number; + keepAlive?: boolean; + containerTimeouts?: NonNullable; +}; + +type CachedSandboxConfiguration = { + sandboxName?: string; + normalizeId?: boolean; + baseUrl?: string; + sleepAfter?: string | number; + keepAlive?: boolean; + containerTimeouts?: NonNullable; +}; + +type ConfigurableSandboxStub = { + configure?: (configuration: SandboxConfiguration) => Promise; + setSandboxName?: (name: string, normalizeId?: boolean) => Promise; + setBaseUrl?: (baseUrl: string) => Promise; + setSleepAfter?: (sleepAfter: string | number) => Promise; + setKeepAlive?: (keepAlive: boolean) => Promise; + setContainerTimeouts?: ( + timeouts: NonNullable + ) => Promise; +}; + +const sandboxConfigurationCache = new WeakMap< + object, + Map +>(); + +function getNamespaceConfigurationCache( + namespace: object +): Map { + const existing = sandboxConfigurationCache.get(namespace); + if (existing) { + return existing; + } + + const created = new Map(); + sandboxConfigurationCache.set(namespace, created); + return created; +} + +function sameContainerTimeouts( + left?: NonNullable, + right?: NonNullable +): boolean { + return ( + left?.instanceGetTimeoutMS === right?.instanceGetTimeoutMS && + left?.portReadyTimeoutMS === right?.portReadyTimeoutMS && + left?.waitIntervalMS === right?.waitIntervalMS + ); +} + +function buildSandboxConfiguration( + effectiveId: string, + options: SandboxOptions | undefined, + cached: CachedSandboxConfiguration | undefined +): SandboxConfiguration { + const configuration: SandboxConfiguration = {}; + + if ( + cached?.sandboxName !== effectiveId || + cached.normalizeId !== options?.normalizeId + ) { + configuration.sandboxName = { + name: effectiveId, + normalizeId: options?.normalizeId + }; + } + + if (options?.baseUrl !== undefined && cached?.baseUrl !== options.baseUrl) { + configuration.baseUrl = options.baseUrl; + } + + if ( + options?.sleepAfter !== undefined && + cached?.sleepAfter !== options.sleepAfter + ) { + configuration.sleepAfter = options.sleepAfter; + } + + if ( + options?.keepAlive !== undefined && + cached?.keepAlive !== options.keepAlive + ) { + configuration.keepAlive = options.keepAlive; + } + + if ( + options?.containerTimeouts && + !sameContainerTimeouts(cached?.containerTimeouts, options.containerTimeouts) + ) { + configuration.containerTimeouts = options.containerTimeouts; + } + + return configuration; +} + +function hasSandboxConfiguration(configuration: SandboxConfiguration): boolean { + return ( + configuration.sandboxName !== undefined || + configuration.baseUrl !== undefined || + configuration.sleepAfter !== undefined || + configuration.keepAlive !== undefined || + configuration.containerTimeouts !== undefined + ); +} + +function mergeSandboxConfiguration( + cached: CachedSandboxConfiguration | undefined, + configuration: SandboxConfiguration +): CachedSandboxConfiguration { + return { + ...cached, + ...(configuration.sandboxName && { + sandboxName: configuration.sandboxName.name, + normalizeId: configuration.sandboxName.normalizeId + }), + ...(configuration.baseUrl !== undefined && { + baseUrl: configuration.baseUrl + }), + ...(configuration.sleepAfter !== undefined && { + sleepAfter: configuration.sleepAfter + }), + ...(configuration.keepAlive !== undefined && { + keepAlive: configuration.keepAlive + }), + ...(configuration.containerTimeouts !== undefined && { + containerTimeouts: configuration.containerTimeouts + }) + }; +} + +function applySandboxConfiguration( + stub: ConfigurableSandboxStub, + configuration: SandboxConfiguration +): Promise { + if (stub.configure) { + return stub.configure(configuration); + } + + const operations: Promise[] = []; + + if (configuration.sandboxName) { + operations.push( + stub.setSandboxName?.( + configuration.sandboxName.name, + configuration.sandboxName.normalizeId + ) ?? Promise.resolve() + ); + } + + if (configuration.baseUrl !== undefined) { + operations.push( + stub.setBaseUrl?.(configuration.baseUrl) ?? Promise.resolve() + ); + } + + if (configuration.sleepAfter !== undefined) { + operations.push( + stub.setSleepAfter?.(configuration.sleepAfter) ?? Promise.resolve() + ); + } + + if (configuration.keepAlive !== undefined) { + operations.push( + stub.setKeepAlive?.(configuration.keepAlive) ?? Promise.resolve() + ); + } + + if (configuration.containerTimeouts !== undefined) { + operations.push( + stub.setContainerTimeouts?.(configuration.containerTimeouts) ?? + Promise.resolve() + ); + } + + return Promise.all(operations).then(() => undefined); +} + export function getSandbox>( ns: DurableObjectNamespace, id: string, @@ -108,24 +290,34 @@ export function getSandbox>( ); } - const stub = getContainer(ns, effectiveId); - - stub.setSandboxName?.(effectiveId, options?.normalizeId); - - if (options?.baseUrl) { - stub.setBaseUrl(options.baseUrl); - } + const stub = getContainer( + ns as unknown as DurableObjectNamespace>, + effectiveId + ) as unknown as T & ConfigurableSandboxStub; + + const namespaceCache = getNamespaceConfigurationCache(ns); + const cachedConfiguration = namespaceCache.get(effectiveId); + const configuration = buildSandboxConfiguration( + effectiveId, + options, + cachedConfiguration + ); - if (options?.sleepAfter !== undefined) { - stub.setSleepAfter(options.sleepAfter); - } + if (hasSandboxConfiguration(configuration)) { + const nextConfiguration = mergeSandboxConfiguration( + cachedConfiguration, + configuration + ); + namespaceCache.set(effectiveId, nextConfiguration); - if (options?.keepAlive !== undefined) { - stub.setKeepAlive(options.keepAlive); - } + void applySandboxConfiguration(stub, configuration).catch(() => { + if (cachedConfiguration) { + namespaceCache.set(effectiveId, cachedConfiguration); + return; + } - if (options?.containerTimeouts) { - stub.setContainerTimeouts(options.containerTimeouts); + namespaceCache.delete(effectiveId); + }); } const defaultSessionId = `sandbox-${effectiveId}`; @@ -454,6 +646,31 @@ export class Sandbox extends Container implements ISandbox { } } + async configure(configuration: SandboxConfiguration): Promise { + if (configuration.sandboxName) { + await this.setSandboxName( + configuration.sandboxName.name, + configuration.sandboxName.normalizeId + ); + } + + if (configuration.baseUrl !== undefined) { + await this.setBaseUrl(configuration.baseUrl); + } + + if (configuration.sleepAfter !== undefined) { + await this.setSleepAfter(configuration.sleepAfter); + } + + if (configuration.keepAlive !== undefined) { + await this.setKeepAlive(configuration.keepAlive); + } + + if (configuration.containerTimeouts !== undefined) { + await this.setContainerTimeouts(configuration.containerTimeouts); + } + } + // RPC method to set the base URL async setBaseUrl(baseUrl: string): Promise { if (!this.baseUrl) { @@ -2637,39 +2854,31 @@ export class Sandbox extends Container implements ISandbox { }); } - async ensureWatch( + /** + * Check whether a path changed while this caller was disconnected. + * + * Pass the `version` returned from a prior call in `options.since` to learn + * whether the path is unchanged, changed, or needs a full resync because the + * retained change state was reset. + * + * @param path - Path to check (absolute or relative to /workspace) + * @param options - Change-check options + */ + async checkChanges( path: string, - options: PersistentWatchOptions = {} - ): Promise { + options: CheckChangesOptions = {} + ): Promise { const sessionId = options.sessionId ?? (await this.ensureDefaultSession()); - return this.client.watch.ensureWatch({ + return this.client.watch.checkChanges({ path, recursive: options.recursive, include: options.include, exclude: options.exclude, - resumeToken: options.resumeToken, + since: options.since, sessionId }); } - async getWatchState(watchId: string): Promise { - return this.client.watch.getWatchState(watchId); - } - - async checkpointWatch( - watchId: string, - request: WatchCheckpointRequest - ): Promise { - return this.client.watch.checkpointWatch(watchId, request); - } - - async stopWatch( - watchId: string, - options: WatchStopOptions = {} - ): Promise { - return this.client.watch.stopWatch(watchId, options); - } - /** * Expose a port and get a preview URL for accessing services running in the sandbox * @@ -3069,17 +3278,9 @@ export class Sandbox extends Container implements ISandbox { readFile: (path, options) => this.readFile(path, { ...options, sessionId }), readFileStream: (path) => this.readFileStream(path, { sessionId }), - watch: (path, options: Omit = {}) => - this.watch(path, { ...options, sessionId }), - ensureWatch: ( - path: string, - options?: Omit - ) => this.ensureWatch(path, { ...options, sessionId }), - getWatchState: (watchId: string) => this.getWatchState(watchId), - checkpointWatch: (watchId: string, request: WatchCheckpointRequest) => - this.checkpointWatch(watchId, request), - stopWatch: (watchId: string, options?: WatchStopOptions) => - this.stopWatch(watchId, options), + watch: (path, options) => this.watch(path, { ...options, sessionId }), + checkChanges: (path, options) => + this.checkChanges(path, { ...options, sessionId }), mkdir: (path, options) => this.mkdir(path, { ...options, sessionId }), deleteFile: (path) => this.deleteFile(path, sessionId), renameFile: (oldPath, newPath) => diff --git a/packages/sandbox/tests/sandbox.test.ts b/packages/sandbox/tests/sandbox.test.ts index e0226d57d..eb419ea4c 100644 --- a/packages/sandbox/tests/sandbox.test.ts +++ b/packages/sandbox/tests/sandbox.test.ts @@ -144,21 +144,10 @@ describe('Sandbox - Automatic Session Management', () => { timestamp: new Date().toISOString() } as any); - vi.spyOn(sandbox.client.watch, 'ensureWatch').mockResolvedValue({ + vi.spyOn(sandbox.client.watch, 'checkChanges').mockResolvedValue({ success: true, - watch: { - watchId: 'watch-1', - path: '/workspace/test', - recursive: true, - cursor: 0, - changed: false, - overflowed: false, - lastEventAt: null, - expiresAt: null, - subscriberCount: 0, - startedAt: new Date().toISOString() - }, - leaseToken: 'lease-1', + status: 'unchanged', + version: 'watch-1:0', timestamp: new Date().toISOString() } as any); }); @@ -213,18 +202,18 @@ describe('Sandbox - Automatic Session Management', () => { ); }); - it('should forward ensureWatch options to the watch client', async () => { - await sandbox.ensureWatch('/workspace/test', { - resumeToken: 'resume-1', + it('should forward checkChanges options to the watch client', async () => { + await sandbox.checkChanges('/workspace/test', { + since: 'watch-1:0', recursive: false }); - expect(sandbox.client.watch.ensureWatch).toHaveBeenCalledWith({ + expect(sandbox.client.watch.checkChanges).toHaveBeenCalledWith({ path: '/workspace/test', recursive: false, include: undefined, exclude: undefined, - resumeToken: 'resume-1', + since: 'watch-1:0', sessionId: expect.stringMatching(/^sandbox-/) }); }); diff --git a/packages/sandbox/tests/watch-client.test.ts b/packages/sandbox/tests/watch-client.test.ts index d60da19f7..1c52eddad 100644 --- a/packages/sandbox/tests/watch-client.test.ts +++ b/packages/sandbox/tests/watch-client.test.ts @@ -21,92 +21,35 @@ describe('WatchClient', () => { vi.restoreAllMocks(); }); - it('should normalize legacy dirty state from ensureWatch', async () => { + it('should post to the retained change check endpoint', async () => { mockFetch.mockResolvedValue( new Response( JSON.stringify({ success: true, - watch: { - watchId: 'watch-1', - path: '/workspace/test', - recursive: true, - cursor: 0, - dirty: false, - overflowed: false, - lastEventAt: null, - expiresAt: null, - subscriberCount: 0, - startedAt: '2023-01-01T00:00:00Z' - }, - leaseToken: 'lease-1', - timestamp: '2023-01-01T00:00:00Z' + status: 'changed', + version: 'watch-1:2', + timestamp: '2026-03-17T00:00:00.000Z' }), { status: 200 } ) ); - const result = await client.ensureWatch({ path: '/workspace/test' }); - - expect(result.watch.changed).toBe(false); - }); - - it('should normalize legacy dirty state from getWatchState', async () => { - mockFetch.mockResolvedValue( - new Response( - JSON.stringify({ - success: true, - watch: { - watchId: 'watch-1', - path: '/workspace/test', - recursive: true, - cursor: 2, - dirty: true, - overflowed: false, - lastEventAt: null, - expiresAt: null, - subscriberCount: 0, - startedAt: '2023-01-01T00:00:00Z' - }, - timestamp: '2023-01-01T00:00:00Z' - }), - { status: 200 } - ) - ); - - const result = await client.getWatchState('watch-1'); - - expect(result.watch.changed).toBe(true); - }); - - it('should normalize legacy dirty state from checkpointWatch', async () => { - mockFetch.mockResolvedValue( - new Response( - JSON.stringify({ - success: true, - checkpointed: true, - watch: { - watchId: 'watch-1', - path: '/workspace/test', - recursive: true, - cursor: 2, - dirty: false, - overflowed: false, - lastEventAt: null, - expiresAt: null, - subscriberCount: 0, - startedAt: '2023-01-01T00:00:00Z' - }, - timestamp: '2023-01-01T00:00:00Z' - }), - { status: 200 } - ) - ); - - const result = await client.checkpointWatch('watch-1', { - cursor: 2, - leaseToken: 'lease-1' + const result = await client.checkChanges({ + path: '/workspace/test', + since: 'watch-1:1' }); - expect(result.watch.changed).toBe(false); + expect(result).toEqual({ + success: true, + status: 'changed', + version: 'watch-1:2', + timestamp: '2026-03-17T00:00:00.000Z' + }); + expect(mockFetch).toHaveBeenCalledWith( + 'http://test.com/api/watch/check', + expect.objectContaining({ + method: 'POST' + }) + ); }); }); diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index c9f05404c..6e54c4e28 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -99,6 +99,9 @@ export type { // Bucket mounting types BucketCredentials, BucketProvider, + CheckChangesOptions, + CheckChangesRequest, + CheckChangesResult, ContextCreateResult, ContextDeleteResult, ContextListResult, @@ -132,8 +135,6 @@ export type { MkdirResult, MountBucketOptions, MoveFileResult, - // File watch types - PersistentWatchOptions, PortCheckRequest, PortCheckResponse, PortCloseResult, @@ -169,15 +170,9 @@ export type { WaitForExitResult, WaitForLogResult, WaitForPortOptions, - WatchCheckpointRequest, - WatchCheckpointResult, - WatchEnsureResult, + // File watch types WatchOptions, WatchRequest, - WatchState, - WatchStateResult, - WatchStopOptions, - WatchStopResult, WriteFileResult } from './types.js'; export { diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 586361f34..ec114eb81 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -726,17 +726,18 @@ export interface WatchOptions { } /** - * Options for creating a persistent watch. + * Options for checking whether a path changed while disconnected. + * + * Pass the `version` returned from a previous `checkChanges()` call to learn + * whether the path is unchanged, changed, or needs a full resync because the + * retained change state was reset. Change state lives only for the current + * container lifetime and may expire while idle. */ -export interface PersistentWatchOptions extends WatchOptions { +export interface CheckChangesOptions extends WatchOptions { /** - * Stable token used to safely reconnect to the same persistent watch. - * - * If provided, repeated `ensureWatch()` calls with the same path and - * `resumeToken` will return the existing lease instead of creating a - * conflicting watch. + * Version returned by a previous `checkChanges()` call. */ - resumeToken?: string; + since?: string; } // Internal types for SSE protocol (not user-facing) @@ -762,7 +763,13 @@ export interface WatchRequest { include?: string[]; exclude?: string[]; sessionId?: string; - resumeToken?: string; +} + +/** + * @internal Request body for checking retained change state. + */ +export interface CheckChangesRequest extends WatchRequest { + since?: string; } /** @@ -776,7 +783,6 @@ export type FileWatchSSEEvent = } | { type: 'event'; - eventId: string; eventType: FileWatchEventType; path: string; isDirectory: boolean; @@ -792,78 +798,22 @@ export type FileWatchSSEEvent = }; /** - * Persistent watch state retained while the container stays alive. - */ -export interface WatchState { - watchId: string; - path: string; - recursive: boolean; - include?: string[]; - exclude?: string[]; - cursor: number; - changed: boolean; - overflowed: boolean; - lastEventAt: string | null; - expiresAt: string | null; - subscriberCount: number; - startedAt: string; -} - -/** - * Request body for acknowledging processed watch state. + * Result returned by `checkChanges()`. */ -export interface WatchCheckpointRequest { - cursor: number; - leaseToken: string; -} - -/** - * Options for stopping a persistent watch. - */ -export interface WatchStopOptions { - /** - * Required when stopping a persistent watch obtained from `ensureWatch()`. - */ - leaseToken?: string; -} - -/** - * Result returned when ensuring a persistent watch exists. - */ -export interface WatchEnsureResult { - success: boolean; - watch: WatchState; - leaseToken: string; - timestamp: string; -} - -/** - * Result returned when retrieving persistent watch state. - */ -export interface WatchStateResult { - success: boolean; - watch: WatchState; - timestamp: string; -} - -/** - * Result returned when acknowledging a watch cursor. - */ -export interface WatchCheckpointResult { - success: boolean; - checkpointed: boolean; - watch: WatchState; - timestamp: string; -} - -/** - * Result returned when stopping a persistent watch. - */ -export interface WatchStopResult { - success: boolean; - watchId: string; - timestamp: string; -} +export type CheckChangesResult = + | { + success: true; + status: 'unchanged' | 'changed'; + version: string; + timestamp: string; + } + | { + success: true; + status: 'resync'; + reason: 'expired' | 'restarted'; + version: string; + timestamp: string; + }; // Process management result types export interface ProcessStartResult { @@ -1060,19 +1010,10 @@ export interface ExecutionSession { path: string, options?: Omit ): Promise>; - ensureWatch( + checkChanges( path: string, - options?: Omit - ): Promise; - getWatchState(watchId: string): Promise; - checkpointWatch( - watchId: string, - request: WatchCheckpointRequest - ): Promise; - stopWatch( - watchId: string, - options?: WatchStopOptions - ): Promise; + options?: Omit + ): Promise; mkdir(path: string, options?: { recursive?: boolean }): Promise; deleteFile(path: string): Promise; renameFile(oldPath: string, newPath: string): Promise; @@ -1330,19 +1271,10 @@ export interface ISandbox { path: string, options?: WatchOptions ): Promise>; - ensureWatch( + checkChanges( path: string, - options?: PersistentWatchOptions - ): Promise; - getWatchState(watchId: string): Promise; - checkpointWatch( - watchId: string, - request: WatchCheckpointRequest - ): Promise; - stopWatch( - watchId: string, - options?: WatchStopOptions - ): Promise; + options?: CheckChangesOptions + ): Promise; mkdir(path: string, options?: { recursive?: boolean }): Promise; deleteFile(path: string): Promise; renameFile(oldPath: string, newPath: string): Promise; diff --git a/tests/e2e/file-watch-workflow.test.ts b/tests/e2e/file-watch-workflow.test.ts index 942233a3f..696098bd5 100644 --- a/tests/e2e/file-watch-workflow.test.ts +++ b/tests/e2e/file-watch-workflow.test.ts @@ -9,7 +9,7 @@ * - Recursive vs non-recursive watching */ -import type { FileWatchSSEEvent } from '@repo/shared'; +import type { CheckChangesResult, FileWatchSSEEvent } from '@repo/shared'; import { afterAll, beforeAll, @@ -26,29 +26,6 @@ import { type TestSandbox } from './helpers/global-sandbox'; -interface PersistentWatchState { - watchId: string; - path: string; - cursor: number; - changed: boolean; - overflowed: boolean; - lastEventAt: string | null; - expiresAt: string | null; -} - -interface WatchStateResponse { - success: boolean; - watch: PersistentWatchState; -} - -interface WatchEnsureResponse extends WatchStateResponse { - leaseToken: string; -} - -interface WatchCheckpointResponse extends WatchStateResponse { - checkpointed: boolean; -} - describe('File Watch Workflow', () => { let sandbox: TestSandbox | null = null; let workerUrl: string; @@ -171,78 +148,6 @@ describe('File Watch Workflow', () => { throw new Error(`${context} failed with ${response.status}: ${body}`); } - async function ensureWatch( - path: string, - options: { - recursive?: boolean; - include?: string[]; - exclude?: string[]; - resumeToken?: string; - } = {} - ): Promise { - const response = await fetch(`${workerUrl}/api/watch/ensure`, { - method: 'POST', - headers, - body: JSON.stringify({ path, ...options }) - }); - await expectOk(response, `ensureWatch(${path})`); - return (await response.json()) as WatchEnsureResponse; - } - - async function getWatchState(watchId: string): Promise { - const response = await fetch(`${workerUrl}/api/watch/${watchId}`, { - method: 'GET', - headers - }); - await expectOk(response, `getWatchState(${watchId})`); - const body = (await response.json()) as WatchStateResponse; - return body.watch; - } - - async function checkpointWatch( - watchId: string, - cursor: number, - leaseToken: string - ): Promise { - const response = await fetch( - `${workerUrl}/api/watch/${watchId}/checkpoint`, - { - method: 'POST', - headers, - body: JSON.stringify({ cursor, leaseToken }) - } - ); - await expectOk(response, `checkpointWatch(${watchId})`); - return (await response.json()) as WatchCheckpointResponse; - } - - async function stopWatch(watchId: string, leaseToken: string): Promise { - const suffix = `?leaseToken=${encodeURIComponent(leaseToken)}`; - const response = await fetch(`${workerUrl}/api/watch/${watchId}${suffix}`, { - method: 'DELETE', - headers - }); - await expectOk(response, `stopWatch(${watchId})`); - } - - async function waitForWatchState( - watchId: string, - predicate: (state: PersistentWatchState) => boolean, - timeoutMs = 8000 - ): Promise { - const startedAt = Date.now(); - - while (Date.now() - startedAt < timeoutMs) { - const state = await getWatchState(watchId); - if (predicate(state)) { - return state; - } - await new Promise((resolve) => setTimeout(resolve, 100)); - } - - throw new Error(`Timed out waiting for watch state ${watchId}`); - } - /** * Helper to create a file via the API. */ @@ -279,6 +184,31 @@ describe('File Watch Workflow', () => { await expectOk(response, `deleteFile(${path})`); } + async function checkChanges( + path: string, + options: { + recursive?: boolean; + include?: string[]; + exclude?: string[]; + since?: string; + } = {} + ): Promise { + const response = await fetch(`${workerUrl}/api/watch/check`, { + method: 'POST', + headers, + body: JSON.stringify({ + path, + recursive: options.recursive, + include: options.include, + exclude: options.exclude, + since: options.since + }) + }); + + await expectOk(response, `checkChanges(${path})`); + return (await response.json()) as CheckChangesResult; + } + test('should establish watch and receive watching event', async () => { testDir = sandbox!.uniquePath('watch-establish'); await createDir(testDir); @@ -297,6 +227,46 @@ describe('File Watch Workflow', () => { } }, 30000); + test('should report unchanged when starting retained change tracking', async () => { + testDir = sandbox!.uniquePath('watch-check-baseline'); + await createDir(testDir); + + const result = await checkChanges(testDir); + + expect(result.status).toBe('unchanged'); + expect(result.version).toMatch(/^watch-\d+-\d+:0$/); + }, 30000); + + test('should retain changed state across reconnect gaps', async () => { + testDir = sandbox!.uniquePath('watch-check-retain'); + await createDir(testDir); + + const first = await checkChanges(testDir); + expect(first.status).toBe('unchanged'); + + await createFile(`${testDir}/changed.txt`, 'hello'); + + const second = await checkChanges(testDir, { + since: first.version + }); + + expect(second.status).toBe('changed'); + expect(second.version).not.toBe(first.version); + }, 30000); + + test('should report unchanged when no files changed since the version', async () => { + testDir = sandbox!.uniquePath('watch-check-unchanged'); + await createDir(testDir); + + const first = await checkChanges(testDir); + const second = await checkChanges(testDir, { + since: first.version + }); + + expect(second.status).toBe('unchanged'); + expect(second.version).toBe(first.version); + }, 30000); + test('should detect file creation', async () => { testDir = sandbox!.uniquePath('watch-create'); await createDir(testDir); @@ -430,45 +400,6 @@ describe('File Watch Workflow', () => { await secondResponse.body?.cancel(); }, 30000); - test('should retain changed state across reconnect gaps', async () => { - testDir = sandbox!.uniquePath('watch-persistent-state'); - await createDir(testDir); - - const resumeToken = `resume-${Date.now()}`; - const ensuredWatch = await ensureWatch(testDir, { resumeToken }); - expect(ensuredWatch.watch.changed).toBe(false); - expect(ensuredWatch.watch.cursor).toBe(0); - expect(ensuredWatch.leaseToken).toBeTruthy(); - expect(ensuredWatch.watch.expiresAt).toBeTruthy(); - - await createFile(`${testDir}/background.txt`, 'hello'); - - const changedState = await waitForWatchState( - ensuredWatch.watch.watchId, - (state) => state.changed && state.cursor > 0 - ); - - expect(changedState.lastEventAt).toBeTruthy(); - - const checkpointResult = await checkpointWatch( - ensuredWatch.watch.watchId, - changedState.cursor, - ensuredWatch.leaseToken - ); - expect(checkpointResult.checkpointed).toBe(true); - expect(checkpointResult.watch.changed).toBe(false); - - await createFile(`${testDir}/second.txt`, 'hello again'); - - const secondChangedState = await waitForWatchState( - ensuredWatch.watch.watchId, - (state) => state.changed && state.cursor > changedState.cursor - ); - expect(secondChangedState.cursor).toBeGreaterThan(changedState.cursor); - - await stopWatch(ensuredWatch.watch.watchId, ensuredWatch.leaseToken); - }, 30000); - test('should return error for non-existent path', async () => { const response = await fetch(`${workerUrl}/api/watch`, { method: 'POST', diff --git a/tests/e2e/test-worker/index.ts b/tests/e2e/test-worker/index.ts index 6c621189f..8b564df0b 100644 --- a/tests/e2e/test-worker/index.ts +++ b/tests/e2e/test-worker/index.ts @@ -1026,12 +1026,13 @@ console.log('Terminal server on port ' + port); }); } - if (url.pathname === '/api/watch/ensure' && request.method === 'POST') { - const result = await sandbox.ensureWatch(body.path, { + // File Watch - Check retained change state via public Sandbox API. + if (url.pathname === '/api/watch/check' && request.method === 'POST') { + const result = await sandbox.checkChanges(body.path, { recursive: body.recursive, include: body.include, exclude: body.exclude, - resumeToken: body.resumeToken, + since: body.since, sessionId: sessionId ?? undefined }); @@ -1040,45 +1041,6 @@ console.log('Terminal server on port ' + port); }); } - if ( - url.pathname.startsWith('/api/watch/') && - request.method === 'GET' && - !url.pathname.endsWith('/checkpoint') - ) { - const watchId = url.pathname.split('/')[3]; - const result = await sandbox.getWatchState(watchId); - - return new Response(JSON.stringify(result), { - headers: { 'Content-Type': 'application/json' } - }); - } - - if (url.pathname.endsWith('/checkpoint') && request.method === 'POST') { - const watchId = url.pathname.split('/')[3]; - const result = await sandbox.checkpointWatch(watchId, { - cursor: body.cursor, - leaseToken: body.leaseToken - }); - - return new Response(JSON.stringify(result), { - headers: { 'Content-Type': 'application/json' } - }); - } - - if ( - url.pathname.startsWith('/api/watch/') && - request.method === 'DELETE' - ) { - const watchId = url.pathname.split('/')[3]; - const result = await sandbox.stopWatch(watchId, { - leaseToken: url.searchParams.get('leaseToken') ?? undefined - }); - - return new Response(JSON.stringify(result), { - headers: { 'Content-Type': 'application/json' } - }); - } - // Cleanup endpoint - destroys the sandbox container // This is used by E2E tests to explicitly clean up after each test if (url.pathname === '/cleanup' && request.method === 'POST') { From e736ac12f03fb7ae8d18515c904b84c2f972b4a5 Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 1 Apr 2026 13:29:04 +0100 Subject: [PATCH 6/6] Use canonical log events in WatchService --- .../src/services/watch-service.ts | 88 +++++++++++-------- packages/sandbox/src/clients/watch-client.ts | 13 +-- 2 files changed, 54 insertions(+), 47 deletions(-) diff --git a/packages/sandbox-container/src/services/watch-service.ts b/packages/sandbox-container/src/services/watch-service.ts index ceb9c834c..0a7e425db 100644 --- a/packages/sandbox-container/src/services/watch-service.ts +++ b/packages/sandbox-container/src/services/watch-service.ts @@ -6,6 +6,7 @@ import type { Logger, WatchRequest } from '@repo/shared'; +import { logCanonicalEvent } from '@repo/shared'; import { ErrorCode } from '@repo/shared/errors'; import type { Subprocess } from 'bun'; import type { ServiceResult } from '../core/types'; @@ -177,8 +178,7 @@ export class WatchService { const watchId = `watch-${++this.watchCounter}-${Date.now()}`; const args = this.buildInotifyArgs(path, options); - const watchLogger = this.logger.child({ watchId, path }); - watchLogger.debug('Starting inotifywait', { args }); + const startTime = Date.now(); try { const proc = Bun.spawn(['inotifywait', ...args], { @@ -207,12 +207,22 @@ export class WatchService { this.activeWatches.set(watchId, watch); this.watchIdsByKey.set(key, watchId); - this.runWatchLoop(watch, watchLogger); + + const watchLogger = this.logger.child({ watchId, path }); + this.runWatchLoop(watch, watchLogger, startTime); return serviceSuccess({ watch, created: true }); } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); - watchLogger.error('Failed to start inotifywait', err); + logCanonicalEvent(this.logger, { + event: 'watch.start', + outcome: 'error', + durationMs: Date.now() - startTime, + path, + watchId, + errorMessage: err.message, + error: err + }); return serviceError({ message: `Failed to start file watcher: ${err.message}`, code: ErrorCode.WATCH_START_ERROR, @@ -601,17 +611,17 @@ export class WatchService { type: 'stopped', reason: 'Watch process ended' }; + const isError = resolvedTerminalEvent.type === 'error'; + const reason = isError + ? resolvedTerminalEvent.error + : resolvedTerminalEvent.reason; this.activeWatches.delete(watchId); this.watchIdsByKey.delete(watch.key); this.clearRetainedWatchExpiry(watch); if (watch.readyState === 'pending') { - const terminalMessage = - resolvedTerminalEvent.type === 'error' - ? resolvedTerminalEvent.error - : resolvedTerminalEvent.reason; - this.rejectWatchReady(watch, new Error(terminalMessage)); + this.rejectWatchReady(watch, new Error(reason)); } this.broadcastTerminalEvent(watch, resolvedTerminalEvent); @@ -641,13 +651,26 @@ export class WatchService { // Process may have already exited. } } + + logCanonicalEvent(this.logger, { + event: 'watch.stop', + outcome: isError ? 'error' : 'success', + durationMs: Date.now() - watch.startedAt.getTime(), + path: watch.path, + watchId, + errorMessage: isError ? reason : undefined + }); }; watch.stopPromise = cleanup(); return watch.stopPromise; } - private runWatchLoop(watch: ActiveWatch, logger: Logger): void { + private runWatchLoop( + watch: ActiveWatch, + logger: Logger, + startTime: number + ): void { const stdout = watch.process.stdout; const stderr = watch.process.stderr; @@ -661,18 +684,26 @@ export class WatchService { void (async () => { try { if (stderr && typeof stderr !== 'number') { - const monitor = await this.waitForWatchesEstablished(stderr, logger); + const monitor = await this.waitForWatchesEstablished(stderr); this.continueStderrMonitoring( monitor.reader, monitor.decoder, monitor.buffer, - watch, - logger + watch ); } this.resolveWatchReady(watch); + logCanonicalEvent(this.logger, { + event: 'watch.start', + outcome: 'success', + durationMs: Date.now() - startTime, + path: watch.path, + watchId: watch.id, + recursive: watch.recursive + }); + const reader = stdout.getReader(); const decoder = new TextDecoder(); let buffer = ''; @@ -702,7 +733,10 @@ export class WatchService { }); } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); - logger.error('Error reading watch output', err); + logger.error('Error reading watch output', err, { + watchId: watch.id, + path: watch.path + }); this.rejectWatchReady(watch, err); await this.stopWatchInternal(watch.id, errorEvent(err.message)); } @@ -878,8 +912,7 @@ export class WatchService { } private async waitForWatchesEstablished( - stderr: ReadableStream, - logger: Logger + stderr: ReadableStream ): Promise<{ reader: { read(): Promise<{ done: boolean; value?: Uint8Array }> }; decoder: TextDecoder; @@ -907,19 +940,12 @@ export class WatchService { } if (trimmed.includes('Watches established')) { - logger.debug('inotifywait watches established'); return 'established'; } if (trimmed.includes('Setting up watches')) { - logger.debug('inotifywait setting up watches', { - message: trimmed - }); continue; } - logger.warn('inotifywait stderr during setup', { - message: trimmed - }); throw new Error(trimmed); } } @@ -939,10 +965,7 @@ export class WatchService { ]); if (result === 'timeout') { - const timeoutMessage = - 'Timed out waiting for file watcher setup to complete'; - logger.warn(timeoutMessage, { timeoutMs: WATCH_SETUP_TIMEOUT_MS }); - throw new Error(timeoutMessage); + throw new Error('Timed out waiting for file watcher setup to complete'); } return { reader, decoder, buffer }; @@ -960,8 +983,7 @@ export class WatchService { reader: { read(): Promise<{ done: boolean; value?: Uint8Array }> }, decoder: TextDecoder, initialBuffer: string, - watch: ActiveWatch, - logger: Logger + watch: ActiveWatch ): void { void (async () => { let buffer = initialBuffer; @@ -986,19 +1008,15 @@ export class WatchService { trimmed.includes('Watches established') || trimmed.includes('Setting up watches') ) { - logger.debug('inotifywait info', { message: trimmed }); continue; } - logger.warn('inotifywait stderr', { message: trimmed }); await this.stopWatchInternal(watch.id, errorEvent(trimmed)); return; } } - } catch (error) { - logger.debug('stderr monitoring ended', { - error: error instanceof Error ? error.message : 'Unknown' - }); + } catch { + // Stream closed or process terminated — expected during cleanup. } })(); } diff --git a/packages/sandbox/src/clients/watch-client.ts b/packages/sandbox/src/clients/watch-client.ts index 05229ec0b..6f84bb475 100644 --- a/packages/sandbox/src/clients/watch-client.ts +++ b/packages/sandbox/src/clients/watch-client.ts @@ -22,18 +22,7 @@ export class WatchClient extends BaseHttpClient { async checkChanges( request: CheckChangesRequest ): Promise { - try { - const response = await this.post( - '/api/watch/check', - request - ); - - this.logSuccess('Checked retained file changes', request.path); - return response; - } catch (error) { - this.logError('checkChanges', error); - throw error; - } + return this.post('/api/watch/check', request); } /**