fix(settings): preserve concurrent updates (#2137)

* fix(settings): preserve concurrent updates

Serialize the complete settings read-merge-write transaction under a physical-target lock with a bounded synchronous contention wait. Read the merge base fresh after ownership, preserve logical symlinks during publication, and route direct settings-sync replacements through the same lock.

* fix(settings): address transaction review feedback

* fix(settings): reject invalid merge bases

* fix(settings): preserve lock ownership and apply outcomes

* fix(settings): address transaction follow-up

* fix(settings): avoid unsafe cleanup control flow

* fix(settings): publish complete lock claims

* test(settings): document pending lock claim

* fix(settings): track lock owner process identity
This commit is contained in:
Bogdan
2026-08-24 10:17:55 +08:00
committed by GitHub
parent bb6d66faa3
commit 34536c6220
6 changed files with 2533 additions and 99 deletions
+115 -26
View File
@@ -33,9 +33,8 @@ import {
getAPIProvider,
isFirstPartyAnthropicBaseUrl,
} from '../../utils/model/providers.js'
import { markInternalWrite } from '../../utils/settings/internalWrites.js'
import { getSettingsFilePathForSource } from '../../utils/settings/settings.js'
import { resetSettingsCache } from '../../utils/settings/settingsCache.js'
import { replaceSettingsFileSync } from '../../utils/settings/settingsFileTransaction.js'
import { sleep } from '../../utils/sleep.js'
import { getClaudeCodeUserAgent } from '../../utils/userAgent.js'
import { getFeatureValue_CACHED_MAY_BE_STALE } from '../analytics/growthbook.js'
@@ -113,12 +112,27 @@ export async function uploadUserSettingsInBackground(): Promise<void> {
// Cached so the fire-and-forget at runHeadless entry and the await in
// installPluginsAndApplyMcpInBackground share one fetch.
let downloadPromise: Promise<boolean> | null = null
let downloadedEntriesForTesting: {
entries: Record<string, string>
projectId: string | null
} | null = null
/** Test-only: clear the cached download promise between tests. */
export function _resetDownloadPromiseForTesting(): void {
downloadPromise = null
}
/** Test-only: bypass eligibility and HTTP while retaining public download flow. */
export function _setDownloadedEntriesForTesting(
value: {
entries: Record<string, string>
projectId: string | null
} | null,
): void {
downloadedEntriesForTesting = value
downloadPromise = null
}
/**
* Download settings from remote for CCR mode.
* Fired fire-and-forget at the top of print.ts runHeadless(); awaited in
@@ -157,6 +171,12 @@ export function redownloadUserSettings(): Promise<boolean> {
async function doDownloadUserSettings(
maxRetries = DEFAULT_MAX_RETRIES,
): Promise<boolean> {
if (downloadedEntriesForTesting) {
return applyDownloadedEntries(
downloadedEntriesForTesting.entries,
downloadedEntriesForTesting.projectId,
)
}
if (feature('DOWNLOAD_USER_SETTINGS')) {
try {
if (
@@ -184,13 +204,7 @@ async function doDownloadUserSettings(
const entries = result.data!.content.entries
const projectId = await getRepoRemoteHash()
const entryCount = Object.keys(entries).length
logForDiagnosticsNoPII('info', 'settings_sync_download_applying', {
entryCount,
})
await applyRemoteEntriesToLocal(entries, projectId)
logEvent('tengu_settings_sync_download_success', { entryCount })
return true
return applyDownloadedEntries(entries, projectId)
} catch {
// Fail-open: log error but don't block CCR startup
logForDiagnosticsNoPII('error', 'settings_sync_download_error')
@@ -477,6 +491,20 @@ async function writeFileForSync(
}
}
function writeSettingsFileForSync(
filePath: string,
content: string,
): boolean {
try {
replaceSettingsFileSync(filePath, content)
logForDiagnosticsNoPII('info', 'settings_sync_file_written')
return true
} catch {
logForDiagnosticsNoPII('warn', 'settings_sync_file_write_failed')
return false
}
}
/**
* Apply remote entries to local files (CCR pull pattern).
* Only writes files that match expected keys.
@@ -488,10 +516,18 @@ async function writeFileForSync(
async function applyRemoteEntriesToLocal(
entries: Record<string, string>,
projectId: string | null,
): Promise<void> {
): Promise<{
appliedCount: number
settingsFilesWritten: number
settingsFilesFailed: number
settingsFilesRejected: number
memoryFilesWritten: number
}> {
let appliedCount = 0
let settingsWritten = false
let memoryWritten = false
let settingsFilesWritten = 0
let settingsFilesFailed = 0
let settingsFilesRejected = 0
let memoryFilesWritten = 0
// Helper to check size limit (defense-in-depth, matches backend limit)
const exceedsSizeLimit = (content: string, _path: string): boolean => {
@@ -514,12 +550,14 @@ async function applyRemoteEntriesToLocal(
userSettingsPath &&
!exceedsSizeLimit(userSettingsContent, userSettingsPath)
) {
// Mark as internal write to prevent spurious change detection
markInternalWrite(userSettingsPath)
if (await writeFileForSync(userSettingsPath, userSettingsContent)) {
if (writeSettingsFileForSync(userSettingsPath, userSettingsContent)) {
appliedCount++
settingsWritten = true
settingsFilesWritten++
} else {
settingsFilesFailed++
}
} else {
settingsFilesRejected++
}
}
@@ -530,7 +568,7 @@ async function applyRemoteEntriesToLocal(
if (!exceedsSizeLimit(userMemoryContent, userMemoryPath)) {
if (await writeFileForSync(userMemoryPath, userMemoryContent)) {
appliedCount++
memoryWritten = true
memoryFilesWritten++
}
}
}
@@ -545,12 +583,16 @@ async function applyRemoteEntriesToLocal(
localSettingsPath &&
!exceedsSizeLimit(projectSettingsContent, localSettingsPath)
) {
// Mark as internal write to prevent spurious change detection
markInternalWrite(localSettingsPath)
if (await writeFileForSync(localSettingsPath, projectSettingsContent)) {
if (
writeSettingsFileForSync(localSettingsPath, projectSettingsContent)
) {
appliedCount++
settingsWritten = true
settingsFilesWritten++
} else {
settingsFilesFailed++
}
} else {
settingsFilesRejected++
}
}
@@ -561,21 +603,68 @@ async function applyRemoteEntriesToLocal(
if (!exceedsSizeLimit(projectMemoryContent, localMemoryPath)) {
if (await writeFileForSync(localMemoryPath, projectMemoryContent)) {
appliedCount++
memoryWritten = true
memoryFilesWritten++
}
}
}
}
// Invalidate caches so subsequent reads pick up new content
if (settingsWritten) {
resetSettingsCache()
}
if (memoryWritten) {
if (memoryFilesWritten > 0) {
clearMemoryFileCaches()
}
logForDiagnosticsNoPII('info', 'settings_sync_applied', {
appliedCount,
settingsFilesWritten,
settingsFilesFailed,
settingsFilesRejected,
memoryFilesWritten,
})
return {
appliedCount,
settingsFilesWritten,
settingsFilesFailed,
settingsFilesRejected,
memoryFilesWritten,
}
}
async function applyDownloadedEntries(
entries: Record<string, string>,
projectId: string | null,
): Promise<boolean> {
const entryCount = Object.keys(entries).length
logForDiagnosticsNoPII('info', 'settings_sync_download_applying', {
entryCount,
})
const result = await applyRemoteEntriesToLocal(entries, projectId)
if (result.settingsFilesFailed > 0) {
logForDiagnosticsNoPII('warn', 'settings_sync_download_apply_failed', {
entryCount,
settingsFilesFailed: result.settingsFilesFailed,
})
logEvent('tengu_settings_sync_download_apply_failed', {
entryCount,
settingsFilesFailed: result.settingsFilesFailed,
})
return false
}
logEvent('tengu_settings_sync_download_success', { entryCount })
return true
}
/** @internal Direct apply seam for focused settings-file transaction tests. */
export function _applyRemoteEntriesToLocalForTesting(
entries: Record<string, string>,
projectId: string | null,
): Promise<{
appliedCount: number
settingsFilesWritten: number
settingsFilesFailed: number
settingsFilesRejected: number
memoryFilesWritten: number
}> {
return applyRemoteEntriesToLocal(entries, projectId)
}
@@ -0,0 +1,371 @@
import { spawn, type ChildProcessByStdio } from 'node:child_process'
import {
existsSync,
mkdtempSync,
mkdirSync,
readFileSync,
rmSync,
writeFileSync,
} from 'node:fs'
import { tmpdir } from 'node:os'
import { dirname, join, resolve } from 'node:path'
import type { Readable } from 'node:stream'
import { expect, test } from 'bun:test'
import { getOriginalCwd, setOriginalCwd } from '../../bootstrap/state.js'
import {
getClaudeConfigHomeDirOverrideForTesting,
setClaudeConfigHomeDirForTesting,
} from '../../utils/envUtils.js'
import { resetSettingsCache } from '../../utils/settings/settingsCache.js'
import {
_applyRemoteEntriesToLocalForTesting,
_setDownloadedEntriesForTesting,
downloadUserSettings,
redownloadUserSettings,
} from './index.js'
import { SYNC_KEYS } from './types.js'
const fixturePath = resolve(
import.meta.dir,
'../../test/fixtures/settingsTransactionWriter.fixture.ts',
)
const CHILD_TIMEOUT_MS = 15_000
const TEST_TIMEOUT_MS = CHILD_TIMEOUT_MS + 5_000
type Holder = {
process: ChildProcessByStdio<null, Readable, Readable>
exited: Promise<{ code: number | null; signal: NodeJS.Signals | null }>
output: () => { stdout: string; stderr: string }
}
function startHolder(
targetPath: string,
holdMs: number,
enteredMarker: string,
completedMarker: string,
): Holder {
const child = spawn(
process.execPath,
[
fixturePath,
'hold-path-for',
targetPath,
'unused',
String(holdMs),
enteredMarker,
completedMarker,
],
{
cwd: process.cwd(),
env: { ...process.env, FORCE_COLOR: '0' },
stdio: ['ignore', 'pipe', 'pipe'],
},
)
let stdout = ''
let stderr = ''
child.stdout.setEncoding('utf8')
child.stderr.setEncoding('utf8')
child.stdout.on('data', chunk => {
stdout += chunk
})
child.stderr.on('data', chunk => {
stderr += chunk
})
return {
process: child,
exited: new Promise(resolveExit => {
child.once('exit', (code, signal) => resolveExit({ code, signal }))
}),
output: () => ({ stdout, stderr }),
}
}
function delay(ms: number): Promise<void> {
return new Promise(resolveDelay => setTimeout(resolveDelay, ms))
}
async function waitForHolder(marker: string, holder: Holder): Promise<void> {
const deadline = performance.now() + CHILD_TIMEOUT_MS
while (!existsSync(marker)) {
if (
holder.process.exitCode !== null ||
holder.process.signalCode !== null
) {
const { stdout, stderr } = holder.output()
throw new Error(
`Holder exited before acquiring the lock\nstdout:\n${stdout}\nstderr:\n${stderr}`,
)
}
if (performance.now() >= deadline) {
throw new Error('Timed out waiting for holder to acquire the lock')
}
await delay(10)
}
}
async function finishHolder(holder: Holder): Promise<void> {
const outcome = await Promise.race([
holder.exited,
delay(CHILD_TIMEOUT_MS).then(() => {
throw new Error('Holder did not exit')
}),
])
const { stdout, stderr } = holder.output()
if (outcome.code !== 0) {
throw new Error(
`Holder exited with code ${outcome.code ?? 'null'} and signal ${outcome.signal ?? 'none'}\nstdout:\n${stdout}\nstderr:\n${stderr}`,
)
}
}
async function terminateHolder(holder: Holder | undefined): Promise<void> {
if (
!holder ||
holder.process.exitCode !== null ||
holder.process.signalCode !== null
) {
return
}
holder.process.kill('SIGTERM')
await Promise.race([holder.exited, delay(500)])
if (
holder.process.exitCode === null &&
holder.process.signalCode === null
) {
holder.process.kill('SIGKILL')
await holder.exited
}
}
async function withSyncEnvironment(
run: (paths: {
root: string
userSettings: string
userMemory: string
localSettings: string
}) => Promise<void>,
): Promise<void> {
const root = mkdtempSync(join(tmpdir(), 'openclaude-settings-sync-'))
const project = join(root, 'project')
const previousConfig = getClaudeConfigHomeDirOverrideForTesting()
const previousCwd = getOriginalCwd()
mkdirSync(project)
setClaudeConfigHomeDirForTesting(root)
setOriginalCwd(project)
resetSettingsCache()
try {
await run({
root,
userSettings: join(root, 'settings.json'),
userMemory: join(root, 'CLAUDE.md'),
localSettings: join(project, '.openclaude', 'settings.local.json'),
})
} finally {
setClaudeConfigHomeDirForTesting(previousConfig)
setOriginalCwd(previousCwd)
resetSettingsCache()
rmSync(root, { recursive: true, force: true })
}
}
test(
'user settings sync waits for the shared transaction lock and succeeds',
async () => {
await withSyncEnvironment(async ({ root, userSettings }) => {
writeFileSync(userSettings, '{}\n')
const entered = join(root, 'user-holder-entered')
const completed = join(root, 'user-holder-completed')
const holder = startHolder(userSettings, 1_000, entered, completed)
try {
await waitForHolder(entered, holder)
const startedAt = performance.now()
const result = await _applyRemoteEntriesToLocalForTesting(
{
[SYNC_KEYS.USER_SETTINGS]: '{"env":{"SYNCED":"yes"}}\n',
},
null,
)
const elapsedMs = performance.now() - startedAt
expect(result).toEqual({
appliedCount: 1,
settingsFilesWritten: 1,
settingsFilesFailed: 0,
settingsFilesRejected: 0,
memoryFilesWritten: 0,
})
expect(elapsedMs).toBeGreaterThanOrEqual(500)
expect(JSON.parse(readFileSync(userSettings, 'utf8')).env).toEqual({
SYNCED: 'yes',
})
await finishHolder(holder)
expect(existsSync(`${userSettings}.lock`)).toBe(false)
} finally {
await terminateHolder(holder)
}
})
},
TEST_TIMEOUT_MS,
)
test(
'local settings sync uses the same physical-target lock',
async () => {
await withSyncEnvironment(async ({ root, localSettings }) => {
mkdirSync(dirname(localSettings), { recursive: true })
writeFileSync(localSettings, '{}\n')
const entered = join(root, 'local-holder-entered')
const completed = join(root, 'local-holder-completed')
const holder = startHolder(localSettings, 1_000, entered, completed)
try {
await waitForHolder(entered, holder)
const result = await _applyRemoteEntriesToLocalForTesting(
{
[SYNC_KEYS.projectSettings('project-id')]:
'{"env":{"LOCAL_SYNCED":"yes"}}\n',
},
'project-id',
)
expect(result).toEqual({
appliedCount: 1,
settingsFilesWritten: 1,
settingsFilesFailed: 0,
settingsFilesRejected: 0,
memoryFilesWritten: 0,
})
expect(JSON.parse(readFileSync(localSettings, 'utf8')).env).toEqual({
LOCAL_SYNCED: 'yes',
})
await finishHolder(holder)
expect(existsSync(`${localSettings}.lock`)).toBe(false)
} finally {
await terminateHolder(holder)
}
})
},
TEST_TIMEOUT_MS,
)
test(
'a timed-out settings entry is not reported applied and memory still syncs',
async () => {
await withSyncEnvironment(async ({ root, userMemory, userSettings }) => {
writeFileSync(userSettings, '{"env":{"ORIGINAL":"yes"}}\n')
const entered = join(root, 'timeout-holder-entered')
const completed = join(root, 'timeout-holder-completed')
const holder = startHolder(userSettings, 4_500, entered, completed)
try {
await waitForHolder(entered, holder)
const result = await _applyRemoteEntriesToLocalForTesting(
{
[SYNC_KEYS.USER_SETTINGS]: '{"env":{"REPLACED":"no"}}\n',
[SYNC_KEYS.USER_MEMORY]: 'synced memory\n',
},
null,
)
expect(result).toEqual({
appliedCount: 1,
settingsFilesWritten: 0,
settingsFilesFailed: 1,
settingsFilesRejected: 0,
memoryFilesWritten: 1,
})
expect(JSON.parse(readFileSync(userSettings, 'utf8')).env).toEqual({
ORIGINAL: 'yes',
})
expect(readFileSync(userMemory, 'utf8')).toBe('synced memory\n')
await finishHolder(holder)
expect(
await _applyRemoteEntriesToLocalForTesting(
{
[SYNC_KEYS.USER_SETTINGS]: '{"env":{"LATER":"yes"}}\n',
},
null,
),
).toEqual({
appliedCount: 1,
settingsFilesWritten: 1,
settingsFilesFailed: 0,
settingsFilesRejected: 0,
memoryFilesWritten: 0,
})
expect(JSON.parse(readFileSync(userSettings, 'utf8')).env).toEqual({
LATER: 'yes',
})
expect(existsSync(`${userSettings}.lock`)).toBe(false)
} finally {
await terminateHolder(holder)
}
})
},
TEST_TIMEOUT_MS,
)
test('permanent settings rejections are reported without retry failure', async () => {
await withSyncEnvironment(async ({ userSettings }) => {
const oversizedSettings = 'x'.repeat(500 * 1024 + 1)
const entries = {
[SYNC_KEYS.USER_SETTINGS]: oversizedSettings,
[SYNC_KEYS.projectSettings('project-id')]: oversizedSettings,
}
expect(
await _applyRemoteEntriesToLocalForTesting(entries, 'project-id'),
).toEqual({
appliedCount: 0,
settingsFilesWritten: 0,
settingsFilesFailed: 0,
settingsFilesRejected: 2,
memoryFilesWritten: 0,
})
expect(existsSync(userSettings)).toBe(false)
try {
_setDownloadedEntriesForTesting({ entries, projectId: 'project-id' })
expect(await redownloadUserSettings()).toBe(true)
} finally {
_setDownloadedEntriesForTesting(null)
}
})
})
test(
'startup and reload downloads return false after a timed-out settings apply',
async () => {
await withSyncEnvironment(async ({ root, userMemory, userSettings }) => {
writeFileSync(userSettings, '{"env":{"ORIGINAL":"yes"}}\n')
const entered = join(root, 'download-holder-entered')
const completed = join(root, 'download-holder-completed')
const holder = startHolder(userSettings, 9_000, entered, completed)
try {
await waitForHolder(entered, holder)
_setDownloadedEntriesForTesting({
entries: {
[SYNC_KEYS.USER_SETTINGS]: '{"env":{"REPLACED":"no"}}\n',
[SYNC_KEYS.USER_MEMORY]: 'best-effort memory\n',
},
projectId: null,
})
const startupDownload = downloadUserSettings()
expect(downloadUserSettings()).toBe(startupDownload)
expect(await startupDownload).toBe(false)
expect(await redownloadUserSettings()).toBe(false)
expect(JSON.parse(readFileSync(userSettings, 'utf8')).env).toEqual({
ORIGINAL: 'yes',
})
expect(readFileSync(userMemory, 'utf8')).toBe('best-effort memory\n')
await finishHolder(holder)
expect(existsSync(`${userSettings}.lock`)).toBe(false)
} finally {
_setDownloadedEntriesForTesting(null)
await terminateHolder(holder)
}
})
},
TEST_TIMEOUT_MS,
)
+171
View File
@@ -0,0 +1,171 @@
import { existsSync, realpathSync, writeFileSync } from 'node:fs'
import { basename, dirname, join, resolve } from 'node:path'
import {
getFsImplementation,
setFsImplementation,
} from '../../utils/fsOperations.js'
const fixtureArgs = process.argv.slice(2)
const [
role,
target,
key,
value,
enteredMarker,
completedMarker,
readMarker,
releaseMarker,
] = fixtureArgs
const supportedRoles: ReadonlySet<string> = new Set([
'normal',
'hold-lock',
'hold-path-for',
'pause-after-read',
'pause-before-lock-owner',
])
if (!role || !supportedRoles.has(role)) {
throw new Error(`Invalid settings transaction fixture role: ${role}`)
}
if (
!target ||
!key ||
!value ||
!enteredMarker ||
!completedMarker
) {
throw new Error('Missing settings transaction fixture arguments')
}
const expectedArgumentCount =
role === 'hold-lock' ||
role === 'pause-after-read' ||
role === 'pause-before-lock-owner'
? 8
: 6
if (fixtureArgs.length !== expectedArgumentCount) {
throw new Error(
`Invalid argument count for ${role}: expected ${expectedArgumentCount}, received ${fixtureArgs.length}`,
)
}
if (role === 'hold-lock' && !releaseMarker) {
throw new Error('Hold-lock fixture requires a release marker')
}
if (role === 'pause-after-read' && (!readMarker || !releaseMarker)) {
throw new Error('Pause-after-read fixture requires read and release markers')
}
if (role === 'pause-before-lock-owner' && (!readMarker || !releaseMarker)) {
throw new Error(
'Pause-before-lock-owner fixture requires pause and release markers',
)
}
const holdMs = role === 'hold-path-for' ? Number(value) : undefined
if (
role === 'hold-path-for' &&
(!Number.isFinite(holdMs) || (holdMs ?? -1) < 0)
) {
throw new Error(`Invalid hold duration: ${value}`)
}
if (role !== 'hold-path-for') {
process.env.OPENCLAUDE_CONFIG_DIR = target
}
const settingsPath =
role === 'hold-path-for'
? resolve(target)
: resolve(target, 'settings.json')
const settingsParentPath = dirname(settingsPath)
const settingsReadPath = existsSync(settingsPath)
? realpathSync(settingsPath)
: existsSync(settingsParentPath)
? join(realpathSync(settingsParentPath), basename(settingsPath))
: settingsPath
const waitBuffer = new Int32Array(new SharedArrayBuffer(4))
function waitForMarker(marker: string): void {
const deadline = performance.now() + 15_000
while (!existsSync(marker)) {
if (performance.now() >= deadline) {
throw new Error(`Timed out waiting for fixture marker: ${marker}`)
}
Atomics.wait(waitBuffer, 0, 0, 10)
}
}
if (role === 'pause-after-read') {
const originalFs = getFsImplementation()
let paused = false
setFsImplementation({
...originalFs,
readFileSync(path, options) {
const content = originalFs.readFileSync(path, options)
if (!paused && resolve(path) === settingsReadPath) {
paused = true
writeFileSync(readMarker, '')
waitForMarker(releaseMarker)
}
return content
},
})
}
if (role === 'pause-before-lock-owner') {
const originalFs = getFsImplementation()
let paused = false
setFsImplementation({
...originalFs,
readlinkSync(path) {
const ownerPath = resolve(path)
const lockPath = `${settingsReadPath}.lock`
const ownerDirectory = dirname(ownerPath)
if (
!paused &&
basename(ownerPath) === 'owner.json' &&
(ownerDirectory === lockPath ||
ownerDirectory.startsWith(`${lockPath}.pending.`))
) {
paused = true
writeFileSync(readMarker, '')
waitForMarker(releaseMarker)
}
return originalFs.readlinkSync(path)
},
})
}
if (role === 'hold-lock' || role === 'hold-path-for') {
const { withSettingsFileTransactionSync } = await import(
'../../utils/settings/settingsFileTransaction.js'
)
withSettingsFileTransactionSync(settingsPath, () => {
writeFileSync(enteredMarker, '')
if (role === 'hold-path-for') {
Atomics.wait(waitBuffer, 0, 0, holdMs!)
} else {
waitForMarker(releaseMarker)
}
})
writeFileSync(completedMarker, '')
process.stdout.write(`${JSON.stringify({ ok: true })}\n`)
} else {
const { updateSettingsForSource } = await import(
'../../utils/settings/settings.js'
)
writeFileSync(enteredMarker, '')
const result = updateSettingsForSource('userSettings', {
env: { [key]: value },
})
writeFileSync(completedMarker, '')
process.stdout.write(
`${JSON.stringify({
ok: result.error === null,
error: result.error?.message,
})}\n`,
)
}
File diff suppressed because it is too large Load Diff
+81 -73
View File
@@ -19,9 +19,10 @@ import { readFileSync } from '../fileRead.js'
import { getFsImplementation, safeResolvePath } from '../fsOperations.js'
import { addFileGlobRuleToGitignore } from '../git/gitignore.js'
import { safeParseJSON } from '../json.js'
import { stripBOM } from '../jsonRead.js'
import { logError } from '../log.js'
import { getPlatform } from '../platform.js'
import { clone, jsonStringify } from '../slowOperations.js'
import { clone, jsonParse, jsonStringify } from '../slowOperations.js'
import { profileCheckpoint } from '../startupProfiler.js'
import {
type EditableSettingSource,
@@ -44,6 +45,7 @@ import {
setCachedSettingsForSource,
setSessionSettingsCache,
} from './settingsCache.js'
import { withSettingsFileTransactionSync } from './settingsFileTransaction.js'
import { type SettingsJson, SettingsSchema } from './types.js'
import {
filterInvalidModelPricing,
@@ -439,91 +441,97 @@ export function updateSettingsForSource(
}
try {
getFsImplementation().mkdirSync(dirname(filePath))
const validationError = withSettingsFileTransactionSync(
filePath,
targetPath => {
// The transaction merge base must bypass both process-local settings
// caches so a peer's completed update cannot be overwritten.
let existingSettings = parseSettingsFileUncached(targetPath).settings
// Try to get existing settings with validation. Bypass the per-source
// cache — mergeWith below mutates its target (including nested refs),
// and mutating the cached object would leak unpersisted state if the
// write fails before resetSettingsCache().
let existingSettings = getSettingsForSourceUncached(source)
// If validation failed, check if file exists with a JSON syntax error
if (!existingSettings) {
let content: string | null = null
try {
content = readFileSync(filePath)
} catch (e) {
if (!isENOENT(e)) {
throw e
}
// File doesn't exist — fall through to merge with empty settings
}
if (content !== null) {
const rawData = safeParseJSON(content)
if (rawData === null) {
// JSON syntax error - return validation error instead of overwriting
// safeParseJSON will already log the error, so we'll just return the error here
return {
error: new Error(
`Invalid JSON syntax in settings file at ${filePath}`,
),
// If validation failed, distinguish syntax errors from schema-invalid
// objects whose unknown fields still need to survive the merge.
if (!existingSettings) {
let content: string | null = null
try {
content = readFileSync(targetPath)
} catch (e) {
if (!isENOENT(e)) throw e
// File doesn't exist — fall through to merge with empty settings.
}
if (content !== null) {
let rawData: unknown
try {
rawData = jsonParse(stripBOM(content))
} catch (parseError) {
logError(parseError)
return new Error(
`Invalid JSON syntax in settings file at ${filePath}`,
)
}
if (
rawData === null ||
typeof rawData !== 'object' ||
Array.isArray(rawData)
) {
return new Error(
`Invalid settings document at ${filePath}: expected a JSON object`,
)
}
existingSettings = rawData as SettingsJson
logForDebugging(
`Using raw settings from ${filePath} due to validation failure`,
)
}
}
if (rawData && typeof rawData === 'object') {
existingSettings = rawData as SettingsJson
logForDebugging(
`Using raw settings from ${filePath} due to validation failure`,
)
}
}
}
const updatedSettings = mergeWith(
existingSettings || {},
settings,
(
_objValue: unknown,
srcValue: unknown,
key: string | number | symbol,
object: Record<string | number | symbol, unknown>,
) => {
// Handle undefined as deletion
if (srcValue === undefined && object && typeof key === 'string') {
delete object[key]
return undefined
}
// For arrays, always replace with the provided array
// This puts the responsibility on the caller to compute the desired final state
if (Array.isArray(srcValue)) {
return srcValue
}
// For non-arrays, let lodash handle the default merge behavior
return undefined
const updatedSettings = mergeWith(
existingSettings || {},
settings,
(
_objValue: unknown,
srcValue: unknown,
key: string | number | symbol,
object: Record<string | number | symbol, unknown>,
) => {
// Handle undefined as deletion
if (srcValue === undefined && object && typeof key === 'string') {
delete object[key]
return undefined
}
// For arrays, always replace with the provided array
// This puts the responsibility on the caller to compute the desired final state
if (Array.isArray(srcValue)) {
return srcValue
}
// For non-arrays, let lodash handle the default merge behavior
return undefined
},
)
writeFileSyncAndFlush_DEPRECATED(
targetPath,
jsonStringify(updatedSettings, null, 2) + '\n',
)
markInternalWrite(filePath)
resetSettingsCache()
return null
},
)
if (validationError) return { error: validationError }
// Mark this as an internal write before writing the file
markInternalWrite(filePath)
writeFileSyncAndFlush_DEPRECATED(
filePath,
jsonStringify(updatedSettings, null, 2) + '\n',
)
// Invalidate the session cache since settings have been updated
resetSettingsCache()
if (source === 'localSettings') {
if (source === 'localSettings' || source === 'projectSettings') {
// Okay to add to gitignore async without awaiting
const relativePath = getRelativeSettingsFilePathForSource(source)
if (source === 'localSettings') {
void addFileGlobRuleToGitignore(relativePath, getOriginalCwd())
}
void addFileGlobRuleToGitignore(
getRelativeSettingsFilePathForSource('localSettings'),
`${relativePath}.lock*`,
getOriginalCwd(),
)
}
} catch (e) {
const error = new Error(
`Failed to read raw settings from ${filePath}: ${e}`,
)
const error = new Error(`Failed to update settings at ${filePath}: ${e}`)
logError(error)
return { error }
}
@@ -0,0 +1,516 @@
import { randomUUID } from 'node:crypto'
import { spawnSync } from 'node:child_process'
import {
mkdirSync as mkdirExclusiveSync,
readFileSync as readProcessFileSync,
renameSync as renameLockSync,
rmSync as removeLockSync,
} from 'node:fs'
import { hostname } from 'node:os'
import { dirname, join, resolve } from 'node:path'
import { logForDebugging } from '../debug.js'
import { getErrnoCode } from '../errors.js'
import { writeFileSyncAndFlush_DEPRECATED } from '../file.js'
import {
getFsImplementation,
resolveDeepestExistingAncestorSync,
} from '../fsOperations.js'
import { markInternalWrite } from './internalWrites.js'
import { resetSettingsCache } from './settingsCache.js'
const SETTINGS_LOCK_RETRY_MS = 25
const SETTINGS_LOCK_CONTENTION_LOG_MS = 100
const SETTINGS_LOCK_WAIT_MS = 2_000
const SETTINGS_LOCK_HOST = hostname()
const SETTINGS_LOCK_OWNER_FILE = 'owner.json'
const waitBuffer = new Int32Array(new SharedArrayBuffer(4))
let settingsLockProcessStartId: string | undefined
type SettingsLockIdentityV1 = {
version: 1
host: string
pid: number
token: string
}
type SettingsLockIdentityV2 = {
version: 2
host: string
pid: number
token: string
processStartId: string
}
type SettingsLockIdentity = SettingsLockIdentityV1 | SettingsLockIdentityV2
type SettingsLockIdentityRead =
| { state: 'missing' }
| { state: 'invalid' }
| { state: 'valid'; identity: SettingsLockIdentity }
function sameIdentity(
left: SettingsLockIdentity,
right: SettingsLockIdentity,
): boolean {
if (
left.version === right.version &&
left.host === right.host &&
left.pid === right.pid &&
left.token === right.token
) {
return (
left.version === 1 ||
(right.version === 2 &&
left.processStartId === right.processStartId)
)
}
return false
}
function readLockIdentity(lockPath: string): SettingsLockIdentityRead {
let raw: string
try {
raw = getFsImplementation().readFileSync(
join(lockPath, SETTINGS_LOCK_OWNER_FILE),
{ encoding: 'utf8' },
)
} catch (error) {
if (getErrnoCode(error) !== 'ENOENT') return { state: 'invalid' }
try {
getFsImplementation().lstatSync(lockPath)
return { state: 'invalid' }
} catch (lockError) {
return getErrnoCode(lockError) === 'ENOENT'
? { state: 'missing' }
: { state: 'invalid' }
}
}
try {
const candidate = JSON.parse(raw) as Record<string, unknown>
if (
(candidate.version !== 1 && candidate.version !== 2) ||
typeof candidate.host !== 'string' ||
candidate.host.length === 0 ||
candidate.host.length > 255 ||
typeof candidate.pid !== 'number' ||
!Number.isSafeInteger(candidate.pid) ||
candidate.pid <= 0 ||
typeof candidate.token !== 'string' ||
!/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(
candidate.token,
)
) {
return { state: 'invalid' }
}
if (
candidate.version === 2 &&
(typeof candidate.processStartId !== 'string' ||
candidate.processStartId.length === 0 ||
candidate.processStartId.length > 512)
) {
return { state: 'invalid' }
}
return {
state: 'valid',
identity:
candidate.version === 1
? {
version: 1,
host: candidate.host,
pid: candidate.pid,
token: candidate.token,
}
: {
version: 2,
host: candidate.host,
pid: candidate.pid,
token: candidate.token,
processStartId: candidate.processStartId as string,
},
}
} catch {
return { state: 'invalid' }
}
}
function readLinuxProcessStartId(pid: number): string | null {
try {
const bootId = readProcessFileSync(
'/proc/sys/kernel/random/boot_id',
'utf8',
).trim()
const stat = readProcessFileSync(`/proc/${pid}/stat`, 'utf8')
const commandEnd = stat.lastIndexOf(')')
if (commandEnd < 0) return null
const fields = stat.slice(commandEnd + 1).trim().split(/\s+/)
const startTicks = fields[19]
if (
!/^[0-9a-f-]{36}$/i.test(bootId) ||
startTicks === undefined ||
!/^\d+$/.test(startTicks)
) {
return null
}
return `linux:${bootId}:${startTicks}`
} catch {
return null
}
}
function readCommandProcessStartId(
command: string,
args: string[],
prefix: string,
): string | null {
try {
const result = spawnSync(command, args, {
encoding: 'utf8',
env: {
...process.env,
LANG: 'C',
LC_ALL: 'C',
TZ: 'UTC',
},
timeout: 1_000,
windowsHide: true,
})
if (result.error || result.status !== 0) return null
const value = result.stdout.trim().replace(/\s+/g, ' ')
return value.length > 0 && value.length <= 480
? `${prefix}:${value}`
: null
} catch {
return null
}
}
function readProcessStartId(pid: number): string | null {
if (process.platform === 'linux') return readLinuxProcessStartId(pid)
if (process.platform === 'win32') {
return readCommandProcessStartId(
'powershell.exe',
[
'-NoProfile',
'-NonInteractive',
'-Command',
`(Get-Process -Id ${pid} -ErrorAction Stop).StartTime.ToUniversalTime().Ticks`,
],
'windows',
)
}
return readCommandProcessStartId(
'ps',
['-p', String(pid), '-o', 'lstart='],
'posix',
)
}
function currentProcessStartId(): string | null {
if (settingsLockProcessStartId !== undefined) {
return settingsLockProcessStartId
}
const processStartId = readProcessStartId(process.pid)
if (processStartId !== null) settingsLockProcessStartId = processStartId
return processStartId
}
function createLockIdentity(): SettingsLockIdentity {
const common = {
host: SETTINGS_LOCK_HOST,
pid: process.pid,
token: randomUUID(),
}
const processStartId = currentProcessStartId()
if (processStartId === null) {
throw Object.assign(
new Error('Unable to determine process identity for settings lock'),
{ code: 'EIDENTITY' },
)
}
return { version: 2, ...common, processStartId }
}
function isIdentityOwnerAlive(
identity: SettingsLockIdentity,
processStartIds: Map<string, string | null>,
): boolean {
// A different host may share this filesystem, but its process namespace is
// not observable here. Never reclaim that ownership on local PID evidence.
if (identity.host !== SETTINGS_LOCK_HOST) return true
try {
process.kill(identity.pid, 0)
} catch (error) {
if (getErrnoCode(error) === 'ESRCH') return false
}
if (identity.version === 1) return true
const cacheKey = `${identity.host}:${identity.pid}:${identity.token}:${identity.processStartId}`
let currentProcessStartId: string | null
if (processStartIds.has(cacheKey)) {
currentProcessStartId = processStartIds.get(cacheKey) ?? null
} else {
currentProcessStartId = readProcessStartId(identity.pid)
processStartIds.set(cacheKey, currentProcessStartId)
}
return (
currentProcessStartId === null ||
currentProcessStartId === identity.processStartId
)
}
function pendingLockPath(
lockPath: string,
owner: SettingsLockIdentity,
): string {
return `${lockPath}.pending.${owner.pid}.${owner.token}`
}
function removePendingLock(pendingPath: string): void {
try {
removeLockSync(pendingPath, { recursive: true, force: true })
} catch (error) {
logForDebugging(`Pending settings lock cleanup failed: ${error}`, {
level: 'error',
})
}
}
function preparePendingLock(
lockPath: string,
owner: SettingsLockIdentity,
): string {
// A crash can leave this private path behind. Its UUID cannot block another
// claimant, and removing it later could race a creator paused before publish.
const pendingPath = pendingLockPath(lockPath, owner)
mkdirExclusiveSync(pendingPath, { mode: 0o700 })
try {
writeFileSyncAndFlush_DEPRECATED(
join(pendingPath, SETTINGS_LOCK_OWNER_FILE),
JSON.stringify(owner),
{ encoding: 'utf8', mode: 0o600 },
)
} catch (error) {
removePendingLock(pendingPath)
throw error
}
return pendingPath
}
function tryPublishPendingLock(
pendingPath: string,
lockPath: string,
): boolean {
try {
renameLockSync(pendingPath, lockPath)
return true
} catch (error) {
const code = getErrnoCode(error)
if (code === 'EEXIST' || code === 'ENOTEMPTY' || code === 'EPERM') {
try {
getFsImplementation().lstatSync(lockPath)
return false
} catch (statError) {
if (getErrnoCode(statError) === 'ENOENT') {
// The observed owner released between rename and confirmation.
return false
}
throw statError
}
}
throw error
}
}
function recoveredLockPath(
lockPath: string,
owner: SettingsLockIdentity,
): string {
return `${lockPath}.recovered.${owner.pid}.${owner.token}`
}
function tryRecoverDeadLock(
lockPath: string,
expectedOwner: SettingsLockIdentity,
): boolean {
const recoveredPath = recoveredLockPath(lockPath, expectedOwner)
try {
renameLockSync(lockPath, recoveredPath)
} catch (error) {
const code = getErrnoCode(error)
if (code === 'ENOENT' || code === 'EEXIST' || code === 'ENOTEMPTY') {
return false
}
if (code === 'EPERM') {
try {
getFsImplementation().lstatSync(recoveredPath)
return false
} catch (statError) {
if (getErrnoCode(statError) !== 'ENOENT') throw statError
}
}
throw error
}
const recoveredOwner = readLockIdentity(recoveredPath)
if (
recoveredOwner.state !== 'valid' ||
!sameIdentity(recoveredOwner.identity, expectedOwner)
) {
throw Object.assign(
new Error('Settings file lock changed during dead-owner recovery'),
{ code: 'ECOMPROMISED' },
)
}
// Keep the deterministic non-empty tombstone. A contender that read this
// dead owner before the rename can only target the same path, so rename will
// refuse to replace the tombstone instead of moving a successor's live lock.
logForDebugging(
`Recovered settings file lock from exited process ${expectedOwner.pid}`,
{ level: 'warn' },
)
return true
}
function tryAcquireSettingsLock(
lockPath: string,
pendingPath: string,
processStartIds: Map<string, string | null>,
): boolean {
if (tryPublishPendingLock(pendingPath, lockPath)) return true
const currentOwner = readLockIdentity(lockPath)
if (
currentOwner.state !== 'valid' ||
isIdentityOwnerAlive(currentOwner.identity, processStartIds)
) {
return false
}
if (!tryRecoverDeadLock(lockPath, currentOwner.identity)) return false
return tryPublishPendingLock(pendingPath, lockPath)
}
function releaseOwnedLock(
lockPath: string,
owner: SettingsLockIdentity,
): void {
const currentOwner = readLockIdentity(lockPath)
if (
currentOwner.state !== 'valid' ||
!sameIdentity(currentOwner.identity, owner)
) {
throw Object.assign(
new Error('Settings file lock ownership changed before release'),
{ code: 'ECOMPROMISED' },
)
}
removeLockSync(lockPath, { recursive: true })
}
function describeCurrentOwner(lockPath: string): string {
const currentOwner = readLockIdentity(lockPath)
if (currentOwner.state !== 'valid') return 'an unknown owner'
return `${currentOwner.identity.host} process ${currentOwner.identity.pid}`
}
function resolveSettingsMutationTarget(requestedPath: string): string {
const fs = getFsImplementation()
const absolutePath = resolve(requestedPath)
try {
return fs.realpathSync(absolutePath)
} catch (error) {
if (getErrnoCode(error) !== 'ENOENT') throw error
return (
resolveDeepestExistingAncestorSync(fs, absolutePath) ?? absolutePath
)
}
}
function acquireSettingsLock(targetPath: string): () => void {
const lockPath = `${targetPath}.lock`
const owner = createLockIdentity()
const pendingPath = preparePendingLock(lockPath, owner)
const startedAt = performance.now()
const deadline = startedAt + SETTINGS_LOCK_WAIT_MS
let reportedContention = false
let acquired = false
const processStartIds = new Map<string, string | null>()
try {
while (true) {
if (tryAcquireSettingsLock(lockPath, pendingPath, processStartIds)) {
acquired = true
return () => releaseOwnedLock(lockPath, owner)
}
const now = performance.now()
const elapsed = now - startedAt
if (!reportedContention && elapsed >= SETTINGS_LOCK_CONTENTION_LOG_MS) {
reportedContention = true
logForDebugging(
`Settings file lock contention has lasted ${Math.round(elapsed)}ms`,
{ level: 'warn' },
)
}
const remaining = deadline - now
if (remaining <= 0) {
throw Object.assign(
new Error(
`Timed out after ${SETTINGS_LOCK_WAIT_MS}ms waiting for settings lock ${lockPath}, held by ${describeCurrentOwner(lockPath)}. If that process is known to have exited, remove the lock directory before retrying.`,
),
{ code: 'ELOCKED' },
)
}
Atomics.wait(
waitBuffer,
0,
0,
Math.min(SETTINGS_LOCK_RETRY_MS, remaining),
)
}
} finally {
if (!acquired) removePendingLock(pendingPath)
}
}
/**
* Run one synchronous settings-file operation under its physical-target lock.
* Calls for the same target must not be nested; contention remains bounded by
* the normal acquisition deadline.
*/
export function withSettingsFileTransactionSync<T>(
requestedPath: string,
operation: (targetPath: string) => T,
): T {
const targetPath = resolveSettingsMutationTarget(requestedPath)
getFsImplementation().mkdirSync(dirname(targetPath))
const release = acquireSettingsLock(targetPath)
let result: T
try {
result = operation(targetPath)
} catch (operationError) {
try {
release()
} catch (releaseError) {
logForDebugging(`Settings lock release failed: ${releaseError}`, {
level: 'error',
})
}
throw operationError
}
release()
return result
}
/** Replace a complete settings document using the shared transaction identity. */
export function replaceSettingsFileSync(
requestedPath: string,
content: string,
): void {
withSettingsFileTransactionSync(requestedPath, targetPath => {
writeFileSyncAndFlush_DEPRECATED(targetPath, content)
markInternalWrite(requestedPath)
resetSettingsCache()
})
}