From 73cd5625d05fe9fadab1c51aebe491ca32b4d8da Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 21 Jul 2026 16:49:55 -0700 Subject: [PATCH] fix(schema-pack): reject YAML block scalars loudly instead of silently truncating manifests (#1750) Takeover of #1799 (fork branch rotted against master; refreshed CI hit an unrelated shard-timeout flake). parseYamlMini now throws a clear error when a bare block-scalar indicator (|, >, |-, >+, |2) reaches parseScalar, instead of mis-parsing it as the literal string and silently dropping every following top-level key. Inlines gbrain-recommended.yaml's description so the bundled pack loads its full page_types. Co-authored-by: harjoth Co-Authored-By: Claude Fable 5 --- .../schema-pack/base/gbrain-recommended.yaml | 7 +--- src/core/schema-pack/loader.ts | 18 +++++++++- test/schema-pack-loader.test.ts | 33 +++++++++++++++++++ 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/core/schema-pack/base/gbrain-recommended.yaml b/src/core/schema-pack/base/gbrain-recommended.yaml index fa17d5e66..a14f4ee08 100644 --- a/src/core/schema-pack/base/gbrain-recommended.yaml +++ b/src/core/schema-pack/base/gbrain-recommended.yaml @@ -26,12 +26,7 @@ name: gbrain-recommended version: 1.0.0 gbrain_min_version: 0.39.0 extends: gbrain-base -description: | - The recommended starter pack for an operational personal brain. - Based on docs/GBRAIN_RECOMMENDED_SCHEMA.md — Karpathy's LLM wiki - pattern extended into a full operational knowledge base. Use this - when you want the documented behavior immediately instead of - authoring your own pack from scratch. +description: "The recommended starter pack for an operational personal brain. Based on docs/GBRAIN_RECOMMENDED_SCHEMA.md — Karpathy's LLM wiki pattern extended into a full operational knowledge base. Use this when you want the documented behavior immediately instead of authoring your own pack from scratch." # Pack-specific page_types (extend gbrain-base's seed types). # Each maps a path prefix to a primitive + opt-in flags. Most types diff --git a/src/core/schema-pack/loader.ts b/src/core/schema-pack/loader.ts index 2f21b5012..b32a5dd4c 100644 --- a/src/core/schema-pack/loader.ts +++ b/src/core/schema-pack/loader.ts @@ -84,7 +84,9 @@ export function loadPackFromString(content: string, hint: string): SchemaPackMan * * Rejected by design: anchors (&), aliases (*), tags (!), flow style * ({...}, [...] except as JSON), block scalars (|, >), multi-document (---). - * Pack authors who need these features should ship JSON. + * Pack authors who need these features should ship JSON. Block scalars in + * particular now throw a clear error (see parseScalar) rather than silently + * mis-parsing and truncating the rest of the manifest. * * This is intentionally narrow. The skill-pack and storage-config parsers * use similar hand-rolled patterns; this one is shape-customized for pack @@ -133,6 +135,20 @@ export function parseYamlMini(content: string): unknown { // Number if (/^-?\d+$/.test(trimmed)) return parseInt(trimmed, 10); if (/^-?\d+\.\d+$/.test(trimmed)) return parseFloat(trimmed); + // Block scalars (`|`, `>`, with optional chomping/indent indicators such + // as `|-`, `>+`, `|2`) are unsupported by design — see the header. A bare + // indicator reaching here is a `key: |` line whose indented body would + // otherwise silently truncate the rest of the mapping: parseScalar would + // return the literal "|"/">", and the over-indented body would trip the + // `indent > baseIndent` break in parseMapping, dropping every following + // top-level key with no diagnostic. Fail loud instead of corrupting. + if (/^[|>][0-9+-]*$/.test(trimmed)) { + throw new Error( + `parseYamlMini: block scalars are not supported (found "${trimmed}"). ` + + `Use an inline or quoted string (e.g. description: "..."), or ship the manifest as JSON. ` + + `Left unhandled, a block scalar silently drops every key after it in the manifest.`, + ); + } // Bare string return trimmed; } diff --git a/test/schema-pack-loader.test.ts b/test/schema-pack-loader.test.ts index e35e5b826..22487eab7 100644 --- a/test/schema-pack-loader.test.ts +++ b/test/schema-pack-loader.test.ts @@ -10,6 +10,7 @@ // module's internal contracts. import { describe, expect, test } from 'bun:test'; +import { readFileSync } from 'node:fs'; import { withEnv } from './helpers/with-env.ts'; import { buildAliasGraph, @@ -349,6 +350,38 @@ describe('YAML mini-parser', () => { const result = parseYamlMini('# top comment\nname: value # inline comment') as Record; expect(result.name).toBe('value'); }); + + // Regression: block scalars (`|`, `>`) are unsupported by design. Before + // the fix, `description: |` was read as the literal "|" and its indented + // body silently truncated the mapping — every key after it was dropped + // with no error (issue #1750). Now it fails loud. + test('throws a clear error on block scalars instead of silently truncating', () => { + expect(() => parseYamlMini('description: |\n multi\n line\npage_types:\n - name: deal')) + .toThrow(/block scalars are not supported/); + expect(() => parseYamlMini('description: >\n folded text')).toThrow(/block scalars/); + // Chomping / indent indicators (`|-`, `>+`, `|2`) are caught too. + expect(() => parseYamlMini('x: |-\n body')).toThrow(/block scalars/); + expect(() => parseYamlMini('x: |2\n body')).toThrow(/block scalars/); + }); + + test('a quoted "|" is still a normal string (not mistaken for a block scalar)', () => { + const result = parseYamlMini('sep: "|"') as Record; + expect(result.sep).toBe('|'); + }); + + // The bundled gbrain-recommended pack uses a (now inline) description and + // declares 12 page_types. Before #1750 it silently resolved to 0 types. + test('bundled gbrain-recommended.yaml parses with its full page_types', () => { + const yaml = readFileSync( + `${import.meta.dir}/../src/core/schema-pack/base/gbrain-recommended.yaml`, + 'utf8', + ); + const parsed = parseYamlMini(yaml) as { page_types?: Array<{ name: string }>; description?: string }; + expect(Array.isArray(parsed.page_types)).toBe(true); + expect(parsed.page_types!.length).toBeGreaterThanOrEqual(12); + expect(parsed.page_types!.map(t => t.name)).toContain('deal'); + expect(parsed.description).not.toBe('|'); + }); }); describe('loadPackFromString end-to-end', () => {