From 1cae4a7ab04525b1c50b89752805d1c7ab2a6192 Mon Sep 17 00:00:00 2001 From: MelmacAlpha Date: Sun, 25 Oct 2026 19:11:20 +0100 Subject: [PATCH] fix: allow sibling placement inside repeated markup containers Use the immediate source parent to distinguish ordinary markup children from JavaScript return roots. This keeps navigator moves within map containers on the byte relocation path instead of falling back to a whole-page diff and hitting its resource limit. Add 50 IPC regression cases for sibling and edge placement, nested scopes, exact Undo, external writers and code-owned return roots. The full local gate passes all 382 suites; the live hpmaas renderer move applies in two splices and Undo restores its original bytes. --- docs/contracts.md | 6 +- shared/engine/planTree.ts | 30 ++- test/electron/documents/repeatedMove.test.ts | 269 +++++++++++++++++++ 3 files changed, 294 insertions(+), 11 deletions(-) create mode 100644 test/electron/documents/repeatedMove.test.ts diff --git a/docs/contracts.md b/docs/contracts.md index 5bc7ee37..ae8897fc 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -262,8 +262,10 @@ no operation replaces a whole file. - `capability.ts` — `editable` | `read-only-opaque` | `repeated-source-node` | `runtime-aggregate` | `unsupported`; `editable` and (since step 7) `repeated-source-node` accept visual intents: a node a loop repeats is one - source node, and an edit of it changes every copy. Nothing is placed beside - it (the loop body is code), and a stale intent whose node changed capability + source node, and an edit of it changes every copy. Sibling placement inside + an element or component preserves that markup list and its loop bindings; + placement beside a direct loop or branch return root remains refused because + that list is JavaScript. A stale intent whose node changed capability since it was authored — wrapped in a loop outside Stacki — is refused `region-externally-modified`. The renderer shows every other capability beside the selection (`src/editor/nodeCapability.ts`, `CapabilityNotice`). diff --git a/shared/engine/planTree.ts b/shared/engine/planTree.ts index 055d2512..0a44f036 100644 --- a/shared/engine/planTree.ts +++ b/shared/engine/planTree.ts @@ -133,7 +133,7 @@ export function planInsertNode( return resolved; } const target = resolved.value; - if (!placeWritable(target.current, operation.placement)) { + if (!placeWritable(currentProjection(context), target.current, operation.placement)) { return err('unsupported-operation'); } const current = currentProjection(context); @@ -233,11 +233,10 @@ function bytesHaveCrlf(bytes: ByteString): boolean { return false; } -// A node inside a loop is one source node the loop repeats (plan §6). Its own -// content is edited like any node's (step 7), but the list it sits in is the -// loop body's code, so nothing is removed from it or placed beside it — with one -// exception: moving it. That gesture exists to take a node out of its loop, and -// the move rewrites exactly what would break (the kind-changing class of §3.3). +// A node inside a loop is one source node the loop repeats (plan §6). A move +// within its markup container preserves those bytes and bindings. Leaving a +// loop rewrites only references the destination cannot satisfy (§3.3). The +// loop's direct return roots remain code-owned, not a markup sibling list. export function planMoveNode( context: PlanContext, anchor: AnchorRef, @@ -260,7 +259,7 @@ export function planMoveNode( return err('unsupported-operation'); } } - if (!placeWritable(destination.value.current, operation.placement)) { + if (!placeWritable(currentProjection(context), destination.value.current, operation.placement)) { return err('unsupported-operation'); } if (startsWith(destination.value.current.path, moved.path)) { @@ -821,13 +820,26 @@ function shifted(path: readonly number[]): readonly number[] { } // Beside a node, the list it sits in is written; inside it, its own content. -function placeWritable(node: ProjectedNode, placement: Placement): boolean { +function placeWritable( + projection: ValidProjection, + node: ProjectedNode, + placement: Placement, +): boolean { if (inside(placement)) { if (node.callback === true) { return false; // Separate return sites cannot receive one shared sibling insertion. } + return nodeEditable(node); + } + if (node.capability === 'repeated-source-node') { + // Repetition is inherited by every descendant. Only the immediate owner + // decides whether siblings are markup or separate JavaScript return sites. + const parent = nodeAtPath(projection, parentPath(node.path)); + assert(parent !== undefined, 'A repeated source node has an enclosing owner'); + assert(node.list === 'markup', 'Repeated source nodes belong to markup syntax'); + return parent.kind === 'element' || parent.kind === 'component'; } - return inside(placement) ? nodeEditable(node) : nodePlaceable(node); + return nodePlaceable(node); } function inside(placement: Placement): boolean { diff --git a/test/electron/documents/repeatedMove.test.ts b/test/electron/documents/repeatedMove.test.ts new file mode 100644 index 00000000..1e4f689e --- /dev/null +++ b/test/electron/documents/repeatedMove.test.ts @@ -0,0 +1,269 @@ +// Goal: navigator moves inside repeated markup use source splices, preserving loop bindings, +// formatting and unrelated writers. Code-owned return roots must still reject sibling placement. +// Method: drive real page IPC on isolated scratch files; compare exact bytes and apply Undo. +import assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { test } from 'node:test'; +import { mainHarness } from '../../helpers/mainHarness.ts'; +import type { Edit, NodeRef } from '#dist/shared/engine/editRequest.js'; +import { parsePageDiskRead, parsePageEditResult } from '#dist/shared/ipc/pageSave.js'; +import type { PageDiskRead } from '#dist/shared/ipc/pageSave.js'; +import type { PageNode } from '#dist/shared/page/pageNode.js'; + +function fixture(source: string) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'stacki-repeated-move-')); + fs.mkdirSync(path.join(root, 'src/pages'), { recursive: true }); + fs.mkdirSync(path.join(root, 'user')); + fs.writeFileSync(path.join(root, 'package.json'), '{"dependencies":{"astro":"*"}}'); + const file = path.join(root, 'src/pages/index.astro'); + fs.writeFileSync(file, source); + const harness = mainHarness(path.join(root, 'user')); + return { + read: async () => parsePageDiskRead(await harness.invoke('page:read', file)), + text: () => fs.readFileSync(file, 'utf8'), + write: (text: string) => fs.writeFileSync(file, text), + edit: async (checksum: string, edit: Edit) => + parsePageEditResult( + await harness.invoke('page:edit', { pagePath: file, authoredChecksum: checksum, edit }), + ), + dispose: () => { + harness.dispose(); + fs.rmSync(root, { recursive: true, force: true }); + }, + }; +} + +function named(page: PageDiskRead, name: string): NodeRef { + assert.ok(page.editable); + const pending: { readonly node: PageNode; readonly path: readonly number[] }[] = + page.model.nodes.map((node, index) => ({ node, path: [index] })); + for (let index = 0; index < pending.length; index++) { + assert.ok(pending.length < 100, 'the synthetic fixture is bounded'); + const entry = pending[index]; + assert.ok(entry !== undefined); + const node = entry.node; + if ('name' in node && node.name === name) { + assert.ok(node.start !== undefined && node.end !== undefined); + return { path: entry.path, kind: node.kind, span: { start: node.start, end: node.end } }; + } + if ('children' in node && node.children !== undefined) { + pending.push( + ...node.children.map((child, at) => ({ node: child, path: [...entry.path, at] })), + ); + } + } + assert.fail(`Missing ${name}`); +} + +const HEADING = ''; +const IMAGE = ''; +const ICON = ""; +const CHILDREN = [HEADING, IMAGE, ICON]; +const WRAPPERS = [ + ['{rechtsgebiete.map((gebiet) => (', '))}'], + ['{rechtsgebiete.map((gebiet) => (
{gebiet.visible && (', ')}
))}'], + ['{rechtsgebiete.map((gebiet) => (
{gebiet.items.map((item) => (', '))}
))}'], + ['{rechtsgebiete.map((gebiet) => { const label = gebiet.title; return (', '); })}'], +] as const; +const PLACES = [ + { source: 'Icon', destination: 'Heading', placement: 'before', order: [ICON, HEADING, IMAGE] }, + { source: 'Icon', destination: 'Image', placement: 'before', order: [HEADING, ICON, IMAGE] }, + { source: 'Icon', destination: 'Image', placement: 'after', order: CHILDREN }, + { source: 'Icon', destination: 'Card', placement: 'first-child', order: [ICON, HEADING, IMAGE] }, + { source: 'Icon', destination: 'Card', placement: 'last-child', order: CHILDREN }, + { source: 'Heading', destination: 'Icon', placement: 'after', order: [IMAGE, ICON, HEADING] }, + { source: 'Heading', destination: 'Icon', placement: 'before', order: [IMAGE, HEADING, ICON] }, + { source: 'Image', destination: 'Heading', placement: 'before', order: [IMAGE, HEADING, ICON] }, +] as const; + +for (const [prefix, suffix] of WRAPPERS) { + for (const place of PLACES) { + test(`repeated move ${place.placement} ${place.destination} in ${prefix}`, async (context) => { + const card = (children: readonly string[]) => + `\r\n ${children.join('\r\n ')}\r\n`; + const source = `${prefix}${card(CHILDREN)}${suffix}\r\n\r\n`; + const harness = fixture(source); + context.after(harness.dispose); + const page = await harness.read(); + const moved = await harness.edit(page.checksum, { + tag: 'move-node', + target: named(page, place.source), + destination: named(page, place.destination), + placement: place.placement, + }); + assert.ok(moved.ok, JSON.stringify(moved)); + assert.equal( + harness.text(), + `${prefix}${card(place.order)}${suffix}\r\n\r\n`, + ); + const undone = await harness.edit(moved.value.checksum, { + tag: 'revert', + hunks: moved.value.inverse, + }); + assert.ok(undone.ok, JSON.stringify(undone)); + assert.equal(harness.text(), source, 'Undo restores every original byte'); + }); + } +} + +test('move out of a nested container retains outer and inner loop bindings', async (context) => { + const icon = ''; + const prefix = '{rechtsgebiete.map((gebiet) => (
{gebiet.items.map((item) => ('; + const suffix = '))}
))}'; + const source = `${prefix}${icon}${HEADING}${suffix}`; + const harness = fixture(source); + context.after(harness.dispose); + const page = await harness.read(); + const moved = await harness.edit(page.checksum, { + tag: 'move-node', + target: named(page, 'Icon'), + destination: named(page, 'Heading'), + placement: 'before', + }); + assert.ok(moved.ok, JSON.stringify(moved)); + assert.equal(harness.text(), `${prefix}${icon}${HEADING}${suffix}`); + const undone = await harness.edit(moved.value.checksum, { + tag: 'revert', + hunks: moved.value.inverse, + }); + assert.ok(undone.ok); + assert.equal(harness.text(), source); +}); + +test('repeated moves and Undo preserve an external change elsewhere', async (context) => { + const filler = '

A paragraph separating the move from other writers and Undo context.

'; + const source = + `{rechtsgebiete.map((gebiet) => (${CHILDREN.join('')}))}` + `${filler}