Skip to content

Commit f2f6bf0

Browse files
fix(metadata-protocol): migrate-stored re-saves a draft-only permission set granting on an object gone since (#29350)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9Cu6rc6Nw1Ty87DTpPRgFnW)_ Fixes #29279 Clause-②: yes (widening) — measured: base `a19759ac` reports the draft-only set `failed` (422 unknown-grant sentence); head reports it `rewritten`. Every author-facing door judges as on `main` (the two author pins below hold on both trees). Metadata-protocol: none — measured: no key, value or type an author can write changes; the only accept-set change is a server-stated `source: 'migrate-stored'` re-save, which no client can claim (the REST layer never forwards `source`). ## 维护者速读 - **改了什么**:`os migrate meta --stored --apply`(以及 `POST /api/v1/meta/_migrate-stored`)重存一个只有草稿、需要转换(如去掉已退役的 `managedBy`)的权限集时,不再因为它授权的对象已经不存在而报 `failed`;发布该草稿时仍然会被拒。发布时的提示文字改成说明发布时查找的范围。 - **为什么改**:#28990 的「授权对象不存在」拒绝用权限集的已发布行判断「已持有的授权」,只有草稿的权限集没有已发布行,于是迁移自己的重存被拒,旧键留在库里。 - **风险与回滚**:低。只放行服务端自己声明的 `migrate-stored` 重存(客户端无法伪造 `source`),和旁边种子 `externalId` 规则(#28952)的豁免同一写法;作者的保存、发布、回滚判定不变。回滚即撤回本 PR。 - **先看**:`packages/metadata-protocol/src/protocol.ts`(`saveMetaItem` 里 #28990 块加了 `writeSource !== 'migrate-stored'`;发布路径传 `'publish'`);`packages/metadata-protocol/src/protocol-save-door-refusals.ts`(提示文字分两种门);`packages/objectql/src/protocol-migrate-stored-draft-permission-grant.test.ts`(真内核用例)。 - **两分钟验证**:`pnpm --filter @objectstack/metadata-protocol exec vitest run src/protocol.unknown-grant-object-refusal.test.ts`,预期 18/18 通过(新增的 `[#29279]` 两条在内)。 - **需要你做的**:无,席位落地。 ## What changes - **Fix 1 (behaviour).** The `unknownGrantObjectSaveRefusal` call in `saveMetaItem` is skipped for a `migrate-stored` re-save, copying the #28952 precedent beside it (`seedExternalIdSaveRefusal`'s `writeSource !== 'migrate-stored'`). Chosen over "read the held keys from the draft row" because it keeps every other door judged exactly as on `main`: `writeSource` is server-stated (only `migrateStoredMetadata` sets it), the publish promotion and the restore doors do not pass through this branch, and the draft is still judged at its publish. Reading the draft row instead would also have widened an author's draft re-save of a draft-only set (its held keys would come from the draft), which the card rules out. - **Fix 2 (wording).** At `promoteDraftForPublish` the finding now reads "…which is not defined in objects: no package registers an object of that name, and no published object, or object draft of this batch, carries it, so the grant applies to nothing." The save door and the restore doors keep "…and none is saved here…". The path (`objects.NAME`) and the leading "Permission 'SET' grants on object 'NAME' which is not defined in objects" (the sentence family `defineStack` and Studio key on) are unchanged. Implemented as a `door: 'save' | 'publish'` parameter (default `'save'`) on the refusal; only the promotion passes `'publish'`. - **Fix 3 (changeset).** `.changeset/29279-migrate-stored-draft-permission-grant.md` (patch) says a draft stored before the rule is refused at its publish, as `26724-object-unknown-datasource-refused.md` does. The pending #28990 changeset is not edited. - Docblocks of `unknownGrantObjectSaveRefusal` (protocol.ts) updated: the "stored-row migration … is not judged" bullet now sits under the carve-out, which is what makes it true for a draft-only set. ## Tests - New real-kernel file `packages/objectql/src/protocol-migrate-stored-draft-permission-grant.test.ts` (`ObjectKernel` + `ObjectQLPlugin`, in-memory driver copied from `protocol-duplicate-package-objects-key-collision.test.ts`): an object draft and a set draft granting on it, the set's stored body given the retired `managedBy` key, the object draft removed. The measured case (→ `rewritten`, key stripped, grant kept, still a draft), its publish still refused, the control with the object draft present (→ `rewritten`), and an author's draft re-save still refused. - `protocol.unknown-grant-object-refusal.test.ts`: new `[#29279]` describe (the pass rewrites a planted draft-only set; its publish is refused; an author re-save is refused). - **Forced test edit:** the same file's three publish-path assertions (`publishMetaItem` ×2, `publishPackageDrafts`) now expect the publish wording; `unknownFinding`/`expectRefusal` take a `door` argument. The `publishPackageDrafts` case now asserts the whole first sentence instead of the `grants on object` fragment. ## Acceptance notes - The premise test needed the planted row's `checksum` cleared: a row whose stored stamp equals the converted body's hash is a no-op write (`storedBodyUnchanged`), so planting the key under the old stamp made the control read `rewritten` while writing nothing. A row really stored with the key carries its own stamp. Noted, not a defect. ## Verification All readings at head `f7ee536e` unless marked. <details><summary>Premise on base a19759a (real-kernel file, before the fix)</summary> ``` × the pass rewrites it: the legacy key is stripped and the grant kept, and the set stays a draft AssertionError: {..."state":"draft",...,"outcome":"failed","reason":"permission/qa_probe failed spec validation: objects.qa_widget: Permission 'qa_probe' grants on object 'qa_widget' which is not defined in objects: no package registers an object of that name and none is saved here, so the grant applies to nothing. ..."} Tests 1 failed | 3 passed (4) (control rewritten, author re-save refused, publish refused: green) ``` </details> <details><summary>Ablations (fix committed first; each leg restored with git checkout HEAD, blob hash equal to HEAD, git status clean)</summary> - A1 carve-out removed (`'migrate-stored'` → `'ablate-29279'`), metadata-protocol rebuilt, `ablation-dist-preflight` marker present in dist/index.js and dist/index.cjs: metadata-protocol file `1 failed | 17 passed` (× `migrate-stored` rewrites it…); objectql real-kernel file `1 failed | 3 passed` (× the pass rewrites it…). Restore leg rebuilt, preflight `--absent` exit 0, tree clean. - A2 carve-out widened to every draft save (`&& mode !== 'draft'`): `3 failed | 15 passed` (× a draft save into a package…, × a draft save naming no package…, × an author re-saving the same draft is still refused). - A3 the promotion stops asking (`if (draftForGate && false)`): `4 failed | 14 passed` (× per-item publish…, × publishMetaItem refuses it…, × publishPackageDrafts answers refused…, × migrate-stored rewrites it…, and its publish is still refused). - Head legs: `18 passed (18)`; `4 passed (4)`. </details> <details><summary>Consumer tests (head f7ee536, after merging origin/main 504ba51; merge-tree clean)</summary> - `pnpm --filter @objectstack/metadata-protocol test`: Test Files 301 passed, 4 skipped (305); Tests 22129 passed, 26 skipped. `typecheck`: exit 0. - `@objectstack/objectql` `src/protocol-duplicate-package-*` (4 files) plus the new real-kernel file: Test Files 5 passed (5), Tests 24 passed (24). `typecheck`: exit 0. - Dogfood (closure built under the lock): `test/permission-unknown-grant-object-refusal.dogfood.test.ts` and `test/wall-catalog-refusal.dogfood.test.ts`: Test Files 2 passed (2), Tests 24 passed (24). - Full dogfood battery: not owed (no `!` in the title, no `(narrowing)` arm). </details> <details><summary>Gates (head f7ee536)</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 62 families derived; all 62 run, exit 0. Two first exited 3 (prerequisite): `check-plugin-teardown-shape --self-test` (pinned commit fetched, then 48 cases pass) and `check:dual-build-cjs-loads` (all packages built, then 115 entry points load). `--ran` reconciliation: "62 run, 0 NOT-MEASURED (a DERIVED zero)". - Lint: neither package has a `lint` script; `pnpm exec eslint --no-inline-config` on the four touched source and test files: exit 0. - CI families: read from this PR's check-runs. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01ADHkmRrBmeYea4FHGY5wro)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3fb74b6 commit f2f6bf0

5 files changed

Lines changed: 299 additions & 19 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 stored-metadata healing pass re-saves a draft-only permission set that grants on an object which exists nowhere now, instead of reporting it `failed`
6+
7+
Clause-②: yes (widening) — `migrateStoredMetadata` (`os migrate meta --stored --apply`, `POST /api/v1/meta/_migrate-stored`) now rewrites a draft-only permission set whose stored body needs a conversion and whose grant names an object with no active row and no draft; it reported that row `failed` with the unknown-grant `422`. Every author-facing save, publish and restore door judges the grant as before.
8+
Metadata-protocol: none
9+
10+
- **Before.** The save door's unknown-grant refusal skips a grant the set already holds, read from the set's active row. A set that is only a draft has none, so the pass re-saving its own draft (for example to strip the retired `managedBy` key) was refused once the object draft it granted on had been discarded, and the legacy key stayed at rest.
11+
- **Now.** A `migrate-stored` re-save is not judged for unknown grants, as it already is not for a seed's keyed field: it rewrites a row the store already holds. A draft stored before the rule is refused at its publish, as any other publish of it is.
12+
- **Wording.** At a draft's publish the finding now says what the publish looked for: "no package registers an object of that name, and no published object, or object draft of this batch, carries it". The save door's sentence, the finding path and the leading "Permission 'SET' grants on object 'NAME' which is not defined in objects" are unchanged.

‎packages/metadata-protocol/src/protocol-save-door-refusals.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,7 @@ export async function unknownGrantObjectSaveRefusal(
541541
name: string,
542542
pending?: 'drafts' | ReadonlySet<string>,
543543
writeFace?: MetadataWriteFace,
544+
door: 'save' | 'publish' = 'save',
544545
): Promise<(Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }) | undefined> {
545546
if (singularType !== 'permission') return undefined;
546547
if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined;
@@ -587,10 +588,15 @@ export async function unknownGrantObjectSaveRefusal(
587588
: new Set<string>();
588589
unknown = unknown.filter((key) => !held.has(key));
589590
if (unknown.length === 0) return undefined;
591+
// [#29279] What was looked for, as this door looked: a save also counts an
592+
// object saved here; a draft's publish counts only what goes live with it.
593+
const looked = door === 'publish'
594+
? 'no package registers an object of that name, and no published object, or object draft of this batch, carries it'
595+
: 'no package registers an object of that name and none is saved here';
590596
const issues: MetadataIssueEntry[] = unknown.map((key) => ({
591597
path: `objects.${key}`,
592-
message: `Permission '${name}' grants on object '${key}' which is not defined in objects: no package registers `
593-
+ 'an object of that name and none is saved here, so the grant applies to nothing. Correct the name to the '
598+
message: `Permission '${name}' grants on object '${key}' which is not defined in objects: ${looked}, `
599+
+ 'so the grant applies to nothing. Correct the name to the '
594600
+ `object's full name (e.g. 'crm_lead', not 'lead'), create the object first, or remove the grant.`,
595601
code: 'custom',
596602
}));

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

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14993,31 +14993,36 @@ export class ObjectStackProtocolImplementation implements
1499314993
* on a package publish, an object draft of the same batch (`pending` as
1499414994
* a set), since the batch promotes as one unit in no type order;
1499514995
* - the set's current definition already grants on it: the grant is not
14996-
* this write's. A re-save, a stored-row migration (`os migrate meta
14997-
* --stored`) or a restore that keeps it is not judged, and neither is
14998-
* an object that has gone since the grant was made.
14996+
* this write's. A re-save or a restore that keeps it is not judged, and
14997+
* neither is an object that has gone since the grant was made.
1499914998
*
1500014999
* Asked in draft and in publish mode by {@link saveMetaItem} after the
15001-
* schema gate, again by {@link promoteDraftForPublish} on the draft body
15000+
* schema gate (except a `migrate-stored` re-save, the platform healing
15001+
* its own storage: a draft-only set holds no active definition, so its
15002+
* own rewrite would be judged as new grants), again by {@link promoteDraftForPublish} on the draft body
1500215003
* (the object can go between a draft's save and its publish), and by the
1500315004
* restore doors ({@link refuseRestoredBodyTheSaveDoorRefuses}). Answered
1500415005
* as the schema gate answers: `422 INVALID_METADATA`, one finding per
15005-
* unknown object at `objects.<name>`, naming it.
15006+
* unknown object at `objects.<name>`, naming it. [#29279] `door` names
15007+
* what the finding says was looked for: a save counts an object saved
15008+
* here, a draft's publish (`'publish'`) only a published object or an
15009+
* object draft of its batch.
1500615010
*/
1500715011
private unknownGrantObjectSaveRefusal(
1500815012
singularType: string,
1500915013
item: unknown,
1501015014
name: string,
1501115015
pending?: 'drafts' | ReadonlySet<string>,
1501215016
writeFace?: MetadataWriteFace,
15017+
door?: 'save' | 'publish',
1501315018
): Promise<(Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }) | undefined> {
1501415019
const readEngine = () => this.engine;
1501515020
const host: ThisParameterType<typeof unknownGrantObjectSaveRefusal> = {
1501615021
get engine() { return readEngine(); },
1501715022
listDrafts: (...a) => this.listDrafts(...a),
1501815023
readMetaItem: (...a) => this.readMetaItem(...a),
1501915024
};
15020-
return unknownGrantObjectSaveRefusal.call(host, singularType, item, name, pending, writeFace);
15025+
return unknownGrantObjectSaveRefusal.call(host, singularType, item, name, pending, writeFace, door);
1502115026
}
1502215027

1502315028
/**
@@ -16766,8 +16771,12 @@ export class ObjectStackProtocolImplementation implements
1676616771
// here is refused, in draft and in publish mode, after the schema and
1676716772
// before the authoring gate and every write: stored, the grant applies
1676816773
// to nothing. A draft may grant on an object that is itself still a
16769-
// draft. See {@link unknownGrantObjectSaveRefusal}.
16770-
{
16774+
// draft. [#29279] A `migrate-stored` re-save rewrites a row the store
16775+
// already holds and is not judged: a draft-only set has no active row
16776+
// to hold its grants, so judging it refused the pass's own rewrite of
16777+
// a draft whose object draft was gone; the draft is judged at its
16778+
// publish. See {@link unknownGrantObjectSaveRefusal}.
16779+
if (writeSource !== 'migrate-stored') {
1677116780
const grantRefusal = await this.unknownGrantObjectSaveRefusal(
1677216781
singularType, request.item, request.name, mode === 'draft' ? 'drafts' : undefined, request.writeFace,
1677316782
);
@@ -18678,7 +18687,7 @@ export class ObjectStackProtocolImplementation implements
1867818687
// {@link unknownGrantObjectSaveRefusal}.
1867918688
if (draftForGate) {
1868018689
const grantRefusal = await this.unknownGrantObjectSaveRefusal(
18681-
singularType, draftForGate.body, request.name, request.batchObjects,
18690+
singularType, draftForGate.body, request.name, request.batchObjects, undefined, 'publish',
1868218691
);
1868318692
if (grantRefusal) throw grantRefusal;
1868418693
}

‎packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,18 +187,25 @@ function plant(rows: Map<string, Row>, body: Record<string, unknown>, state: 'dr
187187
rows.set(keyOf(data), { ...data, id: `planted_${state}_${String(body.name)}` });
188188
}
189189

190-
/** The finding's first sentence, spelled out. */
191-
const unknownFinding = (setName: string, objectName: string) =>
190+
/**
191+
* The finding's first sentence, spelled out. [#29279] A draft's publish says
192+
* what it looked for there: a published object or an object draft of its
193+
* batch, not an object "saved here" (a draft of it may be saved here).
194+
*/
195+
const unknownFinding = (setName: string, objectName: string, door: 'save' | 'publish' = 'save') =>
192196
`Permission '${setName}' grants on object '${objectName}' which is not defined in objects: no package registers `
193-
+ 'an object of that name and none is saved here, so the grant applies to nothing.';
197+
+ (door === 'publish'
198+
? 'an object of that name, and no published object, or object draft of this batch, carries it'
199+
: 'an object of that name and none is saved here')
200+
+ ', so the grant applies to nothing.';
194201

195-
const expectRefusal = (err: any, setName: string, objectNames: string[]) => {
202+
const expectRefusal = (err: any, setName: string, objectNames: string[], door: 'save' | 'publish' = 'save') => {
196203
expect({ code: err?.code, status: err?.status }, String(err?.message)).toEqual({ code: 'INVALID_METADATA', status: 422 });
197204
expect(String(err.message).startsWith(`permission/${setName} failed spec validation: `), String(err.message)).toBe(true);
198205
expect(err.issues.map((i: any) => i.path)).toEqual(objectNames.map((o) => `objects.${o}`));
199206
for (const [index, objectName] of objectNames.entries()) {
200207
expect(err.issues[index].code).toBe('custom');
201-
expect(String(err.issues[index].message).startsWith(unknownFinding(setName, objectName)), err.issues[index].message).toBe(true);
208+
expect(String(err.issues[index].message).startsWith(unknownFinding(setName, objectName, door)), err.issues[index].message).toBe(true);
202209
}
203210
};
204211

@@ -237,7 +244,7 @@ describe('[#28990] the permission-set save door — a grant on an object that do
237244
// The set alone goes live; its object stays a draft.
238245
const err = await refusalOf(protocol.publishMetaItem({ type: 'permission', name: 'qa_probe' }));
239246

240-
expectRefusal(err, 'qa_probe', ['qa_widget']);
247+
expectRefusal(err, 'qa_probe', ['qa_widget'], 'publish');
241248
expect(stored(rows, 'permission', 'qa_probe')).toEqual(['draft|-|qa_widget']);
242249
});
243250
});
@@ -249,7 +256,7 @@ describe('[#28990] the publish doors — a set draft stored before the rule', ()
249256

250257
const err = await refusalOf(protocol.publishMetaItem({ type: 'permission', name: 'qa_probe' }));
251258

252-
expectRefusal(err, 'qa_probe', ['qa_no_such_object']);
259+
expectRefusal(err, 'qa_probe', ['qa_no_such_object'], 'publish');
253260
expect(stored(rows, 'permission', 'qa_probe')).toEqual(['draft|-|qa_no_such_object']);
254261
});
255262

@@ -263,13 +270,42 @@ describe('[#28990] the publish doors — a set draft stored before the rule', ()
263270
expect(res).toMatchObject({ outcome: 'refused', success: false, publishedCount: 0, published: [] });
264271
const causal = res.failed.find((f: any) => f.name === 'qa_probe');
265272
expect(causal, JSON.stringify(res.failed)).toMatchObject({ type: 'permission', code: 'INVALID_METADATA' });
266-
expect(JSON.stringify(causal)).toContain("grants on object 'qa_no_such_object'");
273+
expect(JSON.stringify(causal)).toContain(unknownFinding('qa_probe', 'qa_no_such_object', 'publish'));
267274
// (The double's `transaction` does not roll back, so the sibling's
268275
// promotion is not read here: `publishedCount: 0` is the batch's word.)
269276
expect(stored(rows, 'permission', 'qa_probe')).toEqual([`draft|${PKG}|qa_no_such_object`]);
270277
});
271278
});
272279

280+
describe('[#29279] the stored-metadata healing pass — a draft-only set stored before the rule', () => {
281+
it('`migrate-stored` rewrites it (a legacy key stripped), and its publish is still refused', async () => {
282+
const { protocol, rows } = makeSession();
283+
plant(rows, set('qa_probe', ['qa_no_such_object'], { managedBy: 'user' }), 'draft', null);
284+
285+
const report = await protocol.migrateStoredMetadata({ apply: true, types: ['permission'] });
286+
287+
const row = report.rows.find((r: any) => r.name === 'qa_probe');
288+
expect(row, JSON.stringify(row)).toMatchObject({ state: 'draft', outcome: 'rewritten' });
289+
const bodies = [...rows.values()].filter((r) => r.type === 'permission').map((r) => JSON.parse(r.metadata));
290+
expect(bodies).toHaveLength(1);
291+
expect(bodies[0]).not.toHaveProperty('managedBy');
292+
expect(stored(rows, 'permission', 'qa_probe')).toEqual(['draft|-|qa_no_such_object']);
293+
const err = await refusalOf(protocol.publishMetaItem({ type: 'permission', name: 'qa_probe' }));
294+
expectRefusal(err, 'qa_probe', ['qa_no_such_object'], 'publish');
295+
});
296+
297+
it('an author re-saving the same draft is still refused', async () => {
298+
const { protocol, rows } = makeSession();
299+
plant(rows, set('qa_probe', ['qa_no_such_object'], { managedBy: 'user' }), 'draft', null);
300+
301+
const err = await refusalOf(protocol.saveMetaItem({
302+
type: 'permission', name: 'qa_probe', item: set('qa_probe', ['qa_no_such_object']), mode: 'draft',
303+
}));
304+
305+
expectRefusal(err, 'qa_probe', ['qa_no_such_object']);
306+
});
307+
});
308+
273309
describe('[#28990] controls — a grant on an object that exists is not refused', () => {
274310
it.each([
275311
['an object the registry registers', ['qa_widget']],

0 commit comments

Comments
 (0)