Skip to content

Commit 8169e9a

Browse files
os-zhuangclaude
andcommitted
fix(metadata-protocol): mark and pin the pass-2 operator half; pin the real-driver withhold (#8442)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
1 parent 0c985ed commit 8169e9a

3 files changed

Lines changed: 222 additions & 6 deletions

File tree

‎packages/metadata-protocol/src/seed-loader-driver-text.test.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,8 @@ describe('[#8442] raw driver text is withheld from the seed `errors[].message`',
230230
aggregate: vi.fn(async () => []),
231231
} as unknown as IDataEngine;
232232

233-
const result = await new SeedLoaderService(engine, metadata, createLogger() as never).load({
233+
const logger = createLogger();
234+
const result = await new SeedLoaderService(engine, metadata, logger as never).load({
234235
seeds: [
235236
{ object: 'dept', externalId: 'name', mode: 'insert', env: ['prod', 'dev', 'test'], records: [{ name: 'Engineering', head_id: 'Alice' }] },
236237
{ object: 'worker', externalId: 'name', mode: 'insert', env: ['prod', 'dev', 'test'], records: [{ name: 'Alice', dept_id: 'Engineering' }] },
@@ -245,6 +246,19 @@ describe('[#8442] raw driver text is withheld from the seed `errors[].message`',
245246
expect(backfill!.message).toContain('worker.name');
246247
expect(backfill!.message).toContain(WITHHELD);
247248
expectNothingLeaked(result);
249+
250+
// THE OPERATOR HALF OF *THIS* PASS — pinned here rather than left to
251+
// section 5, which drives pass 1 only. Without this assertion a later
252+
// edit could withhold the pass-2 log line too and every pin in the file
253+
// would stay green while the deferred diagnostic disappeared: a payload
254+
// pinned for both passes and an operator half pinned for one is exactly
255+
// the "green pin narrower than its name" shape this family has already
256+
// recorded once.
257+
const logged = logger.error.mock.calls.map((c) => String(c[0])).join('\n');
258+
expect(logged).toContain(DRIVER_TEXT);
259+
// …under the SAME marked vocabulary pass 1 uses, so an operator reading
260+
// a pass-2 line learns the reporter never received this sentence.
261+
expect(logged).toContain('Cause (withheld from the seed response)');
248262
});
249263
});
250264

‎packages/metadata-protocol/src/seed-loader.ts‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,24 @@ function seedFailureCause(err: unknown): string {
102102
return typeof message === 'string' && message.length > 0 ? message : String(err);
103103
}
104104

105+
/**
106+
* [#8442] How a log line labels the cause it is about to print: marked when the
107+
* payload half withheld that sentence, plain when the caller received it too.
108+
*
109+
* The marker is the half of the operator story that is not about the text. An
110+
* operator reading `Cause: …` and an operator reading
111+
* `Cause (withheld from the seed response): …` are looking at the same
112+
* sentence and a DIFFERENT support situation — in the second the reporter never
113+
* saw it, so "what did the response say?" has a different answer than the log
114+
* suggests. Both passes share this one vocabulary so the two halves of the file
115+
* cannot drift into answering that question differently.
116+
*/
117+
function seedCauseLabel(err: unknown): string {
118+
return quotableSeedFailureDetail(err) === undefined
119+
? 'Cause (withheld from the seed response)'
120+
: 'Cause';
121+
}
122+
105123
/**
106124
* [#8442] The operator half. The log line always carries the caught sentence,
107125
* even when the payload may not quote it — without this, withholding the text
@@ -112,7 +130,7 @@ function seedFailureLogLine(payloadMessage: string, err: unknown): string {
112130
const cause = seedFailureCause(err);
113131
return payloadMessage.includes(cause)
114132
? `[SeedLoader] ${payloadMessage}`
115-
: `[SeedLoader] ${payloadMessage} Cause (withheld from the seed response): ${cause}`;
133+
: `[SeedLoader] ${payloadMessage} ${seedCauseLabel(err)}: ${cause}`;
116134
}
117135

118136
/** The environments a seed dataset can be scoped to — mirrors `SeedSchema.env`. */
@@ -1255,7 +1273,10 @@ export class SeedLoaderService implements ISeedLoaderService {
12551273
`${deferred.targetField} = '${this.formatAttempted(deferred.attemptedValue)}'. Nothing retries this — ` +
12561274
`fix the write error below (a transient failure that outlasted the retry budget, or a validation rule ` +
12571275
`vetoing the update) and re-run the seed to complete the link. ` +
1258-
`Cause: ${err?.message ?? String(err)}`,
1276+
// [#8442] Same cause vocabulary as the pass-1 write sites: the
1277+
// raw sentence always, MARKED when the payload half withheld it
1278+
// so an operator can see the reporter did not receive this line.
1279+
`${seedCauseLabel(err)}: ${seedFailureCause(err)}`,
12591280
err instanceof Error ? err : undefined,
12601281
{
12611282
object: deferred.objectName,
@@ -1267,9 +1288,13 @@ export class SeedLoaderService implements ISeedLoaderService {
12671288
// [#8442] The pass-2 counterpart of `buildWriteError`'s tail, and
12681289
// the same rule: the located structure (which object, which field,
12691290
// which target, which record) is authored here and untouched; only
1270-
// the caught sentence is gated. The `logger.error` above already
1271-
// carries the raw cause for the operator, so nothing is lost from
1272-
// the server's side of this failure.
1291+
// the caught sentence is gated. The `logger.error` above is this
1292+
// site's operator half — it runs on THIS path, in this same catch,
1293+
// and carries the raw cause under {@link seedCauseLabel}, the same
1294+
// marked vocabulary the pass-1 sites use. Both halves are pinned:
1295+
// an assertion on the pass-2 logger lives beside the payload one in
1296+
// `seed-loader-driver-text.test.ts`, so a later edit that withholds
1297+
// here too cannot stay green.
12731298
this.recordDeferredError(deferred, allResults, allErrors,
12741299
`Failed to write deferred reference: ${deferred.objectName}.${deferred.field} = '${this.formatAttempted(deferred.attemptedValue)}' → ${deferred.targetObject}.${deferred.targetField}: ${quotableSeedFailureDetail(err) ?? WITHHELD_WRITE_REASON}`);
12751300
}
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #8442 — the seed loader's `errors[].message` withhold, proven on a REAL
5+
* driver rather than a fixture.
6+
*
7+
* ## The question this file was written to answer
8+
*
9+
* The fix quotes a caught sentence when the error declared itself a client
10+
* refusal, and one of the two accepted declarations is the `VALIDATION_FAILED`
11+
* shape (`@objectstack/types`' `validationFailureDetails`), because the seed
12+
* channel's authoring feedback arrives that way and carries no `status`.
13+
* Importing the canonical recogniser rather than re-spelling it is what stops
14+
* the seed channel drifting from the HTTP boundaries — but it also means the
15+
* limb inherits whatever that recogniser admits.
16+
*
17+
* So: can a DRIVER-originated constraint violation — unique / check / FK, the
18+
* populations where the sentence's author and its shape could come apart —
19+
* reach the loader's catch already wearing the validation shape? If it could,
20+
* the new limb would be a disclosure path that no hand-built fixture in this
21+
* repo would ever reveal, because every fixture constructs its error at the
22+
* layer that authored the sentence.
23+
*
24+
* ## Measured answer: NO — and the disclosure withheld here is worse than the
25+
* one the issue reported
26+
*
27+
* Driven end to end (real `SqlDriver` on better-sqlite3 on disk, real
28+
* `ObjectQL`, real `SeedLoaderService`) with a duplicate on a `unique` column,
29+
* the driver raises:
30+
*
31+
* ```
32+
* SqliteError name: 'SqliteError' code: 'SQLITE_CONSTRAINT_UNIQUE'
33+
* own properties: [stack, message, code] status: undefined
34+
* message: insert into `q2_acct` (…) select 'dup@example.com' as `email`, … - UNIQUE constraint failed: q2_acct.email
35+
* ```
36+
*
37+
* `validationFailureDetails` does NOT recognise it, so nothing converts it on
38+
* the way up: between the driver and this catch there is only ObjectQL, whose
39+
* own `ValidationError` throws are authored (`reference_not_found` from the
40+
* message catalog, and a re-wrap of already-authored fields). The conversions
41+
* that do exist — `mapDataError`, `resolveThrownHttpError` — live at HTTP
42+
* boundaries that CONSUME this loader's output; they are downstream of this
43+
* producer and can never wrap the engine's throw on its way into it.
44+
*
45+
* Note what that raw message contains: the full INSERT statement including the
46+
* seeded VALUES. The issue's example leaked a table name; this path would leak
47+
* row data and schema shape together. It is now withheld, and the assertion
48+
* below is on the whole payload, not just the tail.
49+
*/
50+
51+
import { describe, it, expect, afterEach } from 'vitest';
52+
import { mkdtempSync, rmSync } from 'node:fs';
53+
import { tmpdir } from 'node:os';
54+
import { join } from 'node:path';
55+
import { ObjectQL } from '@objectstack/objectql';
56+
import { SeedLoaderService } from '@objectstack/metadata-protocol';
57+
import { SqlDriver } from '@objectstack/driver-sql';
58+
import { validationFailureDetails } from '@objectstack/types';
59+
60+
/** `email` declares UNIQUE, so the real driver — not a validator — rejects the duplicate. */
61+
const ACCT = {
62+
name: 'dt_acct',
63+
fields: {
64+
name: { type: 'text' },
65+
email: { type: 'text', unique: true },
66+
},
67+
};
68+
69+
const SEED_CONFIG = {
70+
dryRun: false, haltOnError: false, multiPass: true,
71+
defaultMode: 'insert', batchSize: 1000, transaction: false,
72+
} as any;
73+
74+
function metadataFor(objects: any[]) {
75+
const byName = new Map(objects.map((o) => [o.name, o]));
76+
return {
77+
getObject: async (name: string) => byName.get(name),
78+
listObjects: async () => objects,
79+
register: async () => {}, get: async (_t: string, n: string) => byName.get(n),
80+
list: async () => [], unregister: async () => {}, exists: async () => false, listNames: async () => [],
81+
} as any;
82+
}
83+
84+
describe('[#8442] a REAL driver constraint violation is withheld from the seed response', () => {
85+
let dir: string | null = null;
86+
let engine: ObjectQL | null = null;
87+
88+
afterEach(async () => {
89+
try { await engine?.destroy(); } catch { /* noop */ }
90+
engine = null;
91+
if (dir) { rmSync(dir, { recursive: true, force: true }); dir = null; }
92+
});
93+
94+
it('a duplicate on a UNIQUE column leaks neither the SQL nor the seeded values', async () => {
95+
dir = mkdtempSync(join(tmpdir(), 'os-8442-real-'));
96+
const real = new SqlDriver({
97+
client: 'better-sqlite3',
98+
connection: { filename: join(dir, 'data.sqlite') },
99+
useNullAsDefault: true,
100+
});
101+
await real.initObjects([ACCT]);
102+
103+
// Capture the RAW driver error at the seam, then let it propagate
104+
// untouched — so the test can assert on what the driver really threw
105+
// rather than on an assumption about it.
106+
let raw: any = null;
107+
const driver = Object.create(real);
108+
driver.create = async (o: string, d: any, opts: any) => {
109+
try { return await real.create(o, d, opts); } catch (e) { raw ??= e; throw e; }
110+
};
111+
driver.bulkCreate = async (o: string, rows: any[], opts: any) => {
112+
try { return await real.bulkCreate(o, rows, opts); } catch (e) { raw ??= e; throw e; }
113+
};
114+
115+
engine = new ObjectQL();
116+
engine.registerDriver(driver, true);
117+
await engine.init();
118+
engine.registry.registerObject(ACCT as any);
119+
120+
const logger = { info() {}, warn() {}, error: (() => {
121+
const calls: string[] = [];
122+
const fn = (m: string) => { calls.push(String(m)); };
123+
(fn as any).calls = calls;
124+
return fn;
125+
})(), debug() {} };
126+
127+
const svc = new SeedLoaderService(engine as never, metadataFor([ACCT]), logger as never);
128+
const result = await svc.load({
129+
seeds: [{
130+
object: 'dt_acct',
131+
externalId: 'name',
132+
mode: 'insert',
133+
env: ['prod', 'dev', 'test'],
134+
records: [
135+
{ name: 'first', email: 'dup@example.com' },
136+
{ name: 'second', email: 'dup@example.com' }, // duplicate on UNIQUE
137+
],
138+
}],
139+
config: SEED_CONFIG,
140+
} as never);
141+
142+
// ── The population really is what this file claims ──────────────────────
143+
// Non-vacuity: if the driver stopped rejecting duplicates, or the error
144+
// arrived wearing the validation shape, the assertions below would be
145+
// measuring something else entirely.
146+
expect(raw, 'the driver never rejected the duplicate').toBeTruthy();
147+
expect(String(raw.code)).toContain('SQLITE_CONSTRAINT');
148+
expect(String(raw.message)).toContain('UNIQUE constraint failed');
149+
// THE Q2 ANSWER: a driver-originated constraint violation does NOT arrive
150+
// wearing the validation shape, so the quoting limb never opens for it.
151+
expect(validationFailureDetails(raw)).toBeUndefined();
152+
expect(raw.status).toBeUndefined();
153+
154+
// ── The withhold ───────────────────────────────────────────────────────
155+
expect(result.success).toBe(false);
156+
const failed = result.errors.find((e: any) => e.recordIndex === 1);
157+
expect(failed, 'the duplicate row was not reported').toBeDefined();
158+
expect(failed!.message).toContain('the data engine rejected the write; the reason is in the server log');
159+
160+
// Nothing of the driver's sentence reaches the caller — asserted over the
161+
// WHOLE payload, because this message carries the statement AND the values.
162+
const wire = JSON.stringify(result);
163+
expect(wire).not.toContain('UNIQUE constraint failed');
164+
expect(wire).not.toContain('SQLITE_CONSTRAINT');
165+
expect(wire).not.toContain('insert into');
166+
expect(wire).not.toContain('dup@example.com');
167+
168+
// ── …and the operator still gets it ────────────────────────────────────
169+
const logged = ((logger.error as any).calls as string[]).join('\n');
170+
expect(logged).toContain('UNIQUE constraint failed');
171+
expect(logged).toContain('Cause (withheld from the seed response)');
172+
173+
// The structured authoring feedback survives: which row failed.
174+
expect(failed!.sourceObject).toBe('dt_acct');
175+
expect(failed!.attemptedValue).toBe('second');
176+
});
177+
});

0 commit comments

Comments
 (0)