Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 163 additions & 15 deletions packages/rest/src/rest-server-meta-read-org-scope.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object';
/** The value every read assertion looks for. */
const MARKER = 'AUTHORED_AT_RUNTIME';

/** The second revision's marker — two PUTs, two history events. */
const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2';

/**
* A SPEC-VALID body per type, carrying `label` as the marker the reads assert
* on. Real bodies, not `{ label }` stubs: the write door runs full spec
Expand All@@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME';
* nothing to do with org scoping. Each shape was measured against the real
* validator, not guessed.
*/
function bodyFor(type: string, name: string): Record<string, unknown> {
const marker = { name, label: MARKER };
function bodyFor(type: string, name: string, label = MARKER): Record<string, unknown> {
const marker = { name, label };
switch (type) {
case 'view':
// [#7741] the inline arm requires the object-binding pair.
Expand DownExpand Up@@ -114,25 +117,41 @@ interface Row {
state: string; metadata: string; checksum?: string; version?: number;
}

interface HistoryRow {
id: string; type: string; name: string;
organization_id: string | null;
version: number; event_seq: number;
operation_type: string; metadata: string | null;
}

const keyOf = (w: Record<string, unknown>) =>
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;

function matchesWhere(r: Row, where: Record<string, unknown>): boolean {
function matchesWhere(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
for (const [k, v] of Object.entries(where)) {
if (k === '$or') {
const clauses = v as Array<Record<string, unknown>>;
if (!clauses.some((c) => matchesWhere(r, c))) return false;
continue;
}
// ⛔ REFUSE any other combinator rather than reading it as a field
// name. `$or` is the only one the read paths under test emit, and a
// double that answered `$and` by looking for a column literally called
// `$and` would return a well-formed WRONG answer — the same silent
// class as a double that drops the predicate entirely. Refusing loudly
// is the convention the sibling harness already follows.
if (k.startsWith('$')) {
throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``);
}
if (v === undefined) continue;
if ((r as unknown as Record<string, unknown>)[k] !== v) return false;
if (r[k] !== v) return false;
}
return true;
}

function makeStubEngine() {
const rows = new Map<string, Row>();
const historyRows: any[] = [];
const historyRows: HistoryRow[] = [];
let nextId = 0;

const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
Expand All@@ -145,25 +164,63 @@ function makeStubEngine() {
const r = rows.get(k);
if (r) return { key: k, row: r };
}
for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r };
for (const [k, r] of rows) if (matchesWhere(r as unknown as Record<string, unknown>, w)) return { key: k, row: r };
return null;
};

const engine: any = {
async findOne(table: string, opts: { where: Record<string, unknown> }) {
assertEngineFindOnePredicate(table, opts);
if (table === 'sys_metadata_history') return null;
if (table === 'sys_metadata_history') {
// [#13764] Was `return null` UNCONDITIONALLY. Measured before
// changing it: the unconditional null is NOT load-bearing for
// the PUT path this fixture drives — production reaches this
// seam only from `getByHash`, `restoreVersion` and
// `resolveMetaItemOrgScope`, none of which a PUT calls; the
// write path reads history through `find`
// (`nextEventSeq` / `nextItemVersion`).
return historyRows.find(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts.where),
) ?? null;
}
return findRow(opts.where)?.row ?? null;
},
async find(table: string, opts?: { where?: Record<string, unknown> }) {
if (table === 'sys_metadata_history') return historyRows;
return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {}));
async find(table: string, opts?: { where?: Record<string, unknown>; limit?: number }) {
// [#13764] Two silences repaired on one seam, for one reason.
//
// WHERE: this branch used to `return historyRows` UNFILTERED.
// `SysMetadataRepository.history()` and `diffMetaItem` filter
// `organization_id` by STRICT EQUALITY and post-filter nothing, so
// an unfiltered answer made the org predicate a no-op: an
// org-scoping assertion for `/history` was green whether or not the
// door forwarded the organization. Measured, not argued — the
// `#13764` block at the bottom of this file is green over the old
// stub in BOTH states and reddens over this one when the org is
// dropped.
//
// LIMIT: applied AFTER the filter and BY PRESENCE
// (`typeof === 'number'`), so `limit: 0` returns nothing rather
// than everything, and bounding never decides WHICH rows survive
// the predicate — only how many of the survivors come back. Every
// call this fixture makes passes no bound and is untouched.
// `check:objectql-double-limit`. Applied on BOTH tables so the two
// branches cannot disagree.
if (table === 'sys_metadata_history') {
const matched = historyRows.filter(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
}
const matched = Array.from(rows.values()).filter(
(r) => matchesWhere(r as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
},
async insert(table: string, data: Record<string, unknown>) {
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
if (table === 'sys_metadata_history') {
nextId += 1;
historyRows.push({ ...data, id: `h_${nextId}` });
historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` });
return { id: `h_${nextId}` };
}
if (table !== 'sys_metadata') return { id: 'side_effect_skip' };
Expand DownExpand Up@@ -203,7 +260,7 @@ function makeStubEngine() {
isPackageDisabled: () => false,
},
};
return { engine, rows };
return { engine, rows, historyRows };
}

// ── REST harness: real protocol, real routes, one boot ────────────────────
Expand DownExpand Up@@ -236,7 +293,7 @@ function mockRes() {
* can read the same store on the same boot (the cross-tenant control).
*/
function boot() {
const { engine, rows } = makeStubEngine();
const { engine, rows, historyRows } = makeStubEngine();
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
protocol.getDiscovery = async () => ({
version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' },
Expand DownExpand Up@@ -269,17 +326,24 @@ function boot() {

return {
rows,
historyRows,
as(tenantId: string | undefined) {
session = tenantId === undefined
? { userId: 'u1', systemPermissions: ['manage_metadata'] }
: { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId };
},
put: (type: string, name: string) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }),
put: (type: string, name: string, label = MARKER) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }),
get: (type: string, name: string) =>
drive('GET', `${META}/:type/:name`, { params: { type, name } }),
list: (type: string) =>
drive('GET', `${META}/:type`, { params: { type } }),
history: (type: string, name: string) =>
drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }),
/** The fixture proof every history assertion below is gated on. */
historyRowsFor: (type: string, name: string, org: string | null) =>
historyRows.filter((h) => h.type === type && h.name === name
&& (h.organization_id ?? null) === org),
};
}

Expand DownExpand Up@@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted'
});
});
});

// ── #13764 — the instrument's own discriminating power, pinned ────────────
//
// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history`
// seams: `findOne` answered `null` unconditionally and `find` handed back every
// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem`
// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over
// that stub the org predicate was a NO-OP — an org-scoping assertion for
// `/history` was green whether or not the door forwarded the organization.
//
// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to
// org B`. The positive case below cannot do that job: un-partition the stub
// again and it stays GREEN, because an unfiltered read still contains the rows
// it looks for. Only the cross-tenant case reddens, because only it asks for an
// answer the unfiltered stub cannot give. It is here for the stub, not for the
// door.
//
// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in
// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned
// stub is the positive control this repair was calibrated against, and which
// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is
// asserted here is the narrow fact that THIS harness can now tell a forwarded
// org from a dropped one.
describe('#13764 the history seams of this harness honour the org partition', () => {
let b: ReturnType<typeof boot>;
beforeEach(() => { b = boot(); });

it('serves the org-scoped change log of an item the active org authored', async () => {
// The measurement that names the repair: with the door's org dropped
// this reads the ENV partition and answers zero events. Over the old
// unfiltered stub it answered two in BOTH states.
const first = await b.put(CACHED_ARM, 'authored_at_runtime');
expect(first.status, 'the fixture never wrote').toBe(200);
await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2);

// Fixture proof first — "the read is org-scoped" is worthless if the
// fixture never created an org-scoped row.
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length,
'nothing landed in the org partition; the read below would then pass '
+ 'or fail for a reason unrelated to org scoping',
).toBe(2);
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length,
'the write also landed env-wide — the partition is not real',
).toBe(0);

const read = await b.history(CACHED_ARM, 'authored_at_runtime');
expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined();
expect(read.status).toBe(200);
expect(
read.body?.events?.length,
'the door answered an empty change log for an item whose org partition holds two events',
).toBe(2);
});

it('does not serve org A history to org B on the same boot', async () => {
// ⭐ The one that reddens if the stub is ever un-partitioned again.
await b.put(UNCACHED_ARM, 'tenant_bound');
await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2);
expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2);

b.as(ORG_B);
const read = await b.history(UNCACHED_ARM, 'tenant_bound');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'org B was served org A\'s change log',
).toEqual([]);
});

it('does not serve an org-scoped change log to a caller that named no org', async () => {
await b.put(CACHED_ARM, 'org_a_only');
expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1);

b.as(undefined);
const read = await b.history(CACHED_ARM, 'org_a_only');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'an org-less caller was served an org-scoped change log',
).toEqual([]);
});
});
3 changes: 0 additions & 3 deletions scripts/objectql-double-limit.baseline.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -714,9 +714,6 @@
"packages/rest/src/rest-exec-ctx-principal-kind.test.ts": {
"unjudged": 1
},
"packages/rest/src/rest-server-meta-read-org-scope.test.ts": {
"blind": 1
},
"packages/rest/src/rest-server-timing.test.ts": {
"unjudged": 1
},
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
test(rest): the meta read-scope stub honours the where and the limit on both sys_metadata_history seams by claude[bot] · Pull Request #13839 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 163 additions & 15 deletions packages/rest/src/rest-server-meta-read-org-scope.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object';
/** The value every read assertion looks for. */
const MARKER = 'AUTHORED_AT_RUNTIME';

/** The second revision's marker — two PUTs, two history events. */
const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2';

/**
* A SPEC-VALID body per type, carrying `label` as the marker the reads assert
* on. Real bodies, not `{ label }` stubs: the write door runs full spec
Expand All@@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME';
* nothing to do with org scoping. Each shape was measured against the real
* validator, not guessed.
*/
function bodyFor(type: string, name: string): Record<string, unknown> {
const marker = { name, label: MARKER };
function bodyFor(type: string, name: string, label = MARKER): Record<string, unknown> {
const marker = { name, label };
switch (type) {
case 'view':
// [#7741] the inline arm requires the object-binding pair.
Expand DownExpand Up@@ -114,25 +117,41 @@ interface Row {
state: string; metadata: string; checksum?: string; version?: number;
}

interface HistoryRow {
id: string; type: string; name: string;
organization_id: string | null;
version: number; event_seq: number;
operation_type: string; metadata: string | null;
}

const keyOf = (w: Record<string, unknown>) =>
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;

function matchesWhere(r: Row, where: Record<string, unknown>): boolean {
function matchesWhere(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
for (const [k, v] of Object.entries(where)) {
if (k === '$or') {
const clauses = v as Array<Record<string, unknown>>;
if (!clauses.some((c) => matchesWhere(r, c))) return false;
continue;
}
// ⛔ REFUSE any other combinator rather than reading it as a field
// name. `$or` is the only one the read paths under test emit, and a
// double that answered `$and` by looking for a column literally called
// `$and` would return a well-formed WRONG answer — the same silent
// class as a double that drops the predicate entirely. Refusing loudly
// is the convention the sibling harness already follows.
if (k.startsWith('$')) {
throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``);
}
if (v === undefined) continue;
if ((r as unknown as Record<string, unknown>)[k] !== v) return false;
if (r[k] !== v) return false;
}
return true;
}

function makeStubEngine() {
const rows = new Map<string, Row>();
const historyRows: any[] = [];
const historyRows: HistoryRow[] = [];
let nextId = 0;

const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
Expand All@@ -145,25 +164,63 @@ function makeStubEngine() {
const r = rows.get(k);
if (r) return { key: k, row: r };
}
for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r };
for (const [k, r] of rows) if (matchesWhere(r as unknown as Record<string, unknown>, w)) return { key: k, row: r };
return null;
};

const engine: any = {
async findOne(table: string, opts: { where: Record<string, unknown> }) {
assertEngineFindOnePredicate(table, opts);
if (table === 'sys_metadata_history') return null;
if (table === 'sys_metadata_history') {
// [#13764] Was `return null` UNCONDITIONALLY. Measured before
// changing it: the unconditional null is NOT load-bearing for
// the PUT path this fixture drives — production reaches this
// seam only from `getByHash`, `restoreVersion` and
// `resolveMetaItemOrgScope`, none of which a PUT calls; the
// write path reads history through `find`
// (`nextEventSeq` / `nextItemVersion`).
return historyRows.find(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts.where),
) ?? null;
}
return findRow(opts.where)?.row ?? null;
},
async find(table: string, opts?: { where?: Record<string, unknown> }) {
if (table === 'sys_metadata_history') return historyRows;
return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {}));
async find(table: string, opts?: { where?: Record<string, unknown>; limit?: number }) {
// [#13764] Two silences repaired on one seam, for one reason.
//
// WHERE: this branch used to `return historyRows` UNFILTERED.
// `SysMetadataRepository.history()` and `diffMetaItem` filter
// `organization_id` by STRICT EQUALITY and post-filter nothing, so
// an unfiltered answer made the org predicate a no-op: an
// org-scoping assertion for `/history` was green whether or not the
// door forwarded the organization. Measured, not argued — the
// `#13764` block at the bottom of this file is green over the old
// stub in BOTH states and reddens over this one when the org is
// dropped.
//
// LIMIT: applied AFTER the filter and BY PRESENCE
// (`typeof === 'number'`), so `limit: 0` returns nothing rather
// than everything, and bounding never decides WHICH rows survive
// the predicate — only how many of the survivors come back. Every
// call this fixture makes passes no bound and is untouched.
// `check:objectql-double-limit`. Applied on BOTH tables so the two
// branches cannot disagree.
if (table === 'sys_metadata_history') {
const matched = historyRows.filter(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
}
const matched = Array.from(rows.values()).filter(
(r) => matchesWhere(r as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
},
async insert(table: string, data: Record<string, unknown>) {
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
if (table === 'sys_metadata_history') {
nextId += 1;
historyRows.push({ ...data, id: `h_${nextId}` });
historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` });
return { id: `h_${nextId}` };
}
if (table !== 'sys_metadata') return { id: 'side_effect_skip' };
Expand DownExpand Up@@ -203,7 +260,7 @@ function makeStubEngine() {
isPackageDisabled: () => false,
},
};
return { engine, rows };
return { engine, rows, historyRows };
}

// ── REST harness: real protocol, real routes, one boot ────────────────────
Expand DownExpand Up@@ -236,7 +293,7 @@ function mockRes() {
* can read the same store on the same boot (the cross-tenant control).
*/
function boot() {
const { engine, rows } = makeStubEngine();
const { engine, rows, historyRows } = makeStubEngine();
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
protocol.getDiscovery = async () => ({
version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' },
Expand DownExpand Up@@ -269,17 +326,24 @@ function boot() {

return {
rows,
historyRows,
as(tenantId: string | undefined) {
session = tenantId === undefined
? { userId: 'u1', systemPermissions: ['manage_metadata'] }
: { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId };
},
put: (type: string, name: string) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }),
put: (type: string, name: string, label = MARKER) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }),
get: (type: string, name: string) =>
drive('GET', `${META}/:type/:name`, { params: { type, name } }),
list: (type: string) =>
drive('GET', `${META}/:type`, { params: { type } }),
history: (type: string, name: string) =>
drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }),
/** The fixture proof every history assertion below is gated on. */
historyRowsFor: (type: string, name: string, org: string | null) =>
historyRows.filter((h) => h.type === type && h.name === name
&& (h.organization_id ?? null) === org),
};
}

Expand DownExpand Up@@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted'
});
});
});

// ── #13764 — the instrument's own discriminating power, pinned ────────────
//
// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history`
// seams: `findOne` answered `null` unconditionally and `find` handed back every
// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem`
// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over
// that stub the org predicate was a NO-OP — an org-scoping assertion for
// `/history` was green whether or not the door forwarded the organization.
//
// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to
// org B`. The positive case below cannot do that job: un-partition the stub
// again and it stays GREEN, because an unfiltered read still contains the rows
// it looks for. Only the cross-tenant case reddens, because only it asks for an
// answer the unfiltered stub cannot give. It is here for the stub, not for the
// door.
//
// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in
// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned
// stub is the positive control this repair was calibrated against, and which
// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is
// asserted here is the narrow fact that THIS harness can now tell a forwarded
// org from a dropped one.
describe('#13764 the history seams of this harness honour the org partition', () => {
let b: ReturnType<typeof boot>;
beforeEach(() => { b = boot(); });

it('serves the org-scoped change log of an item the active org authored', async () => {
// The measurement that names the repair: with the door's org dropped
// this reads the ENV partition and answers zero events. Over the old
// unfiltered stub it answered two in BOTH states.
const first = await b.put(CACHED_ARM, 'authored_at_runtime');
expect(first.status, 'the fixture never wrote').toBe(200);
await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2);

// Fixture proof first — "the read is org-scoped" is worthless if the
// fixture never created an org-scoped row.
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length,
'nothing landed in the org partition; the read below would then pass '
+ 'or fail for a reason unrelated to org scoping',
).toBe(2);
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length,
'the write also landed env-wide — the partition is not real',
).toBe(0);

const read = await b.history(CACHED_ARM, 'authored_at_runtime');
expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined();
expect(read.status).toBe(200);
expect(
read.body?.events?.length,
'the door answered an empty change log for an item whose org partition holds two events',
).toBe(2);
});

it('does not serve org A history to org B on the same boot', async () => {
// ⭐ The one that reddens if the stub is ever un-partitioned again.
await b.put(UNCACHED_ARM, 'tenant_bound');
await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2);
expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2);

b.as(ORG_B);
const read = await b.history(UNCACHED_ARM, 'tenant_bound');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'org B was served org A\'s change log',
).toEqual([]);
});

it('does not serve an org-scoped change log to a caller that named no org', async () => {
await b.put(CACHED_ARM, 'org_a_only');
expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1);

b.as(undefined);
const read = await b.history(CACHED_ARM, 'org_a_only');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'an org-less caller was served an org-scoped change log',
).toEqual([]);
});
});
3 changes: 0 additions & 3 deletions scripts/objectql-double-limit.baseline.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -714,9 +714,6 @@
"packages/rest/src/rest-exec-ctx-principal-kind.test.ts": {
"unjudged": 1
},
"packages/rest/src/rest-server-meta-read-org-scope.test.ts": {
"blind": 1
},
"packages/rest/src/rest-server-timing.test.ts": {
"unjudged": 1
},
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test(rest): the meta read-scope stub honours the where and the limit on both sys_metadata_history seams by claude[bot] · Pull Request #13839 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 163 additions & 15 deletions packages/rest/src/rest-server-meta-read-org-scope.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object';
/** The value every read assertion looks for. */
const MARKER = 'AUTHORED_AT_RUNTIME';

/** The second revision's marker — two PUTs, two history events. */
const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2';

/**
* A SPEC-VALID body per type, carrying `label` as the marker the reads assert
* on. Real bodies, not `{ label }` stubs: the write door runs full spec
Expand All@@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME';
* nothing to do with org scoping. Each shape was measured against the real
* validator, not guessed.
*/
function bodyFor(type: string, name: string): Record<string, unknown> {
const marker = { name, label: MARKER };
function bodyFor(type: string, name: string, label = MARKER): Record<string, unknown> {
const marker = { name, label };
switch (type) {
case 'view':
// [#7741] the inline arm requires the object-binding pair.
Expand DownExpand Up@@ -114,25 +117,41 @@ interface Row {
state: string; metadata: string; checksum?: string; version?: number;
}

interface HistoryRow {
id: string; type: string; name: string;
organization_id: string | null;
version: number; event_seq: number;
operation_type: string; metadata: string | null;
}

const keyOf = (w: Record<string, unknown>) =>
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;

function matchesWhere(r: Row, where: Record<string, unknown>): boolean {
function matchesWhere(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
for (const [k, v] of Object.entries(where)) {
if (k === '$or') {
const clauses = v as Array<Record<string, unknown>>;
if (!clauses.some((c) => matchesWhere(r, c))) return false;
continue;
}
// ⛔ REFUSE any other combinator rather than reading it as a field
// name. `$or` is the only one the read paths under test emit, and a
// double that answered `$and` by looking for a column literally called
// `$and` would return a well-formed WRONG answer — the same silent
// class as a double that drops the predicate entirely. Refusing loudly
// is the convention the sibling harness already follows.
if (k.startsWith('$')) {
throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``);
}
if (v === undefined) continue;
if ((r as unknown as Record<string, unknown>)[k] !== v) return false;
if (r[k] !== v) return false;
}
return true;
}

function makeStubEngine() {
const rows = new Map<string, Row>();
const historyRows: any[] = [];
const historyRows: HistoryRow[] = [];
let nextId = 0;

const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
Expand All@@ -145,25 +164,63 @@ function makeStubEngine() {
const r = rows.get(k);
if (r) return { key: k, row: r };
}
for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r };
for (const [k, r] of rows) if (matchesWhere(r as unknown as Record<string, unknown>, w)) return { key: k, row: r };
return null;
};

const engine: any = {
async findOne(table: string, opts: { where: Record<string, unknown> }) {
assertEngineFindOnePredicate(table, opts);
if (table === 'sys_metadata_history') return null;
if (table === 'sys_metadata_history') {
// [#13764] Was `return null` UNCONDITIONALLY. Measured before
// changing it: the unconditional null is NOT load-bearing for
// the PUT path this fixture drives — production reaches this
// seam only from `getByHash`, `restoreVersion` and
// `resolveMetaItemOrgScope`, none of which a PUT calls; the
// write path reads history through `find`
// (`nextEventSeq` / `nextItemVersion`).
return historyRows.find(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts.where),
) ?? null;
}
return findRow(opts.where)?.row ?? null;
},
async find(table: string, opts?: { where?: Record<string, unknown> }) {
if (table === 'sys_metadata_history') return historyRows;
return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {}));
async find(table: string, opts?: { where?: Record<string, unknown>; limit?: number }) {
// [#13764] Two silences repaired on one seam, for one reason.
//
// WHERE: this branch used to `return historyRows` UNFILTERED.
// `SysMetadataRepository.history()` and `diffMetaItem` filter
// `organization_id` by STRICT EQUALITY and post-filter nothing, so
// an unfiltered answer made the org predicate a no-op: an
// org-scoping assertion for `/history` was green whether or not the
// door forwarded the organization. Measured, not argued — the
// `#13764` block at the bottom of this file is green over the old
// stub in BOTH states and reddens over this one when the org is
// dropped.
//
// LIMIT: applied AFTER the filter and BY PRESENCE
// (`typeof === 'number'`), so `limit: 0` returns nothing rather
// than everything, and bounding never decides WHICH rows survive
// the predicate — only how many of the survivors come back. Every
// call this fixture makes passes no bound and is untouched.
// `check:objectql-double-limit`. Applied on BOTH tables so the two
// branches cannot disagree.
if (table === 'sys_metadata_history') {
const matched = historyRows.filter(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
}
const matched = Array.from(rows.values()).filter(
(r) => matchesWhere(r as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
},
async insert(table: string, data: Record<string, unknown>) {
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
if (table === 'sys_metadata_history') {
nextId += 1;
historyRows.push({ ...data, id: `h_${nextId}` });
historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` });
return { id: `h_${nextId}` };
}
if (table !== 'sys_metadata') return { id: 'side_effect_skip' };
Expand DownExpand Up@@ -203,7 +260,7 @@ function makeStubEngine() {
isPackageDisabled: () => false,
},
};
return { engine, rows };
return { engine, rows, historyRows };
}

// ── REST harness: real protocol, real routes, one boot ────────────────────
Expand DownExpand Up@@ -236,7 +293,7 @@ function mockRes() {
* can read the same store on the same boot (the cross-tenant control).
*/
function boot() {
const { engine, rows } = makeStubEngine();
const { engine, rows, historyRows } = makeStubEngine();
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
protocol.getDiscovery = async () => ({
version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' },
Expand DownExpand Up@@ -269,17 +326,24 @@ function boot() {

return {
rows,
historyRows,
as(tenantId: string | undefined) {
session = tenantId === undefined
? { userId: 'u1', systemPermissions: ['manage_metadata'] }
: { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId };
},
put: (type: string, name: string) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }),
put: (type: string, name: string, label = MARKER) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }),
get: (type: string, name: string) =>
drive('GET', `${META}/:type/:name`, { params: { type, name } }),
list: (type: string) =>
drive('GET', `${META}/:type`, { params: { type } }),
history: (type: string, name: string) =>
drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }),
/** The fixture proof every history assertion below is gated on. */
historyRowsFor: (type: string, name: string, org: string | null) =>
historyRows.filter((h) => h.type === type && h.name === name
&& (h.organization_id ?? null) === org),
};
}

Expand DownExpand Up@@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted'
});
});
});

// ── #13764 — the instrument's own discriminating power, pinned ────────────
//
// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history`
// seams: `findOne` answered `null` unconditionally and `find` handed back every
// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem`
// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over
// that stub the org predicate was a NO-OP — an org-scoping assertion for
// `/history` was green whether or not the door forwarded the organization.
//
// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to
// org B`. The positive case below cannot do that job: un-partition the stub
// again and it stays GREEN, because an unfiltered read still contains the rows
// it looks for. Only the cross-tenant case reddens, because only it asks for an
// answer the unfiltered stub cannot give. It is here for the stub, not for the
// door.
//
// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in
// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned
// stub is the positive control this repair was calibrated against, and which
// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is
// asserted here is the narrow fact that THIS harness can now tell a forwarded
// org from a dropped one.
describe('#13764 the history seams of this harness honour the org partition', () => {
let b: ReturnType<typeof boot>;
beforeEach(() => { b = boot(); });

it('serves the org-scoped change log of an item the active org authored', async () => {
// The measurement that names the repair: with the door's org dropped
// this reads the ENV partition and answers zero events. Over the old
// unfiltered stub it answered two in BOTH states.
const first = await b.put(CACHED_ARM, 'authored_at_runtime');
expect(first.status, 'the fixture never wrote').toBe(200);
await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2);

// Fixture proof first — "the read is org-scoped" is worthless if the
// fixture never created an org-scoped row.
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length,
'nothing landed in the org partition; the read below would then pass '
+ 'or fail for a reason unrelated to org scoping',
).toBe(2);
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length,
'the write also landed env-wide — the partition is not real',
).toBe(0);

const read = await b.history(CACHED_ARM, 'authored_at_runtime');
expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined();
expect(read.status).toBe(200);
expect(
read.body?.events?.length,
'the door answered an empty change log for an item whose org partition holds two events',
).toBe(2);
});

it('does not serve org A history to org B on the same boot', async () => {
// ⭐ The one that reddens if the stub is ever un-partitioned again.
await b.put(UNCACHED_ARM, 'tenant_bound');
await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2);
expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2);

b.as(ORG_B);
const read = await b.history(UNCACHED_ARM, 'tenant_bound');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'org B was served org A\'s change log',
).toEqual([]);
});

it('does not serve an org-scoped change log to a caller that named no org', async () => {
await b.put(CACHED_ARM, 'org_a_only');
expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1);

b.as(undefined);
const read = await b.history(CACHED_ARM, 'org_a_only');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'an org-less caller was served an org-scoped change log',
).toEqual([]);
});
});
3 changes: 0 additions & 3 deletions scripts/objectql-double-limit.baseline.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -714,9 +714,6 @@
"packages/rest/src/rest-exec-ctx-principal-kind.test.ts": {
"unjudged": 1
},
"packages/rest/src/rest-server-meta-read-org-scope.test.ts": {
"blind": 1
},
"packages/rest/src/rest-server-timing.test.ts": {
"unjudged": 1
},
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test(rest): the meta read-scope stub honours the where and the limit on both sys_metadata_history seams by claude[bot] · Pull Request #13839 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 163 additions & 15 deletions packages/rest/src/rest-server-meta-read-org-scope.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object';
/** The value every read assertion looks for. */
const MARKER = 'AUTHORED_AT_RUNTIME';

/** The second revision's marker — two PUTs, two history events. */
const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2';

/**
* A SPEC-VALID body per type, carrying `label` as the marker the reads assert
* on. Real bodies, not `{ label }` stubs: the write door runs full spec
Expand All@@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME';
* nothing to do with org scoping. Each shape was measured against the real
* validator, not guessed.
*/
function bodyFor(type: string, name: string): Record<string, unknown> {
const marker = { name, label: MARKER };
function bodyFor(type: string, name: string, label = MARKER): Record<string, unknown> {
const marker = { name, label };
switch (type) {
case 'view':
// [#7741] the inline arm requires the object-binding pair.
Expand DownExpand Up@@ -114,25 +117,41 @@ interface Row {
state: string; metadata: string; checksum?: string; version?: number;
}

interface HistoryRow {
id: string; type: string; name: string;
organization_id: string | null;
version: number; event_seq: number;
operation_type: string; metadata: string | null;
}

const keyOf = (w: Record<string, unknown>) =>
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;

function matchesWhere(r: Row, where: Record<string, unknown>): boolean {
function matchesWhere(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
for (const [k, v] of Object.entries(where)) {
if (k === '$or') {
const clauses = v as Array<Record<string, unknown>>;
if (!clauses.some((c) => matchesWhere(r, c))) return false;
continue;
}
// ⛔ REFUSE any other combinator rather than reading it as a field
// name. `$or` is the only one the read paths under test emit, and a
// double that answered `$and` by looking for a column literally called
// `$and` would return a well-formed WRONG answer — the same silent
// class as a double that drops the predicate entirely. Refusing loudly
// is the convention the sibling harness already follows.
if (k.startsWith('$')) {
throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``);
}
if (v === undefined) continue;
if ((r as unknown as Record<string, unknown>)[k] !== v) return false;
if (r[k] !== v) return false;
}
return true;
}

function makeStubEngine() {
const rows = new Map<string, Row>();
const historyRows: any[] = [];
const historyRows: HistoryRow[] = [];
let nextId = 0;

const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
Expand All@@ -145,25 +164,63 @@ function makeStubEngine() {
const r = rows.get(k);
if (r) return { key: k, row: r };
}
for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r };
for (const [k, r] of rows) if (matchesWhere(r as unknown as Record<string, unknown>, w)) return { key: k, row: r };
return null;
};

const engine: any = {
async findOne(table: string, opts: { where: Record<string, unknown> }) {
assertEngineFindOnePredicate(table, opts);
if (table === 'sys_metadata_history') return null;
if (table === 'sys_metadata_history') {
// [#13764] Was `return null` UNCONDITIONALLY. Measured before
// changing it: the unconditional null is NOT load-bearing for
// the PUT path this fixture drives — production reaches this
// seam only from `getByHash`, `restoreVersion` and
// `resolveMetaItemOrgScope`, none of which a PUT calls; the
// write path reads history through `find`
// (`nextEventSeq` / `nextItemVersion`).
return historyRows.find(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts.where),
) ?? null;
}
return findRow(opts.where)?.row ?? null;
},
async find(table: string, opts?: { where?: Record<string, unknown> }) {
if (table === 'sys_metadata_history') return historyRows;
return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {}));
async find(table: string, opts?: { where?: Record<string, unknown>; limit?: number }) {
// [#13764] Two silences repaired on one seam, for one reason.
//
// WHERE: this branch used to `return historyRows` UNFILTERED.
// `SysMetadataRepository.history()` and `diffMetaItem` filter
// `organization_id` by STRICT EQUALITY and post-filter nothing, so
// an unfiltered answer made the org predicate a no-op: an
// org-scoping assertion for `/history` was green whether or not the
// door forwarded the organization. Measured, not argued — the
// `#13764` block at the bottom of this file is green over the old
// stub in BOTH states and reddens over this one when the org is
// dropped.
//
// LIMIT: applied AFTER the filter and BY PRESENCE
// (`typeof === 'number'`), so `limit: 0` returns nothing rather
// than everything, and bounding never decides WHICH rows survive
// the predicate — only how many of the survivors come back. Every
// call this fixture makes passes no bound and is untouched.
// `check:objectql-double-limit`. Applied on BOTH tables so the two
// branches cannot disagree.
if (table === 'sys_metadata_history') {
const matched = historyRows.filter(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
}
const matched = Array.from(rows.values()).filter(
(r) => matchesWhere(r as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
},
async insert(table: string, data: Record<string, unknown>) {
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
if (table === 'sys_metadata_history') {
nextId += 1;
historyRows.push({ ...data, id: `h_${nextId}` });
historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` });
return { id: `h_${nextId}` };
}
if (table !== 'sys_metadata') return { id: 'side_effect_skip' };
Expand DownExpand Up@@ -203,7 +260,7 @@ function makeStubEngine() {
isPackageDisabled: () => false,
},
};
return { engine, rows };
return { engine, rows, historyRows };
}

// ── REST harness: real protocol, real routes, one boot ────────────────────
Expand DownExpand Up@@ -236,7 +293,7 @@ function mockRes() {
* can read the same store on the same boot (the cross-tenant control).
*/
function boot() {
const { engine, rows } = makeStubEngine();
const { engine, rows, historyRows } = makeStubEngine();
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
protocol.getDiscovery = async () => ({
version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' },
Expand DownExpand Up@@ -269,17 +326,24 @@ function boot() {

return {
rows,
historyRows,
as(tenantId: string | undefined) {
session = tenantId === undefined
? { userId: 'u1', systemPermissions: ['manage_metadata'] }
: { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId };
},
put: (type: string, name: string) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }),
put: (type: string, name: string, label = MARKER) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }),
get: (type: string, name: string) =>
drive('GET', `${META}/:type/:name`, { params: { type, name } }),
list: (type: string) =>
drive('GET', `${META}/:type`, { params: { type } }),
history: (type: string, name: string) =>
drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }),
/** The fixture proof every history assertion below is gated on. */
historyRowsFor: (type: string, name: string, org: string | null) =>
historyRows.filter((h) => h.type === type && h.name === name
&& (h.organization_id ?? null) === org),
};
}

Expand DownExpand Up@@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted'
});
});
});

// ── #13764 — the instrument's own discriminating power, pinned ────────────
//
// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history`
// seams: `findOne` answered `null` unconditionally and `find` handed back every
// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem`
// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over
// that stub the org predicate was a NO-OP — an org-scoping assertion for
// `/history` was green whether or not the door forwarded the organization.
//
// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to
// org B`. The positive case below cannot do that job: un-partition the stub
// again and it stays GREEN, because an unfiltered read still contains the rows
// it looks for. Only the cross-tenant case reddens, because only it asks for an
// answer the unfiltered stub cannot give. It is here for the stub, not for the
// door.
//
// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in
// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned
// stub is the positive control this repair was calibrated against, and which
// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is
// asserted here is the narrow fact that THIS harness can now tell a forwarded
// org from a dropped one.
describe('#13764 the history seams of this harness honour the org partition', () => {
let b: ReturnType<typeof boot>;
beforeEach(() => { b = boot(); });

it('serves the org-scoped change log of an item the active org authored', async () => {
// The measurement that names the repair: with the door's org dropped
// this reads the ENV partition and answers zero events. Over the old
// unfiltered stub it answered two in BOTH states.
const first = await b.put(CACHED_ARM, 'authored_at_runtime');
expect(first.status, 'the fixture never wrote').toBe(200);
await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2);

// Fixture proof first — "the read is org-scoped" is worthless if the
// fixture never created an org-scoped row.
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length,
'nothing landed in the org partition; the read below would then pass '
+ 'or fail for a reason unrelated to org scoping',
).toBe(2);
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length,
'the write also landed env-wide — the partition is not real',
).toBe(0);

const read = await b.history(CACHED_ARM, 'authored_at_runtime');
expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined();
expect(read.status).toBe(200);
expect(
read.body?.events?.length,
'the door answered an empty change log for an item whose org partition holds two events',
).toBe(2);
});

it('does not serve org A history to org B on the same boot', async () => {
// ⭐ The one that reddens if the stub is ever un-partitioned again.
await b.put(UNCACHED_ARM, 'tenant_bound');
await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2);
expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2);

b.as(ORG_B);
const read = await b.history(UNCACHED_ARM, 'tenant_bound');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'org B was served org A\'s change log',
).toEqual([]);
});

it('does not serve an org-scoped change log to a caller that named no org', async () => {
await b.put(CACHED_ARM, 'org_a_only');
expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1);

b.as(undefined);
const read = await b.history(CACHED_ARM, 'org_a_only');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'an org-less caller was served an org-scoped change log',
).toEqual([]);
});
});
3 changes: 0 additions & 3 deletions scripts/objectql-double-limit.baseline.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -714,9 +714,6 @@
"packages/rest/src/rest-exec-ctx-principal-kind.test.ts": {
"unjudged": 1
},
"packages/rest/src/rest-server-meta-read-org-scope.test.ts": {
"blind": 1
},
"packages/rest/src/rest-server-timing.test.ts": {
"unjudged": 1
},
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' test(rest): the meta read-scope stub honours the where and the limit on both sys_metadata_history seams by claude[bot] · Pull Request #13839 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 163 additions & 15 deletions packages/rest/src/rest-server-meta-read-org-scope.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object';
/** The value every read assertion looks for. */
const MARKER = 'AUTHORED_AT_RUNTIME';

/** The second revision's marker — two PUTs, two history events. */
const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2';

/**
* A SPEC-VALID body per type, carrying `label` as the marker the reads assert
* on. Real bodies, not `{ label }` stubs: the write door runs full spec
Expand All@@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME';
* nothing to do with org scoping. Each shape was measured against the real
* validator, not guessed.
*/
function bodyFor(type: string, name: string): Record<string, unknown> {
const marker = { name, label: MARKER };
function bodyFor(type: string, name: string, label = MARKER): Record<string, unknown> {
const marker = { name, label };
switch (type) {
case 'view':
// [#7741] the inline arm requires the object-binding pair.
Expand DownExpand Up@@ -114,25 +117,41 @@ interface Row {
state: string; metadata: string; checksum?: string; version?: number;
}

interface HistoryRow {
id: string; type: string; name: string;
organization_id: string | null;
version: number; event_seq: number;
operation_type: string; metadata: string | null;
}

const keyOf = (w: Record<string, unknown>) =>
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;

function matchesWhere(r: Row, where: Record<string, unknown>): boolean {
function matchesWhere(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
for (const [k, v] of Object.entries(where)) {
if (k === '$or') {
const clauses = v as Array<Record<string, unknown>>;
if (!clauses.some((c) => matchesWhere(r, c))) return false;
continue;
}
// ⛔ REFUSE any other combinator rather than reading it as a field
// name. `$or` is the only one the read paths under test emit, and a
// double that answered `$and` by looking for a column literally called
// `$and` would return a well-formed WRONG answer — the same silent
// class as a double that drops the predicate entirely. Refusing loudly
// is the convention the sibling harness already follows.
if (k.startsWith('$')) {
throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``);
}
if (v === undefined) continue;
if ((r as unknown as Record<string, unknown>)[k] !== v) return false;
if (r[k] !== v) return false;
}
return true;
}

function makeStubEngine() {
const rows = new Map<string, Row>();
const historyRows: any[] = [];
const historyRows: HistoryRow[] = [];
let nextId = 0;

const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
Expand All@@ -145,25 +164,63 @@ function makeStubEngine() {
const r = rows.get(k);
if (r) return { key: k, row: r };
}
for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r };
for (const [k, r] of rows) if (matchesWhere(r as unknown as Record<string, unknown>, w)) return { key: k, row: r };
return null;
};

const engine: any = {
async findOne(table: string, opts: { where: Record<string, unknown> }) {
assertEngineFindOnePredicate(table, opts);
if (table === 'sys_metadata_history') return null;
if (table === 'sys_metadata_history') {
// [#13764] Was `return null` UNCONDITIONALLY. Measured before
// changing it: the unconditional null is NOT load-bearing for
// the PUT path this fixture drives — production reaches this
// seam only from `getByHash`, `restoreVersion` and
// `resolveMetaItemOrgScope`, none of which a PUT calls; the
// write path reads history through `find`
// (`nextEventSeq` / `nextItemVersion`).
return historyRows.find(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts.where),
) ?? null;
}
return findRow(opts.where)?.row ?? null;
},
async find(table: string, opts?: { where?: Record<string, unknown> }) {
if (table === 'sys_metadata_history') return historyRows;
return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {}));
async find(table: string, opts?: { where?: Record<string, unknown>; limit?: number }) {
// [#13764] Two silences repaired on one seam, for one reason.
//
// WHERE: this branch used to `return historyRows` UNFILTERED.
// `SysMetadataRepository.history()` and `diffMetaItem` filter
// `organization_id` by STRICT EQUALITY and post-filter nothing, so
// an unfiltered answer made the org predicate a no-op: an
// org-scoping assertion for `/history` was green whether or not the
// door forwarded the organization. Measured, not argued — the
// `#13764` block at the bottom of this file is green over the old
// stub in BOTH states and reddens over this one when the org is
// dropped.
//
// LIMIT: applied AFTER the filter and BY PRESENCE
// (`typeof === 'number'`), so `limit: 0` returns nothing rather
// than everything, and bounding never decides WHICH rows survive
// the predicate — only how many of the survivors come back. Every
// call this fixture makes passes no bound and is untouched.
// `check:objectql-double-limit`. Applied on BOTH tables so the two
// branches cannot disagree.
if (table === 'sys_metadata_history') {
const matched = historyRows.filter(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
}
const matched = Array.from(rows.values()).filter(
(r) => matchesWhere(r as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
},
async insert(table: string, data: Record<string, unknown>) {
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
if (table === 'sys_metadata_history') {
nextId += 1;
historyRows.push({ ...data, id: `h_${nextId}` });
historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` });
return { id: `h_${nextId}` };
}
if (table !== 'sys_metadata') return { id: 'side_effect_skip' };
Expand DownExpand Up@@ -203,7 +260,7 @@ function makeStubEngine() {
isPackageDisabled: () => false,
},
};
return { engine, rows };
return { engine, rows, historyRows };
}

// ── REST harness: real protocol, real routes, one boot ────────────────────
Expand DownExpand Up@@ -236,7 +293,7 @@ function mockRes() {
* can read the same store on the same boot (the cross-tenant control).
*/
function boot() {
const { engine, rows } = makeStubEngine();
const { engine, rows, historyRows } = makeStubEngine();
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
protocol.getDiscovery = async () => ({
version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' },
Expand DownExpand Up@@ -269,17 +326,24 @@ function boot() {

return {
rows,
historyRows,
as(tenantId: string | undefined) {
session = tenantId === undefined
? { userId: 'u1', systemPermissions: ['manage_metadata'] }
: { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId };
},
put: (type: string, name: string) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }),
put: (type: string, name: string, label = MARKER) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }),
get: (type: string, name: string) =>
drive('GET', `${META}/:type/:name`, { params: { type, name } }),
list: (type: string) =>
drive('GET', `${META}/:type`, { params: { type } }),
history: (type: string, name: string) =>
drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }),
/** The fixture proof every history assertion below is gated on. */
historyRowsFor: (type: string, name: string, org: string | null) =>
historyRows.filter((h) => h.type === type && h.name === name
&& (h.organization_id ?? null) === org),
};
}

Expand DownExpand Up@@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted'
});
});
});

// ── #13764 — the instrument's own discriminating power, pinned ────────────
//
// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history`
// seams: `findOne` answered `null` unconditionally and `find` handed back every
// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem`
// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over
// that stub the org predicate was a NO-OP — an org-scoping assertion for
// `/history` was green whether or not the door forwarded the organization.
//
// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to
// org B`. The positive case below cannot do that job: un-partition the stub
// again and it stays GREEN, because an unfiltered read still contains the rows
// it looks for. Only the cross-tenant case reddens, because only it asks for an
// answer the unfiltered stub cannot give. It is here for the stub, not for the
// door.
//
// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in
// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned
// stub is the positive control this repair was calibrated against, and which
// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is
// asserted here is the narrow fact that THIS harness can now tell a forwarded
// org from a dropped one.
describe('#13764 the history seams of this harness honour the org partition', () => {
let b: ReturnType<typeof boot>;
beforeEach(() => { b = boot(); });

it('serves the org-scoped change log of an item the active org authored', async () => {
// The measurement that names the repair: with the door's org dropped
// this reads the ENV partition and answers zero events. Over the old
// unfiltered stub it answered two in BOTH states.
const first = await b.put(CACHED_ARM, 'authored_at_runtime');
expect(first.status, 'the fixture never wrote').toBe(200);
await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2);

// Fixture proof first — "the read is org-scoped" is worthless if the
// fixture never created an org-scoped row.
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length,
'nothing landed in the org partition; the read below would then pass '
+ 'or fail for a reason unrelated to org scoping',
).toBe(2);
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length,
'the write also landed env-wide — the partition is not real',
).toBe(0);

const read = await b.history(CACHED_ARM, 'authored_at_runtime');
expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined();
expect(read.status).toBe(200);
expect(
read.body?.events?.length,
'the door answered an empty change log for an item whose org partition holds two events',
).toBe(2);
});

it('does not serve org A history to org B on the same boot', async () => {
// ⭐ The one that reddens if the stub is ever un-partitioned again.
await b.put(UNCACHED_ARM, 'tenant_bound');
await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2);
expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2);

b.as(ORG_B);
const read = await b.history(UNCACHED_ARM, 'tenant_bound');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'org B was served org A\'s change log',
).toEqual([]);
});

it('does not serve an org-scoped change log to a caller that named no org', async () => {
await b.put(CACHED_ARM, 'org_a_only');
expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1);

b.as(undefined);
const read = await b.history(CACHED_ARM, 'org_a_only');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'an org-less caller was served an org-scoped change log',
).toEqual([]);
});
});
3 changes: 0 additions & 3 deletions scripts/objectql-double-limit.baseline.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -714,9 +714,6 @@
"packages/rest/src/rest-exec-ctx-principal-kind.test.ts": {
"unjudged": 1
},
"packages/rest/src/rest-server-meta-read-org-scope.test.ts": {
"blind": 1
},
"packages/rest/src/rest-server-timing.test.ts": {
"unjudged": 1
},
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test(rest): the meta read-scope stub honours the where and the limit on both sys_metadata_history seams by claude[bot] · Pull Request #13839 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 163 additions & 15 deletions packages/rest/src/rest-server-meta-read-org-scope.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object';
/** The value every read assertion looks for. */
const MARKER = 'AUTHORED_AT_RUNTIME';

/** The second revision's marker — two PUTs, two history events. */
const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2';

/**
* A SPEC-VALID body per type, carrying `label` as the marker the reads assert
* on. Real bodies, not `{ label }` stubs: the write door runs full spec
Expand All@@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME';
* nothing to do with org scoping. Each shape was measured against the real
* validator, not guessed.
*/
function bodyFor(type: string, name: string): Record<string, unknown> {
const marker = { name, label: MARKER };
function bodyFor(type: string, name: string, label = MARKER): Record<string, unknown> {
const marker = { name, label };
switch (type) {
case 'view':
// [#7741] the inline arm requires the object-binding pair.
Expand DownExpand Up@@ -114,25 +117,41 @@ interface Row {
state: string; metadata: string; checksum?: string; version?: number;
}

interface HistoryRow {
id: string; type: string; name: string;
organization_id: string | null;
version: number; event_seq: number;
operation_type: string; metadata: string | null;
}

const keyOf = (w: Record<string, unknown>) =>
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;

function matchesWhere(r: Row, where: Record<string, unknown>): boolean {
function matchesWhere(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
for (const [k, v] of Object.entries(where)) {
if (k === '$or') {
const clauses = v as Array<Record<string, unknown>>;
if (!clauses.some((c) => matchesWhere(r, c))) return false;
continue;
}
// ⛔ REFUSE any other combinator rather than reading it as a field
// name. `$or` is the only one the read paths under test emit, and a
// double that answered `$and` by looking for a column literally called
// `$and` would return a well-formed WRONG answer — the same silent
// class as a double that drops the predicate entirely. Refusing loudly
// is the convention the sibling harness already follows.
if (k.startsWith('$')) {
throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``);
}
if (v === undefined) continue;
if ((r as unknown as Record<string, unknown>)[k] !== v) return false;
if (r[k] !== v) return false;
}
return true;
}

function makeStubEngine() {
const rows = new Map<string, Row>();
const historyRows: any[] = [];
const historyRows: HistoryRow[] = [];
let nextId = 0;

const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
Expand All@@ -145,25 +164,63 @@ function makeStubEngine() {
const r = rows.get(k);
if (r) return { key: k, row: r };
}
for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r };
for (const [k, r] of rows) if (matchesWhere(r as unknown as Record<string, unknown>, w)) return { key: k, row: r };
return null;
};

const engine: any = {
async findOne(table: string, opts: { where: Record<string, unknown> }) {
assertEngineFindOnePredicate(table, opts);
if (table === 'sys_metadata_history') return null;
if (table === 'sys_metadata_history') {
// [#13764] Was `return null` UNCONDITIONALLY. Measured before
// changing it: the unconditional null is NOT load-bearing for
// the PUT path this fixture drives — production reaches this
// seam only from `getByHash`, `restoreVersion` and
// `resolveMetaItemOrgScope`, none of which a PUT calls; the
// write path reads history through `find`
// (`nextEventSeq` / `nextItemVersion`).
return historyRows.find(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts.where),
) ?? null;
}
return findRow(opts.where)?.row ?? null;
},
async find(table: string, opts?: { where?: Record<string, unknown> }) {
if (table === 'sys_metadata_history') return historyRows;
return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {}));
async find(table: string, opts?: { where?: Record<string, unknown>; limit?: number }) {
// [#13764] Two silences repaired on one seam, for one reason.
//
// WHERE: this branch used to `return historyRows` UNFILTERED.
// `SysMetadataRepository.history()` and `diffMetaItem` filter
// `organization_id` by STRICT EQUALITY and post-filter nothing, so
// an unfiltered answer made the org predicate a no-op: an
// org-scoping assertion for `/history` was green whether or not the
// door forwarded the organization. Measured, not argued — the
// `#13764` block at the bottom of this file is green over the old
// stub in BOTH states and reddens over this one when the org is
// dropped.
//
// LIMIT: applied AFTER the filter and BY PRESENCE
// (`typeof === 'number'`), so `limit: 0` returns nothing rather
// than everything, and bounding never decides WHICH rows survive
// the predicate — only how many of the survivors come back. Every
// call this fixture makes passes no bound and is untouched.
// `check:objectql-double-limit`. Applied on BOTH tables so the two
// branches cannot disagree.
if (table === 'sys_metadata_history') {
const matched = historyRows.filter(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
}
const matched = Array.from(rows.values()).filter(
(r) => matchesWhere(r as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
},
async insert(table: string, data: Record<string, unknown>) {
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
if (table === 'sys_metadata_history') {
nextId += 1;
historyRows.push({ ...data, id: `h_${nextId}` });
historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` });
return { id: `h_${nextId}` };
}
if (table !== 'sys_metadata') return { id: 'side_effect_skip' };
Expand DownExpand Up@@ -203,7 +260,7 @@ function makeStubEngine() {
isPackageDisabled: () => false,
},
};
return { engine, rows };
return { engine, rows, historyRows };
}

// ── REST harness: real protocol, real routes, one boot ────────────────────
Expand DownExpand Up@@ -236,7 +293,7 @@ function mockRes() {
* can read the same store on the same boot (the cross-tenant control).
*/
function boot() {
const { engine, rows } = makeStubEngine();
const { engine, rows, historyRows } = makeStubEngine();
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
protocol.getDiscovery = async () => ({
version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' },
Expand DownExpand Up@@ -269,17 +326,24 @@ function boot() {

return {
rows,
historyRows,
as(tenantId: string | undefined) {
session = tenantId === undefined
? { userId: 'u1', systemPermissions: ['manage_metadata'] }
: { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId };
},
put: (type: string, name: string) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }),
put: (type: string, name: string, label = MARKER) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }),
get: (type: string, name: string) =>
drive('GET', `${META}/:type/:name`, { params: { type, name } }),
list: (type: string) =>
drive('GET', `${META}/:type`, { params: { type } }),
history: (type: string, name: string) =>
drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }),
/** The fixture proof every history assertion below is gated on. */
historyRowsFor: (type: string, name: string, org: string | null) =>
historyRows.filter((h) => h.type === type && h.name === name
&& (h.organization_id ?? null) === org),
};
}

Expand DownExpand Up@@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted'
});
});
});

// ── #13764 — the instrument's own discriminating power, pinned ────────────
//
// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history`
// seams: `findOne` answered `null` unconditionally and `find` handed back every
// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem`
// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over
// that stub the org predicate was a NO-OP — an org-scoping assertion for
// `/history` was green whether or not the door forwarded the organization.
//
// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to
// org B`. The positive case below cannot do that job: un-partition the stub
// again and it stays GREEN, because an unfiltered read still contains the rows
// it looks for. Only the cross-tenant case reddens, because only it asks for an
// answer the unfiltered stub cannot give. It is here for the stub, not for the
// door.
//
// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in
// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned
// stub is the positive control this repair was calibrated against, and which
// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is
// asserted here is the narrow fact that THIS harness can now tell a forwarded
// org from a dropped one.
describe('#13764 the history seams of this harness honour the org partition', () => {
let b: ReturnType<typeof boot>;
beforeEach(() => { b = boot(); });

it('serves the org-scoped change log of an item the active org authored', async () => {
// The measurement that names the repair: with the door's org dropped
// this reads the ENV partition and answers zero events. Over the old
// unfiltered stub it answered two in BOTH states.
const first = await b.put(CACHED_ARM, 'authored_at_runtime');
expect(first.status, 'the fixture never wrote').toBe(200);
await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2);

// Fixture proof first — "the read is org-scoped" is worthless if the
// fixture never created an org-scoped row.
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length,
'nothing landed in the org partition; the read below would then pass '
+ 'or fail for a reason unrelated to org scoping',
).toBe(2);
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length,
'the write also landed env-wide — the partition is not real',
).toBe(0);

const read = await b.history(CACHED_ARM, 'authored_at_runtime');
expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined();
expect(read.status).toBe(200);
expect(
read.body?.events?.length,
'the door answered an empty change log for an item whose org partition holds two events',
).toBe(2);
});

it('does not serve org A history to org B on the same boot', async () => {
// ⭐ The one that reddens if the stub is ever un-partitioned again.
await b.put(UNCACHED_ARM, 'tenant_bound');
await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2);
expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2);

b.as(ORG_B);
const read = await b.history(UNCACHED_ARM, 'tenant_bound');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'org B was served org A\'s change log',
).toEqual([]);
});

it('does not serve an org-scoped change log to a caller that named no org', async () => {
await b.put(CACHED_ARM, 'org_a_only');
expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1);

b.as(undefined);
const read = await b.history(CACHED_ARM, 'org_a_only');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'an org-less caller was served an org-scoped change log',
).toEqual([]);
});
});
3 changes: 0 additions & 3 deletions scripts/objectql-double-limit.baseline.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -714,9 +714,6 @@
"packages/rest/src/rest-exec-ctx-principal-kind.test.ts": {
"unjudged": 1
},
"packages/rest/src/rest-server-meta-read-org-scope.test.ts": {
"blind": 1
},
"packages/rest/src/rest-server-timing.test.ts": {
"unjudged": 1
},
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test(rest): the meta read-scope stub honours the where and the limit on both sys_metadata_history seams by claude[bot] · Pull Request #13839 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 163 additions & 15 deletions packages/rest/src/rest-server-meta-read-org-scope.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object';
/** The value every read assertion looks for. */
const MARKER = 'AUTHORED_AT_RUNTIME';

/** The second revision's marker — two PUTs, two history events. */
const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2';

/**
* A SPEC-VALID body per type, carrying `label` as the marker the reads assert
* on. Real bodies, not `{ label }` stubs: the write door runs full spec
Expand All@@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME';
* nothing to do with org scoping. Each shape was measured against the real
* validator, not guessed.
*/
function bodyFor(type: string, name: string): Record<string, unknown> {
const marker = { name, label: MARKER };
function bodyFor(type: string, name: string, label = MARKER): Record<string, unknown> {
const marker = { name, label };
switch (type) {
case 'view':
// [#7741] the inline arm requires the object-binding pair.
Expand DownExpand Up@@ -114,25 +117,41 @@ interface Row {
state: string; metadata: string; checksum?: string; version?: number;
}

interface HistoryRow {
id: string; type: string; name: string;
organization_id: string | null;
version: number; event_seq: number;
operation_type: string; metadata: string | null;
}

const keyOf = (w: Record<string, unknown>) =>
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;

function matchesWhere(r: Row, where: Record<string, unknown>): boolean {
function matchesWhere(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
for (const [k, v] of Object.entries(where)) {
if (k === '$or') {
const clauses = v as Array<Record<string, unknown>>;
if (!clauses.some((c) => matchesWhere(r, c))) return false;
continue;
}
// ⛔ REFUSE any other combinator rather than reading it as a field
// name. `$or` is the only one the read paths under test emit, and a
// double that answered `$and` by looking for a column literally called
// `$and` would return a well-formed WRONG answer — the same silent
// class as a double that drops the predicate entirely. Refusing loudly
// is the convention the sibling harness already follows.
if (k.startsWith('$')) {
throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``);
}
if (v === undefined) continue;
if ((r as unknown as Record<string, unknown>)[k] !== v) return false;
if (r[k] !== v) return false;
}
return true;
}

function makeStubEngine() {
const rows = new Map<string, Row>();
const historyRows: any[] = [];
const historyRows: HistoryRow[] = [];
let nextId = 0;

const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
Expand All@@ -145,25 +164,63 @@ function makeStubEngine() {
const r = rows.get(k);
if (r) return { key: k, row: r };
}
for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r };
for (const [k, r] of rows) if (matchesWhere(r as unknown as Record<string, unknown>, w)) return { key: k, row: r };
return null;
};

const engine: any = {
async findOne(table: string, opts: { where: Record<string, unknown> }) {
assertEngineFindOnePredicate(table, opts);
if (table === 'sys_metadata_history') return null;
if (table === 'sys_metadata_history') {
// [#13764] Was `return null` UNCONDITIONALLY. Measured before
// changing it: the unconditional null is NOT load-bearing for
// the PUT path this fixture drives — production reaches this
// seam only from `getByHash`, `restoreVersion` and
// `resolveMetaItemOrgScope`, none of which a PUT calls; the
// write path reads history through `find`
// (`nextEventSeq` / `nextItemVersion`).
return historyRows.find(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts.where),
) ?? null;
}
return findRow(opts.where)?.row ?? null;
},
async find(table: string, opts?: { where?: Record<string, unknown> }) {
if (table === 'sys_metadata_history') return historyRows;
return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {}));
async find(table: string, opts?: { where?: Record<string, unknown>; limit?: number }) {
// [#13764] Two silences repaired on one seam, for one reason.
//
// WHERE: this branch used to `return historyRows` UNFILTERED.
// `SysMetadataRepository.history()` and `diffMetaItem` filter
// `organization_id` by STRICT EQUALITY and post-filter nothing, so
// an unfiltered answer made the org predicate a no-op: an
// org-scoping assertion for `/history` was green whether or not the
// door forwarded the organization. Measured, not argued — the
// `#13764` block at the bottom of this file is green over the old
// stub in BOTH states and reddens over this one when the org is
// dropped.
//
// LIMIT: applied AFTER the filter and BY PRESENCE
// (`typeof === 'number'`), so `limit: 0` returns nothing rather
// than everything, and bounding never decides WHICH rows survive
// the predicate — only how many of the survivors come back. Every
// call this fixture makes passes no bound and is untouched.
// `check:objectql-double-limit`. Applied on BOTH tables so the two
// branches cannot disagree.
if (table === 'sys_metadata_history') {
const matched = historyRows.filter(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
}
const matched = Array.from(rows.values()).filter(
(r) => matchesWhere(r as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
},
async insert(table: string, data: Record<string, unknown>) {
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
if (table === 'sys_metadata_history') {
nextId += 1;
historyRows.push({ ...data, id: `h_${nextId}` });
historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` });
return { id: `h_${nextId}` };
}
if (table !== 'sys_metadata') return { id: 'side_effect_skip' };
Expand DownExpand Up@@ -203,7 +260,7 @@ function makeStubEngine() {
isPackageDisabled: () => false,
},
};
return { engine, rows };
return { engine, rows, historyRows };
}

// ── REST harness: real protocol, real routes, one boot ────────────────────
Expand DownExpand Up@@ -236,7 +293,7 @@ function mockRes() {
* can read the same store on the same boot (the cross-tenant control).
*/
function boot() {
const { engine, rows } = makeStubEngine();
const { engine, rows, historyRows } = makeStubEngine();
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
protocol.getDiscovery = async () => ({
version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' },
Expand DownExpand Up@@ -269,17 +326,24 @@ function boot() {

return {
rows,
historyRows,
as(tenantId: string | undefined) {
session = tenantId === undefined
? { userId: 'u1', systemPermissions: ['manage_metadata'] }
: { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId };
},
put: (type: string, name: string) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }),
put: (type: string, name: string, label = MARKER) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }),
get: (type: string, name: string) =>
drive('GET', `${META}/:type/:name`, { params: { type, name } }),
list: (type: string) =>
drive('GET', `${META}/:type`, { params: { type } }),
history: (type: string, name: string) =>
drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }),
/** The fixture proof every history assertion below is gated on. */
historyRowsFor: (type: string, name: string, org: string | null) =>
historyRows.filter((h) => h.type === type && h.name === name
&& (h.organization_id ?? null) === org),
};
}

Expand DownExpand Up@@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted'
});
});
});

// ── #13764 — the instrument's own discriminating power, pinned ────────────
//
// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history`
// seams: `findOne` answered `null` unconditionally and `find` handed back every
// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem`
// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over
// that stub the org predicate was a NO-OP — an org-scoping assertion for
// `/history` was green whether or not the door forwarded the organization.
//
// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to
// org B`. The positive case below cannot do that job: un-partition the stub
// again and it stays GREEN, because an unfiltered read still contains the rows
// it looks for. Only the cross-tenant case reddens, because only it asks for an
// answer the unfiltered stub cannot give. It is here for the stub, not for the
// door.
//
// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in
// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned
// stub is the positive control this repair was calibrated against, and which
// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is
// asserted here is the narrow fact that THIS harness can now tell a forwarded
// org from a dropped one.
describe('#13764 the history seams of this harness honour the org partition', () => {
let b: ReturnType<typeof boot>;
beforeEach(() => { b = boot(); });

it('serves the org-scoped change log of an item the active org authored', async () => {
// The measurement that names the repair: with the door's org dropped
// this reads the ENV partition and answers zero events. Over the old
// unfiltered stub it answered two in BOTH states.
const first = await b.put(CACHED_ARM, 'authored_at_runtime');
expect(first.status, 'the fixture never wrote').toBe(200);
await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2);

// Fixture proof first — "the read is org-scoped" is worthless if the
// fixture never created an org-scoped row.
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length,
'nothing landed in the org partition; the read below would then pass '
+ 'or fail for a reason unrelated to org scoping',
).toBe(2);
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length,
'the write also landed env-wide — the partition is not real',
).toBe(0);

const read = await b.history(CACHED_ARM, 'authored_at_runtime');
expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined();
expect(read.status).toBe(200);
expect(
read.body?.events?.length,
'the door answered an empty change log for an item whose org partition holds two events',
).toBe(2);
});

it('does not serve org A history to org B on the same boot', async () => {
// ⭐ The one that reddens if the stub is ever un-partitioned again.
await b.put(UNCACHED_ARM, 'tenant_bound');
await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2);
expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2);

b.as(ORG_B);
const read = await b.history(UNCACHED_ARM, 'tenant_bound');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'org B was served org A\'s change log',
).toEqual([]);
});

it('does not serve an org-scoped change log to a caller that named no org', async () => {
await b.put(CACHED_ARM, 'org_a_only');
expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1);

b.as(undefined);
const read = await b.history(CACHED_ARM, 'org_a_only');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'an org-less caller was served an org-scoped change log',
).toEqual([]);
});
});
3 changes: 0 additions & 3 deletions scripts/objectql-double-limit.baseline.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -714,9 +714,6 @@
"packages/rest/src/rest-exec-ctx-principal-kind.test.ts": {
"unjudged": 1
},
"packages/rest/src/rest-server-meta-read-org-scope.test.ts": {
"blind": 1
},
"packages/rest/src/rest-server-timing.test.ts": {
"unjudged": 1
},
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); test(rest): the meta read-scope stub honours the where and the limit on both sys_metadata_history seams by claude[bot] · Pull Request #13839 · objectstack-ai/objectstack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 163 additions & 15 deletions packages/rest/src/rest-server-meta-read-org-scope.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,9 @@ const NON_OVERRIDABLE = 'object';
/** The value every read assertion looks for. */
const MARKER = 'AUTHORED_AT_RUNTIME';

/** The second revision's marker — two PUTs, two history events. */
const MARKER_2 = 'AUTHORED_AT_RUNTIME_REV2';

/**
* A SPEC-VALID body per type, carrying `label` as the marker the reads assert
* on. Real bodies, not `{ label }` stubs: the write door runs full spec
Expand All@@ -77,8 +80,8 @@ const MARKER = 'AUTHORED_AT_RUNTIME';
* nothing to do with org scoping. Each shape was measured against the real
* validator, not guessed.
*/
function bodyFor(type: string, name: string): Record<string, unknown> {
const marker = { name, label: MARKER };
function bodyFor(type: string, name: string, label = MARKER): Record<string, unknown> {
const marker = { name, label };
switch (type) {
case 'view':
// [#7741] the inline arm requires the object-binding pair.
Expand DownExpand Up@@ -114,25 +117,41 @@ interface Row {
state: string; metadata: string; checksum?: string; version?: number;
}

interface HistoryRow {
id: string; type: string; name: string;
organization_id: string | null;
version: number; event_seq: number;
operation_type: string; metadata: string | null;
}

const keyOf = (w: Record<string, unknown>) =>
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;

function matchesWhere(r: Row, where: Record<string, unknown>): boolean {
function matchesWhere(r: Record<string, unknown>, where: Record<string, unknown>): boolean {
for (const [k, v] of Object.entries(where)) {
if (k === '$or') {
const clauses = v as Array<Record<string, unknown>>;
if (!clauses.some((c) => matchesWhere(r, c))) return false;
continue;
}
// ⛔ REFUSE any other combinator rather than reading it as a field
// name. `$or` is the only one the read paths under test emit, and a
// double that answered `$and` by looking for a column literally called
// `$and` would return a well-formed WRONG answer — the same silent
// class as a double that drops the predicate entirely. Refusing loudly
// is the convention the sibling harness already follows.
if (k.startsWith('$')) {
throw new Error(`stub engine: unsupported WHERE combinator \`${k}\``);
}
if (v === undefined) continue;
if ((r as unknown as Record<string, unknown>)[k] !== v) return false;
if (r[k] !== v) return false;
}
return true;
}

function makeStubEngine() {
const rows = new Map<string, Row>();
const historyRows: any[] = [];
const historyRows: HistoryRow[] = [];
let nextId = 0;

const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
Expand All@@ -145,25 +164,63 @@ function makeStubEngine() {
const r = rows.get(k);
if (r) return { key: k, row: r };
}
for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r };
for (const [k, r] of rows) if (matchesWhere(r as unknown as Record<string, unknown>, w)) return { key: k, row: r };
return null;
};

const engine: any = {
async findOne(table: string, opts: { where: Record<string, unknown> }) {
assertEngineFindOnePredicate(table, opts);
if (table === 'sys_metadata_history') return null;
if (table === 'sys_metadata_history') {
// [#13764] Was `return null` UNCONDITIONALLY. Measured before
// changing it: the unconditional null is NOT load-bearing for
// the PUT path this fixture drives — production reaches this
// seam only from `getByHash`, `restoreVersion` and
// `resolveMetaItemOrgScope`, none of which a PUT calls; the
// write path reads history through `find`
// (`nextEventSeq` / `nextItemVersion`).
return historyRows.find(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts.where),
) ?? null;
}
return findRow(opts.where)?.row ?? null;
},
async find(table: string, opts?: { where?: Record<string, unknown> }) {
if (table === 'sys_metadata_history') return historyRows;
return Array.from(rows.values()).filter((r) => matchesWhere(r, opts?.where ?? {}));
async find(table: string, opts?: { where?: Record<string, unknown>; limit?: number }) {
// [#13764] Two silences repaired on one seam, for one reason.
//
// WHERE: this branch used to `return historyRows` UNFILTERED.
// `SysMetadataRepository.history()` and `diffMetaItem` filter
// `organization_id` by STRICT EQUALITY and post-filter nothing, so
// an unfiltered answer made the org predicate a no-op: an
// org-scoping assertion for `/history` was green whether or not the
// door forwarded the organization. Measured, not argued — the
// `#13764` block at the bottom of this file is green over the old
// stub in BOTH states and reddens over this one when the org is
// dropped.
//
// LIMIT: applied AFTER the filter and BY PRESENCE
// (`typeof === 'number'`), so `limit: 0` returns nothing rather
// than everything, and bounding never decides WHICH rows survive
// the predicate — only how many of the survivors come back. Every
// call this fixture makes passes no bound and is untouched.
// `check:objectql-double-limit`. Applied on BOTH tables so the two
// branches cannot disagree.
if (table === 'sys_metadata_history') {
const matched = historyRows.filter(
(h) => matchesWhere(h as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
}
const matched = Array.from(rows.values()).filter(
(r) => matchesWhere(r as unknown as Record<string, unknown>, opts?.where ?? {}),
);
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
},
async insert(table: string, data: Record<string, unknown>) {
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
if (table === 'sys_metadata_history') {
nextId += 1;
historyRows.push({ ...data, id: `h_${nextId}` });
historyRows.push({ ...(data as unknown as HistoryRow), id: `h_${nextId}` });
return { id: `h_${nextId}` };
}
if (table !== 'sys_metadata') return { id: 'side_effect_skip' };
Expand DownExpand Up@@ -203,7 +260,7 @@ function makeStubEngine() {
isPackageDisabled: () => false,
},
};
return { engine, rows };
return { engine, rows, historyRows };
}

// ── REST harness: real protocol, real routes, one boot ────────────────────
Expand DownExpand Up@@ -236,7 +293,7 @@ function mockRes() {
* can read the same store on the same boot (the cross-tenant control).
*/
function boot() {
const { engine, rows } = makeStubEngine();
const { engine, rows, historyRows } = makeStubEngine();
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
protocol.getDiscovery = async () => ({
version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' },
Expand DownExpand Up@@ -269,17 +326,24 @@ function boot() {

return {
rows,
historyRows,
as(tenantId: string | undefined) {
session = tenantId === undefined
? { userId: 'u1', systemPermissions: ['manage_metadata'] }
: { userId: 'u1', systemPermissions: ['manage_metadata'], tenantId };
},
put: (type: string, name: string) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name) }),
put: (type: string, name: string, label = MARKER) =>
drive('PUT', `${META}/:type/:name`, { params: { type, name }, body: bodyFor(type, name, label) }),
get: (type: string, name: string) =>
drive('GET', `${META}/:type/:name`, { params: { type, name } }),
list: (type: string) =>
drive('GET', `${META}/:type`, { params: { type } }),
history: (type: string, name: string) =>
drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }),
/** The fixture proof every history assertion below is gated on. */
historyRowsFor: (type: string, name: string, org: string | null) =>
historyRows.filter((h) => h.type === type && h.name === name
&& (h.organization_id ?? null) === org),
};
}

Expand DownExpand Up@@ -407,3 +471,87 @@ describe('#9454 every REST /meta read door serves what the write door persisted'
});
});
});

// ── #13764 — the instrument's own discriminating power, pinned ────────────
//
// This file's stub used to DISCARD `opts.where` on both `sys_metadata_history`
// seams: `findOne` answered `null` unconditionally and `find` handed back every
// history row unfiltered. `SysMetadataRepository.history()` and `diffMetaItem`
// filter `organization_id` by STRICT EQUALITY and post-filter nothing, so over
// that stub the org predicate was a NO-OP — an org-scoping assertion for
// `/history` was green whether or not the door forwarded the organization.
//
// ⭐ THE ASSERTION THAT HOLDS THE STUB RIGHT is `does not serve org A history to
// org B`. The positive case below cannot do that job: un-partition the stub
// again and it stays GREEN, because an unfiltered read still contains the rows
// it looks for. Only the cross-tenant case reddens, because only it asks for an
// answer the unfiltered stub cannot give. It is here for the stub, not for the
// door.
//
// ⛔ These are NOT this file's pins for the two doors' behaviour — those live in
// `rest-server-meta-history-diff-org-scope.test.ts` (#13406), whose partitioned
// stub is the positive control this repair was calibrated against, and which
// owns `?limit=`, `/diff`, and the non-overridable env-wide control. What is
// asserted here is the narrow fact that THIS harness can now tell a forwarded
// org from a dropped one.
describe('#13764 the history seams of this harness honour the org partition', () => {
let b: ReturnType<typeof boot>;
beforeEach(() => { b = boot(); });

it('serves the org-scoped change log of an item the active org authored', async () => {
// The measurement that names the repair: with the door's org dropped
// this reads the ENV partition and answers zero events. Over the old
// unfiltered stub it answered two in BOTH states.
const first = await b.put(CACHED_ARM, 'authored_at_runtime');
expect(first.status, 'the fixture never wrote').toBe(200);
await b.put(CACHED_ARM, 'authored_at_runtime', MARKER_2);

// Fixture proof first — "the read is org-scoped" is worthless if the
// fixture never created an org-scoped row.
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', ORG_A).length,
'nothing landed in the org partition; the read below would then pass '
+ 'or fail for a reason unrelated to org scoping',
).toBe(2);
expect(
b.historyRowsFor(CACHED_ARM, 'authored_at_runtime', null).length,
'the write also landed env-wide — the partition is not real',
).toBe(0);

const read = await b.history(CACHED_ARM, 'authored_at_runtime');
expect(read.thrown, `GET /history threw: ${read.thrown?.message}`).toBeUndefined();
expect(read.status).toBe(200);
expect(
read.body?.events?.length,
'the door answered an empty change log for an item whose org partition holds two events',
).toBe(2);
});

it('does not serve org A history to org B on the same boot', async () => {
// ⭐ The one that reddens if the stub is ever un-partitioned again.
await b.put(UNCACHED_ARM, 'tenant_bound');
await b.put(UNCACHED_ARM, 'tenant_bound', MARKER_2);
expect(b.historyRowsFor(UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(2);

b.as(ORG_B);
const read = await b.history(UNCACHED_ARM, 'tenant_bound');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'org B was served org A\'s change log',
).toEqual([]);
});

it('does not serve an org-scoped change log to a caller that named no org', async () => {
await b.put(CACHED_ARM, 'org_a_only');
expect(b.historyRowsFor(CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1);

b.as(undefined);
const read = await b.history(CACHED_ARM, 'org_a_only');
expect(read.status).toBe(200);
expect(
read.body?.events ?? [],
'an org-less caller was served an org-scoped change log',
).toEqual([]);
});
});
3 changes: 0 additions & 3 deletions scripts/objectql-double-limit.baseline.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -714,9 +714,6 @@
"packages/rest/src/rest-exec-ctx-principal-kind.test.ts": {
"unjudged": 1
},
"packages/rest/src/rest-server-meta-read-org-scope.test.ts": {
"blind": 1
},
"packages/rest/src/rest-server-timing.test.ts": {
"unjudged": 1
},
Expand Down
Loading