From dcfb3649ab2da5e1127995c10612d345d43fbfaa Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 08:26:01 +0000 Subject: [PATCH 1/3] test(metadata-protocol): pin the composite-externalId seed diagnostic on BYTES (#16488) WIP: the regression pins land first so the red/green measurement is taken from a committed tree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- ...ed-loader-composite-key-diagnostic.test.ts | 402 ++++++++++++++++++ 1 file changed, 402 insertions(+) create mode 100644 packages/metadata-protocol/src/seed-loader-composite-key-diagnostic.test.ts diff --git a/packages/metadata-protocol/src/seed-loader-composite-key-diagnostic.test.ts b/packages/metadata-protocol/src/seed-loader-composite-key-diagnostic.test.ts new file mode 100644 index 0000000000..0d883433f5 --- /dev/null +++ b/packages/metadata-protocol/src/seed-loader-composite-key-diagnostic.test.ts @@ -0,0 +1,402 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16488 — a composite `externalId`'s NUL joiner must never reach a diagnostic. + * + * ## The ruling this file protects on BOTH sides + * + * `SeedLoaderService.externalIdKey()` joins a COMPOSITE key's parts with + * `\u0000` deliberately. Its own comment is the ruling, quoted here verbatim + * because half of this file exists to keep it true: + * + * > joins the per-field values with a separator (`\u0000`) that cannot occur + * > in a natural-key value, so `('a', 'b')` and `('a\0b', '')` never collide. + * + * That is correct for the KEY, and the key does not change by one character. + * What was wrong is that the SAME string was interpolated into human-readable + * diagnostics — the `(label=value)` parenthetical of `Failed to write ...`, and + * pass 2's `on record '...'` lines — so one failing composite-keyed row put a + * raw NUL byte into the server log. One such byte makes `grep` classify the + * WHOLE log as binary, so every later `grep -n` / `grep -c` over it silently + * returns nothing until the reader remembers `-a`: a single byte disables the + * reader's main instrument at exactly the moment someone is diagnosing a + * failed boot. + * + * Source reading: objectstack-ai/ats#20 — measured there, filed here. + * + * ## Why the assertion counts BYTES and not a substring + * + * An assertion that the message "contains `employer+user`" passes while the NUL + * is still sitting in it — the label side always had its `+` rendering. So + * every diagnostic assertion here counts occurrences of U+0000 and demands + * ZERO, and {@link nulCount} is proved able to answer non-zero on a control + * string in the same file (section 4) so a silently-broken counter cannot green + * the whole suite. + * + * ## The two negative controls (sections 3 and 4) + * + * 1. A SINGLE-key diagnostic line is byte-identical to what it has always been + * — pinned as a whole-string `toBe`, not a `toContain`. + * 2. Map-key behaviour is untouched: `('a','b')` and `('a\0b','')` still do + * not collide, and a pair that WOULD collide under a visible `+` joiner still + * dedupes as two distinct rows across a replay. Swapping the NUL for `+` in + * the key to make the log prettier is the one fix this card forbids, and + * section 4 goes red on it. + */ +import { describe, expect, it, vi } from 'vitest'; +import { SeedLoaderService } from './seed-loader.js'; +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; +import { + assertEngineDeleteDispatch, + assertEngineUpdateDispatch, + assertEngineFindOnePredicate, + type EngineFindOneQueryInput, +} from '@objectstack/metadata-core'; + +// --------------------------------------------------------------------------- +// Byte instrumentation +// --------------------------------------------------------------------------- + +/** + * U+0000, written as an ESCAPE. Never a raw byte in a source file — that is + * `check:nul-bytes`' whole subject, and it is the same byte this card keeps out + * of the log. + */ +const NUL = '\u0000'; + +/** Occurrences of U+0000 in `text`. The card's assertion is that this is 0. */ +function nulCount(text: string): number { + return text.split(NUL).length - 1; +} + +/** Every diagnostic string one load produced: the payload half and the log half. */ +function diagnostics( + result: { errors: Array<{ message: string }> }, + logger: { error: { mock: { calls: unknown[][] } } }, +): string[] { + return [ + ...result.errors.map((e) => e.message), + ...logger.error.mock.calls.map((call) => String(call[0])), + ]; +} + +// --------------------------------------------------------------------------- +// Harness +// --------------------------------------------------------------------------- + +function createLogger() { + return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; +} + +/** The withheld tail a driver fault gets — `WITHHELD_WRITE_REASON` in the loader. */ +const WITHHELD = 'the data engine rejected the write; the reason is in the server log'; + +function createMetadata(objects: Record): IMetadataService { + return { + getObject: vi.fn(async (name: string) => objects[name]), + listObjects: vi.fn(async () => Object.values(objects)), + register: vi.fn(async () => {}), + } as unknown as IMetadataService; +} + +/** An engine whose every write fails with a bare (non-quotable) driver fault. */ +function failingEngine(): IDataEngine { + const fault = () => new Error('boom'); + return { + find: vi.fn(async () => []), + findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => { + assertEngineFindOnePredicate(object, query); + return null; + }), + insert: vi.fn(async () => { throw fault(); }), + update: vi.fn(async (_o: string, data: any, options?: any) => { + assertEngineUpdateDispatch(data, options); + throw fault(); + }), + delete: vi.fn(async (_o: string, options?: any) => { + assertEngineDeleteDispatch(options); + return { deleted: 1 }; + }), + count: vi.fn(async () => 0), + aggregate: vi.fn(async () => []), + } as unknown as IDataEngine; +} + +/** + * A faithful in-memory engine (filters `where`, mints ids) — the same shape + * `seed-loader-composite-external-id.test.ts` uses, because a mock that ignores + * `where` returns the whole table and would mask replay/dedupe behaviour. + */ +function createFaithfulEngine(): { engine: IDataEngine; store: Record } { + const store: Record = {}; + let idCounter = 0; + + const engine = { + find: vi.fn(async (objectName: string, query?: any) => { + let records = store[objectName] || []; + if (query?.where) { + records = records.filter((r) => + Object.entries(query.where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + return r[k] === v; + }), + ); + } + if (typeof query?.limit === 'number') records = records.slice(0, query.limit); + return records; + }), + findOne: vi.fn(async (objectName: string, query?: any) => { + assertEngineFindOnePredicate(objectName, query); + const rows = await (engine.find as any)(objectName, { ...query, limit: 1 }); + return rows[0] ?? null; + }), + insert: vi.fn(async (objectName: string, data: any) => { + if (!store[objectName]) store[objectName] = []; + if (Array.isArray(data)) { + const records = data.map((d) => ({ id: `gen-${++idCounter}`, ...d })); + store[objectName].push(...records); + return records; + } + const record = { id: `gen-${++idCounter}`, ...data }; + store[objectName].push(record); + return record; + }), + update: vi.fn(async (objectName: string, data: any) => { + assertEngineUpdateDispatch(data, undefined); + const records = store[objectName] || []; + const idx = records.findIndex((r) => r.id === data.id); + if (idx >= 0) { + records[idx] = { ...records[idx], ...data }; + return records[idx]; + } + return data; + }), + delete: vi.fn(async (_objectName: string, options?: any) => { + assertEngineDeleteDispatch(options); + return { deleted: 1 }; + }), + count: vi.fn(async (objectName: string) => (store[objectName] || []).length), + aggregate: vi.fn(async () => []), + } as unknown as IDataEngine; + + return { engine, store }; +} + +const CONFIG = { + dryRun: false, + haltOnError: false, + multiPass: true, + defaultMode: 'upsert', + batchSize: 1000, + transaction: false, +} as any; + +// The card's own row: a join table keyed by (employer, user), values shaped +// like the ones that produced the reported log line. +const EMPLOYER = 'ats_employer-1788753956811-1'; +const USER = 'usr_ats_quillstone_admin'; + +const MEMBER_OBJECTS = { + ats_employer_member: { + name: 'ats_employer_member', + fields: { employer: { type: 'text' }, user: { type: 'text' }, role: { type: 'text' } }, + }, +}; + +const MEMBER_SEED = [{ + object: 'ats_employer_member', + externalId: ['employer', 'user'], + mode: 'upsert', + env: ['prod', 'dev', 'test'], + records: [{ employer: EMPLOYER, user: USER, role: 'admin' }], +}]; + +async function loadMemberFailure() { + const logger = createLogger(); + const svc = new SeedLoaderService(failingEngine(), createMetadata(MEMBER_OBJECTS), logger as never); + const result = await svc.load({ seeds: MEMBER_SEED, config: CONFIG } as never); + return { result, logger }; +} + +// =========================================================================== +// 1. THE CARD — the per-record write failure carries ZERO NUL bytes +// =========================================================================== + +describe('[#16488] a composite externalId never puts a raw NUL in a seed diagnostic', () => { + it('the `Failed to write` line — payload AND log — contains U+0000 zero times', async () => { + const { result, logger } = await loadMemberFailure(); + + expect(result.success).toBe(false); + const lines = diagnostics(result as never, logger as never); + // Anti-vacuity: the scenario really did produce the diagnostic under test. + expect(lines.some((l) => l.includes('Failed to write ats_employer_member record #0'))).toBe(true); + + // THE assertion of this card, on BYTES. + for (const line of lines) expect(nulCount(line)).toBe(0); + }); + + it('renders both key parts visibly, so the line is readable and not merely NUL-stripped', async () => { + const { result } = await loadMemberFailure(); + + const message = result.errors[0].message; + // The FIELD-name side is unchanged: `externalIdLabel` has always joined with `+`. + expect(message).toContain('(employer+user='); + // The VALUE side now renders as a JSON array of the parts — unambiguous + // even when a value itself contains the ` + ` the label side uses. + expect(message).toBe( + 'Failed to write ats_employer_member record #0 ' + + `(employer+user=${JSON.stringify([EMPLOYER, USER])}): ${WITHHELD}`, + ); + }); + + it('the structured payload keeps the real key — the rendering is for humans only', async () => { + const { result } = await loadMemberFailure(); + + // `attemptedValue` is the record's EXTERNAL key ("which row") — a datum a + // machine reads, not prose. It keeps the NUL-joined key verbatim; only the + // message is rendered. A consumer that serialises it (JSON, util.inspect) + // escapes the control character rather than emitting the byte. + expect(result.errors[0].attemptedValue).toBe(`${EMPLOYER}${NUL}${USER}`); + expect(JSON.stringify(result.errors[0].attemptedValue)).not.toContain(NUL); + }); +}); + +// =========================================================================== +// 2. THE WIDER SURFACE — pass 2's `on record '...'` lines are the same defect +// =========================================================================== + +describe('[#16488] pass-2 deferred-reference diagnostics name the record NUL-free', () => { + it('the UNRESOLVED-after-pass-2 line carries zero NUL bytes and names both parts', async () => { + // `mate` points at a `demo_other` row that is never seeded, so pass 1 + // defers the reference and pass 2 reports it permanently unresolved — + // naming the source row by its (composite) natural key. + const objects = { + demo_link: { + name: 'demo_link', + fields: { + a: { type: 'text' }, + b: { type: 'text' }, + mate: { type: 'lookup', reference: 'demo_other' }, + }, + }, + demo_other: { name: 'demo_other', fields: { name: { type: 'text' } } }, + }; + const seeds = [{ + object: 'demo_link', + externalId: ['a', 'b'], + mode: 'upsert', + env: ['prod', 'dev', 'test'], + records: [{ a: EMPLOYER, b: USER, mate: 'ghost' }], + }]; + + const logger = createLogger(); + const { engine } = createFaithfulEngine(); + const result = await new SeedLoaderService(engine, createMetadata(objects), logger as never) + .load({ seeds, config: CONFIG } as never); + + const lines = diagnostics(result as never, logger as never); + // Anti-vacuity: the pass-2 diagnostic really fired. + const unresolved = lines.filter((l) => l.includes('UNRESOLVED after pass 2')); + expect(unresolved.length).toBeGreaterThan(0); + // It still names the row — by a rendering, not by the map key. + expect(unresolved[0]).toContain(JSON.stringify([EMPLOYER, USER])); + + for (const line of lines) expect(nulCount(line)).toBe(0); + }); +}); + +// =========================================================================== +// 3. NEGATIVE CONTROL A — a SINGLE-key diagnostic is byte-identical +// =========================================================================== + +describe('[#16488] negative control: a non-composite externalId diagnostic does not move', () => { + it('pins the whole message, byte for byte', async () => { + const objects = { demo_solo: { name: 'demo_solo', fields: { name: { type: 'text' }, plan: { type: 'text' } } } }; + const seeds = [{ + object: 'demo_solo', + externalId: 'name', + mode: 'upsert', + env: ['prod', 'dev', 'test'], + records: [{ name: 'acme', plan: 'pro' }], + }]; + + const logger = createLogger(); + const svc = new SeedLoaderService(failingEngine(), createMetadata(objects), logger as never); + const result = await svc.load({ seeds, config: CONFIG } as never); + + // `toBe`, not `toContain`: "byte-identical" is the claim, so the whole + // string is the assertion. A single-field key never contains U+0000, so the + // renderer returns it unchanged and this line reads exactly as it always did. + expect(result.errors[0].message).toBe( + `Failed to write demo_solo record #0 (name=acme): ${WITHHELD}`, + ); + expect(result.errors[0].attemptedValue).toBe('acme'); + }); +}); + +// =========================================================================== +// 4. NEGATIVE CONTROL B — the MAP KEY is untouched, and the counter can fire +// =========================================================================== + +describe('[#16488] negative control: the U+0000 joiner still separates map keys', () => { + it('nulCount answers non-zero on a control string — the instrument is live', () => { + // Without this, a broken counter would green every assertion above. + expect(nulCount(`${EMPLOYER}${NUL}${USER}`)).toBe(1); + expect(nulCount('a')).toBe(0); + }); + + it("the ruling's own pair — ('a','b') and ('a\\0b','') — still do not collide", async () => { + const objects = { demo_pair: { name: 'demo_pair', fields: { a: { type: 'text' }, b: { type: 'text' } } } }; + const seeds = [{ + object: 'demo_pair', + externalId: ['a', 'b'], + mode: 'upsert', + env: ['prod', 'dev', 'test'], + records: [ + { a: 'a', b: 'b' }, + { a: `a${NUL}b`, b: '' }, + ], + }]; + + const { engine, store } = createFaithfulEngine(); + const result = await new SeedLoaderService(engine, createMetadata(objects), createLogger() as never) + .load({ seeds, config: CONFIG } as never); + + expect(result.success).toBe(true); + // Two rows, not one: the second never matched the first. + expect(store.demo_pair).toHaveLength(2); + expect(store.demo_pair.map((r: any) => r.a).sort()).toEqual([`a${NUL}b`, 'a'].sort()); + }); + + it('a pair that WOULD collide under a visible `+` joiner still dedupes as two rows on replay', async () => { + // This is the test that goes red if anyone swaps the NUL for `+` in the + // KEY to make the log prettier: `('x','y+z')` and `('x+y','z')` both join to + // `x+y+z` under `+`, and the second row would be swallowed by the first. + const objects = { demo_pair: { name: 'demo_pair', fields: { a: { type: 'text' }, b: { type: 'text' } } } }; + const seeds = [{ + object: 'demo_pair', + externalId: ['a', 'b'], + mode: 'upsert', + env: ['prod', 'dev', 'test'], + records: [ + { a: 'x', b: 'y+z' }, + { a: 'x+y', b: 'z' }, + ], + }]; + + const { engine, store } = createFaithfulEngine(); + const metadata = createMetadata(objects); + + const first = await new SeedLoaderService(engine, metadata, createLogger() as never) + .load({ seeds, config: CONFIG } as never); + expect(first.success).toBe(true); + expect(store.demo_pair).toHaveLength(2); + + // Replay: each row matches ITS OWN key, so both skip and the table stays at 2. + const second = await new SeedLoaderService(engine, metadata, createLogger() as never) + .load({ seeds, config: CONFIG } as never); + expect(second.success).toBe(true); + expect(store.demo_pair).toHaveLength(2); + expect(second.results.find((r: any) => r.object === 'demo_pair')!.skipped).toBe(2); + }); +}); From cb1639c79a2fdec65f8481b0dfdb78a9c82d3758 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 08:29:32 +0000 Subject: [PATCH 2/3] fix(metadata-protocol): render a composite externalId in seed diagnostics instead of pasting the NUL-joined key (#16488) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `externalIdKey` joins a composite key's parts with U+0000 on purpose — the byte cannot occur in a natural-key value, so ('a','b') and ('a\\0b','') never collide. That separator is untouched. What changes is that the KEY is no longer interpolated into human-readable diagnostics: one raw NUL makes grep classify the whole server log as binary, so every later grep -n over it silently returns nothing. New private `externalIdDisplay` renders a composite key as a JSON array of its parts and returns a single-field key byte-identical. Applied at the six message interpolations (buildWriteError's parenthetical and pass 2's five `on record '…'` sites); the structured `attemptedValue` and the loggers' structured context keep the real key. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- packages/metadata-protocol/src/seed-loader.ts | 65 ++++++++++++++++--- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index c08c1f0977..45e1db07f2 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -1718,6 +1718,12 @@ export class SeedLoaderService implements ISeedLoaderService { organizationId?: string, ): Promise { for (const deferred of deferredUpdates) { + // [#16488] How the messages below NAME this record. `recordExternalId` + // is a map KEY joined with `\u0000` (see {@link externalIdKey}) and stays + // one — the `insertedRecords` fallback lookup below depends on it — but a + // raw NUL in a log line turns the whole log binary for `grep`, so every + // message renders it instead of pasting it. + const recordName = this.externalIdDisplay(deferred.recordExternalId); // A multi-value field deferred its WHOLE authored array (see pass 1), so // re-resolve every element here; a single-value field has exactly one. const items = Array.isArray(deferred.attemptedValue) @@ -1790,7 +1796,7 @@ export class SeedLoaderService implements ISeedLoaderService { const causeSentence = seedFailureCause(err); this.logger.error( `[SeedLoader] Deferred reference back-fill FAILED — ${deferred.objectName}.${deferred.field} stays NULL ` + - `on record '${deferred.recordExternalId}'. The row itself was seeded, so every row counter looks healthy ` + + `on record '${recordName}'. The row itself was seeded, so every row counter looks healthy ` + `while the circular relationship is HALF-WRITTEN: nothing links it to ${deferred.targetObject}.` + `${deferred.targetField} = '${this.formatAttempted(deferred.attemptedValue)}'. Nothing retries this — ` + `fix the write error below (a transient failure that outlasted the retry budget, or a validation rule ` + @@ -1877,11 +1883,11 @@ export class SeedLoaderService implements ISeedLoaderService { } else { this.logger.error( `[SeedLoader] Deferred reference DROPPED — ${deferred.objectName}.${deferred.field} is never written ` + - `on record '${deferred.recordExternalId}'. Pass 2 RESOLVED the target (${where}) and then had ` + + `on record '${recordName}'. Pass 2 RESOLVED the target (${where}) and then had ` + `nowhere to write it: this load registered no internal id for that ${deferred.objectName} record, ` + `because its pass-1 write FAILED (reported as its own \`error\` above) or returned no id. Nothing ` + `retries this: pass 2 back-fills only rows this load actually seeded, and it is the last pass. ` + - `Fix the pass-1 write error reported for ${deferred.objectName} '${deferred.recordExternalId}' ` + + `Fix the pass-1 write error reported for ${deferred.objectName} '${recordName}' ` + `and re-run the seed — the row and this link land together or not at all.`, undefined, { @@ -1895,7 +1901,7 @@ export class SeedLoaderService implements ISeedLoaderService { this.recordDeferredError(deferred, allResults, allErrors, `Deferred reference dropped: ${deferred.objectName}.${deferred.field} = '${missedTarget}' → ` + `${deferred.targetObject}.${deferred.targetField} resolved, but no internal id was registered for ` + - `${deferred.objectName} '${deferred.recordExternalId}' in this load (its pass-1 write failed), so ` + + `${deferred.objectName} '${recordName}' in this load (its pass-1 write failed), so ` + `the back-fill could not be written`); } } @@ -1914,7 +1920,7 @@ export class SeedLoaderService implements ISeedLoaderService { const missedValue = this.formatAttempted(stillUnresolved ? missingItem : deferred.attemptedValue); this.logger.error( `[SeedLoader] Deferred reference UNRESOLVED after pass 2 — ${deferred.objectName}.${deferred.field} ` + - `stays NULL on record '${deferred.recordExternalId}'. The row itself was seeded, so every row ` + + `stays NULL on record '${recordName}'. The row itself was seeded, so every row ` + `counter looks healthy while the relationship is MISSING: nothing links it to ` + `${deferred.targetObject}.${deferred.targetField} = '${missedValue}', because no such ` + `${deferred.targetObject} row exists — neither seeded in this load nor already in the database. ` + @@ -2288,9 +2294,11 @@ export class SeedLoaderService implements ISeedLoaderService { * [#8442] Every STRUCTURED key is unchanged — `sourceObject`, `field`, * `targetObject`, `targetField`, `attemptedValue`, `recordIndex` are built * from the seed declaration and the record, never from the caught error, so - * "which record, which key" is untouched by the withhold. The authored prefix - * is unchanged byte for byte too (two runtime pins read it). What changes is - * only what follows the colon: a DECLARED refusal — a 4xx, or the data + * "which record, which key" is untouched by the withhold. For a single-field + * key the authored prefix is unchanged byte for byte too (two runtime pins + * read it); #16488 renders the VALUE side of a COMPOSITE key — see + * {@link externalIdDisplay} — so its `\u0000` joiner cannot reach the log. + * What #8442 changes is only what follows the colon: a DECLARED refusal — a 4xx, or the data * engine's `VALIDATION_FAILED` shape, which is where "which field and why" * lives — is quoted whole; a driver fault is replaced by * {@link WITHHELD_WRITE_REASON} and goes to the log instead. See @@ -2311,9 +2319,14 @@ export class SeedLoaderService implements ISeedLoaderService { field: '(write)', targetObject: objectName, targetField: label, + // [#16488] The STRUCTURED key keeps the real key — a datum a machine + // reads, and JSON / util.inspect escape a control character rather than + // emitting it. Only the message is rendered. attemptedValue: keyValue || null, recordIndex, - message: `Failed to write ${objectName} record #${recordIndex} (${label}=${keyValue}): ${detail}`, + message: + `Failed to write ${objectName} record #${recordIndex} ` + + `(${label}=${this.externalIdDisplay(keyValue)}): ${detail}`, }; } @@ -2662,6 +2675,36 @@ export class SeedLoaderService implements ISeedLoaderService { return Array.isArray(externalId) ? externalId.join('+') : externalId; } + /** + * Human-readable rendering of a natural key built by {@link externalIdKey}. + * + * The key is not a display string, and it is not free to become one: + * `externalIdKey` joins a composite key's parts with `\u0000` precisely + * because that byte cannot occur in a natural-key value, so `('a','b')` and + * `('a\0b','')` never collide. That separator stays exactly where it is + * — every Map keyed by this string depends on it (framework#3434). + * + * What must not happen is the KEY reaching a MESSAGE. One raw NUL makes + * `grep` classify the whole server log as binary, so every later `grep -n` / + * `grep -c` over it silently returns nothing until the reader remembers + * `-a`: the reader's main instrument disabled by one byte, at the moment + * someone is diagnosing a failed boot (#16488, measured while investigating + * objectstack-ai/ats#20). + * + * A key with no `\u0000` in it — every single-field key — is returned + * BYTE-IDENTICAL, so non-composite diagnostics do not move. A composite key + * renders as a JSON array of its PARTS rather than a `+`-joined string: + * `externalIdLabel` already spends `+` on the FIELD-name side, and a value + * is an arbitrary string that may contain `+` (or ` + `) itself, so a joined + * form is ambiguous exactly where a composite key is interesting. JSON is + * also NUL-free by construction — it escapes control characters rather than + * passing them through — so a value carrying one of its own cannot + * reintroduce the defect through this path. + */ + private externalIdDisplay(key: string): string { + return key.includes('\u0000') ? JSON.stringify(key.split('\u0000')) : key; + } + private buildEmptyResult(config: SeedLoaderConfigParsed, durationMs: number): SeedLoaderResultParsed { return { success: true, @@ -2772,7 +2815,9 @@ interface DeferredUpdate { * The source record's natural key, as {@link SeedLoaderService.externalIdKey} * computed it in pass 1 — pass 2's FALLBACK lookup into `insertedRecords` * when {@link internalId} is absent, and the name error messages call the - * record by. + * record by. It is the KEY, `\u0000` joiner and all; the messages render it + * through {@link SeedLoaderService.externalIdDisplay} rather than pasting it + * (#16488). * * May legitimately be `''`: `externalIdKey` returns the empty string when the * dataset declares no `externalId` and the row carries no `name`, when the From 9996cb7638279bd3486249ee68a8bc78466b301a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 08:59:42 +0000 Subject: [PATCH 3/3] chore(metadata-protocol): changeset + engine-double ledger rows for the new pin file (#16488) check:engine-double-contract retained the new test file's two engine doubles (delete/findOne/update) as unrecorded; regenerated with --write, 3 rows added, 0 lost. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- .changeset/seed-diagnostic-nul-joiner.md | 23 ++++++++++++++++++++++ scripts/engine-double-contract.pinned.json | 15 ++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 .changeset/seed-diagnostic-nul-joiner.md diff --git a/.changeset/seed-diagnostic-nul-joiner.md b/.changeset/seed-diagnostic-nul-joiner.md new file mode 100644 index 0000000000..1054dd7605 --- /dev/null +++ b/.changeset/seed-diagnostic-nul-joiner.md @@ -0,0 +1,23 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +Seed loader: a composite `externalId` no longer puts a raw NUL byte in a +diagnostic line. + +`SeedLoaderService` joins a composite natural key's parts with U+0000 on +purpose — that byte cannot occur in a natural-key value, so `('a','b')` and +`('a\0b','')` never collide. The map key is unchanged. What changes is that +the key string is no longer interpolated into human-readable messages: the +`Failed to write record #N (=)` parenthetical and pass +2's `on record ''` lines now render a composite value as a JSON array of +its parts (`(employer+user=["emp-1","usr-2"])`). + +A single-field `externalId` renders byte-identically, so non-composite +diagnostics do not move, and the structured `errors[].attemptedValue` still +carries the real key. + +Why it mattered: one raw NUL makes `grep` classify the whole server log as +binary, so every later `grep -n` / `grep -c` over it silently returns nothing +until the reader adds `-a` — the reader's main instrument disabled by one byte, +at the moment someone is diagnosing a failed boot. diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 0a2552f374..bc72a36809 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1361,6 +1361,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/seed-loader-composite-key-diagnostic.test.ts", + "verb": "delete", + "pinned": 2 + }, + { + "file": "packages/metadata-protocol/src/seed-loader-composite-key-diagnostic.test.ts", + "verb": "findOne", + "pinned": 2 + }, + { + "file": "packages/metadata-protocol/src/seed-loader-composite-key-diagnostic.test.ts", + "verb": "update", + "pinned": 2 + }, { "file": "packages/metadata-protocol/src/seed-loader-deferred-dropped.test.ts", "verb": "delete",