Skip to content

Commit 5dbcee8

Browse files
fix(metadata-protocol): the object door never lets a container's expansion displace a stored row of the same name (#21510) (#21557)
Fixes #21510 Clause-②: no ## What this changes Triage's ruling on the card (comment 5964342087): **the stored row wins on both doors.** ADR-0005 keys an overlay by its own name, so a row stored under exactly a name is the sanctioned override for it. An expansion is derived from its container, so it fills only names that have no row of their own. The by-name read has applied that rule since PR #21508. This PR makes the object door's list read apply it too, through the same predicate. - **The defect.** `readFlattenedMetaItems` (the list read behind `GET /api/v1/meta/view?object=OBJECT`) upserted every name a stored view container expands into its answer by bare name (`byName.set(vi.name, vi)`). That ran after the package-aware merge had seated the stored rows, so it replaced a stored row of the same name. The by-name read (`getMetaItem`) answered that row. The two doors disagreed, against #21334's ruling ("the object door and the by-name read answer the same row"). - **One predicate, read by both doors.** The by-name read's test was inline in `resolveRowlessExpandedView`: a `records.some(...)` that compared each row's `name` with `request.name`, over the rows `readActiveOverlayRows` selected for the caller. It is factored out unchanged as `namesWithOwnStoredRow(records)`, which returns the names that have a stored row of their own in that selection. `resolveRowlessExpandedView` now asks it in place of the inline test, with the same answer for every input: a non-string row name matched no request name before and is left out of the set now. The list read asks it over its own `records`, the same selection, and skips an expansion whose name is in the set. There is no second test. - **What still holds.** A name the container expands that has no row of its own is still listed, and on both doors it still replaces a packaged view of the same name (#21442's tenant-overlay case). The predicate reads the caller's own rows, so a row stored for one organization wins for that organization only, and every other caller still gets the expansion on both doors. The by-name read, its layers, history and diff answer as before. Nothing is persisted or registered, and no response shape gains or loses a key. - **Region.** The edits are the list read's expansion pass, the new private method, and the one-line move in `resolveRowlessExpandedView` (plus its docblock). `hydrateExpandedViewItems`, the save door and the data door's existence gate are not touched. ## Measured, in-process at the protocol (the #21334 showcase harness) The dev's setup, as the card describes it: a stored overlay of the showcase's own `showcase_task` container, with a `list` (label `FromContainer`) and a `listViews.in_progress` member (label `FromContainer In Progress`), plus a stored ViewItem row named `showcase_task.default` (label `ByNameRow`), written through the save door. | kernel | scope of both rows | write order | before: object door / by-name, `showcase_task.default` | after: both doors | control `showcase_task.in_progress`, before and after | |---|---|---|---|---|---| | `env_local` | environment-wide | container, then row | `FromContainer` / `ByNameRow` | `ByNameRow` | the expansion, both doors | | `env_local` | environment-wide | row, then container | `FromContainer` / `ByNameRow` | `ByNameRow` | the expansion, both doors | | `env_local` | organization-scoped | either order | `FromContainer` / `ByNameRow` | `ByNameRow` | the expansion, both doors | | unscoped | environment-wide | either order | `FromContainer` / `ByNameRow` | `ByNameRow` | the expansion, both doors | | unscoped | organization-scoped | either order | `FromContainer` / `ByNameRow` | `ByNameRow` | the expansion, both doors | "Before" is `origin/main` at `24dc7c1134`, read with a throwaway test that was deleted afterwards. "After" is this branch. **The public input (PM mechanism assumption 4).** In every cell above, the save door accepts `saveMetaItem` with type `view`, the name `showcase_task.default` and a body whose `name` is `showcase_task.default`, and it stores the row under that name. Since #21470 the save door judges the body `name` against the save name, and these are equal. The pins assert that the row was stored. ## Tests In `packages/metadata-protocol/src/view-container-runtime-expansion.test.ts`, nested in the #21334 block to reuse its faithful-registry harness, there are 10 new cases (the file goes from 119 to 129): - **8 cases: both kernels, environment-wide and organization-scoped, both write orders.** In each, the stored row answers `showcase_task.default` on both doors, and the by-name item equals the listed one (`_diagnostics` excluded). The row's name keeps its own history (every event's `ref.name` is the row's) and its own diff (`name` is the row's), never the container's. The control, `showcase_task.in_progress`, answers the expansion on both doors. Every name the object door lists answers the same item by name. - **2 cases, one per kernel: the predicate reads the caller's own rows.** The container is environment-wide and the row is stored for `org_acme`. `org_acme` gets the row on both doors. A caller with no organization and `org_globex` get the container's expansion on both doors. Results: - At the final head `6a41000f1e`, the full package suite (`vitest run`) gives **206 files passed / 3 skipped, 3173 tests passed / 19 skipped**. - `pnpm --filter @objectstack/metadata-protocol typecheck` is clean, and `tsc --listFiles` includes the edited test file. - Downstream, a narrowed and declared sample against the rebuilt `dist/` (it carries `namesWithOwnStoredRow`): the test files that read the view object door through the real protocol. `@objectstack/objectql` gives 5 files / 71 tests and `@objectstack/rest` gives 1 file / 24 tests, all green. The rest of those packages, and the dogfood suites, are CI's. ## Reverse verification Each mutation was made from committed state (`6a41000f1e`) through `scripts/ablation-replace.mjs`, inside a script with an EXIT/INT/TERM restore trap. After each leg, the restore was proven: blob `f1622d5bdde2` equals HEAD, and `git diff HEAD` is empty. The subject resolves through relative source imports (`./index.js`), so no `dist/` leg applies. The predicted direction was red, and every leg went red. - **A1, the list read's call off** (`… && false) continue;`): **10 failed / 119 passed**. All 10 new cases fail with `expected 'FromContainer' to be 'ByNameRow'`, the card's defect. - **A2, the predicate's body off** (no name is ever added): **10 failed / 119 passed**, the same 10. The list read fails first in each case. - **A3, only the by-name family's call off** (in `resolveRowlessExpandedView`): **8 failed / 121 passed**. Both doors still answer the row, but history now delegates to the container: `every event names the row: expected false to be true`. So the by-name family reads the same predicate and is pinned by it. ## Gates - `dispatch-gates --commands --repo objectstack-ai/objectstack` at the final head `d12a8f6256` derived **64** families, the same set as at `6a41000f1e`. The PM's lead had 56; the changeset adds 8: `check-adr-0087-registration` ×2, `check-empty-changeset` ×2, `release-rehearsal-clone --self-test`, `release-pending-publish --self-test`, `check:objectui-changeset` and `check:pm-changeset-deadline-census`. - After the final commit, **all 64 exited 0**. `pnpm check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET, 44 package entry points with no `dist/`) in the first run at `6a41000f1e`. By the run at `d12a8f6256`, those `dist/` directories were present in this worktree (created at 06:27Z, while the first run's `check:type-check-debt` re-measure was running), and it measured 106 require entry points across 66 packages, which load. - `--ran` reconciliation at `d12a8f6256`: 64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN. - `d12a8f6256` changes only the changeset, so the test, typecheck and ablation readings above (taken at `6a41000f1e`) read the same `protocol.ts` and test file bytes. - Lint, narrowed: `eslint --no-inline-config --format json` over the 2 changed TypeScript files gives 2 files linted, 0 errors and 0 warnings. The config does no type-aware linting (no `parserOptions.project`; see the note at `eslint.config.mjs` line 328), so this diff cannot move a verdict on an untouched file. A repo-wide `pnpm lint` is CI's. ## Acceptance notes - **A container saved under one of its own expanded names** (measured, reported to the seat as a finding, not fixed here). The probe: the save door accepts a container body `{ name: 'showcase_task.default', object: 'showcase_task', list: {...} }` saved under `showcase_task.default`, and its bare `list` expands to that same name. - **Before** (the list read's call off, which is the `origin/main` list): the object door listed the self-expansion, and the by-name read answered the raw container. The doors disagreed. - **After:** the container row is the name's own row, so its expansion does not fill that name. The canonical-shape filter (ADR-0017) never enumerates a container, so the object door lists nothing under `showcase_task.default`, while the by-name read still answers the raw container. That is how every container's own name already behaves. The doors still disagree, now in a different way. - Both kernels were measured. The ruling's predicate gives this result. Neither door can answer a ViewItem for that name while the stored row is a container. The candidate fix is a save-door refusal, which is outside this card's surface. The changeset states the case. - **Declaration bytes.** `dist/index.d.ts` gains one private member line (`private namesWithOwnStoredRow;`). No public member or exported type changes. - **Cost.** The list read builds one `Set` of row names per call, over rows it already holds. There is no extra read. --- _Generated by [Claude Code](https://claude.ai/code/session_01DDZNkDVwPQnevTFcYE47H3)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f83d066 commit 5dbcee8

3 files changed

Lines changed: 172 additions & 2 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): the object door lists a stored view row under its own name even where a stored view container expands that name, as the by-name read already answers
6+
7+
Clause-②: no
8+
9+
- **What changed.** `GET /api/v1/meta/view?object=…` (the object door) no longer lets a stored view container's expansion replace a stored row of the same name. A view item (a row carrying `viewKind`) saved under a name the container also expands, such as `<object>.default` beside a stored overlay of that object's container, is now what the object door lists under that name. Before, the object door listed the container's expansion there while the by-name read (`GET /api/v1/meta/view/NAME`) answered the stored row. Both doors now answer the row.
10+
- **The rule.** A row stored under exactly a name is the override for that name (ADR-0005 keys an overlay by its own name). An expansion fills only the names that have no row of their own. The list read and the by-name read decide this with one test, over the rows each selects for the same caller, so a row stored for one organization does not hide the expansion from any other caller.
11+
- **A container stored under one of its own expanded names.** That row is the name's own row as well, so its expansion no longer fills the name. The object door never lists a container, so it now lists nothing under that name. Before, it listed the container's expansion there. The by-name read answers the stored container, as before.
12+
- **What does not change.** Every name a container expands that has no stored row of its own is still listed, and on both doors it still replaces a packaged view of the same name. The by-name read answers as before. The save door is unchanged. No response shape gains or loses a key.

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

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8353,12 +8353,23 @@ export class ObjectStackProtocolImplementation implements
83538353
// another package's object every name a container expands
83548354
// derives from the container's own name, never one of that
83558355
// package's `<object>.<key>` names.
8356+
//
8357+
// [#21510] …and it never replaces a STORED ROW of that name.
8358+
// A row stored under exactly the name is the sanctioned
8359+
// override for it (ADR-0005 keys an overlay by its own name);
8360+
// an expansion fills only a name with no row of its own. The
8361+
// test is {@link namesWithOwnStoredRow} over this caller's
8362+
// `records`, the one the by-name read asks, so the two doors
8363+
// answer the same row for the name. An item the registry or a
8364+
// package supplies under the name is still replaced, as before.
83568365
if (isView) {
83578366
const byName = new Map<string, unknown>();
83588367
for (const it of items as any[]) {
83598368
if (it && typeof it === 'object' && typeof it.name === 'string') byName.set(it.name, it);
83608369
}
8370+
const ownRowNames = this.namesWithOwnStoredRow(records);
83618371
for (const { item: vi } of this.expandStoredViewContainers(request.type, overlays)) {
8372+
if (ownRowNames.has(vi.name as string)) continue;
83628373
byName.set(vi.name as string, vi);
83638374
}
83648375
items = Array.from(byName.values());
@@ -8819,6 +8830,30 @@ export class ObjectStackProtocolImplementation implements
88198830
return out;
88208831
}
88218832

8833+
/**
8834+
* [#21510] The names that have a stored row of their own among `records`,
8835+
* the active rows {@link readActiveOverlayRows} selected for one caller.
8836+
*
8837+
* ADR-0005 keys an overlay by its own name, so a row stored under exactly a
8838+
* name is the sanctioned override for that name. An expansion is derived
8839+
* from its container, so it fills only a name that is NOT in this set. This
8840+
* is the one predicate both doors ask: the list read
8841+
* ({@link readFlattenedMetaItems}) never lets an expansion displace a
8842+
* stored row of the same name, and the by-name read
8843+
* ({@link resolveRowlessExpandedView}) answers an expansion only for a name
8844+
* outside it. Both doors pass the rows they selected for the same caller,
8845+
* so a name that has a row in one organization only is row-less for every
8846+
* other caller. ⛔ Never a second test of "this name has its own row":
8847+
* two tests are two rules, and the doors would disagree again.
8848+
*/
8849+
private namesWithOwnStoredRow(records: readonly any[]): ReadonlySet<string> {
8850+
const names = new Set<string>();
8851+
for (const record of records) {
8852+
if (typeof record?.name === 'string') names.add(record.name);
8853+
}
8854+
return names;
8855+
}
8856+
88228857
/**
88238858
* [#21442] The item the list read serves under `request.name` when that
88248859
* name is ROW-LESS — no stored row of its own — and a stored view
@@ -8846,7 +8881,8 @@ export class ObjectStackProtocolImplementation implements
88468881
* ⛔ No kernel-specific branch — every kernel answers through this path.
88478882
*
88488883
* A stored row of this very name is the name's own row and is answered
8849-
* as such by the caller's own read, never an expansion. The `container`
8884+
* as such by the caller's own read, never an expansion — the same
8885+
* predicate the list read applies ({@link namesWithOwnStoredRow}). The `container`
88508886
* returned is the stored row the item derives from — its own name, body,
88518887
* package and organization — which the layered read reports as the
88528888
* name's provenance and the history and diff reads resolve to.
@@ -8868,7 +8904,7 @@ export class ObjectStackProtocolImplementation implements
88688904
// this name".
88698905
this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata');
88708906
}
8871-
if (records.some((record) => record?.name === request.name)) return undefined;
8907+
if (this.namesWithOwnStoredRow(records).has(request.name)) return undefined;
88728908
let found: RowlessExpandedView | undefined;
88738909
for (const expanded of this.expandStoredViewContainers(request.type, this.storedOverlayEntries(request, records))) {
88748910
if (expanded.item.name === request.name) found = expanded;

‎packages/metadata-protocol/src/view-container-runtime-expansion.test.ts‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,128 @@ describe('#21334 a container on another package\'s object never takes that packa
940940
}
941941
});
942942
});
943+
944+
/**
945+
* #21510 — a stored row named exactly like a name a stored container
946+
* expands answers that name on BOTH doors.
947+
*
948+
* Triage's ruling: the stored row wins on both doors. ADR-0005 overlays are
949+
* name-keyed, so a row stored under exactly that name is the sanctioned
950+
* override for it; an expansion is derived from its container, so it fills
951+
* only names that have no row of their own. That is the rule #21442 gave
952+
* the by-name read, and the object door's list read adopts it: ⛔ not a
953+
* second rule, both doors ask one predicate over the caller's own row
954+
* selection.
955+
*
956+
* Measured on `origin/main` before this change, with this harness: the
957+
* object door listed the container's expansion under the row's name
958+
* (`FromContainer`) while the by-name read answered the stored row
959+
* (`ByNameRow`), on both kernels, in both scopes and in either write
960+
* order. The name the same container expands with no row of its own still
961+
* answers the expansion on both doors — the control.
962+
*/
963+
describe('#21510 a stored row named exactly like an expansion answers that name on both doors', () => {
964+
const withoutDiagnostics = (item: any) => {
965+
if (!item || typeof item !== 'object') return item;
966+
const { _diagnostics: _drop, ...rest } = item;
967+
return rest;
968+
};
969+
/** The dev's setup: a stored overlay of the showcase's own `showcase_task` container… */
970+
const container = {
971+
name: TASK,
972+
list: { label: 'FromContainer', type: 'grid', data, columns: [{ field: 'title' }] },
973+
listViews: {
974+
in_progress: { label: 'FromContainer In Progress', type: 'grid', data, columns: [{ field: 'title' }] },
975+
},
976+
};
977+
/** …plus a stored row named exactly like the name its bare `list` expands to. */
978+
const row = {
979+
name: DEFAULT, object: TASK, viewKind: 'list', label: 'ByNameRow',
980+
config: { type: 'grid', data, columns: [{ field: 'title' }, { field: 'status' }] },
981+
};
982+
/** The control: a name the container expands that has no row of its own. */
983+
const ROWLESS = `${TASK}.in_progress`;
984+
const saveView = (protocol: Protocol, name: string, item: unknown, organizationId?: string) =>
985+
protocol.saveMetaItem({ type: 'view', name, item, ...scoped(organizationId) } as any);
986+
/** The two doors answer `name` with one item, and that item is the one `expectItem` names. */
987+
const expectBothDoors = async (
988+
protocol: Protocol, name: string, organizationId: string | undefined, expectItem: (v: any) => void,
989+
) => {
990+
const listed = named(await objectDoor(protocol, organizationId), name);
991+
expect(listed, `exactly one item answers ${name} on the object door`).toHaveLength(1);
992+
expectItem(listed[0]);
993+
const read = await byNameDoor(protocol, name, organizationId);
994+
expectItem(read);
995+
expect(withoutDiagnostics(read), `${name}: the by-name read answers the item the object door lists`)
996+
.toEqual(withoutDiagnostics(listed[0]));
997+
};
998+
const expectTheRow = (v: any) => {
999+
expect(v?.label).toBe('ByNameRow');
1000+
expect(v?.config).toEqual(row.config);
1001+
};
1002+
const expectTheExpansion = (v: any) => {
1003+
expect(v?.label).toBe('FromContainer In Progress');
1004+
expect(v?.config?.columns).toEqual([{ field: 'title' }]);
1005+
};
1006+
1007+
for (const [kernel, environmentId] of KERNELS) {
1008+
describe(`on ${kernel}`, () => {
1009+
for (const organizationId of [undefined, ORG]) {
1010+
const scope = organizationId ? 'organization-scoped' : 'environment-wide';
1011+
for (const order of ['the container first', 'the row first'] as const) {
1012+
it(`${scope}, ${order}: the stored row answers its name on both doors; the row-less expanded name answers the expansion`, async () => {
1013+
const { protocol, rows } = showcaseHarness(environmentId);
1014+
const writes = [
1015+
() => saveView(protocol, TASK, container, organizationId),
1016+
// The public input: the save door accepts a write by the expanded name.
1017+
() => saveView(protocol, DEFAULT, row, organizationId),
1018+
];
1019+
for (const write of order === 'the container first' ? writes : [...writes].reverse()) await write();
1020+
expect(
1021+
[...rows.values()].filter((r) => r.name === DEFAULT && r.organization_id === (organizationId ?? null)),
1022+
'the save door stored the row under the expanded name',
1023+
).toHaveLength(1);
1024+
1025+
await expectBothDoors(protocol, DEFAULT, organizationId, expectTheRow);
1026+
// The by-name family asks the same predicate: the
1027+
// row's name keeps its own change log and its own
1028+
// diff, never the container's.
1029+
const events = (await protocol.historyMetaItem({ type: 'view', name: DEFAULT, ...scoped(organizationId) })).events;
1030+
expect(events.length, 'the row has a change log of its own').toBeGreaterThan(0);
1031+
expect(events.every((e: any) => e.ref.name === DEFAULT), 'every event names the row').toBe(true);
1032+
expect((await (protocol as any).diffMetaItem({ type: 'view', name: DEFAULT, ...scoped(organizationId) })).name)
1033+
.toBe(DEFAULT);
1034+
// CONTROL — a row-less name the same container expands.
1035+
await expectBothDoors(protocol, ROWLESS, organizationId, expectTheExpansion);
1036+
// The contract the ruling keeps (#21334): every name
1037+
// the object door lists answers that same item by name.
1038+
for (const listed of await objectDoor(protocol, organizationId)) {
1039+
const read = await byNameDoor(protocol, listed.name, organizationId);
1040+
expect(withoutDiagnostics(read), `${listed.name} by name`).toEqual(withoutDiagnostics(listed));
1041+
}
1042+
});
1043+
}
1044+
}
1045+
1046+
it('the predicate is read over the caller\'s own rows: an organization\'s row wins for that organization only', async () => {
1047+
const { protocol } = showcaseHarness(environmentId);
1048+
await saveView(protocol, TASK, container);
1049+
await saveView(protocol, DEFAULT, row, ORG);
1050+
1051+
// The organization that holds the row: the row, on both doors.
1052+
await expectBothDoors(protocol, DEFAULT, ORG, expectTheRow);
1053+
// A caller for whom the name has no row of its own: the
1054+
// container's expansion, on both doors.
1055+
const expectTheDefaultExpansion = (v: any) => {
1056+
expect(v?.label).toBe('FromContainer');
1057+
expect(v?.config?.columns).toEqual([{ field: 'title' }]);
1058+
};
1059+
await expectBothDoors(protocol, DEFAULT, undefined, expectTheDefaultExpansion);
1060+
await expectBothDoors(protocol, DEFAULT, 'org_globex', expectTheDefaultExpansion);
1061+
});
1062+
});
1063+
}
1064+
});
9431065
});
9441066

9451067
/**

0 commit comments

Comments
 (0)