Skip to content

Commit 8673ec5

Browse files
authored
fix(settings): persist client-first settings updates (#423)
* fix(settings): persist client-first settings updates * fix(settings): await the initial snapshot before saving Creating file-backed settings on first access returns an empty snapshot for a new namespace. If that snapshot arrives after the first write, it can overwrite the new value and persist an empty object. Wait for trust and the initial snapshot before settings operations. Propagate initial-read failures so the next operation can retry. Add project and global regression tests for delayed snapshots and reloads, and align the new comments with the repository terminology. Validation: 1,653 tests passed, 13 skipped; ESLint, knip, typecheck, and the 28-task build passed. * fix(settings): keep empty reads from publishing state Include the remaining empty-state handling and regression tests from PR #424. Read absent settings without sending an empty shared-state snapshot, and initialize the object only on the first setting write.
1 parent a14113f commit 8673ec5

9 files changed

Lines changed: 317 additions & 37 deletions

File tree

‎packages/devframe/src/client/rpc-shared-state.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ export function createRpcSharedStateClientHost(rpc: DevframeRpcClient): RpcShare
121121
}
122122
}
123123

124-
return new Promise<SharedState<T>>((resolve) => {
124+
return new Promise<SharedState<T>>((resolve, reject) => {
125125
if (!rpc.isTrusted) {
126126
resolve(state)
127127
let initialized = false
@@ -133,7 +133,7 @@ export function createRpcSharedStateClientHost(rpc: DevframeRpcClient): RpcShare
133133
})
134134
}
135135
else {
136-
initSharedState().then(resolve)
136+
initSharedState().then(resolve, reject)
137137
}
138138
})
139139
},

‎packages/devframe/src/client/scope.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ function createMockClient() {
1111

1212
// eslint-disable-next-line slop/no-chained-type-assertions -- partial test mock exercises only the members client.scope() touches
1313
const rpc = {
14+
ensureTrusted: vi.fn(async () => true),
1415
call: vi.fn((..._args: any[]) => Promise.resolve('ok')),
1516
callEvent: vi.fn((..._args: any[]) => {}),
1617
callOptional: vi.fn((..._args: any[]) => Promise.resolve('ok')),
@@ -126,7 +127,7 @@ describe('client.scope()', () => {
126127
const { settings } = createScopedClientContext(rpc, 'my-plugin')
127128

128129
await settings.global.set('token', 'abc')
129-
expect(rpc.sharedState.get).toHaveBeenCalledWith('devframe:settings:global:my-plugin', { initialValue: {} })
130+
expect(rpc.sharedState.get).toHaveBeenCalledWith('devframe:settings:global:my-plugin')
130131
expect(await settings.global.get('token')).toBe('abc')
131132

132133
await settings.project.set('theme', 'dark')
Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
import type { DevframeRpcClient } from './rpc'
2+
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
3+
import { tmpdir } from 'node:os'
4+
import { join } from 'node:path'
5+
import { setImmediate } from 'node:timers/promises'
6+
import { createEventEmitter } from 'devframe/utils/events'
7+
import { describe, expect, it, vi } from 'vitest'
8+
import { DEVFRAME_EVENTS } from '../events'
9+
import { createHostContext } from '../node/context'
10+
import { createRpcSharedStateClientHost } from './rpc-shared-state'
11+
import { createClientSettings } from './settings'
12+
13+
function setup(backend = 'live', trusted = true) {
14+
const snapshot = Promise.withResolvers<Record<string, unknown> | undefined>()
15+
const trust = Promise.withResolvers<boolean>()
16+
const handlers = new Map<string, (...args: any[]) => void>()
17+
// eslint-disable-next-line slop/no-chained-type-assertions -- partial RPC mock retains the shared-state implementation
18+
const rpc = {
19+
connectionMeta: { backend },
20+
isTrusted: trusted,
21+
events: createEventEmitter<any>(),
22+
client: { register: (fn: any) => handlers.set(fn.name, fn.handler) },
23+
ensureTrusted: vi.fn(() => trusted ? Promise.resolve(true) : trust.promise),
24+
call: vi.fn(() => snapshot.promise),
25+
callEvent: vi.fn(),
26+
} as unknown as DevframeRpcClient
27+
rpc.sharedState = createRpcSharedStateClientHost(rpc)
28+
const settings = createClientSettings(rpc, 'test')
29+
return { rpc, settings, snapshot, trust, handlers }
30+
}
31+
32+
describe('client settings initialization', () => {
33+
it.each(['project', 'global'] as const)('persists the first %s write after a delayed empty snapshot', async (scope) => {
34+
const dir = mkdtempSync(join(tmpdir(), 'devframe-settings-'))
35+
const host = {
36+
mountStatic: () => {},
37+
resolveOrigin: () => 'http://localhost',
38+
getStorageDir: (storageScope: string) => join(dir, storageScope),
39+
}
40+
const ctx = await createHostContext({ cwd: dir, mode: 'dev', host })
41+
const snapshotReady = Promise.withResolvers<void>()
42+
const releaseSnapshot = Promise.withResolvers<void>()
43+
const writes: Promise<unknown>[] = []
44+
// eslint-disable-next-line slop/no-chained-type-assertions -- control transport timing while retaining both shared-state implementations
45+
const rpc = {
46+
connectionMeta: { backend: 'live' },
47+
isTrusted: true,
48+
ensureTrusted: async () => true,
49+
client: { register: () => {} },
50+
call: async (method: string, key: string) => {
51+
const value = await ctx.rpc.invokeLocal(method as any, key)
52+
snapshotReady.resolve()
53+
await releaseSnapshot.promise
54+
return value
55+
},
56+
callEvent: (method: string, ...args: any[]) => {
57+
if (method !== 'devframe:rpc:server-state:subscribe')
58+
writes.push(ctx.rpc.invokeLocal(method as any, ...args))
59+
},
60+
} as unknown as DevframeRpcClient
61+
rpc.sharedState = createRpcSharedStateClientHost(rpc)
62+
const settings = createClientSettings(rpc, 'new-tool')[scope]
63+
64+
try {
65+
const write = settings.set('theme', 'dark')
66+
await snapshotReady.promise
67+
await setImmediate()
68+
releaseSnapshot.resolve()
69+
await write
70+
await setImmediate()
71+
await Promise.all(writes)
72+
73+
const filepath = join(dir, scope, 'settings/new-tool.json')
74+
await expect.poll(() => existsSync(filepath)).toBe(true)
75+
expect(await settings.all()).toEqual({ theme: 'dark' })
76+
expect(JSON.parse(readFileSync(filepath, 'utf8'))).toEqual({ theme: 'dark' })
77+
const restarted = await createHostContext({ cwd: dir, mode: 'dev', host })
78+
expect(await restarted.scope('new-tool').settings[scope].all()).toEqual({ theme: 'dark' })
79+
}
80+
finally {
81+
rmSync(dir, { recursive: true, force: true })
82+
}
83+
})
84+
85+
it.each(['project', 'global'] as const)('reads the initial %s snapshot before resolving', async (scope) => {
86+
const { settings, snapshot } = setup()
87+
const read = settings[scope].get('theme')
88+
await setImmediate()
89+
snapshot.resolve({ theme: 'light' })
90+
await expect(read).resolves.toBe('light')
91+
})
92+
93+
it.each(['project', 'global'] as const)('preserves an immediate %s write and unrelated settings', async (scope) => {
94+
const { rpc, settings, snapshot } = setup()
95+
const write = settings[scope].set('theme', 'dark')
96+
const read = settings[scope].all()
97+
await setImmediate()
98+
snapshot.resolve({ theme: 'light', language: 'en' })
99+
await write
100+
await expect(settings[scope].get('theme')).resolves.toBe('dark')
101+
await expect(read).resolves.toEqual({ theme: 'dark', language: 'en' })
102+
expect(rpc.call).toHaveBeenCalledTimes(1)
103+
})
104+
105+
it('applies an immediate deletion after the initial snapshot', async () => {
106+
const { settings, snapshot } = setup()
107+
const deletion = settings.project.delete('theme')
108+
await setImmediate()
109+
snapshot.resolve({ theme: 'light', language: 'en' })
110+
await deletion
111+
await expect(settings.project.all()).resolves.toEqual({ language: 'en' })
112+
})
113+
114+
it.each(['live', 'static'])('starts an absent %s store with an empty object', async (backend) => {
115+
const { settings, snapshot } = setup(backend)
116+
const read = settings.project.all()
117+
await setImmediate()
118+
snapshot.resolve(undefined)
119+
await expect(read).resolves.toEqual({})
120+
await settings.project.set('theme', 'dark')
121+
await expect(settings.project.get('theme')).resolves.toBe('dark')
122+
})
123+
124+
it.each(['project', 'global'] as const)('keeps an absent %s store local until a setting is written', async (scope) => {
125+
const { rpc, settings, snapshot } = setup()
126+
const onChange = vi.fn()
127+
const operations = Promise.all([
128+
settings[scope].get('theme'),
129+
settings[scope].all(),
130+
settings[scope].onChange(onChange),
131+
settings[scope].delete('theme'),
132+
])
133+
snapshot.resolve(undefined)
134+
const [value, all, unsubscribe] = await operations
135+
expect(value).toBeUndefined()
136+
expect(all).toEqual({})
137+
expect(vi.mocked(rpc.callEvent).mock.calls).toEqual([
138+
['devframe:rpc:server-state:subscribe', `devframe:settings:${scope}:test`],
139+
])
140+
expect(onChange).not.toHaveBeenCalled()
141+
142+
await settings[scope].set('theme', 'dark')
143+
expect(rpc.callEvent).toHaveBeenLastCalledWith(
144+
'devframe:rpc:server-state:set',
145+
`devframe:settings:${scope}:test`,
146+
{ theme: 'dark' },
147+
expect.any(String),
148+
)
149+
expect(onChange).toHaveBeenCalledExactlyOnceWith({ theme: 'dark' })
150+
unsubscribe()
151+
})
152+
153+
it.each(['project', 'global'] as const)('preserves file storage after a browser-side %s read', async (scope) => {
154+
const dir = mkdtempSync(join(tmpdir(), 'devframe-settings-'))
155+
const host = {
156+
mountStatic: () => {},
157+
resolveOrigin: () => 'http://localhost',
158+
getStorageDir: (storageScope: string) => join(dir, storageScope),
159+
}
160+
const filepath = join(dir, scope, 'settings/test.json')
161+
mkdirSync(join(dir, scope, 'settings'), { recursive: true })
162+
writeFileSync(filepath, JSON.stringify({ theme: 'saved' }))
163+
try {
164+
const ctx = await createHostContext({ cwd: dir, mode: 'dev', host })
165+
// Dispatch RPC handlers directly without a transport session.
166+
vi.spyOn(ctx.rpc, 'getCurrentRpcSession').mockReturnValue(undefined)
167+
const { rpc, settings } = setup()
168+
vi.mocked(rpc.call).mockImplementation(ctx.rpc.invokeLocal.bind(ctx.rpc))
169+
vi.mocked(rpc.callEvent).mockImplementation(ctx.rpc.invokeLocal.bind(ctx.rpc))
170+
171+
await settings[scope].all()
172+
const nodeSettings = ctx.scope('test').settings[scope]
173+
await expect(nodeSettings.get('theme')).resolves.toBe('saved')
174+
await nodeSettings.set('theme', 'changed')
175+
await vi.waitFor(() => {
176+
expect(JSON.parse(readFileSync(filepath, 'utf8'))).toEqual({ theme: 'changed' })
177+
})
178+
const restarted = await createHostContext({ cwd: dir, mode: 'dev', host })
179+
await expect(restarted.scope('test').settings[scope].get('theme')).resolves.toBe('changed')
180+
}
181+
finally {
182+
rmSync(dir, { recursive: true, force: true })
183+
}
184+
})
185+
186+
it('waits for trust before requesting the initial snapshot', async () => {
187+
const { rpc, settings, snapshot, trust } = setup('live', false)
188+
const write = settings.project.set('theme', 'dark')
189+
await setImmediate()
190+
expect(rpc.call).not.toHaveBeenCalled()
191+
expect(rpc.callEvent).not.toHaveBeenCalled()
192+
Object.defineProperty(rpc, 'isTrusted', { value: true })
193+
trust.resolve(true)
194+
snapshot.resolve({ language: 'en' })
195+
await write
196+
await expect(settings.project.all()).resolves.toEqual({ theme: 'dark', language: 'en' })
197+
})
198+
199+
it('rejects a failed initial snapshot and retries on the next operation', async () => {
200+
const { rpc, settings } = setup()
201+
vi.mocked(rpc.call).mockRejectedValueOnce(new Error('snapshot failed'))
202+
await expect(settings.project.get('theme')).rejects.toThrow('snapshot failed')
203+
vi.mocked(rpc.call).mockResolvedValueOnce({ theme: 'light' })
204+
await expect(settings.project.get('theme')).resolves.toBe('light')
205+
})
206+
207+
it('continues to accept remote updates after initialization', async () => {
208+
const { settings, snapshot, handlers } = setup()
209+
await setImmediate()
210+
snapshot.resolve({ theme: 'light' })
211+
await settings.project.set('theme', 'dark')
212+
handlers.get(DEVFRAME_EVENTS.broadcast.clientStateUpdated)!('devframe:settings:project:test', { theme: 'system' }, 'remote')
213+
await expect(settings.project.get('theme')).resolves.toBe('system')
214+
})
215+
})

‎packages/devframe/src/client/settings.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,17 @@ function createClientSettingsStore<T extends Record<string, any>>(
1111
const stateKey = `devframe:settings:${scope}:${namespace}`
1212
let statePromise: Promise<SharedState<T>> | undefined
1313

14-
// The client mirrors the server's file-backed settings store over the
15-
// shared-state sync protocol: providing an empty initial value lets the
16-
// client subscribe and merge the authoritative server snapshot, and any
17-
// local `set` is pushed back to (and persisted by) the server.
14+
// Wait for the initial snapshot before reading or changing settings.
15+
// This keeps initialization from overwriting the first local operation.
1816
function store(): Promise<SharedState<T>> {
1917
if (!statePromise) {
20-
statePromise = (rpc.sharedState.get as any)(stateKey, { initialValue: {} }) as Promise<SharedState<T>>
18+
statePromise = (async () => {
19+
await rpc.ensureTrusted()
20+
return rpc.sharedState.get<T>(stateKey)
21+
})().catch((error) => {
22+
statePromise = undefined
23+
throw error
24+
})
2125
}
2226
return statePromise
2327
}

‎packages/devframe/src/node/__tests__/scope.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,42 @@ describe('ctx.scope()', () => {
180180
expect(JSON.parse(readFileSync(globalFile, 'utf-8'))).toEqual({ token: 'abc' })
181181
})
182182

183+
it.each(['project', 'global'] as const)('persists client-first %s settings and loads them on first RPC read', async (scope) => {
184+
const { ctx, dir } = await createCtx()
185+
const key = `devframe:settings:${scope}:client-only`
186+
// Exercise the handlers used by RPC clients without touching node settings first.
187+
await ctx.rpc.invokeLocal('devframe:rpc:server-state:set', key, { theme: 'dark' }, 'client')
188+
await sleep(250)
189+
const restarted = await createHostContext({ cwd: dir, mode: 'dev', host: createTestHost(dir) })
190+
expect(await restarted.rpc.invokeLocal('devframe:rpc:server-state:get', key)).toEqual({ theme: 'dark' })
191+
expect(await restarted.scope('client-only').settings[scope].all()).toEqual({ theme: 'dark' })
192+
})
193+
194+
it('loads existing settings for a client-first patch and shares the same state with node settings', async () => {
195+
const { ctx, dir } = await createCtx()
196+
await ctx.scope('my-plugin').settings.project.set('theme', 'dark')
197+
await sleep(250)
198+
const restarted = await createHostContext({ cwd: dir, mode: 'dev', host: createTestHost(dir) })
199+
const key = 'devframe:settings:project:my-plugin'
200+
await restarted.rpc.invokeLocal('devframe:rpc:server-state:patch', key, [{ op: 'add', path: ['zoom'], value: 2 }], 'client')
201+
expect(await restarted.scope('my-plugin').settings.project.all()).toEqual({ theme: 'dark', zoom: 2 })
202+
expect(await restarted.scope('other-plugin').settings.project.all()).toEqual({})
203+
expect(await restarted.scope('my-plugin').settings.global.all()).toEqual({})
204+
await sleep(250)
205+
expect(JSON.parse(readFileSync(join(dir, 'project/settings/my-plugin.json'), 'utf-8'))).toEqual({ theme: 'dark', zoom: 2 })
206+
})
207+
208+
it.each(['ordinary:state', 'devframe:settings:workspace:plugin', 'devframe:settings:project:../escaped'])('keeps %s in memory', async (key) => {
209+
const { ctx, dir } = await createCtx()
210+
expect(await ctx.rpc.invokeLocal('devframe:rpc:server-state:get', key)).toBeUndefined()
211+
await ctx.rpc.invokeLocal('devframe:rpc:server-state:set', key, { value: 1 }, 'client')
212+
expect(await ctx.rpc.invokeLocal('devframe:rpc:server-state:get', key)).toEqual({ value: 1 })
213+
await sleep(250)
214+
expect(existsSync(join(dir, 'project'))).toBe(false)
215+
const restarted = await createHostContext({ cwd: dir, mode: 'dev', host: createTestHost(dir) })
216+
expect(await restarted.rpc.invokeLocal('devframe:rpc:server-state:get', key)).toBeUndefined()
217+
})
218+
183219
it('notifies onChange subscribers', async () => {
184220
const { ctx } = await createCtx()
185221
const { settings } = ctx.scope('my-plugin')

‎packages/devframe/src/node/host-functions.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { removeClientAgentSession } from './client-agent'
77
import { diagnostics } from './diagnostics'
88
import { createRpcSharedStateServerHost } from './rpc-shared-state'
99
import { createRpcStreamingServerHost } from './rpc-streaming'
10+
import { resolveSettingsState } from './settings'
1011

1112
const debugBroadcast = createDebug('devframe:rpc:broadcast')
1213

@@ -38,7 +39,7 @@ export class RpcFunctionsHostImpl extends RpcFunctionsCollectorBase<DevframeRpcS
3839
constructor(context: DevframeNodeContext) {
3940
super(context)
4041

41-
this.sharedState = createRpcSharedStateServerHost(this)
42+
this.sharedState = createRpcSharedStateServerHost(this, key => resolveSettingsState(context, key))
4243
this.streaming = createRpcStreamingServerHost(this)
4344
}
4445

0 commit comments

Comments
 (0)