From f44b4db1f22b44b5d280065ad0bd8170d36b8d1a Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Tue, 28 Jul 2026 18:18:34 +0800 Subject: [PATCH] fix(settings): harden seam and provider per review findings Confirmed and fixed, each with a regression test that failed first: - Concurrent update() lost patches (merge over one stale snapshot): per-namespace serialized write queues; a failed write cannot poison the queue for later writers. - Fixed-name .tmp write followed planted symlinks and kept stale modes: random-suffix sibling, exclusive-create (wx), 0600, cleanup on failure, then rename. - A throwing settings/updated listener escaped commit and permanently wedged the provider reload chain (rejected refreshTask): commit now contains listener failures (INVARIANT-coded errors still propagate), async watcher rejections are adopted and contained (watch callbacks are officially void | Promise), and the provider chains refreshes on a settled tail with an error log. - No way to remove a user override: scope/service replace(section) sets the user section wholesale; replace({}) re-inherits base and schema defaults. - The three-primitive provider contract did not hold (base never called load()): the base Service.init loads and publishes once; settings-local delegates via yield* super[Service.init](). - Dispose did not quiesce: teardown flags closed, closes the watcher, then awaits queued/in-flight reloads; closed is re-checked across await points. - Invariant now checks the authoritative relation with the seam's own deepEqualJson: emitted next must equal settings.get(ns), and next/prev must differ structurally (cosmokit dependency dropped). - New docs/core-data-structures/settings.{md,zh.md} with type-equiv blocks + manifest entries; catalog types moved from exemptions to LINK_MAP; website page registered. Both packages stay at per-file 100% coverage. --- docs/config-catalog.md | 2 +- docs/cordis-catalog/events.md | 4 +- docs/cordis-catalog/services.md | 18 ++- docs/core-data-structures/settings.i18n.yaml | 6 + docs/core-data-structures/settings.md | 94 +++++++++++++ docs/core-data-structures/settings.zh.md | 94 +++++++++++++ docs/event-producer-consumer.md | 2 +- .../cordis/tool-cordis/src/api-catalog.ts | 8 +- .../settings/settings-local/README.i18n.yaml | 4 +- packages/settings/settings-local/README.md | 3 +- packages/settings/settings-local/README.zh.md | 3 +- packages/settings/settings-local/src/index.ts | 58 +++++++-- .../tests/loader-composition.spec.ts | 2 +- .../settings-local/tests/local.spec.ts | 33 ++++- .../settings-local/tests/watcher.spec.ts | 54 ++++++++ packages/settings/settings/README.i18n.yaml | 4 +- packages/settings/settings/README.md | 9 +- packages/settings/settings/README.zh.md | 9 +- packages/settings/settings/src/index.ts | 123 +++++++++++++++--- packages/settings/settings/src/invariant.ts | 13 +- .../settings/settings/tests/invariant.spec.ts | 11 ++ packages/settings/settings/tests/memory.ts | 17 +-- .../settings/settings/tests/settings.spec.ts | 114 +++++++++++++++- scripts/gen-cordis-catalog.ts | 10 +- scripts/project-doc-site.spec.ts | 2 +- scripts/type-equiv.manifest.json | 30 +++++ website/docs.ts | 1 + 27 files changed, 655 insertions(+), 73 deletions(-) create mode 100644 docs/core-data-structures/settings.i18n.yaml create mode 100644 docs/core-data-structures/settings.md create mode 100644 docs/core-data-structures/settings.zh.md diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 1305dd7270..2a1eb47da5 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -1193,7 +1193,7 @@ export interface Config { } ``` -Source: [`packages/settings/settings-local/src/index.ts:18`](../packages/settings/settings-local/src/index.ts) +Source: [`packages/settings/settings-local/src/index.ts:19`](../packages/settings/settings-local/src/index.ts) ## `@deepseek-ai/dsh-skill` diff --git a/docs/cordis-catalog/events.md b/docs/cordis-catalog/events.md index aea04f4acd..74e1e6cfda 100644 --- a/docs/cordis-catalog/events.md +++ b/docs/cordis-catalog/events.md @@ -660,7 +660,9 @@ Committed change to one registered namespace's resolved value. Emitted after the 'settings/updated'(ns: SettingsNamespace, next: unknown, prev: unknown, source: SettingsUpdateSource): void ``` -Source: [`packages/settings/settings/src/index.ts:90`](../../packages/settings/settings/src/index.ts) +Types: [SettingsNamespace](../core-data-structures/settings.md) · [SettingsUpdateSource](../core-data-structures/settings.md) + +Source: [`packages/settings/settings/src/index.ts:96`](../../packages/settings/settings/src/index.ts) ## `slash/*` diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index a0271083d6..e915952ffb 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -1421,14 +1421,28 @@ get(ns: SettingsNamespace): unknown /** * Merge a patch into one registered namespace's user layer, validate the * resolved candidate, persist through the provider, then commit and emit. - * A validation failure rejects before anything is persisted. + * A validation failure rejects before anything is persisted. Writes to one + * namespace are serialized: concurrent updates apply in call order, each + * merging over the previous write's committed section. * @param ns - the registered namespace to update. * @param patch - plain-object patch over the user section. */ async update(ns: SettingsNamespace, patch: object): Promise + +/** + * Replace one registered namespace's user section wholesale, validate, + * persist, then commit and emit. Keys absent from `section` fall back to the + * composition `base` and schema defaults — this is the removal/reset path a + * merge-only patch cannot express (`replace({})` re-inherits everything). + * @param ns - the registered namespace to replace. + * @param section - the complete next user section. + */ +async replace(ns: SettingsNamespace, section: object): Promise ``` -Source: [`packages/settings/settings/src/index.ts:140`](../../packages/settings/settings/src/index.ts) +Types: [SettingsDescriptor](../core-data-structures/settings.md) · [SettingsNamespace](../core-data-structures/settings.md) · [SettingsRegisterOptions](../core-data-structures/settings.md) · [SettingsScope](../core-data-structures/settings.md) + +Source: [`packages/settings/settings/src/index.ts:168`](../../packages/settings/settings/src/index.ts) ## `ctx.skills` — `SkillService` diff --git a/docs/core-data-structures/settings.i18n.yaml b/docs/core-data-structures/settings.i18n.yaml new file mode 100644 index 0000000000..6dc8750dfb --- /dev/null +++ b/docs/core-data-structures/settings.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write docs/core-data-structures/settings.md +settings.md: 851087065c627f2390041dd6e69273e31be3917d +settings.zh.md: 955c4fbf7c147b3d0a8be62c33c031d7bd2c4ba2 diff --git a/docs/core-data-structures/settings.md b/docs/core-data-structures/settings.md new file mode 100644 index 0000000000..851087065c --- /dev/null +++ b/docs/core-data-structures/settings.md @@ -0,0 +1,94 @@ +# User Settings + +English | [中文](settings.zh.md) + +The user-settings seam of [dsh-settings](../../packages/settings/settings) holds one user-owned document of per-namespace sections and resolves each registered namespace as schema defaults, then the registrant's composition `base`, then the user section. Providers such as [dsh-settings-local](../../packages/settings/settings-local) store the raw document and push external edits; consumer plugins register a schema and read or observe the resolved value. Composition config stays in `cordis.yml` — a namespace carries only the user-editable subset. + +Source: [`packages/settings/settings/src/index.ts`](../../packages/settings/settings/src/index.ts) + +## Identity + +A namespace names one plugin-owned section of the user document. The brand keeps namespaces from mixing with other cross-boundary ids; construction validates the lowercase kebab-case shape. + +```ts type-equiv +/** Nominal id of one registered settings namespace. */ +type SettingsNamespace = Branded<'SettingsNamespace'> +``` + +## Registration + +Registration binds a schemastery schema to a namespace on the calling plugin's fiber — disposing that fiber removes the namespace and its observers. The options carry the composition layer and the owner's effect timing. + +```ts type-equiv +/** Registration options beyond the namespace schema. */ +interface SettingsRegisterOptions { + /** Composition-layer values resolved below the user layer (entry-config subset). */ + base?: Partial + /** Owner's effect timing, surfaced to configuration UIs; defaults to `live`. */ + applies?: SettingsApplies +} +``` + +`applies` is a UI hint, not a mechanism: a `restart` owner simply never watches, so its value is read once at construction and configuration surfaces can badge the pending change. + +```ts type-equiv +/** When a namespace's changes take effect for its owner. */ +type SettingsApplies = 'live' | 'restart' +``` + +## Owner scope + +The scope is the owner-facing handle. `update` merges a sparse patch over the user section only (never into `base`); `replace` sets the section wholesale, which is the removal/reset path — keys absent from the replacement re-inherit `base` and schema defaults. Writes to one namespace are serialized in call order, and resolved values are deep-frozen snapshots. + +```ts type-equiv +/** Owner-facing handle for one registered namespace. */ +interface SettingsScope { + /** Current resolved value: schema defaults, then `base`, then the user layer. */ + get(): T + /** + * Observe committed changes to this namespace's resolved value. A callback + * may be async; a rejection is contained and logged like a sync throw. + * @param callback - invoked after each commit with the next and previous values. + * @returns the disposer removing this observer. + */ + watch(callback: (next: T, prev: T) => void | Promise): () => void + /** + * Merge a partial patch into this namespace's user layer and persist it. + * @param patch - plain-object patch over the user section. + */ + update(patch: object): Promise + /** + * Replace this namespace's user section wholesale; absent keys re-inherit + * the composition `base` and schema defaults (`replace({})` resets all). + * @param section - the complete next user section. + */ + replace(section: object): Promise +} +``` + +## Descriptors + +`describe()` serializes every registered namespace for configuration surfaces: the schemastery `toJSON()` envelope drives schema-rendered forms, and the resolved value fills them. + +```ts type-equiv +/** One registered namespace as surfaced to configuration UIs. */ +interface SettingsDescriptor { + /** The registered namespace. */ + ns: SettingsNamespace + /** Serialized schemastery schema (`schema.toJSON()`). */ + schema: unknown + /** Current resolved value. */ + value: unknown + /** Owner's declared effect timing. */ + applies: SettingsApplies +} +``` + +## Change commits + +Every committed change — an in-process write or an externally observed provider edit — emits `settings/updated (ns, next, prev, source)` after the new value is authoritative, and never when the resolved value is deep-equal. The source tag separates the two entry paths. + +```ts type-equiv +/** Origin of one committed settings change. */ +type SettingsUpdateSource = 'update' | 'provider' +``` diff --git a/docs/core-data-structures/settings.zh.md b/docs/core-data-structures/settings.zh.md new file mode 100644 index 0000000000..955c4fbf7c --- /dev/null +++ b/docs/core-data-structures/settings.zh.md @@ -0,0 +1,94 @@ +# 用户设置 + +[English](settings.md) | 中文 + +[dsh-settings](../../packages/settings/settings) 的用户设置 seam 持有一份按 namespace 分节的用户文档，并把每个已注册 namespace 解析为：schema 默认值，然后注册方的组合 `base`，最后用户分节。[dsh-settings-local](../../packages/settings/settings-local) 这类 provider 存储原始文档并推送外部编辑；消费插件注册 schema 后读取或观察解析值。组合配置仍留在 `cordis.yml`——namespace 只承载用户可编辑子集。 + +Source: [`packages/settings/settings/src/index.ts`](../../packages/settings/settings/src/index.ts) + +## 标识 + +namespace 命名用户文档中一个插件所有的分节。brand 使其不与其他跨边界 id 混用；构造时校验小写 kebab-case 形态。 + +```ts type-equiv +/** Nominal id of one registered settings namespace. */ +type SettingsNamespace = Branded<'SettingsNamespace'> +``` + +## 注册 + +注册把 schemastery schema 绑定到调用方插件 fiber 上的 namespace——dispose 该 fiber 即移除 namespace 及其观察者。options 携带组合层与 owner 的生效时机。 + +```ts type-equiv +/** Registration options beyond the namespace schema. */ +interface SettingsRegisterOptions { + /** Composition-layer values resolved below the user layer (entry-config subset). */ + base?: Partial + /** Owner's effect timing, surfaced to configuration UIs; defaults to `live`. */ + applies?: SettingsApplies +} +``` + +`applies` 是 UI 提示而非机制：`restart` 的 owner 只是从不 watch，其值在构造期读取一次，配置界面可为待生效变更加标。 + +```ts type-equiv +/** When a namespace's changes take effect for its owner. */ +type SettingsApplies = 'live' | 'restart' +``` + +## Owner scope + +scope 是面向 owner 的句柄。`update` 把稀疏 patch 只合并进用户分节（绝不进 `base`）；`replace` 整体替换分节，是删除/重置路径——替换中缺席的键重新继承 `base` 与 schema 默认值。同一 namespace 的写入按调用顺序串行，解析值是深冻结快照。 + +```ts type-equiv +/** Owner-facing handle for one registered namespace. */ +interface SettingsScope { + /** Current resolved value: schema defaults, then `base`, then the user layer. */ + get(): T + /** + * Observe committed changes to this namespace's resolved value. A callback + * may be async; a rejection is contained and logged like a sync throw. + * @param callback - invoked after each commit with the next and previous values. + * @returns the disposer removing this observer. + */ + watch(callback: (next: T, prev: T) => void | Promise): () => void + /** + * Merge a partial patch into this namespace's user layer and persist it. + * @param patch - plain-object patch over the user section. + */ + update(patch: object): Promise + /** + * Replace this namespace's user section wholesale; absent keys re-inherit + * the composition `base` and schema defaults (`replace({})` resets all). + * @param section - the complete next user section. + */ + replace(section: object): Promise +} +``` + +## 描述符 + +`describe()` 为配置界面序列化每个已注册 namespace：schemastery 的 `toJSON()` 信封驱动 schema 渲染的表单，解析值填充表单。 + +```ts type-equiv +/** One registered namespace as surfaced to configuration UIs. */ +interface SettingsDescriptor { + /** The registered namespace. */ + ns: SettingsNamespace + /** Serialized schemastery schema (`schema.toJSON()`). */ + schema: unknown + /** Current resolved value. */ + value: unknown + /** Owner's declared effect timing. */ + applies: SettingsApplies +} +``` + +## 变更提交 + +每次提交的变更——进程内写入或 provider 观察到的外部编辑——在新值成为权威值之后发出 `settings/updated (ns, next, prev, source)`，解析值深相等时绝不发出。source 标记区分两条入口路径。 + +```ts type-equiv +/** Origin of one committed settings change. */ +type SettingsUpdateSource = 'update' | 'provider' +``` diff --git a/docs/event-producer-consumer.md b/docs/event-producer-consumer.md index 49bdf3fe35..e3eb1d9435 100644 --- a/docs/event-producer-consumer.md +++ b/docs/event-producer-consumer.md @@ -35,7 +35,7 @@ This matrix shows which packages dispatch each harness-owned event and which pac | `session/disposed` | `emit` | [`packages/core/session/src/index.ts:80`](../packages/core/session/src/index.ts) | [`session`](../packages/core/session) (`events.dispatch`) | [`agent-loop`](../packages/core/agent-loop), `apiproxy`, [`session-persistence`](../packages/session-persistence/session-persistence), [`session-telemetry`](../packages/telemetry/session-telemetry), [`session-title`](../packages/session-title/session-title) | | `session/event` | `emit` | [`packages/core/session/src/index.ts:92`](../packages/core/session/src/index.ts) | [`session`](../packages/core/session) (`events.dispatch`) | [`acp`](../packages/acp/acp), `apiproxy`, [`cli-demo`](../packages/examples/cli-demo), [`compact`](../packages/compact/compact), [`compact-basic`](../packages/compact/compact-basic), [`goal`](../packages/goal/goal), [`goal-session`](../packages/goal/goal-session), [`hook-protocol`](../packages/hooks/hook-protocol), [`jsonrpc`](../packages/ui/jsonrpc), [`plan-mode`](../packages/plan/plan-mode), [`session`](../packages/core/session), [`session-persistence`](../packages/session-persistence/session-persistence), [`session-telemetry`](../packages/telemetry/session-telemetry), [`session-title`](../packages/session-title/session-title), [`token-meter`](../packages/llm/token-meter), [`tools`](../packages/core/tools), [`tui`](../packages/ui/tui), [`user-approval`](../packages/ui/user-approval), [`workspace-context`](../packages/context/workspace-context) | | `session/flush` | `parallel` | [`packages/core/session/src/index.ts:102`](../packages/core/session/src/index.ts) | [`session`](../packages/core/session) (`events.dispatch`) | [`session-persistence`](../packages/session-persistence/session-persistence), [`session-telemetry`](../packages/telemetry/session-telemetry) | -| `settings/updated` | `emit` | [`packages/settings/settings/src/index.ts:90`](../packages/settings/settings/src/index.ts) | [`settings`](../packages/settings/settings) (`emit`) | [`settings`](../packages/settings/settings) | +| `settings/updated` | `emit` | [`packages/settings/settings/src/index.ts:96`](../packages/settings/settings/src/index.ts) | [`settings`](../packages/settings/settings) (`emit`) | [`settings`](../packages/settings/settings) | | `slash/input-begin-command` | `bail` | [`packages/client/ui-slash/src/types.ts:230`](../packages/client/ui-slash/src/types.ts) | - | `ui-conversation` | | `slash/input-consume-token` | `bail` | [`packages/client/ui-slash/src/types.ts:244`](../packages/client/ui-slash/src/types.ts) | - | `ui-conversation` | | `slash/input-insert-reference` | `bail` | [`packages/client/ui-slash/src/types.ts:237`](../packages/client/ui-slash/src/types.ts) | - | `ui-conversation` | diff --git a/packages/cordis/tool-cordis/src/api-catalog.ts b/packages/cordis/tool-cordis/src/api-catalog.ts index 81e3c79d05..4190a94a90 100644 --- a/packages/cordis/tool-cordis/src/api-catalog.ts +++ b/packages/cordis/tool-cordis/src/api-catalog.ts @@ -684,7 +684,11 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ }, { signature: 'async update(ns: SettingsNamespace, patch: object): Promise', - jsDoc: '/**\n * Merge a patch into one registered namespace\'s user layer, validate the\n * resolved candidate, persist through the provider, then commit and emit.\n * A validation failure rejects before anything is persisted.\n * @param ns - the registered namespace to update.\n * @param patch - plain-object patch over the user section.\n */', + jsDoc: '/**\n * Merge a patch into one registered namespace\'s user layer, validate the\n * resolved candidate, persist through the provider, then commit and emit.\n * A validation failure rejects before anything is persisted. Writes to one\n * namespace are serialized: concurrent updates apply in call order, each\n * merging over the previous write\'s committed section.\n * @param ns - the registered namespace to update.\n * @param patch - plain-object patch over the user section.\n */', + }, + { + signature: 'async replace(ns: SettingsNamespace, section: object): Promise', + jsDoc: '/**\n * Replace one registered namespace\'s user section wholesale, validate,\n * persist, then commit and emit. Keys absent from `section` fall back to the\n * composition `base` and schema defaults — this is the removal/reset path a\n * merge-only patch cannot express (`replace({})` re-inherits everything).\n * @param ns - the registered namespace to replace.\n * @param section - the complete next user section.\n */', }, ], }, @@ -2228,7 +2232,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [ }, { name: 'SettingsScope', - declaration: 'export interface SettingsScope {\n get(): T;\n watch(callback: (next: T, prev: T) => void): () => void;\n update(patch: object): Promise;\n}', + declaration: 'export interface SettingsScope {\n get(): T;\n watch(callback: (next: T, prev: T) => void | Promise): () => void;\n update(patch: object): Promise;\n replace(section: object): Promise;\n}', }, { name: 'SkillCandidate', diff --git a/packages/settings/settings-local/README.i18n.yaml b/packages/settings/settings-local/README.i18n.yaml index f7ca57e86e..455255941a 100644 --- a/packages/settings/settings-local/README.i18n.yaml +++ b/packages/settings/settings-local/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/settings/settings-local/README.md -README.md: 90428f98055d8e49fa1ec54e571453db2e0a5054 -README.zh.md: 3532d6cee99cb46f54e23889ef1bb54b2548ecfa +README.md: 9d0aa3982507ca16b382aaa918ca1536a98e29ea +README.zh.md: 075de7ee5b3e0ef0a4ee27eeb8cb098ca0d73456 diff --git a/packages/settings/settings-local/README.md b/packages/settings/settings-local/README.md index 90428f9805..9d0aa39825 100644 --- a/packages/settings/settings-local/README.md +++ b/packages/settings/settings-local/README.md @@ -18,7 +18,8 @@ Defaulting is one explicit `resolveSpec(config)` step; an unsupported extension ## Behavior - **Boot fails loud, reload keeps last-good.** An existing-but-invalid document fails plugin load; once live, an unreadable or unparsable edit warns and keeps the last good sections. A missing document resolves every namespace from defaults and `base`; deleting it publishes the same empty state. -- **Write-back is atomic and owner-only.** `persist` writes `.tmp` with mode `0600` and renames over the target. YAML writes patch one namespace in the comment-preserving document; JSON re-serializes. +- **Write-back is atomic, owner-only, and symlink-proof.** `persist` exclusive-creates a random-suffix temp sibling with mode `0600` (`wx` refuses to follow a planted symlink) and renames over the target, cleaning the temp up on failure. YAML writes patch one namespace in the comment-preserving document; JSON re-serializes. +- **Dispose quiesces.** Teardown stops accepting watcher events, closes the watcher, then waits out any queued or in-flight reload, so nothing publishes after disposal. - **Self-write suppression by content.** The provider caches the last good text; a watcher event whose content equals the cache (its own write included) is a no-op. ## Model Experience diff --git a/packages/settings/settings-local/README.zh.md b/packages/settings/settings-local/README.zh.md index 3532d6cee9..075de7ee5b 100644 --- a/packages/settings/settings-local/README.zh.md +++ b/packages/settings/settings-local/README.zh.md @@ -18,7 +18,8 @@ ## 行为 - **启动报错响亮，重载保留最后可用值。** 存在但非法的文档使插件加载失败；运行中不可读或不可解析的编辑只告警并保留最后可用分节。文档缺失时所有 namespace 按默认值与 `base` 解析；删除文档发布同样的空状态。 -- **写回原子且仅属主可读。** `persist` 以 `0600` 权限写 `.tmp` 后 rename 覆盖目标。YAML 写回在保留注释的文档里只修补目标 namespace；JSON 重新序列化。 +- **写回原子、仅属主可读、抗符号链接。** `persist` 以 `0600` 权限独占创建随机后缀临时同级文件（`wx` 拒绝跟随预埋符号链接）后 rename 覆盖目标，失败时清理临时文件。YAML 写回在保留注释的文档里只修补目标 namespace；JSON 重新序列化。 +- **Dispose 保证静止。** 卸载先停止接收 watcher 事件、关闭 watcher，再等完排队与进行中的重载，之后不再有任何发布。 - **按内容抑制自写。** provider 缓存最后可用文本；watcher 事件内容与缓存相同（含自己的写入）即为 no-op。 ## Model Experience diff --git a/packages/settings/settings-local/src/index.ts b/packages/settings/settings-local/src/index.ts index 421411ea50..2c020da63c 100644 --- a/packages/settings/settings-local/src/index.ts +++ b/packages/settings/settings-local/src/index.ts @@ -8,7 +8,8 @@ import { Context, Service } from 'cordis' import z from 'schemastery' import { watch as chokidarWatch } from 'chokidar' -import { mkdir, readFile, rename, writeFile } from 'node:fs/promises' +import { randomBytes } from 'node:crypto' +import { mkdir, readFile, rename, rm, writeFile } from 'node:fs/promises' import { dirname, extname, join, resolve } from 'node:path' import { Document, parseDocument } from 'yaml' import { resolveDshHome } from '@deepseek-ai/dsh-paths' @@ -86,6 +87,13 @@ export class SettingsLocal extends Settings { private text: string | undefined /** Serializes watcher-triggered reloads so reads never interleave. */ private refreshTask: Promise = Promise.resolve() + /** Set at dispose: refuse new watcher events and let in-flight work no-op. */ + private closed = false + + /** Opaque read of {@link closed}: control flow cannot narrow it across awaits. */ + private isClosed(): boolean { + return this.closed + } constructor(ctx: Context, public config: Config) { super(ctx) @@ -118,18 +126,26 @@ export class SettingsLocal extends Settings { ? this.renderYaml(ns, section) : this.renderJson(ns, section) await mkdir(dirname(this.spec.filename), { recursive: true }) - const temp = `${this.spec.filename}.tmp` - // Owner-only permissions apply to the temp file and survive the rename, so - // a document that may carry personal values is never world-readable. - await writeFile(temp, output, { mode: 0o600 }) - await rename(temp, this.spec.filename) + // Exclusive-create (`wx`) a random-suffix sibling: the open refuses to + // follow any planted symlink at a guessable temp path, and the fresh inode + // 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` + try { + await writeFile(temp, output, { mode: 0o600, flag: 'wx' }) + await rename(temp, this.spec.filename) + } catch (error) { + await rm(temp, { force: true }) + throw error + } this.text = output } - async* [Service.init](): AsyncGenerator<() => Promise | void, void, void> { - // A parse failure here is a boot failure: an existing-but-invalid document - // must fail loud, never be silently ignored or overwritten. - this.publish(await this.load()) + override async* [Service.init](): AsyncGenerator<() => Promise | void, void, void> { + // The base init loads and publishes; a parse failure there is a boot + // failure: an existing-but-invalid document must fail loud, never be + // silently ignored or overwritten. + yield* super[Service.init]() if (!this.spec.watch) return const watcher = chokidarWatch(this.spec.filename, { ignoreInitial: true, @@ -139,13 +155,26 @@ export class SettingsLocal extends Settings { }, }) watcher.on('all', () => { - this.refreshTask = this.refreshTask.then(() => this.refresh()) + if (this.closed) return + this.refreshTask = this.refreshTask.then(() => this.refresh()).catch((error: unknown) => { + // Only an invariant violation escaping the commit path can reject a + // refresh; keep the reload queue alive and surface it as an error so + // one poisoned commit cannot silently end hot reloading forever. + this.ctx.logger.error('settings-local: reload commit failed at %s', this.spec.filename) + this.ctx.logger.error(error) + }) }) watcher.on('error', (error) => { this.ctx.logger.warn('settings-local: watcher error on %s', this.spec.filename) this.ctx.logger.warn(error) }) - yield () => watcher.close() + yield async () => { + // Quiesce: stop accepting events, close the watcher, then wait out any + // queued or in-flight refresh so nothing publishes after disposal. + this.closed = true + await watcher.close() + await this.refreshTask + } } /** Parse one document text into raw sections, failing on a non-map root. */ @@ -174,6 +203,7 @@ export class SettingsLocal extends Settings { * never take the process down. */ private async refresh(): Promise { + if (this.closed) return let text: string try { text = await readFile(this.spec.filename, 'utf8') @@ -183,12 +213,12 @@ export class SettingsLocal extends Settings { this.ctx.logger.warn(error) return } - if (this.text === undefined) return + if (this.text === undefined || this.isClosed()) return this.text = undefined this.publish({}) return } - if (text === this.text) return + if (text === this.text || this.isClosed()) return let doc: Record try { doc = this.parse(text) diff --git a/packages/settings/settings-local/tests/loader-composition.spec.ts b/packages/settings/settings-local/tests/loader-composition.spec.ts index 5744e92f46..d584c11899 100644 --- a/packages/settings/settings-local/tests/loader-composition.spec.ts +++ b/packages/settings/settings-local/tests/loader-composition.spec.ts @@ -55,7 +55,7 @@ async function loadComposition(): Promise<{ ctx: Context; state: ConsumerState; base: { fontSize: 16 }, }) state.scope = scope - scope.watch(next => state.seen.push(next)) + scope.watch((next) => { state.seen.push(next) }) }, } diff --git a/packages/settings/settings-local/tests/local.spec.ts b/packages/settings/settings-local/tests/local.spec.ts index 9c9d472bac..1c74429153 100644 --- a/packages/settings/settings-local/tests/local.spec.ts +++ b/packages/settings/settings-local/tests/local.spec.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { Context } from 'cordis' import z from 'schemastery' -import { chmod, mkdtemp, readFile, readdir, rm, stat, writeFile } from 'node:fs/promises' +import { chmod, lstat, mkdtemp, readFile, readdir, rm, stat, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { settingsNamespace } from '@deepseek-ai/dsh-settings' @@ -146,6 +146,23 @@ describe('persist', () => { expect((await readdir(dir)).sort()).toEqual(['settings.yaml']) }) + it('never follows a planted symlink at a temp path and never leaves the document a symlink', async () => { + const dir = await tempDir() + const path = join(dir, 'settings.yaml') + const victim = join(dir, 'victim.txt') + await writeFile(victim, 'precious') + // A hostile sibling plants the historic fixed temp name as a symlink. + await symlink(victim, `${path}.tmp`) + const ctx = await boot({ path, watch: false }) + const scope = ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) + await scope.update({ theme: 'light' }) + + expect(await readFile(victim, 'utf8')).toBe('precious') + expect((await lstat(path)).isSymbolicLink()).toBe(false) + expect((await stat(path)).mode & 0o777).toBe(0o600) + expect(await readFile(path, 'utf8')).toContain('theme: light') + }) + it('preserves comments and unregistered sections across updates', async () => { const dir = await tempDir() const path = join(dir, 'settings.yaml') @@ -180,6 +197,20 @@ describe('persist', () => { expect(written).toEqual({ 'ui-theme': { theme: 'light' } }) }) + it('rejects and leaves no temp residue when the directory turns unwritable', async () => { + const dir = await tempDir() + const path = join(dir, 'settings.yaml') + await writeFile(path, 'ui-theme:\n theme: light\n') + const ctx = await boot({ path, watch: false }) + const scope = ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) + await chmod(dir, 0o500) + cleanups.push(() => chmod(dir, 0o700)) + await expect(scope.update({ theme: 'dark' })).rejects.toThrow() + await chmod(dir, 0o700) + expect((await readdir(dir)).sort()).toEqual(['settings.yaml']) + expect(scope.get().theme).toBe('light') + }) + it('round-trips a json document', async () => { const dir = await tempDir() const path = join(dir, 'settings.json') diff --git a/packages/settings/settings-local/tests/watcher.spec.ts b/packages/settings/settings-local/tests/watcher.spec.ts index 00c67eacc7..439f213473 100644 --- a/packages/settings/settings-local/tests/watcher.spec.ts +++ b/packages/settings/settings-local/tests/watcher.spec.ts @@ -105,6 +105,60 @@ describe('watcher pipeline', () => { expect(scope.get()).toEqual({ theme: 'light' }) }) + it('keeps the reload queue alive after an invariant violation escapes a commit', async () => { + const dir = await tempDir() + const path = join(dir, 'settings.yaml') + await writeFile(path, 'ui-theme:\n theme: light\n') + const ctx = await boot({ path, debounceMs: 5 }) + const scope = ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) + let arm = true + ctx.on('settings/updated', () => { + if (!arm) return + throw Object.assign(new Error('forged relation'), { code: 'INVARIANT' }) + }) + const [instance] = await fakeInstances() + + await writeFile(path, 'ui-theme:\n theme: broken-commit\n') + instance!.watcher.emit('all', 'change', path) + await vi.waitFor(() => { + expect(scope.get().theme).toBe('broken-commit') + }) + + arm = false + await writeFile(path, 'ui-theme:\n theme: recovered\n') + instance!.watcher.emit('all', 'change', path) + await vi.waitFor(() => { + expect(scope.get().theme).toBe('recovered') + }) + }) + + it('quiesces the refresh pipeline before dispose completes', async () => { + const dir = await tempDir() + const path = join(dir, 'settings.yaml') + await writeFile(path, 'ui-theme:\n theme: light\n') + const ctx = new Context() + const fiber = ctx.plugin(SettingsLocal, { path, debounceMs: 5 }) + await fiber + ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) + let disposed = false + let postDisposeCommits = 0 + ctx.on('settings/updated', () => { + if (disposed) postDisposeCommits += 1 + }) + + await writeFile(path, 'ui-theme:\n theme: darker\n') + const [instance] = await fakeInstances() + // Two queued refreshes: dispose interrupts one mid-flight and the other + // before it starts, so both closed guards must hold. + instance!.watcher.emit('all', 'change', path) + instance!.watcher.emit('all', 'change', path) + await fiber.dispose() + disposed = true + instance!.watcher.emit('all', 'change', path) + await new Promise(resolve => setTimeout(resolve, 100)) + expect(postDisposeCommits).toBe(0) + }) + it('treats an event for a still-absent file as a no-op', async () => { const dir = await tempDir() const path = join(dir, 'settings.yaml') diff --git a/packages/settings/settings/README.i18n.yaml b/packages/settings/settings/README.i18n.yaml index 51f30f2ca6..6f5e5c6beb 100644 --- a/packages/settings/settings/README.i18n.yaml +++ b/packages/settings/settings/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/settings/settings/README.md -README.md: e57db00c095a87f9f0b51397e030dec364f48e62 -README.zh.md: 8733823106f0ef3880cbe2c567de2edcdeff2c86 +README.md: f7858a247f6011cd0654a73b5325d81c118441e5 +README.zh.md: 67ecba695066389bfe3a69f517d52f91b48b6c75 diff --git a/packages/settings/settings/README.md b/packages/settings/settings/README.md index e57db00c09..f7858a247f 100644 --- a/packages/settings/settings/README.md +++ b/packages/settings/settings/README.md @@ -9,12 +9,13 @@ Abstract user-settings seam (`ctx.settings`). One provider holds a raw document - `register(ns, schema, { base?, applies? })` — returns the owner `SettingsScope` (`get`/`watch`/`update`). The registration is an effect on the calling plugin's fiber: disposing that fiber removes the namespace and its observers. A stored section the schema rejects fails the registration itself; a duplicate namespace fails loud. - `describe()` — one descriptor per namespace (`schema.toJSON()` envelope, resolved value, `applies`) for configuration surfaces. - `get(ns)` — resolved value, `undefined` while unregistered. -- `update(ns, patch)` — deep-merges the plain-object patch into the user section only (never the `base`), validates the resolved candidate, persists through the provider, then commits. Validation failure rejects before anything is persisted; a read-only provider (`writable: false`) rejects every update. -- Resolved values are deep-frozen snapshots; watchers receive `(next, prev)` after each commit, and watcher failures are contained. +- `update(ns, patch)` — deep-merges the plain-object patch into the user section only (never the `base`), validates the resolved candidate, persists through the provider, then commits. Validation failure rejects before anything is persisted; a read-only provider (`writable: false`) rejects every write. Writes to one namespace are serialized in call order. +- `replace(ns, section)` — sets the user section wholesale: the removal/reset path a merge cannot express (`replace({})` re-inherits `base` and schema defaults). +- Resolved values are deep-frozen snapshots; watchers receive `(next, prev)` after each commit, and watcher failures — sync throws and async rejections alike — are contained. ## Provider contract -Subclasses implement `writable`, `load()`, and `persist(ns, section)`, and push externally observed documents through the protected `publish(doc)`. At publish, each registered namespace re-resolves independently: an invalid section keeps that namespace's last good value and warns — a live reload never takes the process down — while boot-time and registration-time validation fail loud. +Subclasses implement `writable`, `load()`, and `persist(ns, section)`, and push externally observed documents through the protected `publish(doc)`. The base service init loads and publishes the document once before the service becomes injectable; a provider with its own init (watcher, connection) delegates first via `yield* super[Service.init]()`. At publish, each registered namespace re-resolves independently: an invalid section keeps that namespace's last good value and warns — a live reload never takes the process down — while boot-time and registration-time validation fail loud. ## Events @@ -31,5 +32,5 @@ No direct invalidation; a consumer that folds a settings value into the request ## Known Limitations and Deferred Work - **Single user layer** — resolution knows schema defaults, one composition `base`, and one user document; there is no project/managed layering or per-value provenance yet. -- **Cross-process concurrency is provider-defined** — the seam serializes nothing across processes; concurrent writers converge by provider behavior (the local file provider is last-write-wins). +- **Cross-process concurrency is provider-defined** — the seam serializes writes per namespace in-process only; concurrent processes converge by provider behavior (the local file provider is last-write-wins). - **No secret-field redaction** — `describe()` returns resolved values verbatim; a wire surface (RPC/UI) must redact `role('secret')` fields before exposure. diff --git a/packages/settings/settings/README.zh.md b/packages/settings/settings/README.zh.md index 8733823106..67ecba6950 100644 --- a/packages/settings/settings/README.zh.md +++ b/packages/settings/settings/README.zh.md @@ -9,12 +9,13 @@ - `register(ns, schema, { base?, applies? })` — 返回 owner 的 `SettingsScope`（`get`/`watch`/`update`）。注册是调用方插件 fiber 上的 effect：dispose 该 fiber 即移除 namespace 及其观察者。schema 拒绝的存量分节会使注册本身失败；重复 namespace 立即报错。 - `describe()` — 每个 namespace 一条描述（`schema.toJSON()` 信封、解析值、`applies`），供配置界面使用。 - `get(ns)` — 解析值；未注册时为 `undefined`。 -- `update(ns, patch)` — 把普通对象 patch 深合并进用户分节（绝不合并进 `base`），校验解析候选值，经 provider 持久化后提交。校验失败在持久化前拒绝；只读 provider（`writable: false`）拒绝一切更新。 -- 解析值是深冻结快照；每次提交后观察者收到 `(next, prev)`，观察者异常被隔离。 +- `update(ns, patch)` — 把普通对象 patch 深合并进用户分节（绝不合并进 `base`），校验解析候选值，经 provider 持久化后提交。校验失败在持久化前拒绝；只读 provider（`writable: false`）拒绝一切写入。同一 namespace 的写入按调用顺序串行。 +- `replace(ns, section)` — 整体替换用户分节：merge 表达不了的删除/重置路径（`replace({})` 重新继承 `base` 与 schema 默认值）。 +- 解析值是深冻结快照；每次提交后观察者收到 `(next, prev)`；观察者异常——同步抛出与异步拒绝——均被隔离。 ## Provider 契约 -子类实现 `writable`、`load()`、`persist(ns, section)`，并通过受保护的 `publish(doc)` 推入外部观察到的文档。publish 时每个已注册 namespace 独立重解析：非法分节保留该 namespace 的最后可用值并告警——热重载绝不拖垮进程；启动期与注册期校验则立即报错。 +子类实现 `writable`、`load()`、`persist(ns, section)`，并通过受保护的 `publish(doc)` 推入外部观察到的文档。基类 service init 在服务可注入前加载并发布一次文档；自有 init（watcher、连接）的 provider 先经 `yield* super[Service.init]()` 委托。publish 时每个已注册 namespace 独立重解析：非法分节保留该 namespace 的最后可用值并告警——热重载绝不拖垮进程；启动期与注册期校验则立即报错。 ## 事件 @@ -31,5 +32,5 @@ ## Known Limitations and Deferred Work - **单一用户层** — 解析只认识 schema 默认值、一个组合 `base` 与一个用户文档；尚无 project/managed 分层或按值溯源。 -- **跨进程并发由 provider 定义** — seam 不做跨进程串行化；并发写入者按 provider 行为收敛（本地文件 provider 为后写胜出）。 +- **跨进程并发由 provider 定义** — seam 仅在进程内按 namespace 串行化写入；跨进程并发按 provider 行为收敛（本地文件 provider 为后写胜出）。 - **无 secret 字段脱敏** — `describe()` 原样返回解析值；wire 面（RPC/UI）在暴露前必须对 `role('secret')` 字段脱敏。 diff --git a/packages/settings/settings/src/index.ts b/packages/settings/settings/src/index.ts index bec60dfdea..10b21c2154 100644 --- a/packages/settings/settings/src/index.ts +++ b/packages/settings/settings/src/index.ts @@ -7,7 +7,6 @@ */ import { Context, Service } from 'cordis' -import { deepEqual } from 'cosmokit' import type z from 'schemastery' import type { Branded } from '@deepseek-ai/dsh-brand' @@ -59,16 +58,23 @@ export interface SettingsScope { /** Current resolved value: schema defaults, then `base`, then the user layer. */ get(): T /** - * Observe committed changes to this namespace's resolved value. + * Observe committed changes to this namespace's resolved value. A callback + * may be async; a rejection is contained and logged like a sync throw. * @param callback - invoked after each commit with the next and previous values. * @returns the disposer removing this observer. */ - watch(callback: (next: T, prev: T) => void): () => void + watch(callback: (next: T, prev: T) => void | Promise): () => void /** * Merge a partial patch into this namespace's user layer and persist it. * @param patch - plain-object patch over the user section. */ update(patch: object): Promise + /** + * Replace this namespace's user section wholesale; absent keys re-inherit + * the composition `base` and schema defaults (`replace({})` resets all). + * @param section - the complete next user section. + */ + replace(section: object): Promise } declare module 'cordis' { @@ -91,6 +97,28 @@ declare module 'cordis' { } } +/** + * Deep equality over JSON-shaped data (objects, arrays, primitives) — the + * seam's single change-detection predicate, exported so the invariant + * companion checks exactly the implementation's relation. + * @param a - one JSON-shaped value. + * @param b - the other JSON-shaped value. + * @returns whether the two values are structurally equal. + */ +export function deepEqualJson(a: unknown, b: unknown): boolean { + if (a === b) return true + if (typeof a !== 'object' || typeof b !== 'object' || a === null || b === null) return false + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) return false + return a.every((entry, index) => deepEqualJson(entry, b[index])) + } + const left = a as Record + const right = b as Record + const keys = Object.keys(left) + if (keys.length !== Object.keys(right).length) return false + return keys.every(key => key in right && deepEqualJson(left[key], right[key])) +} + /** Whether a value is a plain data object (not an array, null, or class instance). */ function isPlainObject(value: unknown): value is Record { if (typeof value !== 'object' || value === null || Array.isArray(value)) return false @@ -128,7 +156,7 @@ interface SettingsRegistration { base: unknown applies: SettingsApplies resolved: unknown - watchers: Set<(next: never, prev: never) => void> + watchers: Set<(next: never, prev: never) => void | Promise> } /** @@ -141,11 +169,22 @@ export abstract class Settings extends Service { private readonly registrations = new Map() /** Latest published raw document; empty until the provider's first publish. */ private document: Record = {} + /** Per-namespace write chains; settled tails, so a failure never poisons the queue. */ + private readonly writeQueues = new Map>() constructor(ctx: Context) { super(ctx, 'settings') } + /** + * Load the provider's document once and publish it before the service + * becomes injectable. Providers with their own init (watchers, connections) + * delegate here first via `yield* super[Service.init]()`. + */ + async* [Service.init](): AsyncGenerator<() => void, void, void> { + this.publish(await this.load()) + } + /** Whether {@link update} may persist through this provider. */ abstract readonly writable: boolean @@ -195,6 +234,7 @@ export abstract class Settings extends Service { return () => registration.watchers.delete(callback) }, update: patch => this.update(ns, patch), + replace: section => this.replace(ns, section), } } @@ -223,11 +263,30 @@ export abstract class Settings extends Service { /** * Merge a patch into one registered namespace's user layer, validate the * resolved candidate, persist through the provider, then commit and emit. - * A validation failure rejects before anything is persisted. + * A validation failure rejects before anything is persisted. Writes to one + * namespace are serialized: concurrent updates apply in call order, each + * merging over the previous write's committed section. * @param ns - the registered namespace to update. * @param patch - plain-object patch over the user section. */ async update(ns: SettingsNamespace, patch: object): Promise { + return this.write(ns, patch, 'merge') + } + + /** + * Replace one registered namespace's user section wholesale, validate, + * persist, then commit and emit. Keys absent from `section` fall back to the + * composition `base` and schema defaults — this is the removal/reset path a + * merge-only patch cannot express (`replace({})` re-inherits everything). + * @param ns - the registered namespace to replace. + * @param section - the complete next user section. + */ + async replace(ns: SettingsNamespace, section: object): Promise { + return this.write(ns, section, 'replace') + } + + /** Validate a write, then queue it on the namespace's serialized write chain. */ + private write(ns: SettingsNamespace, input: object, mode: 'merge' | 'replace'): Promise { const registration = this.registrations.get(ns) if (registration === undefined) { throw new Error(`settings namespace "${ns}" is not registered`) @@ -235,14 +294,23 @@ export abstract class Settings extends Service { if (!this.writable) { throw new Error(`settings provider is read-only: "${ns}" cannot be updated in-process`) } - if (!isPlainObject(patch)) { - throw new TypeError(`settings update for "${ns}" must be a plain object patch`) + if (!isPlainObject(input)) { + throw new TypeError(`settings ${mode === 'merge' ? 'update' : 'replace'} for "${ns}" must be a plain object`) } - const section = mergeLayers(this.section(ns) ?? {}, patch) as Record - const next = deepFreeze(this.resolve(registration.schema, registration.base, section)) - await this.persist(ns, section) - this.document[ns] = section - this.commit(registration, next, 'update') + const previous = this.writeQueues.get(ns) ?? Promise.resolve() + // Chain past a failed predecessor: one rejected write must not poison the + // namespace queue for every later caller. + const run = previous.catch(() => undefined).then(async () => { + const section = mode === 'merge' + ? mergeLayers(this.section(ns) ?? {}, input) as Record + : structuredClone(input) + const next = deepFreeze(this.resolve(registration.schema, registration.base, section)) + await this.persist(ns, section) + this.document[ns] = section + this.commit(registration, next, 'update') + }) + this.writeQueues.set(ns, run) + return run } /** @@ -287,17 +355,38 @@ export abstract class Settings extends Service { /** Commit a resolved value when changed: swap, notify watchers, emit the event. */ private commit(registration: SettingsRegistration, next: unknown, source: SettingsUpdateSource): void { const prev = registration.resolved - if (deepEqual(next, prev)) return + if (deepEqualJson(next, prev)) return registration.resolved = next for (const watcher of [...registration.watchers]) { try { - watcher(next as never, prev as never) + // A watcher may be async: adopt its promise so a rejection is contained + // here instead of surfacing as an unhandled rejection. + const outcome = watcher(next as never, prev as never) as unknown + if (outcome instanceof Promise) { + outcome.catch((error: unknown) => { + this.warnWatcherFailure(registration.ns, error) + }) + } } catch (error) { - this.ctx.logger.warn('settings: watcher for "%s" failed', registration.ns) - this.ctx.logger.warn(error) + this.warnWatcherFailure(registration.ns, error) } } - this.ctx.emit('settings/updated', registration.ns, next, prev, source) + try { + this.ctx.emit('settings/updated', registration.ns, next, prev, source) + } catch (error) { + // Invariant violations are harness-fatal by design; any other listener + // failure is contained so one broken observer cannot wedge the commit + // path (and, through it, a provider's reload loop). + if ((error as { code?: unknown } | null)?.code === 'INVARIANT') throw error + this.ctx.logger.warn('settings: a settings/updated listener for "%s" failed', registration.ns) + this.ctx.logger.warn(error) + } + } + + /** Contained-watcher diagnostic shared by the sync and async failure paths. */ + private warnWatcherFailure(ns: SettingsNamespace, error: unknown): void { + this.ctx.logger.warn('settings: watcher for "%s" failed', ns) + this.ctx.logger.warn(error) } } diff --git a/packages/settings/settings/src/invariant.ts b/packages/settings/settings/src/invariant.ts index 235f413aae..d8db41bce4 100644 --- a/packages/settings/settings/src/invariant.ts +++ b/packages/settings/settings/src/invariant.ts @@ -5,6 +5,7 @@ import type { Context } from 'cordis' import type { InvariantFailure, InvariantInstaller } from '@deepseek-ai/dsh-invariants' +import { deepEqualJson } from './index.ts' const PACKAGE_NAME = '@deepseek-ai/dsh-settings' @@ -15,7 +16,9 @@ export const inject = ['invariants'] /** * Install the commit-event contract: `settings/updated` fires only for a - * currently registered namespace and only when the resolved value changed. + * currently registered namespace, only when the resolved value changed, and + * only with the service's authoritative resolved value — all judged with the + * seam's own equality predicate. */ const install: InvariantInstaller = (ctx: Context, fail: InvariantFailure) => { ctx.on('settings/updated', (ns, next, prev) => { @@ -23,10 +26,14 @@ const install: InvariantInstaller = (ctx: Context, fail: InvariantFailure) => { if (settings === undefined) { fail(`settings/updated for "${ns}" emitted without a live settings service`) } - if (settings.get(ns) === undefined) { + const current = settings.get(ns) + if (current === undefined) { fail(`settings/updated for "${ns}" emitted while the namespace is unregistered`) } - if (JSON.stringify(next) === JSON.stringify(prev)) { + if (!deepEqualJson(current, next)) { + fail(`settings/updated for "${ns}" does not match the authoritative resolved value`) + } + if (deepEqualJson(next, prev)) { fail(`settings/updated for "${ns}" emitted without a resolved-value change`) } }) diff --git a/packages/settings/settings/tests/invariant.spec.ts b/packages/settings/settings/tests/invariant.spec.ts index f12ff6126c..0976827368 100644 --- a/packages/settings/settings/tests/invariant.spec.ts +++ b/packages/settings/settings/tests/invariant.spec.ts @@ -38,4 +38,15 @@ describe('settings invariants', () => { ctx.emit('settings/updated', settingsNamespace('ui-theme'), { theme: 'dark' }, { theme: 'dark' }, 'update') }).toThrow(/without a resolved-value change/) }) + + it('fails a settings/updated emission whose value diverges from the authoritative state', async () => { + const ctx = await setup(true) + ctx.settings.register(settingsNamespace('ui-theme'), z.object({ + theme: z.string().default('dark'), + })) + // Fabricated next ≠ the service's current resolved value ({theme: 'dark'}). + expect(() => { + ctx.emit('settings/updated', settingsNamespace('ui-theme'), { theme: 'forged' }, { theme: 'dark' }, 'update') + }).toThrow(/authoritative/) + }) }) diff --git a/packages/settings/settings/tests/memory.ts b/packages/settings/settings/tests/memory.ts index 0b20771cbe..bb310c939d 100644 --- a/packages/settings/settings/tests/memory.ts +++ b/packages/settings/settings/tests/memory.ts @@ -5,7 +5,6 @@ * packages. */ -import { Service } from 'cordis' import { Settings, type SettingsNamespace } from '../src/index.ts' /** In-memory provider exposing the protected seam hooks to tests. */ @@ -17,13 +16,18 @@ export class MemorySettings extends Settings { /** When false, update() must reject before reaching persist(). */ writableFlag: boolean + /** Artificial persist latency so tests can interleave concurrent updates. */ + persistDelayMs: number + constructor(ctx: ConstructorParameters[0], options?: { doc?: Record writable?: boolean + persistDelayMs?: number }) { super(ctx) this.doc = structuredClone(options?.doc ?? {}) this.writableFlag = options?.writable ?? true + this.persistDelayMs = options?.persistDelayMs ?? 0 } get writable(): boolean { @@ -34,10 +38,12 @@ export class MemorySettings extends Settings { return Promise.resolve(structuredClone(this.doc)) } - protected persist(ns: SettingsNamespace, section: Record): Promise { + protected async persist(ns: SettingsNamespace, section: Record): Promise { + if (this.persistDelayMs > 0) { + await new Promise(resolve => setTimeout(resolve, this.persistDelayMs)) + } this.persisted.push({ ns, section: structuredClone(section) }) this.doc[ns] = structuredClone(section) - return Promise.resolve() } /** Simulate an external storage change reaching the provider. */ @@ -45,9 +51,4 @@ export class MemorySettings extends Settings { this.doc = structuredClone(doc) this.publish(structuredClone(doc)) } - - async* [Service.init](): AsyncGenerator<() => void, void, void> { - this.publish(await this.load()) - yield () => { this.persisted.length = 0 } - } } diff --git a/packages/settings/settings/tests/settings.spec.ts b/packages/settings/settings/tests/settings.spec.ts index a3d223c3f7..c413e948ed 100644 --- a/packages/settings/settings/tests/settings.spec.ts +++ b/packages/settings/settings/tests/settings.spec.ts @@ -1,9 +1,32 @@ import { describe, expect, it, vi } from 'vitest' import { Context } from 'cordis' import z from 'schemastery' -import { settingsNamespace, type SettingsScope, type SettingsUpdateSource } from '../src/index.ts' +import { Settings, deepEqualJson, settingsNamespace, type SettingsNamespace, type SettingsScope, type SettingsUpdateSource } from '../src/index.ts' import { MemorySettings } from './memory.ts' +/** A provider implementing only the three primitives: the seam owns init. */ +class BareProvider extends Settings { + doc: Record + + constructor(ctx: ConstructorParameters[0], options?: { doc?: Record }) { + super(ctx) + this.doc = structuredClone(options?.doc ?? {}) + } + + get writable(): boolean { + return true + } + + protected load(): Promise> { + return Promise.resolve(structuredClone(this.doc)) + } + + protected persist(ns: SettingsNamespace, section: Record): Promise { + this.doc[ns] = structuredClone(section) + return Promise.resolve() + } +} + interface ThemeConfig { theme: 'dark' | 'light' fontSize: number @@ -119,7 +142,7 @@ describe('registration', () => { inject: ['settings'], apply: (child: Context) => { scope = child.settings.register(settingsNamespace('ui-theme'), ThemeSchema) - scope.watch(next => seen.push(next)) + scope.watch((next) => { seen.push(next) }) }, }) await fiber @@ -191,6 +214,9 @@ describe('update', () => { expect(provider.persisted).toEqual([]) expect(events).toEqual([]) expect(scope.get()).toEqual({ theme: 'dark', fontSize: 14 }) + // The failed write must not poison the namespace queue for later writers. + await scope.update({ fontSize: 18 }) + expect(scope.get()).toEqual({ theme: 'dark', fontSize: 18 }) }) it('ignores explicit undefined entries so a sparse patch cannot erase keys', async () => { @@ -206,6 +232,7 @@ describe('update', () => { const scope = ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) await expect(scope.update([1])).rejects.toThrow(TypeError) await expect(scope.update(new Date() as unknown as object)).rejects.toThrow(TypeError) + await expect(scope.replace([1])).rejects.toThrow(/replace for "ui-theme"/) }) it('accepts a null-prototype patch object', async () => { @@ -231,6 +258,89 @@ describe('update', () => { }) }) +describe('deepEqualJson', () => { + it.each([ + [{ a: [1, 2] }, { a: [1, 2] }, true], + [{ a: [1, 2] }, { a: [1] }, false], + [{ a: [1] }, { a: { 0: 1 } }, false], + [{ a: 1 }, { b: 1 }, false], + [{ a: 1 }, {}, false], + [{ a: null }, { a: null }, true], + [{ a: null }, { a: {} }, false], + ])('compares %j vs %j as %s', (a, b, equal) => { + expect(deepEqualJson(a, b)).toBe(equal) + }) +}) + +describe('review regressions', () => { + it('propagates an invariant-coded listener failure instead of containing it', async () => { + const { ctx, provider } = await boot() + ctx.on('settings/updated', () => { + throw Object.assign(new Error('forged relation'), { code: 'INVARIANT' }) + }) + ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) + expect(() => { provider.pushExternal({ 'ui-theme': { theme: 'light' } }) }) + .toThrow(/forged relation/) + }) + + it('serializes concurrent updates so neither patch is lost', async () => { + const { ctx, provider } = await boot({ persistDelayMs: 10 }) + const scope = ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) + await Promise.all([ + scope.update({ theme: 'light' }), + scope.update({ fontSize: 20 }), + ]) + expect(provider.doc['ui-theme']).toEqual({ theme: 'light', fontSize: 20 }) + expect(scope.get()).toEqual({ theme: 'light', fontSize: 20 }) + }) + + it('contains a throwing settings/updated listener and keeps later commits alive', async () => { + const { ctx, provider } = await boot() + ctx.on('settings/updated', () => { + throw new Error('listener boom') + }) + const scope = ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) + expect(() => { provider.pushExternal({ 'ui-theme': { theme: 'light' } }) }).not.toThrow() + expect(scope.get().theme).toBe('light') + provider.pushExternal({ 'ui-theme': { theme: 'dark' } }) + expect(scope.get().theme).toBe('dark') + }) + + it('contains an async watcher rejection', async () => { + const { ctx, provider } = await boot() + const scope = ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) + scope.watch(async () => { + throw new Error('async watcher boom') + }) + provider.pushExternal({ 'ui-theme': { theme: 'light' } }) + expect(scope.get().theme).toBe('light') + // Give the rejected watcher promise a microtask turn; containment means + // vitest observes no unhandled rejection out of this test. + await new Promise(resolve => setTimeout(resolve, 10)) + }) + + it('loads the provider document through the base init without provider boilerplate', async () => { + const ctx = new Context() + await ctx.plugin(BareProvider, { doc: { 'ui-theme': { fontSize: 7 } } }) + const scope = ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema) + expect(scope.get()).toEqual({ theme: 'dark', fontSize: 7 }) + }) + + it('replaces the user section wholesale so overrides can be removed', async () => { + const { ctx, provider } = await boot({ doc: { 'ui-theme': { theme: 'light', fontSize: 20 } } }) + const scope = ctx.settings.register(settingsNamespace('ui-theme'), ThemeSchema, { + base: { fontSize: 16 }, + }) + await scope.replace({ theme: 'light' }) + // fontSize override is gone: resolution falls back to the base layer. + expect(scope.get()).toEqual({ theme: 'light', fontSize: 16 }) + expect(provider.doc['ui-theme']).toEqual({ theme: 'light' }) + await scope.replace({}) + expect(scope.get()).toEqual({ theme: 'dark', fontSize: 16 }) + expect(provider.doc['ui-theme']).toEqual({}) + }) +}) + describe('publish', () => { it('notifies watchers of an external change with source provider', async () => { const { ctx, provider } = await boot() diff --git a/scripts/gen-cordis-catalog.ts b/scripts/gen-cordis-catalog.ts index b7df3dc622..971576f35f 100644 --- a/scripts/gen-cordis-catalog.ts +++ b/scripts/gen-cordis-catalog.ts @@ -189,6 +189,11 @@ export const LINK_MAP: Record = { ToolRegistry: 'tools.md', ToolRestriction: 'tools.md', ToolSchema: 'tools.md', + SettingsNamespace: 'settings.md', + SettingsRegisterOptions: 'settings.md', + SettingsScope: 'settings.md', + SettingsDescriptor: 'settings.md', + SettingsUpdateSource: 'settings.md', AskUserQuestionAnswer: 'user-interaction.md', AskUserQuestionRequest: 'user-interaction.md', UserInteractionProvider: 'user-interaction.md', @@ -217,11 +222,6 @@ const FOUNDATION_TYPE_NAMES = new Set([ /** Project types deliberately documented outside the core-data catalog. */ const TYPE_LINK_EXEMPTIONS: Readonly> = { AgentFactory: 'agent creation seam is owned by packages/core/agent/README.md', - SettingsNamespace: 'settings seam vocabulary is owned by packages/settings/settings/README.md', - SettingsUpdateSource: 'settings seam vocabulary is owned by packages/settings/settings/README.md', - SettingsRegisterOptions: 'settings seam vocabulary is owned by packages/settings/settings/README.md', - SettingsScope: 'settings seam vocabulary is owned by packages/settings/settings/README.md', - SettingsDescriptor: 'settings seam vocabulary is owned by packages/settings/settings/README.md', z: 'schemastery schema constructor is owned by vendor/schemastery (vendored upstream)', BeginCommandRequest: 'event-local request contract is owned by packages/client/ui-slash/src/types.ts', InsertReferenceRequest: 'event-local request contract is owned by packages/client/ui-slash/src/types.ts', diff --git a/scripts/project-doc-site.spec.ts b/scripts/project-doc-site.spec.ts index e6cc11ae6e..28f854fe9b 100644 --- a/scripts/project-doc-site.spec.ts +++ b/scripts/project-doc-site.spec.ts @@ -197,7 +197,7 @@ describe('docsPages locale routes', () => { const translated = rootPages.filter(page => page.contentLocale === 'zh-CN') const fallbacks = rootPages.filter(page => page.contentLocale === 'en-US') - expect(translated).toHaveLength(18) + expect(translated).toHaveLength(19) expect(translated.every(page => page.source.endsWith('.zh.md'))).toBe(true) expect(fallbacks.map(page => page.source).sort()).toEqual([ 'docs/core-data-structures/commands.md', diff --git a/scripts/type-equiv.manifest.json b/scripts/type-equiv.manifest.json index b607995003..7bd9c09177 100644 --- a/scripts/type-equiv.manifest.json +++ b/scripts/type-equiv.manifest.json @@ -1308,6 +1308,36 @@ "doc": "docs/core-data-structures/subprocess.md", "symbol": "SubprocessCollectedOutputs", "source": "packages/subprocess/subprocess/src/types.ts" + }, + { + "doc": "docs/core-data-structures/settings.md", + "symbol": "SettingsNamespace", + "source": "packages/settings/settings/src/index.ts" + }, + { + "doc": "docs/core-data-structures/settings.md", + "symbol": "SettingsRegisterOptions", + "source": "packages/settings/settings/src/index.ts" + }, + { + "doc": "docs/core-data-structures/settings.md", + "symbol": "SettingsApplies", + "source": "packages/settings/settings/src/index.ts" + }, + { + "doc": "docs/core-data-structures/settings.md", + "symbol": "SettingsScope", + "source": "packages/settings/settings/src/index.ts" + }, + { + "doc": "docs/core-data-structures/settings.md", + "symbol": "SettingsDescriptor", + "source": "packages/settings/settings/src/index.ts" + }, + { + "doc": "docs/core-data-structures/settings.md", + "symbol": "SettingsUpdateSource", + "source": "packages/settings/settings/src/index.ts" } ] } diff --git a/website/docs.ts b/website/docs.ts index 1888cd908c..cbc8d9e627 100644 --- a/website/docs.ts +++ b/website/docs.ts @@ -256,6 +256,7 @@ const coreDataReference = pairedPages(([ ['sandbox.md', '沙箱', 'Sandboxing', 18], ['web.md', 'Web 访问', 'Web access', 19], ['persistence.md', '会话持久化', 'Session persistence', 20], + ['settings.md', '用户设置', 'User settings', 21], ] as const).map(([file, rootLabel, enLabel, order]): PairedPage => ({ source: `docs/core-data-structures/${file}`, route: `reference/core-data-structures/${file}`,