diff --git a/.changeset/rfc45-step5-mcp.md b/.changeset/rfc45-step5-mcp.md index 69c8510c..3067eabd 100644 --- a/.changeset/rfc45-step5-mcp.md +++ b/.changeset/rfc45-step5-mcp.md @@ -12,7 +12,7 @@ RFC-0045 rollout step 5: reference and repeater fields over MCP. The two types s - **Approval prompts** name targets readably, from the dry run's planned entry (canonical ids): `venue → "Main hall" (entry_…, Places)`, `speakers: 2 items → 3 items (added "Ada Lovelace", Speaker; changed item 1: person → "Grace Hopper" (entry_…, People))`, quoted and truncated. - **New read-only tool `list_references`** (`content:read`) with `direction: "to"` (referrers, published first, with the members that show them) or `"from"` (the groups an entry names, its original's inherited ones included). 110 → 111 tools. The API's `GET …/entries/:entryId/references` and the tool share core-ts's new `listReferrers` (and `listReferenceTargets`, `referenceCard`). - **`list_entries`** returns each entry's `groupId`, the value a reference stores. -- **Translation tools**: `get_translation_source` lists a localized repeater's text and rich-text sub-values by item path (`speakers/0/talk`) with the source's `sourceRev`; `save_translation` takes them back with that `sourceRev` (`conflict` when the source changed) and folds them into a copy of the source's list, keeping each item's other sub-values (core-ts `translatedItemValues`). Once the translation holds its own list for that repeater, item paths are refused and the list is sent whole; a whole list of another length than the source's is refused too (`translation_shape_mismatch`, new code, 409 in the API). A path naming no such value is refused. Keys whose first part is not a repeater (`og/image`) stay plain keys. +- **Translation tools**: `get_translation_source` lists a localized repeater's text and rich-text sub-values by item path (`speakers/0/talk`) with the source's `sourceRev`; `save_translation` takes them back with that `sourceRev` (`conflict` when the source changed) and folds them into a copy of the source's list, keeping each item's other sub-values (core-ts `translatedItemValues`). Once the translation holds its own list for that repeater, item paths are refused and the list is sent whole; a whole list given together with item paths must have the source's length (`translation_shape_mismatch`, new code, 409 in the API). A path naming no such value is refused. Keys whose first part is not a repeater (`og/image`) stay plain keys. - **`normalizeEntryFields`** is exported from `@cloudhal/core-ts`: MCP's approval prompts read stored values through the markers exactly as the store does. - **`continueFieldOperation`** with `onInvalid: "clear"` reports the values it drops as cleared, without the stored request's `field_values_unconvertible` refusal. - **Removals during `rename_field`** over MCP: the old and the new key are one field; removing either removes both, so the writer's move cannot bring the value back, and setting one while removing the other is `invalid`. diff --git a/.changeset/rfc45-step7a-fixes.md b/.changeset/rfc45-step7a-fixes.md new file mode 100644 index 00000000..1834fb6a --- /dev/null +++ b/.changeset/rfc45-step7a-fixes.md @@ -0,0 +1,10 @@ +--- +"@cloudhal/core-ts": patch +"@cloudhal/mcp-server": patch +--- + +RFC-0045 rollout step 7a: two fixes ahead of enabling reference and repeater fields. Both types stay refused on every production write path until step 7b; nothing changes for the nine existing types. + +- **core-ts (#215).** While a top-level conversion from references runs without a text ↔ reference path (reference (multiple) ↔ repeater, reference single ↔ multiple, a repeater converting away), a stored value still in the old shape that does not convert keeps its `entry_refs` rows through a person's unrelated write, such as a title edit. Before, the write dropped them, and the target's delete guard under-blocked. The writer holds the rows at the old type's paths, the same ones the import rebuild keeps (one helper, `oldShapePairs`), so the rows no longer depend on which touched the entry last. +- **core-ts: held rows follow their holder's status.** A row a write holds (a value a running conversion did not accept, here or during a text ↔ reference conversion) kept the `source_status` of its last insert, so publishing the holder left the target deletable and unpublishing left it locked. The write now updates the held rows' `source_status` in the same batch, anchored on the entry's new hash. +- **mcp-server, core-ts (#218).** Once a translation holds its own list for a localized repeater, `get_translation_source` returns the source's whole list for that key under `fields` (in the source's order, with every sub-value) instead of item paths, and names it in the new `wholeLists` output: send it back whole, translated. `save_translation` already refused item paths there (`translation_shape_mismatch`); its hint and the tool descriptions now say what to send. `save_translation` also checks a given `sourceRev` against whole repeater lists (`conflict` when the source moved on); without one they save as before. New core-ts export: `translationSourceValues`. diff --git a/docs/rfcs/0045-collections-stage-4-references-and-repeaters.md b/docs/rfcs/0045-collections-stage-4-references-and-repeaters.md index a1c200d7..2287660f 100644 --- a/docs/rfcs/0045-collections-stage-4-references-and-repeaters.md +++ b/docs/rfcs/0045-collections-stage-4-references-and-repeaters.md @@ -1381,7 +1381,7 @@ Round 1 dropped the "fits" shortcut for text ↔ reference, but then the table r - A key is an item path only when the part before its first `/` names a repeater of the collection (`itemPathRepeater`); any other key, such as `og/image` in a collection without definitions, is a plain key, as before. - Item paths write into a copy of the source's list (or into a list given whole in the same call), so each item keeps the source's other sub-values: targets, images, selects. - **Once the same-locale translation holds its own list for that repeater, item paths are refused** (`translation_shape_mismatch`, 409, "send the whole list"): re-translate by sending the whole list, in the source's order. Either side may have been reordered or edited since the first translation, and no revision check can tell whether an index still names the same item, so the rule does not try (review round 3 replaced a `translation.source.rev` check that had holes). - - A list given whole must have the source's length (`translation_shape_mismatch`). + - A list given whole must have the source's length (`translation_shape_mismatch`). *Amended in 7a:* only when item paths come in the same call; a whole list sent alone is the translation's own and is stored as sent. - Paths name items by index, so `get_translation_source` returns `sourceRev`, and `save_translation` requires it with item paths (`invalid` without, `conflict` when the source moved on). The handler refuses paths that no longer fit, as the precheck does; nothing is dropped silently. - **`set_field_localized`** takes no `sub` over MCP, since the store refuses it (§7 "Amended in 4"). - **The dry run's reach.** It returns after the asset references, the release hold (`scheduled_locked`), time gates and the term invariants (`required_term`), with the planned entry. It does not plan the path change (`path_conflict`, redirects), a collection's first entry, list values or source-forward, and slug uniqueness is decided by the write. The prechecks build the same entry the handler writes (terms, cover, title and body included). @@ -1446,3 +1446,11 @@ Round 1 dropped the "fits" shortcut for text ↔ reference, but then the table r - **A type change's target settings lock** (a disabled `fieldset`) whenever its inputs do: running, stuck or finished. - **"Other entries left" is decided per referring field**, as the store decides: referrers whose field has no usable other member are listed as showing nothing, the others name the language that loses the card; both can show. - **The blocked count is the store's** (`total`, never capped at the list's 100), the "more references" note shows whenever rows remain unlisted, and a late refusal reads the referrers again. + +### Amended in 7a (2026-11-13, rollout step 7, first PR: #215 and #218) + +- **Old-shape values under a reshape from references** ([#215](https://github.com/cloudhal-dev/cloudhal/issues/215)). A running top-level conversion from references that no text ↔ reference rule reads (reference (multiple) ↔ repeater, reference single ↔ multiple, a repeater converting away) has no row path (`convertingPath`). A stored value still in the old shape that does not convert, such as a malformed list for reference → repeater, now keeps its rows through a person's unrelated write: `normalizeWrite` holds them at the old type's paths, each id-shaped string at `` from a reference and each item's id-shaped sub-value at `.` from a repeater. The rebuild (`reshapedRows`) and the writer share one helper (`references/old-shape.ts`, `oldShapePairs`), so the rows are the same whichever touched the node last, and the target's delete guard holds. +- **Re-translating a repeater the translation already holds its own list of** ([#218](https://github.com/cloudhal-dev/cloudhal/issues/218)). `save_translation` refuses item paths for it (Amended in 5), so `get_translation_source` no longer offers them. For each such repeater it returns the source's whole list under the repeater's key in `fields`, in the source's order and with every sub-value, and names it in a new `wholeLists` output (core-ts `translationSourceValues`). The agent translates its text and rich-text sub-values and sends the whole list back. The `translation_shape_mismatch` hint and the tool descriptions say so. +- **Held rows follow their holder's status** (PR #221 review). A row a person's write holds kept the `source_status` of its last insert, since the writer leaves it out of its `DELETE` and `INSERT`, while the delete guard reads that column. The write now sets the held pairs' `source_status` to the entry's new status in the same batch, anchored on its new hash (`references/held-status.ts`), for reshapes and text ↔ reference conversions alike; the rebuild writes the same value. +- **`sourceRev` with whole lists.** A given `sourceRev` is checked for a whole repeater list too (`conflict` when the source moved on), so a list read before the source changed is not linked to the new revision. It stays optional there. +- **The length rule, qualified.** Amended in 5 said a whole list of another length than the source's is refused. That holds only when item paths come in the same call, where the paths index into the given list (`baseList`). A whole list sent alone is the translation's own, like any other localized value, and is stored as sent. §3's "only into a list of the original's length" describes item paths. diff --git a/packages/core-ts/src/fields/index.ts b/packages/core-ts/src/fields/index.ts index 65bbede0..c81a548b 100644 --- a/packages/core-ts/src/fields/index.ts +++ b/packages/core-ts/src/fields/index.ts @@ -142,8 +142,10 @@ export { sharedFieldIssues, sharedFieldKeys, type TranslatedItemsInput, + type TranslationSourceValues, translatableFieldValues, translatedItemValues, + translationSourceValues, } from "./translation.ts"; export { ALL_FIELD_TYPES, diff --git a/packages/core-ts/src/fields/translation.ts b/packages/core-ts/src/fields/translation.ts index a28ca528..d6c5702c 100644 --- a/packages/core-ts/src/fields/translation.ts +++ b/packages/core-ts/src/fields/translation.ts @@ -5,8 +5,10 @@ * the localized `text` and `richText` values to translate: top-level by key, and inside a * localized repeater by item path, `speakers/0/talk` (RFC-0045 §3). Item paths write into a copy * of the source's list, so an item's other sub-values (references, selects, images) come along. - * Once the translation holds its own list, that repeater is sent whole; other lengths are - * `translation_shape_mismatch`. Worker-safe. + * Once the translation holds its own list, that repeater is sent whole: the source hands it over + * whole (`translationSourceValues`), and item paths for it are `translation_shape_mismatch`. A + * list given whole together with item paths must have the source's length; a whole list sent + * alone is the translation's own and is stored as sent (RFC-0045 "Amended in 7a"). Worker-safe. */ import type { FieldDefinition } from "@cloudhal/core-contracts"; import { type FieldIssue, isRecord } from "./types.ts"; @@ -62,6 +64,40 @@ export function translatableFieldValues( return out; } +/** What a translation source hands over to translate (`translationSourceValues`). */ +export interface TranslationSourceValues { + /** `translatableFieldValues`, with a list given whole for each repeater in `whole`. */ + readonly fields: Record; + /** The repeaters sent whole: the existing translation holds its own list of them. */ + readonly whole: string[]; +} + +/** + * RFC-0045 §3 (#218): the values to translate for a source and the existing same-locale + * translation, if any. As `translatableFieldValues`, except a localized repeater the translation + * already holds its own list of: item paths would be refused for it (`baseList`), so the source's + * whole list is handed over under its key instead, in the source's order and with every + * sub-value, to be translated and sent back whole. + */ +export function translationSourceValues( + defs: readonly FieldDefinition[], + source: Values | undefined, + existing: Values | undefined, +): TranslationSourceValues { + const fields: Record = translatableFieldValues(defs, source); + const whole: string[] = []; + for (const def of defs) { + const list = source?.[def.key]; + if (translatableSubs(def).size === 0 || !Array.isArray(list)) continue; + if (!Array.isArray(existing?.[def.key])) continue; + for (const key of Object.keys(fields)) + if (itemPathRepeater(defs, key) === def.key) delete fields[key]; + fields[def.key] = list; + whole.push(def.key); + } + return { fields, whole }; +} + /** * The repeater an item path (`speakers/0/talk`) names: the part before the key's first `/`, when * it is a repeater of `defs`. Any other key is a plain key, `/` included (a collection without @@ -133,6 +169,8 @@ const shapeIssue = (key: string, message: string): FieldIssue => ({ * source's length), else a copy of the source's. When the translation already holds its own list, * a path cannot know which of its items matches the source's by index (either side may have been * reordered or edited since), so that repeater must be sent whole (`translation_shape_mismatch`). + * Only paths reach here: a whole list sent with no path for it is the translation's own and is + * stored as sent, whatever its length. */ function baseList( key: string, diff --git a/packages/core-ts/src/references/held-status.ts b/packages/core-ts/src/references/held-status.ts new file mode 100644 index 00000000..f5bd3c97 --- /dev/null +++ b/packages/core-ts/src/references/held-status.ts @@ -0,0 +1,21 @@ +/** + * RFC-0045 §7: a held row follows its holder's status. A person's write leaves the rows of values + * a running conversion did not accept as they are (`HeldRef`): `entryRefStatements` keeps them out + * of its `DELETE` and its `INSERT`. Their `source_status` is what the delete guard reads (`SHOWN`, + * `references/sql.ts`), so the write sets it to the entry's new status, in the same batch and + * anchored on the entry's new hash like the other rows statements (rule 4). Worker-safe. + */ +import type { SqlStatement } from "@cloudhal/storage"; + +/** The `UPDATE` of the held pairs' `source_status` (`kept`: `" "` keys). */ +export function heldStatusStatement( + at: { siteId: string; nodeId: string; status: string; hash: string }, + kept: ReadonlySet, +): SqlStatement { + return { + sql: `UPDATE entry_refs SET source_status = ? + WHERE site_id = ? AND source_id = ? AND field || ' ' || target_id IN (SELECT value FROM json_each(?)) + AND EXISTS (SELECT 1 FROM nodes WHERE id = ? AND hash = ?)`, + params: [at.status, at.siteId, at.nodeId, JSON.stringify([...kept]), at.nodeId, at.hash], + }; +} diff --git a/packages/core-ts/src/references/old-shape.ts b/packages/core-ts/src/references/old-shape.ts new file mode 100644 index 00000000..99986e4e --- /dev/null +++ b/packages/core-ts/src/references/old-shape.ts @@ -0,0 +1,48 @@ +/** + * RFC-0045 §7: the rows a stored value keeps under a running top-level conversion from references + * that no text ↔ reference rule reads (`convertingPath` is undefined): reference (multiple) ↔ + * repeater, reference single ↔ multiple, a repeater converting away. A value still in the old + * shape is read through the old type, so its rows stand until phase B rewrites it. + * + * One rule for both sides that write rows (#215): the rebuild (`rebuildRows`) keeps these rows + * for every stored value, and a person's write holds them for a stored value that did not + * convert (`normalizeWrite`), so whichever touched the node last leaves the same rows. Pure. + */ +import type { FieldDefinition } from "@cloudhal/core-contracts"; +import { GROUP_ID_PATTERN } from "../fields/props.ts"; +import { isRecord } from "../fields/types.ts"; +import { convertingPath } from "./conversion-rows.ts"; + +const GROUP_ID = new RegExp(GROUP_ID_PATTERN); +const isId = (v: unknown): v is string => typeof v === "string" && GROUP_ID.test(v); + +/** One old-type (field path, group id) pair. */ +export interface OldShapePair { + readonly f: string; + readonly t: string; +} + +/** Whether `def` carries a top-level marker converting from references with no row path. */ +export function readsOldShape(def: FieldDefinition): boolean { + const m = def.migration; + if (m === undefined || m.sub !== undefined) return false; + if (m.fromType !== "reference" && m.fromType !== "repeater") return false; + return convertingPath(def) === undefined; +} + +/** + * The pairs `value`, stored under `key`, holds under `def`'s old type, in order, duplicates + * included: from a reference, each id-shaped string at `key`; from a repeater, each item's + * id-shaped sub-value at `.`. Empty when `def` carries no such marker. + */ +export function oldShapePairs(def: FieldDefinition, key: string, value: unknown): OldShapePair[] { + if (!readsOldShape(def)) return []; + const from = def.migration?.fromType; + const out: OldShapePair[] = []; + for (const v of Array.isArray(value) ? value : [value]) { + if (from === "reference" && isId(v)) out.push({ f: key, t: v }); + if (from === "repeater" && isRecord(v)) + for (const [sub, t] of Object.entries(v)) if (isId(t)) out.push({ f: `${key}.${sub}`, t }); + } + return out; +} diff --git a/packages/core-ts/src/references/rebuild.ts b/packages/core-ts/src/references/rebuild.ts index 2d23f4c4..d732d045 100644 --- a/packages/core-ts/src/references/rebuild.ts +++ b/packages/core-ts/src/references/rebuild.ts @@ -17,6 +17,7 @@ import { GROUP_ID_PATTERN } from "../fields/props.ts"; import { fieldDefinitions, isRecord } from "../fields/types.ts"; import { chunks, REFS_STATEMENT_BYTES } from "../media/refs-rebuild.ts"; import { convertingPath } from "./conversion-rows.ts"; +import { oldShapePairs } from "./old-shape.ts"; import { type EntryRefRow, entryRefRows, sharedOf, storedKey } from "./rows.ts"; const GROUP_ID = new RegExp(GROUP_ID_PATTERN); @@ -64,30 +65,20 @@ export function rebuildRows(site: Pick, entry: ContentEntry } /** - * A running reference (multiple) ↔ repeater conversion (top level): a stored value still in the - * old shape is read through the old type, so its rows stand until phase B rewrites it. From a - * reference, each id-shaped string of the list gets a row at the field path; from a repeater, - * each item's id-shaped sub-value at `.`. + * A running top-level conversion from references with no text ↔ reference path (reference + * (multiple) ↔ repeater, among others): a stored value still in the old shape is read through the + * old type (`oldShapePairs`, the same pairs a person's write holds for a value that does not + * convert), so its rows stand until phase B rewrites it. */ function reshapedRows( def: FieldDefinition, values: Readonly>, have: readonly EntryRefRow[], ): EntryRefRow[] { - const from = def.migration?.fromType; - if (def.migration?.sub !== undefined || (from !== "reference" && from !== "repeater")) return []; const k = storedKey(def, values) ?? def.key; - const stored = values[k]; - const found: { f: string; t: string }[] = []; - const id = (v: unknown): v is string => typeof v === "string" && GROUP_ID.test(v); - for (const v of Array.isArray(stored) ? stored : [stored]) { - if (from === "reference" && id(v)) found.push({ f: k, t: v }); - if (from === "repeater" && isRecord(v)) - for (const [sub, t] of Object.entries(v)) if (id(t)) found.push({ f: `${k}.${sub}`, t }); - } const seen = new Set(have.map((r) => `${r.f} ${r.t}`)); const out: EntryRefRow[] = []; - for (const { f, t } of found) { + for (const { f, t } of oldShapePairs(def, k, values[k])) { if (seen.has(`${f} ${t}`)) continue; seen.add(`${f} ${t}`); out.push({ t, f, k, shared: sharedOf(def) }); diff --git a/packages/core-ts/src/references/writer.ts b/packages/core-ts/src/references/writer.ts index 8cb4a911..d3f2c625 100644 --- a/packages/core-ts/src/references/writer.ts +++ b/packages/core-ts/src/references/writer.ts @@ -23,6 +23,7 @@ import { refuseFieldIssues } from "../fields/refusal.ts"; import type { FieldRows } from "../fields/text-reference.ts"; import { type FieldIssue, fieldDefinitions, isRecord, sameJson } from "../fields/types.ts"; import { and, type Cond } from "../node-sql.ts"; +import { heldStatusStatement } from "./held-status.ts"; import { hasReferenceFields } from "./issues.ts"; import { memberGroup } from "./members.ts"; import { definitionsPath, entryRefRows, holdsReferences, storedKey } from "./rows.ts"; @@ -229,6 +230,8 @@ export function entryRefStatements( params: [draft.id, ...(kept.size > 0 ? [JSON.stringify([...kept])] : []), draft.id, hash], }, ]; + const at = { siteId: site.id, nodeId: draft.id, status: draft.status, hash }; + if (kept.size > 0) out.push(heldStatusStatement(at, kept)); if (rows.length === 0) return out; out.push({ sql: `INSERT INTO entry_refs (site_id, target_id, source_id, source_collection, field, root_key, shared, source_status) diff --git a/packages/core-ts/src/store/entry-fields.ts b/packages/core-ts/src/store/entry-fields.ts index 1f68d9c0..1ed16648 100644 --- a/packages/core-ts/src/store/entry-fields.ts +++ b/packages/core-ts/src/store/entry-fields.ts @@ -33,6 +33,7 @@ import type { Cond } from "../node-sql.ts"; import { loadConversionEntries } from "../references/conversion-load.ts"; import type { ConversionEntries } from "../references/conversion-maps.ts"; import { convertingPath, loadNodeRows } from "../references/conversion-rows.ts"; +import { oldShapePairs } from "../references/old-shape.ts"; import type { HeldRef } from "../references/writer.ts"; import type { StoreContext } from "./context.ts"; @@ -143,6 +144,17 @@ function heldRefs( return out; } +/** A stored old-shape value's failures, per old-type field path (`oldShapePairs`). */ +function oldShapeFailures( + def: FieldDefinition, + stored: readonly FailedValue[], +): { def: FieldDefinition; f: string; ids: string[] }[] { + const by = new Map(); + for (const { value } of stored) + for (const { f, t } of oldShapePairs(def, def.key, value)) by.set(f, [...(by.get(f) ?? []), t]); + return [...by].map(([f, ids]) => ({ def, f, ids })); +} + /** * RFC-0045 §7: a write's draft and its stored entry through the markers. A running text ↔ * reference conversion's maps, and the node's rows for its path, are read only while one runs: @@ -179,10 +191,12 @@ export async function normalizeWrite( // Per value: only a stored value that did not convert keeps its rows as they are; one the // person newly wrote is guarded like any choice. onUnconvertible: (def, failed) => { + const kept = failed.filter((x) => x.stored); const f = convertingPath(def); - if (f === undefined) return; - const ids = failed.filter((x) => x.stored).flatMap((x) => idsOf(x.value)); - failures.push({ def, f, ids }); + if (f !== undefined) failures.push({ def, f, ids: kept.flatMap((x) => idsOf(x.value)) }); + // #215: a reshape from references (reference ↔ repeater, …) has no row path; its stored + // old-shape value keeps the rows the rebuild gives it, at the old type's paths. + else failures.push(...oldShapeFailures(def, kept)); }, }); const held = heldRefs(failures, draft.fields); diff --git a/packages/core-ts/src/testing-field-operations-old-shape.ts b/packages/core-ts/src/testing-field-operations-old-shape.ts new file mode 100644 index 00000000..f16aa965 --- /dev/null +++ b/packages/core-ts/src/testing-field-operations-old-shape.ts @@ -0,0 +1,249 @@ +/** + * RFC-0045 §7, #215: a stored value still in the old shape that does not convert under a running + * top-level conversion from references (reference ↔ repeater) keeps its `entry_refs` rows through + * a person's unrelated write, at the paths the rebuild gives it (`oldShapePairs`), so the target + * stays locked whichever touched the node last. A held row, of a reshape or a text ↔ reference + * conversion, follows its holder's status (`heldStatusStatement`), as the rebuild writes it. + * Every rebuild here is real (`rebuildingImport`). Runs on every backend + * (`describeSiteGraphStore`); the types are enabled with `SiteGraphStoreOptions.fieldTypes`. + */ +import type { Actor, ContentEntry, FieldDefinition, Site } from "@cloudhal/core-contracts"; +import { describe, expect, it } from "vitest"; +import { GATES_ENFORCED } from "./availability.ts"; +import { ConflictError, InvariantError } from "./errors.ts"; +import { type FieldOperation, runFieldOperation } from "./fields-operations/index.ts"; +import { SiteGraphStore } from "./store.ts"; +import { rebuildingImport } from "./testing-reimport.ts"; +import type { SuiteContext } from "./testing-taxonomy.ts"; + +const person: Actor = { kind: "human", id: "user_01HZX8K3Q9CD", name: "Halil" }; +const L = (label: string) => ({ en: { label } }); +const ENABLED = { supported: () => true }; +const PERSON = { key: "person", type: "reference", labels: L("Person"), collections: ["places"] }; + +async function code(p: Promise): Promise { + try { + await p; + } catch (e) { + if (e instanceof InvariantError) return e.code; + if (e instanceof ConflictError) return "conflict"; + throw e; + } + return "ok"; +} + +export function fieldOperationsOldShapeSuite(ctx: SuiteContext): void { + describe("RFC-0045 §7 held rows: old-shape values and status (#215)", () => { + let n = 0; + let tick = 0; + const now = () => new Date(Date.UTC(2026, 10, 13, 9, 0, tick++)); + const store = () => + new SiteGraphStore({ + sql: ctx.sql(), + blobs: ctx.blobs(), + now, + gates: GATES_ENFORCED, + fieldTypes: ENABLED, + }); + + async function world(field: FieldDefinition): Promise { + n++; + const project = await store().createProject({ + name: "Shapes", + slug: `shapes-${n}`, + ownerId: person.id, + siteIds: [], + }); + return store().createSite({ + projectId: project.id, + name: "Shapes", + slug: `shapes-site-${n}`, + defaultLocale: "en", + locales: ["en"], + domains: [], + pageIds: [], + theme: { name: "blog" }, + collections: { events: { fields: [field] }, places: {} }, + }); + } + const entry = (site: Site, collection: string, over: Partial = {}) => + store().createContentEntry({ + siteId: site.id, + collection, + slug: `${collection}-${Math.random().toString(36).slice(2, 8)}`, + locale: "en", + status: "draft", + title: `A ${collection}`, + body: { format: "markdown", value: "Body" }, + fields: {}, + tags: [], + author: person, + ...over, + } as never); + const fresh = async (e: ContentEntry) => (await store().getContentEntry(e.id)) as ContentEntry; + /** The node's rows, `source_status` included (the delete guard reads it). */ + const rows = async (sourceId: string) => + ( + await ctx + .sql() + .all<{ t: string; f: string; k: string; s: number; st: string }>( + "SELECT target_id AS t, field AS f, root_key AS k, shared AS s, source_status AS st FROM entry_refs WHERE source_id = ? ORDER BY field, target_id", + [sourceId], + ) + ).map((r) => ({ ...r, s: Number(r.s) })); + /** A stored value as an import could hold it, written past the store's checks. */ + const storeRaw = (e: ContentEntry, value: unknown) => + ctx + .sql() + .run("UPDATE nodes SET body = json_set(body, '$.fields.hosts', json(?)) WHERE id = ?", [ + JSON.stringify(value), + e.id, + ]); + /** A replace import that really rebuilds the rows (`rebuildingImport`). */ + const reimport = (site: Site) => + rebuildingImport( + { sql: ctx.sql(), blobs: ctx.blobs() }, + { siteId: site.id, fieldTypes: ENABLED, addNode: () => entry(site, "places") }, + ); + /** The operation's marker, with no entry rewritten yet (budget 0). */ + const start = (site: Site, op: FieldOperation) => + runFieldOperation(store(), site.id, op, { actor: person, budget: { entries: 0 } }); + const editTitle = async (e: ContentEntry, title: string) => + code(store().updateContentEntry({ ...(await fresh(e)), title })); + const setStatus = async (e: ContentEntry, status: ContentEntry["status"]) => + code(store().updateContentEntry({ ...(await fresh(e)), status })); + const HOSTS = { + key: "hosts", + type: "reference", + labels: L("Hosts"), + localized: false, + collections: ["places"], + multiple: true, + } as FieldDefinition; + const TO_REPEATER: FieldOperation = { + op: "change_field_type", + collection: "events", + key: "hosts", + type: "repeater", + subFields: [PERSON as never], + }; + + it("reference → repeater: a malformed stored list keeps its rows through a title edit; the target stays locked", async () => { + const site = await world(HOSTS); + const p = await entry(site, "places", { title: "P place", slug: "p-place" }); + const e = await entry(site, "events", { status: "published", fields: { hosts: [p.id] } }); + const expected = [{ t: p.id, f: "hosts", k: "hosts", s: 1, st: "published" }]; + expect(await rows(e.id)).toEqual(expected); + await start(site, TO_REPEATER); + // an old-shape value the reshape cannot convert (not a list of ids) + await storeRaw(e, [p.id, 5]); + + // a write touches the node last + expect(await editTitle(e, "Edited")).toBe("ok"); + expect((await fresh(e)).fields).toEqual({ hosts: [p.id, 5] }); + expect(await rows(e.id)).toEqual(expected); + expect(await code(store().deleteContentEntry(await fresh(p)))).toBe("entry_referenced"); + + // the rebuild touches it last: the same rows + await reimport(site); + expect(await rows(e.id)).toEqual(expected); + expect(await code(store().deleteContentEntry(await fresh(p)))).toBe("entry_referenced"); + + // and a write again after the rebuild + expect(await editTitle(e, "Edited again")).toBe("ok"); + expect(await rows(e.id)).toEqual(expected); + expect(await code(store().deleteContentEntry(await fresh(p)))).toBe("entry_referenced"); + }); + + it("repeater → reference: a stored item of another shape keeps its sub-field rows through a title edit", async () => { + const site = await world({ + key: "hosts", + type: "repeater", + labels: L("Hosts"), + localized: false, + fields: [PERSON], + } as FieldDefinition); + const p = await entry(site, "places", { title: "P place", slug: "p-place" }); + const q = await entry(site, "places", { title: "Q place", slug: "q-place" }); + const e = await entry(site, "events", { + status: "published", + fields: { hosts: [{ person: p.id }, { person: q.id }] }, + }); + await start(site, { + op: "change_field_type", + collection: "events", + key: "hosts", + type: "reference", + collections: ["places"], + }); + // an item holding two values: the repeater does not convert + await storeRaw(e, [{ person: p.id }, { person: q.id, note: "x" }]); + const expected = [ + { t: p.id, f: "hosts.person", k: "hosts", s: 1, st: "published" }, + { t: q.id, f: "hosts.person", k: "hosts", s: 1, st: "published" }, + ].sort((a, b) => (a.t < b.t ? -1 : 1)); + + // a conversion into references holds every target delete meanwhile (R12), so the rows, + // which lock the targets once it ends, are what a write must not drop + expect(await editTitle(e, "Edited")).toBe("ok"); + expect(await rows(e.id)).toEqual(expected); + expect(await code(store().deleteContentEntry(await fresh(q)))).toBe( + "field_operation_running", + ); + + await reimport(site); + expect(await rows(e.id)).toEqual(expected); + expect(await editTitle(e, "Edited again")).toBe("ok"); + expect(await rows(e.id)).toEqual(expected); + }); + + it("a held row follows its holder's status: publish locks the target, unpublish frees it", async () => { + const site = await world(HOSTS); + const p = await entry(site, "places", { title: "P place", slug: "p-place" }); + const e = await entry(site, "events", { fields: { hosts: [p.id] } }); + await start(site, TO_REPEATER); + await storeRaw(e, [p.id, 5]); + const held = (st: string) => [{ t: p.id, f: "hosts", k: "hosts", s: 1, st }]; + + expect(await setStatus(e, "published")).toBe("ok"); + expect(await rows(e.id)).toEqual(held("published")); + expect(await code(store().deleteContentEntry(await fresh(p)))).toBe("entry_referenced"); + // the rebuild writes exactly these rows + await reimport(site); + expect(await rows(e.id)).toEqual(held("published")); + + expect(await setStatus(e, "draft")).toBe("ok"); + expect(await rows(e.id)).toEqual(held("draft")); + expect(await code(store().deleteContentEntry(await fresh(p)))).toBe("ok"); + }); + + it("a held reference → text value's row follows its holder's status too", async () => { + const site = await world({ + key: "venue", + type: "reference", + labels: L("Venue"), + localized: false, + collections: ["places"], + } as FieldDefinition); + const p = await entry(site, "places", { title: "P place", slug: "p-place" }); + const e = await entry(site, "events", { fields: { venue: p.id } }); + await start(site, { + op: "change_field_type", + collection: "events", + key: "venue", + type: "text", + }); + // the target goes while the holder is a draft: its value no longer converts to a title + expect(await code(store().deleteContentEntry(await fresh(p)))).toBe("ok"); + const held = (st: string) => [{ t: p.id, f: "venue", k: "venue", s: 1, st }]; + + expect(await setStatus(e, "published")).toBe("ok"); + expect((await fresh(e)).fields).toEqual({ venue: p.id }); + expect(await rows(e.id)).toEqual(held("published")); + await reimport(site); + expect(await rows(e.id)).toEqual(held("published")); + expect(await setStatus(e, "draft")).toBe("ok"); + expect(await rows(e.id)).toEqual(held("draft")); + }); + }); +} diff --git a/packages/core-ts/src/testing-field-operations-references.ts b/packages/core-ts/src/testing-field-operations-references.ts index 76753611..94f2d9ef 100644 --- a/packages/core-ts/src/testing-field-operations-references.ts +++ b/packages/core-ts/src/testing-field-operations-references.ts @@ -24,7 +24,7 @@ import { import { nodeHash } from "./hash.ts"; import { PNG_1X1 } from "./media/fixtures.ts"; import { SiteGraphStore } from "./store.ts"; -import { exportSite, importSite } from "./sync.ts"; +import { rebuildingImport } from "./testing-reimport.ts"; import { racingDriver, type SuiteContext } from "./testing-taxonomy.ts"; const person: Actor = { kind: "human", id: "user_01HZX8K3Q9CD", name: "Halil" }; @@ -134,6 +134,12 @@ export function fieldOperationsReferencesSuite(ctx: SuiteContext): void { const place = (site: Site, title: string, over: Partial = {}) => entry(site, "places", { title, slug: title.toLowerCase().replace(/ /g, "-"), ...over }); const fresh = async (e: ContentEntry) => (await store().getContentEntry(e.id)) as ContentEntry; + /** A replace import that really rebuilds the rows (`rebuildingImport`). */ + const reimport = (site: Site) => + rebuildingImport( + { sql: ctx.sql(), blobs: ctx.blobs() }, + { siteId: site.id, fieldTypes: ENABLED, addNode: () => place(site, `Scratch ${++n}`) }, + ); const freshSite = async (site: Site) => (await store().getSite(site.id)) as Site; const def = async (site: Site, key: string) => ((await store().getSite(site.id)) as Site).collections?.events?.fields?.find( @@ -1036,11 +1042,7 @@ export function fieldOperationsReferencesSuite(ctx: SuiteContext): void { const legacy = await entry(a, "events", { fields: { venue: gone.id } }); const toRefOp = await run(a, toRef(), { budget: { entries: 0 } }); await store().updateContentEntry({ ...(await fresh(gone)), status: "archived" }); - const storage = { sql: ctx.sql(), blobs: ctx.blobs() }; - await importSite(storage, await exportSite(storage, a.id), { - mode: "replace", - fieldTypes: ENABLED, - }); + await reimport(a); expect(await rows(legacy.id)).toEqual([]); const stuck = await continueFieldOperation(store(), a.id, toRefOp.operationId, actor); expect(stuck.unconvertible.map((u) => u.entryId)).toEqual([legacy.id]); @@ -1053,10 +1055,7 @@ export function fieldOperationsReferencesSuite(ctx: SuiteContext): void { { op: "change_field_type", collection: "events", key: "venue", type: "text" }, { budget: { entries: 0 } }, ); - await importSite(storage, await exportSite(storage, b.id), { - mode: "replace", - fieldTypes: ENABLED, - }); + await reimport(b); expect(await rows(e.id)).toEqual([{ t: hall.id, f: "venue", k: "venue", s: 1 }]); const done = await continueFieldOperation(store(), b.id, toText.operationId, actor); expect(done.done).toBe(true); @@ -1075,12 +1074,6 @@ export function fieldOperationsReferencesSuite(ctx: SuiteContext): void { collections: ["places"], ...over, }) as FieldOperation; - const storage = () => ({ sql: ctx.sql(), blobs: ctx.blobs() }); - const reimport = async (site: Site) => - importSite(storage(), await exportSite(storage(), site.id), { - mode: "replace", - fieldTypes: ENABLED, - }); it("a rebuild keeps an accepted list's rows; only a bare string is checked again", async () => { const site = await world([text("venue")]); @@ -1266,11 +1259,7 @@ export function fieldOperationsReferencesSuite(ctx: SuiteContext): void { }, { budget: { entries: 0 } }, ); - const storage = { sql: ctx.sql(), blobs: ctx.blobs() }; - await importSite(storage, await exportSite(storage, site.id), { - mode: "replace", - fieldTypes: ENABLED, - }); + await reimport(site); expect(await rows(e.id)).toEqual([{ t: p.id, f: "hosts", k: "hosts", s: 1 }]); expect(await code(store().deleteContentEntry(await fresh(p)))).toBe("entry_referenced"); }); diff --git a/packages/core-ts/src/testing-reimport.ts b/packages/core-ts/src/testing-reimport.ts new file mode 100644 index 00000000..9b5e538e --- /dev/null +++ b/packages/core-ts/src/testing-reimport.ts @@ -0,0 +1,29 @@ +/** + * Test helper for the shared suites: a replace import that really rebuilds `entry_refs`. + * Re-importing a site's own export is planned `unchanged` (`sync-plan.ts`), so the batch writes + * nothing and no rebuild runs. Here a node is added after the export (the import removes it), and + * the site's rows are dropped first, so every row the test reads afterwards came from the rebuild. + */ +import type { BlobStore, SqlDriver } from "@cloudhal/storage"; +import { expect } from "vitest"; +import type { FieldTypeSupport } from "./references/definitions.ts"; +import { exportSite, importSite } from "./sync.ts"; + +export async function rebuildingImport( + storage: { sql: SqlDriver; blobs: BlobStore }, + at: { + siteId: string; + fieldTypes: FieldTypeSupport; + /** A store write that adds a node to the site (it is not in the export). */ + addNode: () => Promise; + }, +): Promise { + const bundle = await exportSite(storage, at.siteId); + await at.addNode(); + await storage.sql.run("DELETE FROM entry_refs WHERE site_id = ?", [at.siteId]); + const result = await importSite(storage, bundle, { + mode: "replace", + fieldTypes: at.fieldTypes, + }); + expect(result.nodes.unchanged).toBe(false); +} diff --git a/packages/core-ts/src/testing.ts b/packages/core-ts/src/testing.ts index d62b9b5e..1a8a005c 100644 --- a/packages/core-ts/src/testing.ts +++ b/packages/core-ts/src/testing.ts @@ -13,6 +13,7 @@ import { assetsSuite } from "./testing-assets.ts"; import { collectionsSuite } from "./testing-collections.ts"; import { entryQuerySuite } from "./testing-entry-query.ts"; import { fieldOperationsSuite } from "./testing-field-operations.ts"; +import { fieldOperationsOldShapeSuite } from "./testing-field-operations-old-shape.ts"; import { fieldOperationsReferencesSuite } from "./testing-field-operations-references.ts"; import { fieldsSuite } from "./testing-fields.ts"; import { listingsSuite } from "./testing-listings.ts"; @@ -280,6 +281,7 @@ export function describeSiteGraphStore(label: string, factory: StoreFactory): vo fieldsSuite(suite); fieldOperationsSuite(suite); fieldOperationsReferencesSuite(suite); + fieldOperationsOldShapeSuite(suite); referenceGateSuite(suite); referencesSuite(suite); referenceReadsSuite(suite); diff --git a/packages/core-ts/test/fields-translation.test.ts b/packages/core-ts/test/fields-translation.test.ts index f8a0c069..6a0a2e53 100644 --- a/packages/core-ts/test/fields-translation.test.ts +++ b/packages/core-ts/test/fields-translation.test.ts @@ -7,6 +7,7 @@ import { sharedFieldKeys, translatableFieldValues, translatedItemValues, + translationSourceValues, } from "../src/fields/index.ts"; const label = (en: string) => ({ en: { label: en } }); @@ -159,6 +160,34 @@ describe("repeater paths (RFC-0045)", () => { expect(given.values.agenda).toEqual([{ topic: "a" }]); }); + it("a whole list sent alone is the translation's own: stored as sent, whatever its length", () => { + const alone = translatedItemValues(AGENDA, { source: SOURCE }, { agenda: [{ topic: "a" }] }); + expect(alone).toEqual({ values: { agenda: [{ topic: "a" }] }, issues: [] }); + }); + + it("hands over the source's whole list once the translation holds its own (#218)", () => { + // no translation yet, or one without its own list: item paths, as before + for (const existing of [undefined, {}, { agenda: "not a list" }]) + expect(translationSourceValues(AGENDA, SOURCE, existing)).toEqual({ + fields: translatableFieldValues(AGENDA, SOURCE), + whole: [], + }); + // its own list: the source's list, in its order, with every sub-value, and no item paths + const own = translationSourceValues(AGENDA, { ...SOURCE, title: "x" }, { agenda: [] }); + expect(own).toEqual({ fields: { agenda: SOURCE.agenda }, whole: ["agenda"] }); + // what it hands over, translated and sent back whole, is accepted + const back = (own.fields.agenda as Record[]).map((i) => ({ ...i })); + expect( + translatedItemValues(AGENDA, { source: SOURCE, existing: { agenda: [] } }, { agenda: back }) + .issues, + ).toEqual([]); + // a shared repeater, or a source without the list, is never handed over + expect(translationSourceValues(AGENDA, { shared: SOURCE.shared }, SOURCE)).toEqual({ + fields: {}, + whole: [], + }); + }); + it("scans a long path in linear time", () => { const long = `agenda/${"9".repeat(100_000)}/topic`; const started = performance.now(); diff --git a/packages/mcp-server/src/field-hints.ts b/packages/mcp-server/src/field-hints.ts index f90fc33f..97111a63 100644 --- a/packages/mcp-server/src/field-hints.ts +++ b/packages/mcp-server/src/field-hints.ts @@ -51,7 +51,7 @@ const HINTS: Partial> = { entry_referenced: "published entries (details.referrers) show this entry, and it is the last one of its group here: ask a person to unpublish or edit them first; archiving it does not release them", translation_shape_mismatch: - "the translation's list has another length than the source's: translate that repeater by hand, or send it whole in the source's order (get_translation_source)", + "send that repeater as get_translation_source returns it, translated: by item paths, or as the whole list when it is named in `wholeLists` (the translation already holds its own list); a whole list given beside item paths must have the source's length and order", translation_group_in_use: "other language versions use this entry as their original: unlink or move them first, then change its translation", field_operation_running: diff --git a/packages/mcp-server/src/translation-fields.ts b/packages/mcp-server/src/translation-fields.ts index 2766e851..9807f7c9 100644 --- a/packages/mcp-server/src/translation-fields.ts +++ b/packages/mcp-server/src/translation-fields.ts @@ -10,8 +10,11 @@ * RFC-0045 §3: a localized repeater's text and rich-text sub-values are translated by item path * (`speakers/0/talk`), and folded into a copy of the source's list (`translatedItemValues`), so * each item keeps its other sub-values. Once the translation holds its own list, that repeater - * is sent whole; a path or a list of another length is `translation_shape_mismatch`. A path names an item by index, so a save with paths - * names the source revision it was read from (`sourceRev`, `conflict` when it moved on). + * is sent whole: `get_translation_source` hands over the source's whole list for it and names it + * in `wholeLists`, and a path for it is `translation_shape_mismatch`. A list given whole together + * with paths must have the source's length; a whole list sent alone is the translation's own and + * is stored as sent (RFC-0045 "Amended in 7a"). A path names an item by index, so a save with + * paths names the source revision it was read from (`sourceRev`, `conflict` when it moved on). */ import type { ContentEntry, FieldDefinition, Site } from "@cloudhal/core-contracts"; import { @@ -20,8 +23,8 @@ import { refuseFieldIssues, sharedFieldIssues, sharedFieldKeys, - translatableFieldValues, translatedItemValues, + translationSourceValues, } from "@cloudhal/core-ts/fields"; import { type JsonSchema, ToolError } from "./common.ts"; @@ -31,7 +34,7 @@ type Values = Record; export const TRANSLATED_FIELDS: JsonSchema = { type: "object", description: - "Entries: your translation of each value in get_translation_source's `fields` (localized text and rich-text fields), by the same key: a field key, or a repeater item's path such as speakers/0/talk (the item's other values are kept). Shared fields (`sharedFields`) are read from the original and refused here", + "Entries: your translation of each value in get_translation_source's `fields` (localized text and rich-text fields), by the same key: a field key, or a repeater item's path such as speakers/0/talk (the item's other values are kept). A repeater named in get_translation_source's `wholeLists` is sent whole instead: the list it returned, its text and rich-text sub-values translated and every other sub-value kept, in the same order, under the repeater's key; item paths for it are refused (translation_shape_mismatch). A whole list sent alone is stored as sent. Shared fields (`sharedFields`) are read from the original and refused here", }; /** `save_translation`'s `sourceRev`. */ @@ -39,7 +42,7 @@ export const SOURCE_REV_INPUT: JsonSchema = { type: "integer", minimum: 0, description: - "Entries: get_translation_source's sourceRev. Required when fields has repeater item paths, which name items by index: refused (conflict) when the source changed since", + "Entries: get_translation_source's sourceRev. Required when fields has repeater item paths, which name items by index: refused (conflict) when the source changed since. Send it with a repeater's whole list (wholeLists) too: given, it is checked the same way, so a list read before the source changed is not saved as current", }; /** True when `values` names a repeater item (`speakers/0/talk`); other keys with `/` are plain. */ @@ -48,14 +51,28 @@ const hasItemPaths = (defs: readonly FieldDefinition[], values: unknown): boolea typeof values === "object" && Object.keys(values).some((k) => itemPathRepeater(defs, k) !== undefined); -/** Item paths need the source revision they were read from, unchanged (RFC-0045 §3). */ +/** True when `values` gives a repeater's list whole (`wholeLists`). */ +const hasWholeLists = (defs: readonly FieldDefinition[], values: unknown): boolean => + !!values && + typeof values === "object" && + Object.entries(values).some( + ([k, v]) => Array.isArray(v) && defs.some((d) => d.key === k && d.type === "repeater"), + ); + +/** + * Item paths need the source revision they were read from, unchanged (RFC-0045 §3). A whole + * repeater list is checked against `sourceRev` when one is given (#221), so a list read before the + * source changed is not linked to its new revision; without one it is saved as before. + */ export function checkSourceRev( a: Record, site: Site, source: { id: string; rev: number; collection?: string }, ) { if (source.collection === undefined) return; - if (!hasItemPaths(fieldDefinitions(site, source.collection), a.fields)) return; + const defs = fieldDefinitions(site, source.collection); + const paths = hasItemPaths(defs, a.fields); + if (!paths && !(typeof a.sourceRev === "number" && hasWholeLists(defs, a.fields))) return; if (typeof a.sourceRev !== "number") throw new ToolError( "invalid", @@ -68,20 +85,33 @@ export function checkSourceRev( ); } -/** What `get_translation_source` returns about an entry's typed fields (nothing for a page). */ +/** + * What `get_translation_source` returns about an entry's typed fields (nothing for a page). A + * localized repeater the existing translation (`existing`, the one `save_translation` writes) + * already holds its own list of comes whole, named in `wholeLists` (#218). + */ export function translationSourceFields( site: Site, source: object, -): { source: object; fields?: Record; sharedFields?: string[] } { + existing?: object | undefined, +): { + source: object; + fields?: Record; + wholeLists?: string[]; + sharedFields?: string[]; +} { if (!("collection" in source)) return { source }; const entry = source as ContentEntry; const defs = fieldDefinitions(site, entry.collection); if (defs.length === 0) return { source }; const { fields, ...rest } = entry; const shared = sharedFieldKeys(defs); + const own = existing && "fields" in existing ? (existing as ContentEntry).fields : undefined; + const values = translationSourceValues(defs, fields, own); return { source: rest, - fields: translatableFieldValues(defs, fields), + fields: values.fields, + ...(values.whole.length > 0 ? { wholeLists: values.whole } : {}), ...(shared.length > 0 ? { sharedFields: shared } : {}), }; } diff --git a/packages/mcp-server/src/translation-tools.ts b/packages/mcp-server/src/translation-tools.ts index a3efb698..db296533 100644 --- a/packages/mcp-server/src/translation-tools.ts +++ b/packages/mcp-server/src/translation-tools.ts @@ -109,7 +109,7 @@ const getSourceTool: ToolDefinition = { name: "get_translation_source", title: "Get translation source", description: - "What you need to translate a page or entry into `locale`: the source (translate its title, text, summary and SEO; keep section ids, kinds, links, numbers, images and bindings), for an entry with typed fields the localized text and rich-text values to translate (`fields`, repeater items by path such as speakers/0/talk) and the shared fields a translation reads from the original (`sharedFields`, never translated), the glossary terms it uses (doNotTranslate terms stay as written, given translations are used), and the current translation in that locale if there is one (with `stale` when the source changed since, and for a linked page whose source changed its layout, `layout`: the translation's sections on the new layout, with the `untranslated` ones to translate). Then call save_translation.", + "What you need to translate a page or entry into `locale`: the source (translate its title, text, summary and SEO; keep section ids, kinds, links, numbers, images and bindings), for an entry with typed fields the localized text and rich-text values to translate (`fields`, repeater items by path such as speakers/0/talk; a localized repeater whose translation already holds its own list comes whole under its key and is named in `wholeLists`: send it back whole, with sourceRev) and the shared fields a translation reads from the original (`sharedFields`, never translated), the glossary terms it uses (doNotTranslate terms stay as written, given translations are used), and the current translation in that locale if there is one (with `stale` when the source changed since, and for a linked page whose source changed its layout, `layout`: the translation's sections on the new layout, with the `untranslated` ones to translate). Then call save_translation.", inputSchema: obj( { siteId: id, @@ -130,7 +130,13 @@ const getSourceTool: ToolDefinition = { fields: { type: "object", description: - "Entries: the localized text and rich-text field values to translate, by key, and a localized repeater's by item path (speakers/0/talk); send them translated, under the same keys, as save_translation's `fields`", + "Entries: the localized text and rich-text field values to translate, by key, and a localized repeater's by item path (speakers/0/talk); send them translated, under the same keys, as save_translation's `fields`. A repeater named in `wholeLists` comes whole instead: the source's list under its key, in the source's order, with every sub-value", + }, + wholeLists: { + type: "array", + items: { type: "string" }, + description: + "Entries: the localized repeaters the existing translation already holds its own list of. Item paths cannot name their items, so `fields` has the source's whole list for each: translate its text and rich-text sub-values, keep the others, and send the whole list back under the same key, in the same order, with sourceRev (item paths for it are refused, translation_shape_mismatch)", }, sourceRev: { type: "integer", @@ -163,7 +169,7 @@ const getSourceTool: ToolDefinition = { throw new ToolError("locale_not_enabled", `${locale} is not one of the site's locales`); const source = await nodeOf(ctx, site.id, a.nodeId as string); const { member, existing } = await translationOf(ctx, { site, target, source, locale }); - const typed = translationSourceFields(site, source); + const typed = translationSourceFields(site, source, existing); const text = JSON.stringify( "sections" in source ? source.sections : [source.body.value, typed.fields ?? {}], ); @@ -193,7 +199,7 @@ const saveTranslationTool: ToolDefinition = { name: "save_translation", title: "Save translation", description: - "Save your translation of a page or entry into `locale` as a draft linked to the source's current revision, with your model as its provenance. Pages: send title, path and sections (same ids, kinds and order as the source; translated text). Entries: title, slug, body, summary and the translated `fields` (only the localized text and rich-text ones get_translation_source returns, under its keys, repeater items by path with get_translation_source's sourceRev; shared fields are refused with field_not_localized). The checks (structure, links, numbers, glossary, length) run and are returned; a failed check keeps it from being published until fixed. A person reviews it in Studio before it goes live where the site asks for review. Refused for a translation a person wrote or reviewed. Snapshots afterwards.", + "Save your translation of a page or entry into `locale` as a draft linked to the source's current revision, with your model as its provenance. Pages: send title, path and sections (same ids, kinds and order as the source; translated text). Entries: title, slug, body, summary and the translated `fields` (only the localized text and rich-text ones get_translation_source returns, under its keys, repeater items by path with get_translation_source's sourceRev, or a repeater named in its `wholeLists` as the whole list; shared fields are refused with field_not_localized). The checks (structure, links, numbers, glossary, length) run and are returned; a failed check keeps it from being published until fixed. A person reviews it in Studio before it goes live where the site asks for review. Refused for a translation a person wrote or reviewed. Snapshots afterwards.", inputSchema: obj( { siteId: id, diff --git a/packages/mcp-server/test/references.test.ts b/packages/mcp-server/test/references.test.ts index 99a483dd..9fb4607d 100644 --- a/packages/mcp-server/test/references.test.ts +++ b/packages/mcp-server/test/references.test.ts @@ -599,6 +599,90 @@ describe("RFC-0045 over MCP (rollout step 5)", () => { expect((await w.store.getContentEntry(tr.id))?.fields).toEqual({ agenda: whole }); }); + it("re-translates a stale repeater: list_stale_translations → get_translation_source → save_translation (#218)", async () => { + const bob = await entry(w, "people", { title: "Bob" }); + const session = await entry(w, "sessions", { + title: "Day one", + fields: { agenda: [{ topic: "Opening", host: ada.id }] }, + }); + const get = async () => + (await call(w, "get_translation_source")({ nodeId: session.id, locale: "tr" })).result as { + fields: Record; + wholeLists?: string[]; + sourceRev: number; + stale?: boolean; + }; + const save = (fields: Record, sourceRev?: number) => + call( + w, + "save_translation", + )({ + nodeId: session.id, + locale: "tr", + title: "Birinci gün", + slug: "birinci-gun", + body: "Gövde", + fields, + ...(sourceRev === undefined ? {} : { sourceRev }), + }); + // The first translation goes by item path; the translation then holds its own list. + const first = await get(); + expect(first.fields).toEqual({ "agenda/0/topic": "Opening" }); + expect(first.wholeLists).toBeUndefined(); + const tr = ( + (await save({ "agenda/0/topic": "Açılış" }, first.sourceRev)).result as { + node: ContentEntry; + } + ).node; + // The source changes: an item is reworded and one is added. + const agenda = [ + { topic: "Welcome", host: ada.id }, + { topic: "Keynote", host: bob.id }, + ]; + const current = (await w.store.getContentEntry(session.id)) as ContentEntry; + await w.store.updateContentEntry({ ...current, fields: { agenda } }); + + const stale = (await call(w, "list_stale_translations")({ locale: "tr" })).result as { + stale: { id: string }[]; + }; + expect(stale.stale.map((m) => m.id)).toEqual([tr.id]); + const translation = (await w.store.getContentEntry(tr.id)) as ContentEntry; + expect(translation.translation?.source?.id).toBe(session.id); + + // The source hands over its whole list, in its order with every sub-value, not item paths. + const src = await get(); + expect(src.stale).toBe(true); + expect(src.wholeLists).toEqual(["agenda"]); + expect(src.fields).toEqual({ agenda }); + // Item paths are still refused, with a hint that names what to send. + const paths = await rejection(save({ "agenda/0/topic": "Hoş geldiniz" }, src.sourceRev)); + expect(paths.code).toBe("translation_shape_mismatch"); + expect(paths.message).toContain("wholeLists"); + + // The list as handed over, its text translated, saves. + const translated = (src.fields.agenda as { topic: string; host: string }[]).map((item, i) => ({ + ...item, + topic: ["Hoş geldiniz", "Açılış konuşması"][i] as string, + })); + // A whole list read before the source changed is refused when its sourceRev is given... + const late = await rejection(save({ agenda: translated }, src.sourceRev - 1)); + expect(late.code).toBe("conflict"); + // ...and saves with the revision it was read at. + const saved = (await save({ agenda: translated }, src.sourceRev)).result as { + node: ContentEntry; + }; + expect(saved.node.fields).toEqual({ + agenda: [ + { topic: "Hoş geldiniz", host: ada.id }, + { topic: "Açılış konuşması", host: bob.id }, + ], + }); + const after = (await call(w, "list_stale_translations")({ locale: "tr" })).result as { + stale: unknown[]; + }; + expect(after.stale).toEqual([]); + }); + it("reads an edition member's stored items as the store does while a localize marker runs", async () => { const items = [{ topic: "A" }, { topic: "B" }, { topic: "C" }]; const sessions = w.site.collections?.sessions;