From 49fae2a3cddede3cc976a94ed82b26b9dc6c0cdc Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Tue, 1 Sep 2026 05:20:59 +0000 Subject: [PATCH 1/2] fix(dev): include owner URL in lock diagnostics --- .changeset/include-dev-owner-url.md | 5 + packages/agent-bundle/src/dev/coordinator.ts | 5 + packages/agent-bundle/src/dev/dev-lock.ts | 95 +++++++++++++++++-- .../agent-bundle/src/dev/foreground-server.ts | 2 + packages/agent-bundle/tests/dev-lock.test.ts | 8 +- .../agent-bundle/tests/dev-server.test.ts | 6 ++ 6 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 .changeset/include-dev-owner-url.md diff --git a/.changeset/include-dev-owner-url.md b/.changeset/include-dev-owner-url.md new file mode 100644 index 000000000..d42a2b52f --- /dev/null +++ b/.changeset/include-dev-owner-url.md @@ -0,0 +1,5 @@ +--- +"agent-bundle": patch +--- + +Include the existing development workbench URL when another process already owns the project. diff --git a/packages/agent-bundle/src/dev/coordinator.ts b/packages/agent-bundle/src/dev/coordinator.ts index 9e2702d41..e571cc532 100644 --- a/packages/agent-bundle/src/dev/coordinator.ts +++ b/packages/agent-bundle/src/dev/coordinator.ts @@ -27,6 +27,7 @@ import { export interface DevLockHandle { close(): Promise; + publishServerUrl?(url: string): Promise; } export interface ProjectPreparer { @@ -265,6 +266,10 @@ export class DevCoordinator { return this.#rebuild(invalidation); } + async publishServerUrl(url: string): Promise { + await this.#lock?.publishServerUrl?.(url); + } + async #rebuild( invalidation: Invalidation, token?: symbol, diff --git a/packages/agent-bundle/src/dev/dev-lock.ts b/packages/agent-bundle/src/dev/dev-lock.ts index c742f57a4..8ac05659c 100644 --- a/packages/agent-bundle/src/dev/dev-lock.ts +++ b/packages/agent-bundle/src/dev/dev-lock.ts @@ -3,7 +3,7 @@ import { link, lstat, mkdir, open, readFile, rm } from 'node:fs/promises'; import { basename, dirname, join, resolve } from 'node:path'; import { stableJson } from '../core/digest.ts'; -import { publishFileByLink } from '../core/durable-fs.ts'; +import { publishFileByLink, writeJsonFileAtomically } from '../core/durable-fs.ts'; import { CodedError, isErrno } from '../core/errors.ts'; import { acquireOwnerLockFile, isProcessAlive, ownerLockRaceLost } from '../core/owner-lock.ts'; @@ -12,6 +12,7 @@ export interface DevLockOwner { readonly nonce: string; readonly pid: number; readonly projectRoot: string; + readonly url?: string; } export interface DevLockOptions { @@ -49,13 +50,27 @@ interface RecoveryRecord { readonly owner: DevLockOwner; } +const isLoopbackServerUrl = (value: unknown): value is string => { + if (typeof value !== 'string') return false; + try { + const parsed = new URL(value); + return parsed.protocol === 'http:' && + (parsed.hostname === '127.0.0.1' || parsed.hostname === '[::1]') && + parsed.origin === value; + } catch { + return false; + } +}; + const parseOwnerValue = (value: unknown, projectRoot: string): DevLockOwner | undefined => { try { if (typeof value !== 'object' || value === null || Array.isArray(value)) return undefined; const parsed = value as Partial; const pid = parsed.pid; + const hasUrl = Object.hasOwn(parsed, 'url'); + const url = hasUrl && isLoopbackServerUrl(parsed.url) ? parsed.url : undefined; if ( - Object.keys(parsed).length !== 4 || + Object.keys(parsed).length !== (hasUrl ? 5 : 4) || !Object.hasOwn(parsed, 'createdAt') || !Object.hasOwn(parsed, 'nonce') || !Object.hasOwn(parsed, 'pid') || @@ -66,7 +81,8 @@ const parseOwnerValue = (value: unknown, projectRoot: string): DevLockOwner | un typeof pid !== 'number' || !Number.isSafeInteger(pid) || pid <= 0 || - parsed.projectRoot !== projectRoot + parsed.projectRoot !== projectRoot || + (hasUrl && url === undefined) ) { return undefined; } @@ -76,6 +92,7 @@ const parseOwnerValue = (value: unknown, projectRoot: string): DevLockOwner | un nonce: parsed.nonce, pid, projectRoot: parsed.projectRoot, + ...(url === undefined ? {} : { url }), }); } catch { return undefined; @@ -224,13 +241,16 @@ const acquireRecoveryGate = async ( }; export class DevLock { - readonly #contents: string; + #contents: string; + #owner: DevLockOwner; readonly #path: string; readonly #probeProcess: (pid: number) => boolean; readonly #recoveryPath: string; readonly #storage: DevLockStorage; #closed = false; #closePromise: Promise | undefined; + #publishPromise: Promise | undefined; + #publishingUrl: string | undefined; constructor( path: string, @@ -245,16 +265,74 @@ export class DevLock { this.#recoveryPath = recoveryPath; this.#storage = storage; this.#contents = contents; - this.owner = owner; + this.#owner = owner; } - readonly owner: DevLockOwner; + get owner(): DevLockOwner { + return this.#owner; + } + + publishServerUrl(url: string): Promise { + if (!isLoopbackServerUrl(url)) return Promise.reject(new TypeError('Development server URL must be a loopback HTTP origin.')); + if (this.#closed || this.#closePromise !== undefined) return Promise.reject(new Error('Development lock is closing.')); + if (this.#owner.url === url) return Promise.resolve(); + if (this.#owner.url !== undefined) return Promise.reject(new Error('Development lock already published a different server URL.')); + if (this.#publishPromise !== undefined) { + return this.#publishingUrl === url + ? this.#publishPromise + : Promise.reject(new Error('Development lock is publishing a different server URL.')); + } + + const owner = Object.freeze({ ...this.#owner, url }); + const contents = `${stableJson(owner)}\n`; + const publishPromise = (async () => { + const currentContents = await this.#storage.readFile(this.#path, 'utf8'); + if (currentContents !== this.#contents) { + throw new DevLockError('DEV_LOCK_INVALID', 'Development lock ownership changed before its server URL could be published.'); + } + const temporaryPath = `${this.#path}.update-${randomUUID()}`; + try { + const temporary = await this.#storage.open(temporaryPath, 'wx', 0o600); + await temporary.close(); + await writeJsonFileAtomically(this.#path, contents, { temporaryPath }); + } catch (error) { + if (await this.#storage.readFile(this.#path, 'utf8') === contents) { + this.#contents = contents; + this.#owner = owner; + } + throw error; + } finally { + await this.#storage.remove(temporaryPath, { force: true }); + } + this.#contents = contents; + this.#owner = owner; + })(); + this.#publishingUrl = url; + this.#publishPromise = publishPromise; + void publishPromise.then( + () => { + if (this.#publishPromise === publishPromise) { + this.#publishPromise = undefined; + this.#publishingUrl = undefined; + } + }, + () => { + if (this.#publishPromise === publishPromise) { + this.#publishPromise = undefined; + this.#publishingUrl = undefined; + } + }, + ); + return publishPromise; + } close(): Promise { if (this.#closed) return Promise.resolve(); if (this.#closePromise !== undefined) return this.#closePromise; + const pendingPublication = this.#publishPromise; const closePromise = (async () => { + await pendingPublication?.catch(() => undefined); const recoveryContents = await acquireRecoveryGate( this.#storage, this.#recoveryPath, @@ -332,9 +410,12 @@ export const acquireDevLock = async (options: DevLockOptions): Promise ); } if (probeProcess(currentOwner.pid)) { + const ownerDetails = currentOwner.url === undefined + ? `pid ${currentOwner.pid}` + : `pid ${currentOwner.pid}, ${currentOwner.url}`; throw new DevLockError( 'DEV_LOCK_HELD', - `Another agent-bundle dev process owns this project (pid ${currentOwner.pid}).`, + `Another agent-bundle dev process owns this project (${ownerDetails}).`, currentOwner, ); } diff --git a/packages/agent-bundle/src/dev/foreground-server.ts b/packages/agent-bundle/src/dev/foreground-server.ts index ed365acc6..bf076beb2 100644 --- a/packages/agent-bundle/src/dev/foreground-server.ts +++ b/packages/agent-bundle/src/dev/foreground-server.ts @@ -81,6 +81,7 @@ export class ForegroundServerStartError extends Error { /** The small coordinator surface required by foreground HTTP routes. */ export interface ForegroundCoordinator { close(): Promise; + publishServerUrl?(url: string): Promise; rebuild(invalidation: Invalidation): Promise; start(): Promise; status(): ProjectStatus; @@ -599,6 +600,7 @@ export class ForegroundServer { throw new Error('Foreground server did not report a TCP address.'); } this.#url = `http://${addressToHost(address)}:${address.port}`; + await this.#coordinator.publishServerUrl?.(this.#url); } catch (error) { if (this.#closePromise !== undefined) throw error; this.#closing = true; diff --git a/packages/agent-bundle/tests/dev-lock.test.ts b/packages/agent-bundle/tests/dev-lock.test.ts index f7d594427..6093a34f1 100644 --- a/packages/agent-bundle/tests/dev-lock.test.ts +++ b/packages/agent-bundle/tests/dev-lock.test.ts @@ -9,7 +9,7 @@ import { acquireDevLock } from '../src/dev/dev-lock.ts'; const lockPathFor = (root: string): string => join(root, '.agent-bundle', 'dev.lock'); const recoveryPathFor = (root: string): string => `${lockPathFor(root)}.recovery`; -it('rejects a second writer with stable metadata for the live owning process', async () => { +it('rejects a second writer with the live owning process URL', async () => { const root = await mkdtemp(join(tmpdir(), 'agent bundle dev lock with spaces ')); try { @@ -17,13 +17,16 @@ it('rejects a second writer with stable metadata for the live owning process', a now: () => new Date('2026-08-14T12:00:00.000Z'), projectRoot: root, }); + await first.publishServerUrl('http://127.0.0.1:48721'); await expect(acquireDevLock({ projectRoot: root })).rejects.toMatchObject({ code: 'DEV_LOCK_HELD', + message: `Another agent-bundle dev process owns this project (pid ${process.pid}, http://127.0.0.1:48721).`, owner: { createdAt: '2026-08-14T12:00:00.000Z', pid: process.pid, projectRoot: root, + url: 'http://127.0.0.1:48721', }, }); expect(first.owner).not.toHaveProperty('version'); @@ -32,13 +35,16 @@ it('rejects a second writer with stable metadata for the live owning process', a readonly nonce: unknown; readonly pid: number; readonly projectRoot: string; + readonly url: string; }; expect(published).toMatchObject({ createdAt: '2026-08-14T12:00:00.000Z', pid: process.pid, projectRoot: root, + url: 'http://127.0.0.1:48721', }); expect(published).not.toHaveProperty('version'); + expect((await lstat(lockPathFor(root))).mode & 0o777).toBe(0o600); expect(typeof published.nonce).toBe('string'); expect(published.nonce).not.toBe(''); diff --git a/packages/agent-bundle/tests/dev-server.test.ts b/packages/agent-bundle/tests/dev-server.test.ts index 707389189..d552b59f7 100644 --- a/packages/agent-bundle/tests/dev-server.test.ts +++ b/packages/agent-bundle/tests/dev-server.test.ts @@ -27,6 +27,7 @@ const status = (): ProjectStatus => ({ class RecordingCoordinator { readonly invalidations: Invalidation[] = []; + readonly publishedUrls: string[] = []; closeCalls = 0; failClose = false; startCalls = 0; @@ -36,6 +37,10 @@ class RecordingCoordinator { if (this.failClose) throw new Error('coordinator close failure'); } + async publishServerUrl(url: string): Promise { + this.publishedUrls.push(url); + } + async rebuild(invalidation: Invalidation): Promise<{ readonly diagnostics: readonly []; readonly outcome: 'failed' }> { this.invalidations.push(invalidation); return { diagnostics: [], outcome: 'failed' }; @@ -242,6 +247,7 @@ it('serves typed project status and supplied prebuilt assets after starting the try { expect(coordinator.startCalls).toBe(1); + expect(coordinator.publishedUrls).toEqual([server.url]); const [statusResponse, assetResponse] = await Promise.all([ fetch(`${server.url}/api/project/status`), From e8de7a147aae4178b0b6581e03c9c97d0212065f Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Tue, 1 Sep 2026 05:28:38 +0000 Subject: [PATCH 2/2] fix(dev): preserve replacement locks during URL publish --- packages/agent-bundle/src/dev/dev-lock.ts | 83 +++++++++++++++----- packages/agent-bundle/tests/dev-lock.test.ts | 40 +++++++++- 2 files changed, 101 insertions(+), 22 deletions(-) diff --git a/packages/agent-bundle/src/dev/dev-lock.ts b/packages/agent-bundle/src/dev/dev-lock.ts index 8ac05659c..ef0959d71 100644 --- a/packages/agent-bundle/src/dev/dev-lock.ts +++ b/packages/agent-bundle/src/dev/dev-lock.ts @@ -1,9 +1,9 @@ -import { randomUUID } from 'node:crypto'; +import { createHash, randomUUID } from 'node:crypto'; import { link, lstat, mkdir, open, readFile, rm } from 'node:fs/promises'; import { basename, dirname, join, resolve } from 'node:path'; import { stableJson } from '../core/digest.ts'; -import { publishFileByLink, writeJsonFileAtomically } from '../core/durable-fs.ts'; +import { publishFileByLink } from '../core/durable-fs.ts'; import { CodedError, isErrno } from '../core/errors.ts'; import { acquireOwnerLockFile, isProcessAlive, ownerLockRaceLost } from '../core/owner-lock.ts'; @@ -108,6 +108,47 @@ const parseOwner = (value: string, projectRoot: string): DevLockOwner | undefine } }; +interface DevServerUrlRecord { + readonly nonce: string; + readonly url: string; +} + +const serverUrlPathFor = (path: string, nonce: string): string => + join(dirname(path), `.${basename(path)}.server-${createHash('sha256').update(nonce).digest('hex')}`); + +const serverUrlContentsFor = (owner: DevLockOwner, url: string): string => + `${stableJson({ nonce: owner.nonce, url } satisfies DevServerUrlRecord)}\n`; + +const parseServerUrl = (contents: string, owner: DevLockOwner): string | undefined => { + try { + const value: unknown = JSON.parse(contents); + if (typeof value !== 'object' || value === null || Array.isArray(value)) return undefined; + const record = value as Partial; + if ( + Object.keys(record).length !== 2 || + record.nonce !== owner.nonce || + !isLoopbackServerUrl(record.url) + ) return undefined; + const canonical = `${stableJson({ nonce: record.nonce, url: record.url })}\n`; + return contents === canonical ? record.url : undefined; + } catch { + return undefined; + } +}; + +const readServerUrl = async ( + storage: DevLockStorage, + path: string, + owner: DevLockOwner, +): Promise => { + try { + return parseServerUrl(await storage.readFile(serverUrlPathFor(path, owner.nonce), 'utf8'), owner); + } catch (error) { + if (isErrno(error, 'ENOENT')) return undefined; + throw error; + } +}; + const parseRecoveryRecord = (value: string, projectRoot: string): RecoveryRecord | undefined => { try { const parsedValue: unknown = JSON.parse(value); @@ -241,7 +282,7 @@ const acquireRecoveryGate = async ( }; export class DevLock { - #contents: string; + readonly #contents: string; #owner: DevLockOwner; readonly #path: string; readonly #probeProcess: (pid: number) => boolean; @@ -284,27 +325,23 @@ export class DevLock { } const owner = Object.freeze({ ...this.#owner, url }); - const contents = `${stableJson(owner)}\n`; + const serverUrlPath = serverUrlPathFor(this.#path, this.#owner.nonce); + const serverUrlContents = serverUrlContentsFor(this.#owner, url); const publishPromise = (async () => { const currentContents = await this.#storage.readFile(this.#path, 'utf8'); if (currentContents !== this.#contents) { throw new DevLockError('DEV_LOCK_INVALID', 'Development lock ownership changed before its server URL could be published.'); } - const temporaryPath = `${this.#path}.update-${randomUUID()}`; - try { - const temporary = await this.#storage.open(temporaryPath, 'wx', 0o600); - await temporary.close(); - await writeJsonFileAtomically(this.#path, contents, { temporaryPath }); - } catch (error) { - if (await this.#storage.readFile(this.#path, 'utf8') === contents) { - this.#contents = contents; - this.#owner = owner; + if (!(await writeCompleteExclusive(this.#storage, serverUrlPath, serverUrlContents, this.#owner.nonce))) { + const existing = await this.#storage.readFile(serverUrlPath, 'utf8'); + if (existing !== serverUrlContents) { + throw new DevLockError('DEV_LOCK_INVALID', 'The development server URL record belongs to a different owner.'); } - throw error; - } finally { - await this.#storage.remove(temporaryPath, { force: true }); } - this.#contents = contents; + if (await this.#storage.readFile(this.#path, 'utf8') !== this.#contents) { + await removeIfOwned(this.#storage, serverUrlPath, serverUrlContents); + throw new DevLockError('DEV_LOCK_INVALID', 'Development lock ownership changed while its server URL was published.'); + } this.#owner = owner; })(); this.#publishingUrl = url; @@ -342,6 +379,7 @@ export class DevLock { try { await removeIfOwned(this.#storage, this.#path, this.#contents); await removeIfOwned(this.#storage, candidatePathFor(this.#path, this.owner.nonce), this.#contents); + await this.#storage.remove(serverUrlPathFor(this.#path, this.owner.nonce), { force: true }); } finally { await removeIfOwned(this.#storage, this.#recoveryPath, recoveryContents); } @@ -410,13 +448,17 @@ export const acquireDevLock = async (options: DevLockOptions): Promise ); } if (probeProcess(currentOwner.pid)) { - const ownerDetails = currentOwner.url === undefined + const serverUrl = await readServerUrl(storage, path, currentOwner); + const reportedOwner = serverUrl === undefined + ? currentOwner + : Object.freeze({ ...currentOwner, url: serverUrl }); + const ownerDetails = serverUrl === undefined ? `pid ${currentOwner.pid}` - : `pid ${currentOwner.pid}, ${currentOwner.url}`; + : `pid ${currentOwner.pid}, ${serverUrl}`; throw new DevLockError( 'DEV_LOCK_HELD', `Another agent-bundle dev process owns this project (${ownerDetails}).`, - currentOwner, + reportedOwner, ); } @@ -439,6 +481,7 @@ export const acquireDevLock = async (options: DevLockOptions): Promise } if (probeProcess(ownerDuringRecovery.pid)) return undefined; await removeIfOwned(storage, path, currentContents); + await storage.remove(serverUrlPathFor(path, ownerDuringRecovery.nonce), { force: true }); await removeIfOwned( storage, candidatePathFor(path, ownerDuringRecovery.nonce), diff --git a/packages/agent-bundle/tests/dev-lock.test.ts b/packages/agent-bundle/tests/dev-lock.test.ts index 6093a34f1..f7b0e3d13 100644 --- a/packages/agent-bundle/tests/dev-lock.test.ts +++ b/packages/agent-bundle/tests/dev-lock.test.ts @@ -35,15 +35,15 @@ it('rejects a second writer with the live owning process URL', async () => { readonly nonce: unknown; readonly pid: number; readonly projectRoot: string; - readonly url: string; }; expect(published).toMatchObject({ createdAt: '2026-08-14T12:00:00.000Z', pid: process.pid, projectRoot: root, - url: 'http://127.0.0.1:48721', }); + expect(published).not.toHaveProperty('url'); expect(published).not.toHaveProperty('version'); + expect(first.owner).toMatchObject({ url: 'http://127.0.0.1:48721' }); expect((await lstat(lockPathFor(root))).mode & 0o777).toBe(0o600); expect(typeof published.nonce).toBe('string'); expect(published.nonce).not.toBe(''); @@ -56,6 +56,42 @@ it('rejects a second writer with the live owning process URL', async () => { } }); +it('does not overwrite a replacement lock while publishing the server URL', async () => { + const root = await mkdtemp(join(tmpdir(), 'agent bundle dev lock publication race ')); + let replaceOnRead = false; + let replacement: Awaited> | undefined; + const storage = { + link, + lstat, + mkdir, + open, + readFile: async (...args: Parameters) => { + const contents = await readFile(...args); + if (replaceOnRead && args[0] === lockPathFor(root)) { + replaceOnRead = false; + await rm(lockPathFor(root), { force: true }); + replacement = await acquireDevLock({ projectRoot: root }); + } + return contents; + }, + remove: rm, + }; + + const first = await acquireDevLock({ projectRoot: root, storage }); + try { + replaceOnRead = true; + await expect(first.publishServerUrl('http://127.0.0.1:48721')).rejects.toMatchObject({ + code: 'DEV_LOCK_INVALID', + }); + + const published = JSON.parse(await readFile(lockPathFor(root), 'utf8')) as { readonly nonce: string }; + expect(published.nonce).toBe(replacement?.owner.nonce); + } finally { + await Promise.allSettled([first.close(), replacement?.close()]); + await rm(root, { force: true, recursive: true }); + } +}); + it('recovers a dead lock only after probing its recorded pid', async () => { const root = await mkdtemp(join(tmpdir(), 'agent bundle dev lock recovery ')); const stalePid = 2_147_483_647;