diff --git a/packages/core/src/evaluator/__tests__/listConditional.test.ts b/packages/core/src/evaluator/__tests__/listConditional.test.ts index 6a86d22b94..d9c6aedfc8 100644 --- a/packages/core/src/evaluator/__tests__/listConditional.test.ts +++ b/packages/core/src/evaluator/__tests__/listConditional.test.ts @@ -117,6 +117,30 @@ describe('evalRowPredicate', () => { expect(r).toBe(false); expect(warn).not.toHaveBeenCalled(); }); + + // The warning is deduped per (label, source) pair. These two pin the two + // halves of that contract across the objectstack#5450 key change: the key + // used to be the pair joined on a raw U+0000 byte (which is what made this + // file invisible to grep) and is now JSON.stringify of the pair. + it('warns only once for the same (label, source) pair', () => { + const faulty = 'record.dedupe_once =='; + evalRowPredicate(faulty, { a: 1 }, { warnOnError: true, label: 'twice' }); + evalRowPredicate(faulty, { a: 2 }, { warnOnError: true, label: 'twice' }); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('keeps the label/source boundary unambiguous', () => { + // The obvious repair for an "impossible character" separator is to pick a + // printable one, which merely relocates the collision. These two pairs + // share a label+source CONCATENATION and would collapse into one key under + // a space separator, so the second warning would be swallowed. JSON needs + // no impossible character and no such assumption. + evalRowPredicate('beta record.boundary ==', { a: 1 }, { warnOnError: true, label: 'alpha' }); + evalRowPredicate('record.boundary ==', { a: 1 }, { warnOnError: true, label: 'alpha beta' }); + expect(warn).toHaveBeenCalledTimes(2); + expect(String(warn.mock.calls[0][0])).toContain('(alpha)'); + expect(String(warn.mock.calls[1][0])).toContain('(alpha beta)'); + }); }); }); diff --git a/packages/core/src/evaluator/listConditional.ts b/packages/core/src/evaluator/listConditional.ts index ae8ec04d2e..675a91c1ec 100644 Binary files a/packages/core/src/evaluator/listConditional.ts and b/packages/core/src/evaluator/listConditional.ts differ diff --git a/packages/core/src/utils/__tests__/record-title.test.ts b/packages/core/src/utils/__tests__/record-title.test.ts index 86e9210b19..871eec473d 100644 --- a/packages/core/src/utils/__tests__/record-title.test.ts +++ b/packages/core/src/utils/__tests__/record-title.test.ts @@ -384,3 +384,46 @@ describe('formatTitleTemplate', () => { expect(formatTitleTemplate(undefined, { id: '1' })).toBe(''); }); }); + +/** + * objectstack#5450. The empty-placeholder sentinel `EMPTY_TOKEN` is a U+0000 + * that used to be written into the source as the raw byte, which made this + * module binary to grep. Re-spelling it as an escape leaves the runtime value + * identical — these pin the behaviour that identity is responsible for, so a + * later attempt to "make it printable" cannot pass quietly. + * + * Byte discipline: the assertions test CODE POINTS via charCodeAt. No control + * character is written into this file, as a literal or as an escape. + */ +describe('formatTitleTemplate — the empty-placeholder sentinel', () => { + /** True when every code point is printable (no C0 control, no U+007F). */ + const isPrintable = (s: string) => + Array.from(s).every((c) => { + const cp = c.codePointAt(0) as number; + return cp >= 0x20 && cp !== 0x7f; + }); + + it('never leaks the sentinel into the rendered title', () => { + const out = formatTitleTemplate('{full_name} - {company}', { company: 'Acme' }); + expect(out).toBe('Acme'); + expect(isPrintable(out), 'the sentinel must not survive into a rendered title').toBe(true); + }); + + it('strips several empty placeholders and their orphan separators at once', () => { + const out = formatTitleTemplate('{a} - {b} | {c}', { b: 'Only' }); + expect(out).toBe('Only'); + expect(isPrintable(out)).toBe(true); + }); + + it('leaves a separator that sits between two RESOLVED fields alone', () => { + // The strip passes are anchored on the sentinel, so a real separator with + // content on both sides must survive — otherwise "A - B" would become "AB". + expect(formatTitleTemplate('{a} - {b}', { a: 'A', b: 'B' })).toBe('A - B'); + }); + + it('does not swallow a record value just because it neighbours an empty one', () => { + const out = formatTitleTemplate('{missing}·{kept}', { kept: 'Kept' }); + expect(out).toBe('Kept'); + expect(isPrintable(out)).toBe(true); + }); +}); diff --git a/packages/core/src/utils/record-title.ts b/packages/core/src/utils/record-title.ts index aa8fcd4718..72d15c3878 100644 Binary files a/packages/core/src/utils/record-title.ts and b/packages/core/src/utils/record-title.ts differ diff --git a/packages/fields/src/widgets/PeoplePicker.tsx b/packages/fields/src/widgets/PeoplePicker.tsx index 76fbd4e209..04290ad252 100644 --- a/packages/fields/src/widgets/PeoplePicker.tsx +++ b/packages/fields/src/widgets/PeoplePicker.tsx @@ -316,8 +316,14 @@ export function PeoplePicker({ // returns the same records (StrictMode double-effect, refetch-on-focus) // re-emits a new array; resetting on identity alone yanked the cursor to -1 // mid-navigation (flaky ArrowDown→Enter, and a real UX nit). + // + // The id separator is the escaped U+0000 spelling, not the byte itself + // (objectstack#5450): one raw NUL made this whole file binary to grep and + // ripgrep — a content search printed `binary file matches` and no line. The + // runtime value is unchanged, and it is the same separator the `cursorEpoch` + // key below already spelled this way. const recordsSignature = useMemo( - () => query.records.map(r => String(getPersonId(r, idField))).join(''), + () => query.records.map(r => String(getPersonId(r, idField))).join('\u0000'), [query.records, idField], ); // The reset runs in the RENDER PHASE (the "adjusting state during render" diff --git a/packages/plugin-dashboard/src/DatasetWidget.tsx b/packages/plugin-dashboard/src/DatasetWidget.tsx index adf6e1c889..e1bdbdf2a0 100644 --- a/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -86,7 +86,13 @@ export function buildPivot( const colSeen = new Set(); const cellIndex = new Map(); rows.forEach((row, index) => { - const rid = rowDims.map((d) => String(row[d] ?? '∅')).join(''); + // The row-dimension separator below is the escaped U+0001 spelling, not the + // byte itself (objectstack#5450). U+0001 does not blind grep the way U+0000 + // does, but written raw it is invisible in every editor and every diff, so no + // reviewer could tell what this separator actually was. The runtime value is + // unchanged: a character no dimension value can carry, which is what keeps + // two dimension values from merging into one ambiguous row id. + const rid = rowDims.map((d) => String(row[d] ?? '∅')).join('\u0001'); const cid = String(row[colDim] ?? '∅'); if (!rowSeen.has(rid)) { rowSeen.add(rid); rowHeaders.push({ id: rid, labels: rowDims.map((d) => formatDimensionValue(row[d])) }); } if (!colSeen.has(cid)) { colSeen.add(cid); colHeaders.push({ id: cid, label: formatDimensionValue(row[colDim]) }); } diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx index e9c65d2531..5630055655 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx @@ -319,6 +319,37 @@ describe('DatasetWidget', () => { expect(p.cellIndex.get('Done Low')).toBeUndefined(); // sparse combo absent }); + // objectstack#5450. Every case above pivots on ONE row dimension, so the + // separator that joins several of them was never exercised — it could have + // been anything, including the empty string, and the suite stayed green. It + // was a raw U+0001 in the source (invisible in every editor and diff) and is + // now the same character written as an escape; these pin what that character + // is responsible for. + // + // Byte discipline: the expected id is built from a NUMBER via fromCharCode. + // No control character is written into this file, as a literal or an escape. + it('buildPivot keeps two row dimensions apart that would merge without a separator', () => { + const rows = [ + // "x" + "yz" and "xy" + "z" concatenate to the same string, so an empty + // (or absent) separator collapses these into ONE row header and the second + // row's cell overwrites the first. + { region: 'x', segment: 'yz', priority: 'High', total: 1 }, + { region: 'xy', segment: 'z', priority: 'High', total: 2 }, + ]; + const p = buildPivot(rows, ['region', 'segment'], 'priority'); + + expect(p.rowHeaders).toHaveLength(2); + expect(p.rowHeaders.map((r) => r.labels)).toEqual([ + ['x', 'yz'], + ['xy', 'z'], + ]); + + const SEP = String.fromCharCode(0x01); + expect(p.rowHeaders.map((r) => r.id)).toEqual([`x${SEP}yz`, `xy${SEP}z`]); + expect(p.cellIndex.get(`x${SEP}yz High`)).toBe(0); + expect(p.cellIndex.get(`xy${SEP}z High`)).toBe(1); + }); + it('renders a pivot (≥2 dims) as a true cross-tab, not a flat table', async () => { const src = { queryDataset: vi.fn(async () => ({ rows: [ diff --git a/scripts/__tests__/check-control-bytes.test.ts b/scripts/__tests__/check-control-bytes.test.ts index 24c307df77..2ecc14e9db 100644 --- a/scripts/__tests__/check-control-bytes.test.ts +++ b/scripts/__tests__/check-control-bytes.test.ts @@ -303,6 +303,49 @@ describe('objectstack#5425 — the file that started this is readable again', () }); }); +/** + * objectstack#5450 — the four files the baseline shipped with are now clean, and + * KNOWN_OFFENDERS is empty. + * + * Two harms, so two assertion shapes. Three of the four carried U+0000 and were + * genuinely invisible to content search, so the pin for those is a real grep + * that must print a line. The fourth carried U+0001, which (measured on GNU grep + * 3.11 and ripgrep 14) never blinded grep at all — claiming a search outage + * there would be a fabricated pin, so it is asserted only on the byte's absence, + * which is the harm it actually had: an unreviewable literal. + */ +describe('objectstack#5450 — the four baselined files are clean', () => { + const cleaned = [ + { file: 'packages/core/src/evaluator/listConditional.ts', grepFor: 'warnedError' }, + { file: 'packages/core/src/utils/record-title.ts', grepFor: 'EMPTY_TOKEN' }, + { file: 'packages/fields/src/widgets/PeoplePicker.tsx', grepFor: 'recordsSignature' }, + // U+0001, not U+0000: never a search outage, so no grep assertion below. + { file: 'packages/plugin-dashboard/src/DatasetWidget.tsx', grepFor: null }, + ]; + + it.each(cleaned.map((c) => c.file))('%s carries no control byte at all', (file) => { + const buf = fs.readFileSync(path.join(repoRoot, file)); + const found = [...buf].filter((b) => SCANNED_BYTES.has(b)); + expect(found).toEqual([]); + }); + + it.each(cleaned.filter((c) => c.grepFor).map((c) => [c.file, c.grepFor as string]))( + '%s is visible to a content search again', + (file, needle) => { + const out = execFileSync('grep', ['-n', needle, file], { cwd: repoRoot, encoding: 'utf8' }); + expect(out).toMatch(new RegExp(needle)); + expect(out).not.toMatch(/binary file matches/); + }, + ); + + it('has no baseline entry left for any of them', () => { + const baseline = KNOWN_OFFENDERS as Map; + for (const { file } of cleaned) { + expect(baseline.has(file), `${file} is clean; its KNOWN_OFFENDERS entry must be gone`).toBe(false); + } + }); +}); + describe('wiring — the gate is actually reachable and actually runs', () => { const pkg = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')); const workflowPath = path.join(repoRoot, '.github/workflows/control-bytes.yml'); diff --git a/scripts/check-control-bytes.mjs b/scripts/check-control-bytes.mjs index 3a874e1008..0534bf32cc 100644 --- a/scripts/check-control-bytes.mjs +++ b/scripts/check-control-bytes.mjs @@ -148,10 +148,11 @@ const WIDE_BOMS = [ * The map is expected to reach empty and stay there. */ export const KNOWN_OFFENDERS = new Map([ - ['packages/core/src/evaluator/listConditional.ts', { bytes: [NUL], issue: 'objectstack#5450' }], - ['packages/core/src/utils/record-title.ts', { bytes: [NUL], issue: 'objectstack#5450' }], - ['packages/fields/src/widgets/PeoplePicker.tsx', { bytes: [NUL], issue: 'objectstack#5450' }], - ['packages/plugin-dashboard/src/DatasetWidget.tsx', { bytes: [0x01], issue: 'objectstack#5450' }], + // Empty, and expected to stay that way. The four entries this map shipped + // with (two in `@object-ui/core`, one in `@object-ui/fields`, one in + // `@object-ui/plugin-dashboard`) were cleaned by objectstack#5450 and deleted + // in the same PR, because `scan()` reports a stale entry as loudly as a new + // offender. Add one back only with an issue number, and only as debt. ]); function hasWideBom(buf) {