diff --git a/packages/settings/settings-local/src/index.ts b/packages/settings/settings-local/src/index.ts index 04ba6808a3..d0d1497b16 100644 --- a/packages/settings/settings-local/src/index.ts +++ b/packages/settings/settings-local/src/index.ts @@ -214,6 +214,8 @@ export class SettingsLocal extends Settings { // carries owner-only permissions that survive the rename — a document that // may hold personal values is never world-readable and never a symlink. const temp = `${this.spec.filename}.${randomBytes(6).toString('hex')}.tmp` + // TODO(settings-atomic-durability): Use a replacement that fsyncs the file + // and parent directory and preserves owner-only permissions on Windows. try { await writeFile(temp, output, { mode: 0o600, flag: 'wx' }) await rename(temp, this.spec.filename) @@ -248,6 +250,8 @@ export class SettingsLocal extends Settings { // is free right now, so retry without burning backoff or deadline. if (ageMs === undefined) continue if (ageMs > LOCK_STALE_MS) { + // TODO(settings-lock-ownership): Replace age-only takeover with ownership-safe + // acquisition and release so a slow writer cannot remove a successor's lock. this.ctx.logger.warn('settings-local: breaking a stale writer lock at %s', lockPath) await rm(lockPath, { force: true }) continue diff --git a/packages/settings/settings/src/index.ts b/packages/settings/settings/src/index.ts index 27decc7db5..76c1082f3f 100644 --- a/packages/settings/settings/src/index.ts +++ b/packages/settings/settings/src/index.ts @@ -43,6 +43,8 @@ export interface SettingsRegisterOptions { /** One registered namespace as surfaced to configuration UIs. */ export interface SettingsDescriptor { + // TODO(settings-namespace-vocabulary): Rename `ns` to `namespace` across the + // public seam, provider contract, implementations, tests, and consumers. /** The registered namespace. */ ns: SettingsNamespace /** Serialized schemastery schema (`schema.toJSON()`). */ @@ -181,6 +183,8 @@ function cloneJsonShaped( if (isPlainObject(value)) { if (visiting.has(value)) throw reject('a circular reference', path) visiting.add(value) + // TODO(settings-json-properties): Use property-safe construction here and + // in mergeLayers so valid JSON keys such as "__proto__" remain own data. const out: Record = {} for (const [key, entry] of Object.entries(value)) { if (entry === undefined) continue @@ -321,6 +325,8 @@ export abstract class Settings extends Service { } this.ctx.effect(() => { this.registrations.set(ns, registration) + // TODO(settings-registration-quiescence): Deactivate every watcher and await + // its tail on disposal so callbacks cannot outlive the registrant fiber. return () => this.registrations.delete(ns) }, `settings.register(${JSON.stringify(String(ns))})`) return { @@ -425,6 +431,8 @@ export abstract class Settings extends Service { // only when this registration is still the namespace owner — a fiber // disposed (or replaced) mid-persist must not receive the notification. this.document[ns] = section + // TODO(settings-replacement-resync): Re-resolve any replacement registration + // from this persisted section so an old in-flight write cannot leave it stale. if (this.registrations.get(ns) === registration && !this.isStopped()) { this.commit(registration, next, 'update') }