diff --git a/docs/agent-conversations.md b/docs/agent-conversations.md index 3e73954..0188ced 100644 --- a/docs/agent-conversations.md +++ b/docs/agent-conversations.md @@ -98,6 +98,9 @@ marker, and dispatches once. `wait` waits at most 25 seconds by default; `get` a `list` return saved status. A caller's AbortSignal stops its wait, while accepted work continues under a separate 12-minute deadline. `cancel` applies only before acceptance. Repeating delivery returns the existing job or exact receipt; it never replays a send. +The lock appears with its owner's process already recorded in it, so a process killed at +any moment leaves no lock, or one naming a dead owner that the next call reclaims. An +empty lock left by an older version is removed once it is ten seconds old. State is stored under `MainSequenceCLI/agent-delivery-v1` in the platform application data directory, with mode 0700 directories and 0600 atomic files. It contains message diff --git a/docs/workspace-environments.md b/docs/workspace-environments.md index 4c78121..9dd981f 100644 --- a/docs/workspace-environments.md +++ b/docs/workspace-environments.md @@ -4,7 +4,13 @@ URL and repositories folder. It never signs in or rewrites configuration. The backend uses `MAINSEQUENCE_ENDPOINT`, then existing `backend_url`, then `https://api.main-sequence.app`. The folder uses `mainsequence_path`, otherwise -`/mainsequence`. Host-saved overrides remain the host's responsibility. +`/mainsequence`; `baseDirSaved` says which. Host-saved overrides remain the host's +responsibility. + +`writeWorkspacePreferences({ backend?, baseDir?, commandCenterUrl? })` saves only the +fields it is given into the same file the `mainsequence` CLI reads, keeping every other +setting. The Codex plugin saves all three; Claude Desktop saves `{ baseDir }` alone, so +both and the CLI share one repositories folder. Every action reads a repositories folder as the MainSequence CLI does (`repositoriesFolder(value, home?)`): `~` is the home folder, a bare name such as diff --git a/package-lock.json b/package-lock.json index 8104327..83e081a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@dev-mainsequence/devtools", - "version": "0.3.16", + "version": "0.3.17", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@dev-mainsequence/devtools", - "version": "0.3.16", + "version": "0.3.17", "license": "MIT", "bin": { "ms-devtools": "dist/runner.js" diff --git a/package.json b/package.json index a0f4c2d..25d6585 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@dev-mainsequence/devtools", - "version": "0.3.16", + "version": "0.3.17", "description": "Shared MainSequence developer actions, project operations, MCP client and command runner", "license": "MIT", "engines": { diff --git a/src/shared/agentDelivery.ts b/src/shared/agentDelivery.ts index 7d678b7..0c4d5e7 100644 --- a/src/shared/agentDelivery.ts +++ b/src/shared/agentDelivery.ts @@ -34,6 +34,8 @@ const uuid = (value: unknown): string => { const failStore = () => new ToolingError('STORE_ERROR', 'Saved agent requests could not be read safely. Existing requests were kept; no message was resent.'); const live = (pid: number) => { try { process.kill(pid, 0); return true; } catch (e) { return (e as NodeJS.ErrnoException).code !== 'ESRCH'; } }; const view = (r: StoredRequest): AgentDeliveryRequest => { const { owner: _, ...publicRequest } = r; return structuredClone(publicRequest); }; +/** A live owner writes its record within moments of creating the lock; an empty lock this old was left by a killed process. */ +const EMPTY_LOCK_STALE_MS = 10_000; const finished = (r: StoredRequest) => !['pending', 'preparing', 'sending'].includes(r.state); const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); const defaultDirectory = () => path.join(process.platform === 'darwin' ? path.join(os.homedir(), 'Library', 'Application Support', 'MainSequenceCLI') @@ -75,23 +77,33 @@ export class AgentDelivery { try { const stat = await handle.stat(); if (lockRead && stat.nlink === 0) throw Object.assign(new Error('The lock was released while opening it.'), { code: 'ENOENT' }); - if (!stat.isFile() || stat.nlink !== 1 || stat.size > 36_000_000 || (process.getuid && stat.uid !== process.getuid())) throw failStore(); + // A lock is linked into place from its owner's private file, so it briefly has two names. + if (!stat.isFile() || stat.nlink !== 1 && !(lockRead && stat.nlink === 2) || stat.size > 36_000_000 || (process.getuid && stat.uid !== process.getuid())) throw failStore(); return await handle.readFile('utf8'); } finally { await handle.close(); } } private async locked(key: string, run: (state: State, save: () => Promise) => Promise): Promise { await this.secureDirectory(); const file = path.join(this.directory, key + '.json'), lock = file + '.lock'; - const owner = { pid: process.pid, token: randomUUID() }; let guard; + const owner = { pid: process.pid, token: randomUUID() }; let acquired = false; const deadline = Date.now() + 3000; - while (!guard) { - try { guard = await fs.open(lock, 'wx', 0o600); } - catch (error) { + while (!acquired) { + // The lock appears with its owner already in it: the record is written to a private file, which is + // then linked into place, failing while another lock exists. A process killed at any moment leaves + // no lock, or one naming a dead owner that the next caller reclaims; never an empty one. + const pending = `${lock}.${owner.token}.tmp`; + try { + const handle = await fs.open(pending, 'wx', 0o600); + try { await handle.writeFile(JSON.stringify(owner)); await handle.sync(); } finally { await handle.close(); } + await fs.link(pending, lock); acquired = true; + } catch (error) { if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw failStore(); let previous: Owner; try { previous = JSON.parse(await this.readFile(lock, true)); } catch (e) { - // A new exclusive file can briefly precede its owner record. Never reclaim it. + // An empty lock is an older version's, made before its owner record was written: still being + // written while it is new, left by a killed process once it is stale. + if (e instanceof SyntaxError && await this.reclaimEmptyLock(lock)) continue; if (Date.now() >= deadline) throw failStore(); if (e instanceof SyntaxError || (e as NodeJS.ErrnoException).code === 'ENOENT') { await sleep(25); continue; } throw failStore(); @@ -102,15 +114,16 @@ export class AgentDelivery { const claim = lock + '.reclaim-' + uuid(previous.token); try { const handle = await fs.open(claim, 'wx', 0o600); await handle.close(); } catch { throw new ToolingError('PROJECT_BUSY', 'Agent storage recovery is already running. Try reading status again.'); } + // An owner killed between linking its lock and removing its private file left that second name. + await fs.unlink(`${lock}.${uuid(previous.token)}.tmp`).catch(() => {}); if (await this.readFile(lock) !== JSON.stringify(previous)) throw failStore(); await fs.unlink(lock); continue; } if (Date.now() >= deadline) throw new ToolingError('PROJECT_BUSY', 'Another agent request is updating local storage. Try reading status again.'); await sleep(25); - } + } finally { await fs.unlink(pending).catch(() => {}); } } try { - await guard.writeFile(JSON.stringify(owner)); await guard.sync(); let state: State; try { state = JSON.parse(await this.readFile(file)); } catch (e) { if ((e as NodeJS.ErrnoException).code !== 'ENOENT') throw failStore(); state = { version: 1, scope: key, requests: {} }; } @@ -148,7 +161,18 @@ export class AgentDelivery { finally { await fs.unlink(temporary).catch(() => {}); } }; const result = await run(state, save); await save(); return result; - } finally { await guard.close(); await fs.unlink(lock); } + } finally { await fs.unlink(lock); } + } + /** Removes an empty lock older than any write of its owner record could take, once, under a durable claim. */ + private async reclaimEmptyLock(lock: string): Promise { + const stat = await fs.lstat(lock).catch(() => null); + if (!stat || !stat.isFile() || stat.size !== 0 || Date.now() - stat.mtimeMs < EMPTY_LOCK_STALE_MS) return false; + const claim = `${lock}.reclaim-empty-${stat.ino}-${Math.floor(stat.mtimeMs)}`; + try { const handle = await fs.open(claim, 'wx', 0o600); await handle.close(); } + catch { throw new ToolingError('PROJECT_BUSY', 'Agent storage recovery is already running. Try reading status again.'); } + const current = await fs.lstat(lock).catch(() => null); + if (!current || current.ino !== stat.ino || current.size !== 0 || current.mtimeMs !== stat.mtimeMs) return true; + await fs.unlink(lock); return true; } /** Prepare a stable conversation without sending an introductory message. */ async prepare(input: AgentDeliveryScope, selection: {environmentUid:string;agentUid:string;conversationKey?:string}, signal?:AbortSignal) { diff --git a/src/shared/repositories.ts b/src/shared/repositories.ts index 7d55c0f..bb5aa58 100644 --- a/src/shared/repositories.ts +++ b/src/shared/repositories.ts @@ -35,14 +35,16 @@ export async function resolveBackend(explicit?: string, root?: string, env: Node } /** Resolve non-secret workspace settings without requiring a session or writing configuration. */ -export async function readWorkspacePreferences(options: { env?: NodeJS.ProcessEnv; configPath?: string; homeDir?: string } = {}): Promise<{ backend: string; baseDir: string }> { +export async function readWorkspacePreferences(options: { env?: NodeJS.ProcessEnv; configPath?: string; homeDir?: string } = {}): Promise<{ backend: string; baseDir: string; baseDirSaved: boolean }> { const env = options.env ?? process.env, home = options.homeDir ?? os.homedir(); const directory = mainSequenceStateDirectory(env, home); const config = await fs.readFile(options.configPath ?? path.join(directory, 'config.json'), 'utf8') .then(text => JSON.parse(text)).catch(() => ({})); const folder = config?.mainsequence_path; + // `baseDirSaved`: the folder comes from the shared configuration, not the default. + const baseDirSaved = typeof folder === 'string' && !!folder.trim() && !/[\0\r\n]/.test(folder); return { backend: requireBackend(env.MAINSEQUENCE_ENDPOINT || (typeof config?.backend_url === 'string' && config.backend_url.trim()) || 'https://api.main-sequence.app'), - baseDir: typeof folder === 'string' && folder.trim() && !/[\0\r\n]/.test(folder) ? repositoriesFolder(folder, home) : path.join(home, 'mainsequence') }; + baseDir: baseDirSaved ? repositoriesFolder(folder, home) : path.join(home, 'mainsequence'), baseDirSaved }; } export async function git(host: HostServices, cwd: string, args: string[], signal?: AbortSignal): Promise { diff --git a/src/shared/workspaceSettings.ts b/src/shared/workspaceSettings.ts index 9e5df56..45eb805 100644 --- a/src/shared/workspaceSettings.ts +++ b/src/shared/workspaceSettings.ts @@ -23,11 +23,14 @@ export async function readDesktopPreferences(options: Options = {}) { const config = await read(location(options)); return { ...await readWorkspacePreferences(options), commandCenterUrl: typeof config.command_center_url === 'string' ? config.command_center_url : '' }; } -/** Updates only non-secret workspace fields, preserving existing CLI settings. */ -export async function writeWorkspacePreferences(input: { backend: string; baseDir: string; commandCenterUrl?: string }, options: Options = {}) { - const backend = requireBackend(input.backend); - const baseDir = repositoriesFolder(input.baseDir, options.homeDir, 'Repositories folder'); - const commandCenterUrl = input.commandCenterUrl ? requireBackend(input.commandCenterUrl) : ''; +/** Updates only the non-secret workspace fields given, preserving every other CLI setting: a host that + * shares only the repositories folder (Claude Desktop) saves `{ baseDir }` alone. */ +export async function writeWorkspacePreferences(input: { backend?: string; baseDir?: string; commandCenterUrl?: string }, options: Options = {}) { + if (input.backend === undefined && input.baseDir === undefined && input.commandCenterUrl === undefined) throw new ToolingError('INVALID_REQUEST', 'Name a workspace setting to save.'); + const updates: Record = {}; + if (input.backend !== undefined) updates.backend_url = requireBackend(input.backend); + if (input.baseDir !== undefined) updates.mainsequence_path = repositoriesFolder(input.baseDir, options.homeDir, 'Repositories folder'); + if (input.commandCenterUrl !== undefined) updates.command_center_url = input.commandCenterUrl ? requireBackend(input.commandCenterUrl) : ''; const file = location(options); await fs.mkdir(path.dirname(file), { recursive: true }); let lock; try { lock = await fs.open(file + '.preferences.lock', 'wx', 0o600); } @@ -35,7 +38,7 @@ export async function writeWorkspacePreferences(input: { backend: string; baseDi const temporary = file + '.' + randomUUID() + '.tmp'; try { const config = await read(file); - await fs.writeFile(temporary, JSON.stringify({ ...config, backend_url: backend, mainsequence_path: baseDir, command_center_url: commandCenterUrl }, null, 2) + '\n', { flag: 'wx', mode: 0o600 }); + await fs.writeFile(temporary, JSON.stringify({ ...config, ...updates }, null, 2) + '\n', { flag: 'wx', mode: 0o600 }); await fs.rename(temporary, file); return readDesktopPreferences(options); } finally { await fs.rm(temporary, { force: true }); await lock.close(); await fs.unlink(file + '.preferences.lock'); } diff --git a/tests/agent-delivery.test.cjs b/tests/agent-delivery.test.cjs index 052d24c..5898592 100644 --- a/tests/agent-delivery.test.cjs +++ b/tests/agent-delivery.test.cjs @@ -80,6 +80,22 @@ test('a process killed after send leaves an uncertain record; restarting cannot const recovered=await f.service().deliver(scope,r.requestId);assert.equal(recovered.state,'uncertain');assert.equal(recovered.sendAttempts,1);assert.equal(f.sends().length,0); assert.equal(await fs.readFile(sentFile,'utf8'),r.requestId+'\n'); }); +test('a process killed while taking the storage lock never blocks the next one',async t=>{ + const f=await fixture(t),r=await f.api.stage(scope,f.draft()); + const store=path.join(f.directory,(await fs.readdir(f.directory)).find(n=>n.endsWith('.json'))),lock=store+'.lock'; + const exited=spawn(process.execPath,['-e','']);const dead=exited.pid;await new Promise(resolve=>exited.once('exit',resolve)); + // Killed between linking its lock and removing its private file: the lock still has that second name. + const token=randomUUID(),pending=`${lock}.${token}.tmp`; + await fs.writeFile(pending,JSON.stringify({pid:dead,token}),{mode:0o600});await fs.link(pending,lock); + assert.equal((await f.api.get(scope,r.requestId)).state,'pending'); + await assert.rejects(fs.stat(lock),{code:'ENOENT'});await assert.rejects(fs.stat(pending),{code:'ENOENT'}); + // An older version's empty lock: left alone while new, since its owner may still be writing; removed once stale. + await fs.writeFile(lock,'',{mode:0o600}); + await assert.rejects(f.api.get(scope,r.requestId),{code:'STORE_ERROR'}); + const old=new Date(Date.now()-60_000);await fs.utimes(lock,old,old); + assert.equal((await f.api.get(scope,r.requestId)).state,'pending'); + await assert.rejects(fs.stat(lock),{code:'ENOENT'});assert.equal(f.sends().length,0); +}); test('corrupt and symlinked storage fail closed rather than discarding deduplication history',async t=>{ const f=await fixture(t),r=await f.api.stage(scope,f.draft()),file=(await fs.readdir(f.directory)).find(n=>n.endsWith('.json')),p=path.join(f.directory,file); await fs.writeFile(p,'broken');await assert.rejects(f.api.deliver(scope,r.requestId));assert.equal(await fs.readFile(p,'utf8'),'broken'); diff --git a/tests/workspace-environments.test.cjs b/tests/workspace-environments.test.cjs index 2c4d624..bacef49 100644 --- a/tests/workspace-environments.test.cjs +++ b/tests/workspace-environments.test.cjs @@ -11,10 +11,10 @@ const row=(n,environment=envA)=>({backend,name:'Project',resource_release_uid:ui async function temporary(t){const dir=await fs.mkdtemp(path.join(os.tmpdir(),'ms-settings-env-'));t.after(()=>fs.rm(dir,{recursive:true,force:true}));return dir;} test('workspace values resolve without authentication, preserve configuration, and honor precedence', async t=>{ const dir=await temporary(t),configPath=path.join(dir,'config.json'); - assert.deepEqual(await readWorkspacePreferences({env:{},homeDir:dir,configPath}),{backend:'https://api.main-sequence.app',baseDir:path.join(dir,'mainsequence')}); + assert.deepEqual(await readWorkspacePreferences({env:{},homeDir:dir,configPath}),{backend:'https://api.main-sequence.app',baseDir:path.join(dir,'mainsequence'),baseDirSaved:false}); const original=JSON.stringify({backend_url:'http://127.0.0.1:8000',mainsequence_path:dir,provider:{secret:'fixture-secret'}}); await fs.writeFile(configPath,original); - assert.deepEqual(await readWorkspacePreferences({env:{},homeDir:dir,configPath}),{backend:'http://127.0.0.1:8000',baseDir:dir}); + assert.deepEqual(await readWorkspacePreferences({env:{},homeDir:dir,configPath}),{backend:'http://127.0.0.1:8000',baseDir:dir,baseDirSaved:true}); assert.equal((await readWorkspacePreferences({env:{MAINSEQUENCE_ENDPOINT:'https://override.example'},homeDir:dir,configPath})).backend,'https://override.example'); assert.equal(await fs.readFile(configPath,'utf8'),original); }); diff --git a/tests/workspace-settings.test.cjs b/tests/workspace-settings.test.cjs index 846b242..0ceea4a 100644 --- a/tests/workspace-settings.test.cjs +++ b/tests/workspace-settings.test.cjs @@ -10,3 +10,13 @@ test('workspace settings preserve unrelated fields, save a bare folder name unde await fs.writeFile(configPath,'broken');await assert.rejects(writeWorkspacePreferences(input,options),/preserved/);assert.equal(await fs.readFile(configPath,'utf8'),'broken'); await assert.rejects(readDesktopPreferences(options),/preserved/); }); +test('a host that shares only the repositories folder saves it alone; reads say whether the folder was saved',async t=>{ + const {readWorkspacePreferences}=require('../dist'); + const root=await fs.mkdtemp(path.join(os.tmpdir(),'ms-settings-'));t.after(()=>fs.rm(root,{recursive:true,force:true}));const configPath=path.join(root,'config.json'),options={configPath,homeDir:root,env:{}}; + assert.deepEqual(await readWorkspacePreferences(options),{backend:'https://api.main-sequence.app',baseDir:path.join(root,'mainsequence'),baseDirSaved:false}); + await fs.writeFile(configPath,JSON.stringify({backend_url:'http://local-backend:8000',command_center_url:'https://command.example',custom:{keep:true}})); + const saved=await writeWorkspacePreferences({baseDir:'mainsequence-dev'},options); + assert.equal(saved.baseDir,path.join(root,'mainsequence-dev'));assert.equal(saved.baseDirSaved,true);assert.equal(saved.backend,'http://local-backend:8000'); + assert.deepEqual(JSON.parse(await fs.readFile(configPath)),{backend_url:'http://local-backend:8000',command_center_url:'https://command.example',custom:{keep:true},mainsequence_path:path.join(root,'mainsequence-dev')}); + await assert.rejects(writeWorkspacePreferences({},options),/Name a workspace setting/); +});