From f7d9a1094ea00f538193110e7ee10cd9b9227c30 Mon Sep 17 00:00:00 2001 From: Jose Antonio Alatorre Date: Thu, 5 Nov 2026 14:22:23 +0100 Subject: [PATCH 1/2] Save the repositories folder alone into the shared settings (0.3.17) So Claude Desktop, the Codex plugin and the mainsequence CLI can keep one repositories folder: the CLI's `mainsequence_path` in its config.json. - `writeWorkspacePreferences({ backend?, baseDir?, commandCenterUrl? })` saves only the fields given and keeps every other setting. A call with none is refused. The Codex plugin still passes all three, so it behaves as before. - `readWorkspacePreferences` also returns `baseDirSaved`: true when the folder comes from the shared configuration rather than the `/mainsequence` default. Co-Authored-By: Claude Opus 5.5 --- docs/workspace-environments.md | 8 +++++++- package-lock.json | 4 ++-- package.json | 2 +- src/shared/repositories.ts | 6 ++++-- src/shared/workspaceSettings.ts | 15 +++++++++------ tests/workspace-environments.test.cjs | 4 ++-- tests/workspace-settings.test.cjs | 10 ++++++++++ 7 files changed, 35 insertions(+), 14 deletions(-) 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/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/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/); +}); From 5459813c51344bbe798a11c910c3522a32ea4e2d Mon Sep 17 00:00:00 2001 From: Jose Antonio Alatorre Date: Thu, 5 Nov 2026 15:02:49 +0100 Subject: [PATCH 2/2] Never leave an empty agent storage lock behind a killed process CI failed once on this branch in `a process killed after send leaves an uncertain record` (STORE_ERROR after the 3-second lock wait); the same commit passed in the other run. The cause is in the store, not the test: the lock was created empty and its owner written after, and `wait` takes it every 250 ms while a send runs. A process killed between the two left an empty lock, which by design is never reclaimed, so every later call failed with "Saved agent requests could not be read safely" until the file was removed by hand. - The owner record is written to a private file first and linked into place, which fails while another lock exists, so the lock never exists without its owner. A lock left with that second name by an owner killed between linking and cleaning up is reclaimed with the dead owner's lock, as before. - An empty lock (left by an older version) is removed once it is ten seconds old, under a durable claim; a new one is still waited for, since its owner may be writing. Tests: a dead owner's lock with its second name is reclaimed; a fresh empty lock is waited for and refused, a stale one removed. The killed-process test passed 30 times in a row. Co-Authored-By: Claude Opus 5.5 --- docs/agent-conversations.md | 3 +++ src/shared/agentDelivery.ts | 42 +++++++++++++++++++++++++++-------- tests/agent-delivery.test.cjs | 16 +++++++++++++ 3 files changed, 52 insertions(+), 9 deletions(-) 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/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/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');