Bug: a settings write deletes an external edit that added a key inside the namespace it writes (still live on 0.1.5-rc.2) #6221
Replies: 2 comments
|
Verified against current upstream master and this is exactly right, both halves of it. packages/settings/settings/src/index.ts's write() (around line 664-677) computes const current = this.section(ns) ?? {} from the cached document, derives the next section from that, and calls this.persist(ns, section) with no way for the provider to react to anything newer. The abstract persist(ns, section) signature (line 406) only ever takes that one precomputed section. packages/settings/settings-file/src/index.ts's persistSection (around line 211-226) then does exactly what you describe: it calls await this.reconcileFromDisk() first, which folds the external edit into the seam correctly, and then immediately renders using the stale section it was handed (renderYaml(ns, section) or renderJson(ns, section)), throwing away the reconciliation it just did. The write genuinely observes the external edit and then overwrites it anyway. Your fix is the right shape for this because it moves derivation to after reconciliation instead of before it, which is the only way to keep last-write-wins semantics for a real conflict while not touching keys the write never intended to touch. Since PRs are disabled on this repo, having the branch, the Agent Note, and the coverage/test numbers already in the discussion is about as ready as a report can be for someone to pick up. |
|
核实结论:四层断言全部成立.
【未核实】fork 自报 225 tests/覆盖/typecheck 未复跑;has_pull_requests=false 无字段可验(仅确认 hasIssuesEnabled=false);复现未实跑,静态路径已充分. |
Uh oh!
There was an error while loading. Please reload this page.
Summary
A file-backed settings write can delete user data it never observed: an external edit that adds or changes a key inside the namespace being written is silently removed by that write. Still reproducible on
masteras of0.1.5-rc.2.Reproduction
With the shipped file provider (
@deepseek-ai/dsh-settings-file,watch: true, defaultdebounceMs: 100):settings.yamlcontainsui-theme: { theme: light }, and a plugin has registered a namespace with a schema that also acceptsfontSize.Something outside the process edits the file and adds a key inside that same namespace:
Before the watcher's debounce window elapses, the process performs an ordinary write:
The external key is gone from disk, and the resolved value no longer contains it:
An unobserved edit adding a different namespace is unaffected, because the diff only touches the written namespace's path — which is why the existing sibling-namespace fold case in
watcher.spec.tspasses and this one doesn't.Root cause
SettingsProvider.writederives the namespace's next section from the cached document at the front of the namespace's write queue, then hands that section topersist:packages/settings/settings/src/index.ts—write()computessectionfromthis.section(ns)and callsthis.persist(ns, section).FileSettingsProvider.persistSectionthen reconciles the on-disk text under its writer lock — correctly folding in the unobserved edit and publishing it into the seam — but renders the queue-front section:packages/settings/settings-file/src/index.ts—await this.reconcileFromDisk()followed byrenderYaml(ns, section)/renderJson(ns, section).The YAML renderer's leaf diff treats a key that is present in the stored document and absent from the section it is given as a removal, so the externally added key is deleted. The seam diverges too: the reconciled document
publishjust committed is overwritten by the stale section. The JSON renderer replaces the whole namespace key, so it loses the key the same way.Impact
Silent loss of a user's manual edit to the settings document, plus a memory/disk divergence after the write — on the configuration surface, where a hand-edit inside a debounce window is the normal way to change a value.
A fix, if useful
On a fork branch based on current
master:persist(ns, section, derive)— wherederive(base)re-applies the write's own intent (merge / replace / path ops) to a base — and the file provider applies it to the section of the document it reconciled, returning what it stored for the service to adopt for the in-memory document, the revision bump, and the commit. Untouched keys of the external edit survive; a key both sides changed still resolves to the later write, so same-namespace write-write races keep their last-write-wins outcome..agents/notes/implemented/bug-fix/2026-09-10-settings-write-derives-its-reconciled-base.mdVerification on that branch: two new
watcher.spec.tscases (this one, and one where the reconciled section cannot satisfy its schema) fail on the pre-fix render and pass after; 225 tests pass across the settings, settings-file, and consumer settings suites; per-file 100% coverage on both changed source files;pnpm run typecheckandpnpm run test:docs(16/16 gates) pass.I could not open a pull request here —
has_pull_requestsisfalsefor this repository andCreatePullRequestis refused — so this is posted as a Discussion instead. Happy to rebase, split, or hand it over in any form if it is useful.All reactions