docs(settings): mark deferred review follow-ups
This commit is contained in:
@@ -214,6 +214,8 @@ export class SettingsLocal extends Settings {
|
|||||||
// carries owner-only permissions that survive the rename — a document that
|
// carries owner-only permissions that survive the rename — a document that
|
||||||
// may hold personal values is never world-readable and never a symlink.
|
// may hold personal values is never world-readable and never a symlink.
|
||||||
const temp = `${this.spec.filename}.${randomBytes(6).toString('hex')}.tmp`
|
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 {
|
try {
|
||||||
await writeFile(temp, output, { mode: 0o600, flag: 'wx' })
|
await writeFile(temp, output, { mode: 0o600, flag: 'wx' })
|
||||||
await rename(temp, this.spec.filename)
|
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.
|
// is free right now, so retry without burning backoff or deadline.
|
||||||
if (ageMs === undefined) continue
|
if (ageMs === undefined) continue
|
||||||
if (ageMs > LOCK_STALE_MS) {
|
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)
|
this.ctx.logger.warn('settings-local: breaking a stale writer lock at %s', lockPath)
|
||||||
await rm(lockPath, { force: true })
|
await rm(lockPath, { force: true })
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -43,6 +43,8 @@ export interface SettingsRegisterOptions<T> {
|
|||||||
|
|
||||||
/** One registered namespace as surfaced to configuration UIs. */
|
/** One registered namespace as surfaced to configuration UIs. */
|
||||||
export interface SettingsDescriptor {
|
export interface SettingsDescriptor {
|
||||||
|
// TODO(settings-namespace-vocabulary): Rename `ns` to `namespace` across the
|
||||||
|
// public seam, provider contract, implementations, tests, and consumers.
|
||||||
/** The registered namespace. */
|
/** The registered namespace. */
|
||||||
ns: SettingsNamespace
|
ns: SettingsNamespace
|
||||||
/** Serialized schemastery schema (`schema.toJSON()`). */
|
/** Serialized schemastery schema (`schema.toJSON()`). */
|
||||||
@@ -181,6 +183,8 @@ function cloneJsonShaped(
|
|||||||
if (isPlainObject(value)) {
|
if (isPlainObject(value)) {
|
||||||
if (visiting.has(value)) throw reject('a circular reference', path)
|
if (visiting.has(value)) throw reject('a circular reference', path)
|
||||||
visiting.add(value)
|
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<string, unknown> = {}
|
const out: Record<string, unknown> = {}
|
||||||
for (const [key, entry] of Object.entries(value)) {
|
for (const [key, entry] of Object.entries(value)) {
|
||||||
if (entry === undefined) continue
|
if (entry === undefined) continue
|
||||||
@@ -321,6 +325,8 @@ export abstract class Settings extends Service {
|
|||||||
}
|
}
|
||||||
this.ctx.effect(() => {
|
this.ctx.effect(() => {
|
||||||
this.registrations.set(ns, registration)
|
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)
|
return () => this.registrations.delete(ns)
|
||||||
}, `settings.register(${JSON.stringify(String(ns))})`)
|
}, `settings.register(${JSON.stringify(String(ns))})`)
|
||||||
return {
|
return {
|
||||||
@@ -425,6 +431,8 @@ export abstract class Settings extends Service {
|
|||||||
// only when this registration is still the namespace owner — a fiber
|
// only when this registration is still the namespace owner — a fiber
|
||||||
// disposed (or replaced) mid-persist must not receive the notification.
|
// disposed (or replaced) mid-persist must not receive the notification.
|
||||||
this.document[ns] = section
|
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()) {
|
if (this.registrations.get(ns) === registration && !this.isStopped()) {
|
||||||
this.commit(registration, next, 'update')
|
this.commit(registration, next, 'update')
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user