From 05f30446481b52b9af60d79fc4d26dca348d44bf Mon Sep 17 00:00:00 2001 From: Fini Date: Tue, 21 Apr 2026 22:03:11 +0800 Subject: [PATCH] fix(ai): batch_design parser accepts pretty-printed multi-line JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit executeLine's three parse regexes (assign / bindless / call) missed the `s` flag, so `.+` stopped at the first newline and rejected any pretty-printed JSON body — even though splitOperations already groups balanced `()`/`[]`/`{}` spans into one logical line. Kimi K2.5 was primed into this style by elements.md examples, tripping the latent bug on 3/24 A/B prompts; baseline models never pretty-printed so the bug stayed masked. Regression test locks the fix: bound + bindless insert + bound update with newline-embedded bodies now parse, and a genuinely malformed single-line input still surfaces as an error instead of being silently swallowed. See openpencil-docs/superpowers/notes/2026-04-21-kimi-k25-regression-rca.md. --- .../batch-design-multiline-body.test.ts | 104 ++++++++++++++++++ packages/pen-mcp/src/tools/batch-design.ts | 16 ++- 2 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 packages/pen-mcp/src/__tests__/batch-design-multiline-body.test.ts diff --git a/packages/pen-mcp/src/__tests__/batch-design-multiline-body.test.ts b/packages/pen-mcp/src/__tests__/batch-design-multiline-body.test.ts new file mode 100644 index 000000000..3cda97faf --- /dev/null +++ b/packages/pen-mcp/src/__tests__/batch-design-multiline-body.test.ts @@ -0,0 +1,104 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { writeFile, unlink, readFile, mkdir } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { handleBatchDesign } from '../tools/batch-design'; +import { invalidateCache } from '../document-manager'; + +/** + * Regression for the 2026-04-21 Kimi K2.5 A/B regression: pretty-printed + * multi-line JSON inside `I(...)` / `U(...)` / etc. must parse. `splitOperations` + * already preserves multi-line balanced bodies; `executeLine`'s parse regex + * had `.+` without the `s` flag, so `.` stopped at the first `\n` and the + * whole line was mis-classified as unparseable. See + * openpencil-docs/superpowers/notes/2026-04-21-kimi-k25-regression-rca.md. + */ +describe('batch_design — multi-line pretty-printed body (Kimi K2.5 regression)', () => { + const TMP = join(tmpdir(), 'openpencil-batch-design-multiline'); + const EMPTY = JSON.stringify({ version: '1.0.0', children: [] }); + + beforeEach(async () => { + await mkdir(TMP, { recursive: true }); + }); + afterEach(async () => { + for (const f of ['a.op']) { + try { + const fp = join(TMP, f); + invalidateCache(fp); + await unlink(fp); + } catch {} + } + }); + + async function fresh(name: string): Promise { + const fp = join(TMP, name); + await writeFile(fp, EMPTY, 'utf-8'); + return fp; + } + + it('accepts an insert with pretty-printed multi-line JSON body', async () => { + const fp = await fresh('a.op'); + const operations = [ + 'root=I(null, {', + ' "type": "frame",', + ' "name": "Root",', + ' "width": 375,', + ' "height": 200,', + ' "layout": "vertical"', + '})', + ].join('\n'); + const res = await handleBatchDesign({ filePath: fp, operations, postProcess: false }); + expect(res.errors).toBeUndefined(); + expect(res.results.length).toBe(1); + const saved = JSON.parse(await readFile(fp, 'utf-8')); + const children = (saved.children ?? saved.pages?.[0]?.children) as Record[]; + expect(children.length).toBe(1); + expect(children[0].name).toBe('Root'); + expect(children[0].layout).toBe('vertical'); + }); + + it('accepts bindless insert with multi-line body (Kimi style)', async () => { + const fp = await fresh('a.op'); + const operations = + 'I(null, {\n "type": "frame",\n "name": "NoBind",\n "width": 100,\n "height": 100\n})'; + const res = await handleBatchDesign({ filePath: fp, operations, postProcess: false }); + expect(res.errors).toBeUndefined(); + expect(res.results.length).toBe(1); + const saved = JSON.parse(await readFile(fp, 'utf-8')); + const children = (saved.children ?? saved.pages?.[0]?.children) as Record[]; + expect(children[0].name).toBe('NoBind'); + }); + + it('accepts update call with multi-line patch body (insert + update in one batch)', async () => { + const fp = await fresh('a.op'); + // Bindings live in the per-handleBatchDesign Map — insert + update must + // be in the same call for the `root` binding to resolve on U(). In a + // real agent flow the two lines arrive concatenated in one payload. + const operations = [ + 'root=I(null, {"type":"frame","name":"Pre","width":100,"height":100})', + 'U(root, {', + ' "name": "After",', + ' "width": 200', + '})', + ].join('\n'); + const res = await handleBatchDesign({ filePath: fp, operations, postProcess: false }); + expect(res.errors).toBeUndefined(); + const saved = JSON.parse(await readFile(fp, 'utf-8')); + const children = (saved.children ?? saved.pages?.[0]?.children) as Record[]; + expect(children[0].name).toBe('After'); + expect(children[0].width).toBe(200); + }); + + it('still rejects genuinely malformed single-line input', async () => { + const fp = await fresh('a.op'); + // Missing the closing `)` — should surface as an error, not silently pass + const res = await handleBatchDesign({ + filePath: fp, + operations: 'x=I(null, {"type":"frame","name":"Bad"', + postProcess: false, + }); + // splitOperations keeps unclosed lines; executeLine can't match → error collected + expect(res.errors).toBeDefined(); + expect(res.errors!.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/pen-mcp/src/tools/batch-design.ts b/packages/pen-mcp/src/tools/batch-design.ts index 736844981..2e66b0374 100644 --- a/packages/pen-mcp/src/tools/batch-design.ts +++ b/packages/pen-mcp/src/tools/batch-design.ts @@ -165,9 +165,19 @@ async function executeLine( // Parse: binding=OP(args) or OP(args) // Binding is optional for I/C/R/G — auto-generated when omitted (agents // sometimes write `I(parent, data)` without the binding prefix). - const assignMatch = line.match(/^(\w+)\s*=\s*([ICRMG])\((.+)\)$/); - const bindlessAssignMatch = !assignMatch && line.match(/^([ICRG])\((.+)\)$/); - const callMatch = line.match(/^([UDM])\((.+)\)$/); + // + // `s` flag (dotAll) is REQUIRED: splitOperations above yields one logical + // line per top-level op and does NOT split on newlines inside balanced + // `()`/`[]`/`{}`. Pretty-printed JSON bodies (e.g. Kimi K2.5 in the + // 2026-04-21 A/B regression) carry literal `\n` inside the arg list; a + // `.+` without `s` stops at the first newline and mis-fails the whole + // line. See + // openpencil-docs/superpowers/notes/2026-04-21-kimi-k25-regression-rca.md + // for the exact failure mode. `^`/`$` still anchor to string boundaries + // (no `m` flag), which is what we want. + const assignMatch = line.match(/^(\w+)\s*=\s*([ICRMG])\((.+)\)$/s); + const bindlessAssignMatch = !assignMatch && line.match(/^([ICRG])\((.+)\)$/s); + const callMatch = line.match(/^([UDM])\((.+)\)$/s); // Normalize bindless form to an auto-generated binding so the rest of the // logic below can treat it uniformly.