Skip to content

Commit fd40a7b

Browse files
authored
fix(agent): install a colour palette in one mutation, not one per token (#317)
`site_set_color_tokens` looped `createFrameworkColorToken` once per token. Each of those is its own site mutation, and any mutation that ensures a not-yet-bound collab doc registers a write gate at `synced: false` — so every remaining write in the same synchronous tick was refused with `syncing`. The action returned a token object either way and the runner recorded it as created, so the tool answered "14 tokens created" for a palette of which only the first 7 reached the relay and the database. Authored CSS then referenced `var(--case)` and `var(--rock)` that did not exist. Nothing in the flow said so: the header simply rendered white. Colour tokens now go through a single `upsertFrameworkColorTokens` action — one mutation, one gate check — which plans slugs against a growing copy so in-batch uniqueness and category canonicalization match what a sequence of single-token calls produced. It returns `accepted`, and the runner reports a refused batch as an error instead of a list of tokens nobody stored. The font and scale runners had the same silent-success shape; both now read their writes back and fail loudly when the value is absent. Test: 8 tokens land in one mutation; slugs stay unique inside a batch; re-running updates in place; a refused batch returns ok:false and stores nothing.
1 parent c35fd8f commit fd40a7b

4 files changed

Lines changed: 258 additions & 18 deletions

File tree

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
/**
2+
* Installing a palette must be ONE mutation, and a refused write must be
3+
* reported as a failure.
4+
*
5+
* `site_set_color_tokens` used to loop one `createFrameworkColorToken` per
6+
* token. Each of those is its own site mutation, and any mutation that ensures
7+
* a not-yet-bound collab doc registers a write gate at `synced: false` — so
8+
* every remaining write in the same synchronous tick was refused with
9+
* `syncing`. The action still returned a token object regardless, so the tool
10+
* answered "14 tokens created" for a palette of which only a prefix reached
11+
* the relay and the database. Colours the CSS then referenced simply did not
12+
* exist, and nothing in the authoring flow said so.
13+
*/
14+
import { afterEach, describe, expect, it } from 'bun:test'
15+
import * as Y from 'yjs'
16+
import * as awarenessProtocol from 'y-protocols/awareness'
17+
import {
18+
connectCollabProvider,
19+
disconnectCollabProvider,
20+
} from '@site/store/slices/site/collabBinding'
21+
import type {
22+
BoundCollabDoc,
23+
CollabProvider,
24+
CollabResetListener,
25+
} from '@site/collab/collabProvider'
26+
import { clearCollabBlockNotice } from '@site/store/slices/site/collabNotices'
27+
import { useEditorStore } from '@site/store/store'
28+
import { runSetColorTokens } from '@site/agent/tokenRunners'
29+
import '@modules/base/index'
30+
31+
/** Provider whose docs stay unsynced until the test releases them. */
32+
function deferredProvider(): CollabProvider & { releaseAll: () => void } {
33+
const presenceDoc = new Y.Doc()
34+
const awareness = new awarenessProtocol.Awareness(presenceDoc)
35+
const bound = new Map<string, BoundCollabDoc>()
36+
const releases: Array<() => void> = []
37+
const resetListeners = new Set<CollabResetListener>()
38+
return {
39+
bind: (docId) => {
40+
let entry = bound.get(docId)
41+
if (!entry) {
42+
let release = (): void => {}
43+
const whenSynced = new Promise<void>((resolve) => {
44+
release = () => resolve()
45+
})
46+
entry = { doc: new Y.Doc(), synced: false, whenSynced }
47+
releases.push(() => {
48+
entry!.synced = true
49+
release()
50+
})
51+
bound.set(docId, entry)
52+
}
53+
return entry
54+
},
55+
unbind: (docId) => {
56+
bound.get(docId)?.doc.destroy()
57+
bound.delete(docId)
58+
},
59+
awareness,
60+
status: () => 'connected',
61+
canSend: () => true,
62+
reconnectNow: () => {},
63+
onStatus: () => () => {},
64+
onReset: (listener) => {
65+
resetListeners.add(listener)
66+
return () => resetListeners.delete(listener)
67+
},
68+
releaseAll: () => {
69+
for (const release of releases) release()
70+
},
71+
destroy: () => {
72+
awareness.destroy()
73+
presenceDoc.destroy()
74+
},
75+
}
76+
}
77+
78+
const PALETTE = Array.from({ length: 8 }, (_, i) => ({
79+
slug: `hue-${i + 1}`,
80+
lightValue: `hsla(${i * 40}, 60%, 50%, 1)`,
81+
}))
82+
83+
function storedSlugs(): string[] {
84+
return (useEditorStore.getState().site?.settings.framework?.colors.tokens ?? []).map(
85+
(token) => token.slug,
86+
)
87+
}
88+
89+
afterEach(() => {
90+
disconnectCollabProvider()
91+
// These tests load a site into the shared editor store; leaving it behind
92+
// leaks into every later suite that reads the same singleton.
93+
useEditorStore.getState().clearSite()
94+
// A refused write latches the "one toast per outage" flag in module scope.
95+
// Left set, the next suite's first outage is silently swallowed.
96+
clearCollabBlockNotice()
97+
})
98+
99+
describe('installing a colour palette in one call', () => {
100+
it('writes every token in a single mutation once docs are synced', async () => {
101+
useEditorStore.getState().createSite('Palette Site')
102+
const provider = deferredProvider()
103+
connectCollabProvider(provider)
104+
provider.releaseAll()
105+
// Let the whenSynced promises settle so the gates are open.
106+
await new Promise((resolve) => setTimeout(resolve, 10))
107+
108+
const result = useEditorStore.getState().upsertFrameworkColorTokens(PALETTE)
109+
110+
expect(result.accepted).toBe(true)
111+
expect(result.tokens).toHaveLength(8)
112+
expect(storedSlugs()).toEqual(PALETTE.map((t) => t.slug))
113+
})
114+
115+
it('keeps slugs unique against tokens created earlier in the same batch', async () => {
116+
useEditorStore.getState().createSite('Dedup Site')
117+
const provider = deferredProvider()
118+
connectCollabProvider(provider)
119+
provider.releaseAll()
120+
await new Promise((resolve) => setTimeout(resolve, 10))
121+
122+
// Two DIFFERENT entries normalizing to the same slug must not collapse:
123+
// the second is a distinct token and gets a suffixed slug, exactly as a
124+
// sequence of single-token calls would have produced.
125+
useEditorStore.getState().upsertFrameworkColorTokens([
126+
{ slug: 'brand', lightValue: 'hsla(0, 0%, 0%, 1)' },
127+
{ slug: 'Brand Two', lightValue: 'hsla(0, 0%, 50%, 1)' },
128+
])
129+
130+
expect(storedSlugs()).toEqual(['brand', 'brand-two'])
131+
})
132+
133+
it('re-running the same palette updates in place instead of suffixing', async () => {
134+
useEditorStore.getState().createSite('Rerun Site')
135+
const provider = deferredProvider()
136+
connectCollabProvider(provider)
137+
provider.releaseAll()
138+
await new Promise((resolve) => setTimeout(resolve, 10))
139+
140+
useEditorStore.getState().upsertFrameworkColorTokens(PALETTE)
141+
const second = useEditorStore.getState().upsertFrameworkColorTokens(PALETTE)
142+
143+
expect(second.tokens.every((t) => t.action === 'updated')).toBe(true)
144+
expect(storedSlugs()).toEqual(PALETTE.map((t) => t.slug))
145+
})
146+
147+
it('reports a refused batch as an error instead of a list of created tokens', () => {
148+
useEditorStore.getState().createSite('Blocked Site')
149+
// Never released: every doc stays unsynced, so the write path refuses.
150+
connectCollabProvider(deferredProvider())
151+
152+
const output = runSetColorTokens({ tokens: PALETTE })
153+
154+
expect(output.ok).toBe(false)
155+
expect(String(output.error)).toContain('not saved')
156+
expect(storedSlugs()).toEqual([])
157+
})
158+
})

src/admin/pages/site/agent/tokenRunners.ts

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import {
2727
} from '@core/ai'
2828
import { apiRequest } from '@core/http'
2929
import { getErrorMessage } from '@core/utils/errorMessage'
30-
import { normalizeFrameworkColorSlug } from '@core/framework'
3130
import { FontEntrySchema, normalizeFontTokenVariable } from '@core/fonts'
3231
import type { EditorStore } from '@site/store/types'
3332
import { getAgentStoreApi } from './storeRef'
@@ -46,6 +45,9 @@ const FontInstallResponseSchema = Type.Object({ font: FontEntrySchema })
4645
// Runners
4746
// ---------------------------------------------------------------------------
4847

48+
const SCALE_NOT_SAVED =
49+
'The scale was not saved: the editor could not reach the collaboration server. Try again in a moment.'
50+
4951
/** Build the `--<prefix>-<step>` variable list a scale group generates. */
5052
function generatedScaleVars(namingConvention: string, steps: string): string[] {
5153
return steps
@@ -59,29 +61,29 @@ export function runSetColorTokens(rawInput: unknown): AiToolOutput {
5961
const input = parseValue(SetColorTokensInputSchema, rawInput)
6062
const store = getStoreState()
6163
if (!store.site) return aiToolError('No active site.')
62-
const results: Array<{ slug: string; ref: string; action: 'created' | 'updated' }> = []
6364

64-
for (const t of input.tokens) {
65-
// Create-or-update by normalized slug so re-runs patch the existing token
66-
// instead of minting `primary-2`.
67-
const norm = normalizeFrameworkColorSlug(t.slug)
68-
const existing = getStoreState().site?.settings.framework?.colors.tokens ?? []
69-
const match = existing.find((e) => normalizeFrameworkColorSlug(e.slug) === norm)
70-
const patch = {
65+
// One mutation for the whole palette, not one per token. Create-or-update is
66+
// keyed by normalized slug inside the batch, so re-runs patch the existing
67+
// token instead of minting `primary-2`. Batching is a correctness
68+
// requirement, not an optimization: a per-token loop lets the collab write
69+
// gate close midway and silently drop every remaining token.
70+
const { tokens, accepted } = store.upsertFrameworkColorTokens(
71+
input.tokens.map((t) => ({
72+
slug: t.slug,
7173
lightValue: t.lightValue,
7274
...(t.category !== undefined ? { category: t.category } : {}),
7375
...(t.darkValue !== undefined ? { darkValue: t.darkValue } : {}),
7476
...(t.darkModeEnabled !== undefined ? { darkModeEnabled: t.darkModeEnabled } : {}),
75-
}
76-
if (match) {
77-
store.updateFrameworkColorToken(match.id, patch)
78-
results.push({ slug: match.slug, ref: `var(--${match.slug})`, action: 'updated' })
79-
} else {
80-
const created = store.createFrameworkColorToken({ slug: t.slug, ...patch })
81-
results.push({ slug: created.slug, ref: `var(--${created.slug})`, action: 'created' })
82-
}
77+
})),
78+
)
79+
if (!accepted) {
80+
return aiToolError(
81+
'Color tokens were not saved: the editor could not reach the collaboration server. Try again in a moment.',
82+
)
8383
}
84-
return aiToolOk({ tokens: results })
84+
return aiToolOk({
85+
tokens: tokens.map((t) => ({ ...t, ref: `var(--${t.slug})` })),
86+
})
8587
}
8688

8789
export async function runSetFontTokens(rawInput: unknown): Promise<AiToolOutput> {
@@ -158,6 +160,19 @@ export async function runSetFontTokens(rawInput: unknown): Promise<AiToolOutput>
158160
})
159161
}
160162
}
163+
164+
// Read back rather than trusting the writes: a refused mutation leaves the
165+
// token absent while the loop above still recorded it as created, which is
166+
// how a site ends up referencing `var(--font-x)` that was never installed.
167+
const saved = new Set(
168+
(getStoreState().site?.settings.fonts?.tokens ?? []).map((token) => token.variable),
169+
)
170+
const lost = results.filter((r) => !saved.has(r.variable)).map((r) => r.variable)
171+
if (lost.length > 0) {
172+
return aiToolError(
173+
`Font tokens were not saved (${lost.join(', ')}): the editor could not reach the collaboration server. Try again in a moment.`,
174+
)
175+
}
161176
return aiToolOk({ tokens: results })
162177
}
163178

@@ -197,6 +212,10 @@ export function runSetTypeScale(rawInput: unknown): AiToolOutput {
197212
const group = getStoreState().site?.settings.framework?.typography?.groups.find(
198213
(g) => g.id === groupId,
199214
)
215+
// Read back rather than trusting the write: a mutation the collab path
216+
// refused leaves no group behind, and answering with the requested steps
217+
// would report a scale the site does not have.
218+
if (!group) return aiToolError(SCALE_NOT_SAVED)
200219
const namingConvention = group?.namingConvention ?? input.namingConvention ?? 'text'
201220
const steps = group?.steps ?? input.steps ?? ''
202221
return aiToolOk({
@@ -243,6 +262,8 @@ export function runSetSpacingScale(rawInput: unknown): AiToolOutput {
243262
const group = getStoreState().site?.settings.framework?.spacing?.groups.find(
244263
(g) => g.id === groupId,
245264
)
265+
// See the matching read-back in `runSetTypeScale`.
266+
if (!group) return aiToolError(SCALE_NOT_SAVED)
246267
const namingConvention = group?.namingConvention ?? input.namingConvention ?? 'space'
247268
const steps = group?.steps ?? input.steps ?? ''
248269
return aiToolOk({

src/admin/pages/site/store/slices/site/framework/colors.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ function reorderFrameworkColorTokenInGroup(
245245
type FrameworkColorActions = Pick<
246246
SiteSlice,
247247
| 'createFrameworkColorToken'
248+
| 'upsertFrameworkColorTokens'
248249
| 'updateFrameworkColorToken'
249250
| 'duplicateFrameworkColorToken'
250251
| 'reorderFrameworkColorToken'
@@ -275,6 +276,57 @@ export function createFrameworkColorActions({
275276
return token
276277
},
277278

279+
/**
280+
* Create-or-update a whole token list in ONE site mutation.
281+
*
282+
* A loop of single-token actions is not equivalent: each one is its own
283+
* mutation, and a mutation that ensures a not-yet-bound collab doc
284+
* registers an unsynced write gate, so every LATER write in the same tick
285+
* is refused with `syncing`. Callers that install a palette in one go
286+
* (`site_set_color_tokens`, theme presets) then keep only the prefix that
287+
* ran before the gate closed — with each individual action still returning
288+
* a token, so the loss is silent. One mutation means one gate check, and
289+
* the boolean below reports the outcome honestly.
290+
*/
291+
upsertFrameworkColorTokens: (inputs) => {
292+
const { site } = get()
293+
if (!site) throw new Error('[siteSlice] Site document is not initialized')
294+
const results: Array<{ slug: string; action: 'created' | 'updated' }> = []
295+
// Plan against a growing copy so slug uniqueness and category
296+
// canonicalization see the tokens earlier entries in THIS batch added —
297+
// the same view a sequence of single-token calls would have had.
298+
const planned = [...(site.settings.framework?.colors?.tokens ?? [])]
299+
const creations: Array<{ token: FrameworkColorToken }> = []
300+
const updates: Array<{ id: string; patch: UpdateFrameworkColorTokenPatch }> = []
301+
302+
for (const input of inputs) {
303+
const norm = normalizeFrameworkColorSlug(input.slug)
304+
const match = planned.find((t) => normalizeFrameworkColorSlug(t.slug) === norm)
305+
if (match) {
306+
updates.push({ id: match.id, patch: input })
307+
results.push({ slug: match.slug, action: 'updated' })
308+
continue
309+
}
310+
const token = createFrameworkColorTokenFromInput(input, { tokens: planned })
311+
planned.push(token)
312+
creations.push({ token })
313+
results.push({ slug: token.slug, action: 'created' })
314+
}
315+
316+
const accepted = mutateSite((draftSite) => {
317+
const draftColors = ensureFrameworkColors(draftSite)
318+
for (const { id, patch } of updates) {
319+
const token = draftColors.tokens.find((candidate) => candidate.id === id)
320+
if (token) applyFrameworkColorTokenPatch(token, patch, draftColors)
321+
}
322+
for (const { token } of creations) draftColors.tokens.push(token)
323+
reconcileFrameworkClasses(draftSite)
324+
return true
325+
})
326+
327+
return { tokens: results, accepted }
328+
},
329+
278330
updateFrameworkColorToken: (tokenId, patch) => {
279331
mutateSite((site) => {
280332
const colors = site.settings.framework?.colors

src/admin/pages/site/store/slices/site/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,15 @@ export interface SiteSlice {
245245

246246
// Framework color mutations
247247
createFrameworkColorToken: (input: CreateFrameworkColorTokenInput) => FrameworkColorToken
248+
/**
249+
* Create-or-update many tokens in ONE mutation. `accepted` is false when the
250+
* collab write path refused the whole batch (offline / still syncing) — the
251+
* caller must not report those tokens as installed.
252+
*/
253+
upsertFrameworkColorTokens: (inputs: readonly CreateFrameworkColorTokenInput[]) => {
254+
tokens: Array<{ slug: string; action: 'created' | 'updated' }>
255+
accepted: boolean
256+
}
248257
updateFrameworkColorToken: (tokenId: string, patch: UpdateFrameworkColorTokenPatch) => void
249258
duplicateFrameworkColorToken: (tokenId: string) => FrameworkColorToken | null
250259
reorderFrameworkColorToken: (tokenId: string, direction: 'up' | 'down') => void

0 commit comments

Comments
 (0)