Skip to content

Commit 1030a80

Browse files
fix(metadata-protocol, cli, cloud-connection): a declared action override moved to another object is told which object the base ships the action on (#30697)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9D6Ehjp6oVrWQjMzb1AkDfRk)_ Fixes #30623 Clause-②: no Metadata-protocol: none ## 维护者速读 1. 改了什么:`.custom` 动作覆盖把 `objectName` 写成了与基包动作不同的对象时,`declared_package_ships_no_such_item` 拒绝信息现在点名两个对象,并告诉作者保留基包动作的 `objectName`;三个入口(保存/发布门、`os validate`、托管安装)用同一句话。 2. 为什么改:#30623。原话说基包「没有叫 X 的动作」、让作者「改名」,可名字是对的、错的是对象,作者会去改错地方。 3. 风险与回滚:只改文案。规则、错误码(`NOT_OVERRIDABLE`)、状态码(403)、判定结果全不变;基包根本没有这个名字时,文案与今天一字不差。回滚即 revert。 4. 先看:`protocol-packaged-base-refusal.ts` 的 `shipsNoSuchItemMessage`(唯一的句子构造器)和 `OverrideWriteClosure.baseItemsOf`(三个入口各自递进基包的同类条目)。 5. 两分钟验证:`pnpm --filter @objectstack/cloud-connection exec vitest run src/marketplace-install-local-override-gate.test.ts`,预期 14/14 绿,其中 `[#30623]` 两条(换对象、CONTROL)。 6. 需要你做的:无,席位落地。 ## What changes - **One builder** — `shipsNoSuchItemMessage` in `packages/metadata-protocol/src/protocol-packaged-base-refusal.ts` writes the `declared_package_ships_no_such_item` sentence. `judgeOverrideWrite` already serves all three doors, so the builder lives there, in the lowest package all three already import; no new cross-package export. - When a declared `action` override names a base that ships an action of that name on **another object**, the sentence becomes: `'action/NAME' declares an override of 'BASE' on object 'OVERRIDE_OBJECT', but 'BASE' ships its action 'NAME' on object 'BASE_OBJECT'. An override replaces the base's action on that action's own object: keep the base action's objectName: 'BASE_OBJECT' in the override, or remove overrides to keep it as 'PAIR''s own item (ADR-0146 D2).` - Otherwise (another type, a name the base does not ship at all, or no base items in hand) the sentence is today's, byte for byte. - `OverrideWriteClosure` gains an optional `baseItemsOf(base)`: the base's items of this type at any object. Each door hands it the items it already holds: - save/publish door (`overrideWriteVerdict`): the artifact lookup's items under that base's `_packageId`; - `os validate` (`packages/cli/src/utils/override-declarations.ts`): the base package's walked items, when the artifact carries it; - managed install (`packages/cloud-connection/src/marketplace-install-local-plugin.ts`): the base's listing it already reads (`heldBy`). - The packageless overlay migration (`protocol-packageless-overlay-migration.ts`, `cli migrate packageless-overlays`) only names the code in a comment and asks `overrideWriteVerdict`, so it gets the new sentence with no change. - The rule, code and status are unchanged. No `packages/spec/**`. Changed lines: 174 (+165 / -9, 7 files). ## Tests - Save door: `packages/objectql/src/protocol-flow-custom-override.test.ts`: `[#30623]` pins the full sentence through `saveMetaItem` (403 `NOT_OVERRIDABLE`) for `showcase_task_close` moved to `showcase_other`, plus a CONTROL for an unshipped name (today's sentence, whole verdict). - `os validate`: `packages/cli/src/utils/override-declarations.test.ts`: `log_call` (base on `crm_account`) declared on `crm_lead`, first sentence plus the `objectName: 'crm_account'` remedy; CONTROL `no_such_call`. - Managed install: `packages/cloud-connection/src/marketplace-install-local-override-gate.test.ts`: `crm_close` (base on `crm_account`) declared on `crm_lead` → 403, nothing saved or ledgered, first sentence plus remedy; separate CONTROL `crm_reopen`. <details><summary>Ablation (one, on the builder)</summary> `node scripts/ablation-replace.mjs` on `protocol-packaged-base-refusal.ts`, anchor `if (others.length > 0) {` → `if (others.length > 0 && String('ABLATE30623') === 'on') {`; `pnpm --filter @objectstack/metadata-protocol build`; `ablation-dist-preflight.mjs @objectstack/metadata-protocol ABLATE30623` exit 0 (marker in dist). - objectql: `Tests 1 failed | 14 passed (15)`; red: `[#30623] the moved override's refusal names both objects…` - cli: `Tests 1 failed | 11 passed (12)`; red: `[#30623] an action override moved to another object…` - cloud-connection: `Tests 1 failed | 13 passed (14)`; red: the moved override; the CONTROL stays green. Restore: `blob == HEAD (153a824)`, `git diff HEAD` empty; rebuild; preflight `--absent` ✓; the three files 15/15, 12/12, 14/14. </details> ## Acceptance notes - `os-dev-report` on #30623 lists the local gate readings; CI runs the full farm. ## Verification <details><summary>Local runs</summary> Head after the gates: `01adff4f` (the fix commit); `9569f388` adds only a merge of `origin/main` touching none of the 7 files. - Build: `turbo run build` closure for cli / cloud-connection / objectql + metadata-protocol: 62/62 tasks, lock VERDICT command-exit 0. - Typecheck: `pnpm --filter metadata-protocol --filter cli --filter cloud-connection --filter objectql typecheck`: VERDICT command-exit 0. - `@objectstack/metadata-protocol` vitest: Test Files 317 passed | 4 skipped; Tests 22454 passed | 26 skipped; exit 0. - `@objectstack/cloud-connection` vitest: Test Files 70 passed; Tests 892 passed; exit 0. - `@objectstack/cli` `--project unit` (after `pnpm --filter @objectstack/cli build`): Test Files 389 passed; Tests 5432 passed; VERDICT command-exit 0. Integration layer declared to CI (no spawn entry touched). - `@objectstack/objectql`: `src/protocol-flow-custom-override.test.ts` 15/15; the full package run is CI's. - `dispatch-gates --commands`: 64 families; 63 exit 0; `check:dual-build-cjs-loads` exit 3 then 1 on an unrelated `@objectstack/organizations` dist that this targeted build did not emit (needs a whole-tree build): NOT MEASURED, CI runs it. `--ran`: `64 derived famil(ies) accounted for — 63 run, 1 NOT-MEASURED`. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016WV4cx3aafYJDsJo3siYFw Co-authored-by: Claude <noreply@anthropic.com>
1 parent de6fbd4 commit 1030a80

7 files changed

Lines changed: 165 additions & 9 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+
'@objectstack/cli': patch
4+
'@objectstack/cloud-connection': patch
5+
---
6+
7+
fix(metadata-protocol, cli, cloud-connection): a declared action override moved to another object is told which object the base ships the action on
8+
9+
Clause-②: no
10+
Metadata-protocol: none
11+
12+
The `declared_package_ships_no_such_item` refusal (`403 NOT_OVERRIDABLE`) for a `.custom` action override whose `objectName` differs from the base action's used to say the base "ships no action named 'X'" and to "correct the name", though the name was right. It now names both objects and tells the author to keep the base action's `objectName`, at all three doors (the save and publish door, `os validate`, the managed install), from one sentence builder in `judgeOverrideWrite`. A name the base does not ship at all reads as before. The rule, code and status are unchanged.

‎packages/cli/src/utils/override-declarations.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,29 @@ describe('#28410 — findOverrideDeclarationRefusals walks the artifact under th
145145
]);
146146
});
147147

148+
it('[#30623] an action override moved to another object is told both objects and to keep the base action\'s objectName; CONTROL: an unshipped name keeps the name sentence', () => {
149+
const overrides = { package: BASE, basedOn: '1.0.0' };
150+
const rows = findOverrideDeclarationRefusals({
151+
manifest: manifest('com.acme.bundle'),
152+
packages: [base, pkg(PAIR, [BASE], {
153+
actions: [
154+
{ name: 'log_call', objectName: 'crm_lead', label: 'Ours', overrides },
155+
{ name: 'no_such_call', objectName: 'crm_account', label: 'Ours', overrides },
156+
],
157+
})],
158+
});
159+
expect(rows.map((r) => [r.path, r.code, r.rule])).toEqual([
160+
['packages[1].manifest.actions[0]', 'NOT_OVERRIDABLE', 'declared_package_ships_no_such_item'],
161+
['packages[1].manifest.actions[1]', 'NOT_OVERRIDABLE', 'declared_package_ships_no_such_item'],
162+
]);
163+
expect(rows[0].message.split('. ')[0]).toBe(
164+
`'action/log_call' declares an override of '${BASE}' on object 'crm_lead', but '${BASE}' ships its action 'log_call' on object 'crm_account'`);
165+
expect(rows[0].message).toContain("keep the base action's `objectName: 'crm_account'` in the override");
166+
expect(rows[1].message.split('. ')[0]).toBe(
167+
`'action/no_such_call' declares an override of '${BASE}', which ships no action named 'no_such_call'`);
168+
expect(rows[1].message).toContain('or correct the name');
169+
});
170+
148171
it('a pair that does not depend on its base is told to declare the dependency', () => {
149172
const rows = findOverrideDeclarationRefusals({
150173
manifest: manifest('com.acme.bundle'),

‎packages/cli/src/utils/override-declarations.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,14 @@ export function findOverrideDeclarationRefusals(parsed: Record<string, unknown>)
126126
// carries the base: what a declared action override is judged against.
127127
const baseItemOf = (baseId: string): unknown =>
128128
byId.get(baseId)?.items.find((i) => `${i.type}\u0000${i.identity}` === key)?.item;
129+
// [#30623] The base's items of this type, when this artifact carries
130+
// the base: what the ships-no-such-item refusal names an action's
131+
// object from.
132+
const baseItemsOf = (baseId: string): unknown[] =>
133+
(byId.get(baseId)?.items ?? []).filter((i) => i.type === walked.type).map((i) => i.item);
129134
const verdict = judgeOverrideWrite(
130135
{ type: walked.type, name: walked.name, packageId: pkg.id, item: walked.item },
131-
{ dependencies, shippers, baseItemOf },
136+
{ dependencies, shippers, baseItemOf, baseItemsOf },
132137
);
133138
if (verdict.verdict !== 'refuse') continue;
134139
rows.push({ path: walked.path, code: 'NOT_OVERRIDABLE', rule: verdict.rule, packageId: pkg.id, message: verdict.message });

‎packages/cloud-connection/src/marketplace-install-local-override-gate.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,36 @@ describe('[#30542] the install door judges a declared action override against th
341341
expect([saved, ledger]).toEqual([[], []]);
342342
});
343343
});
344+
345+
/**
346+
* [#30623] An `action` is keyed `<objectName>:<name>`: a pair's declared
347+
* override of an action its base ships on another object is refused
348+
* `declared_package_ships_no_such_item`, and the refusal names both objects
349+
* and the base action's `objectName` to keep, in the judge's one sentence.
350+
*/
351+
describe('[#30623] the install door names the object a moved action override left', () => {
352+
const overrides = { package: BASE, basedOn: '1.0.0' };
353+
const holdings: Holdings = { [BASE]: { action: [{ name: 'crm_close', objectName: 'crm_account', label: 'Close' }] } };
354+
const pairOf = (action: Record<string, unknown>) =>
355+
({ id: PAIR, version: '1.0.0', namespace: 'crm_custom', dependencies: deps(BASE), actions: [action] });
356+
const messageOf = (res: any) => res.payload?.error?.details?.refusals?.[0]?.message as string;
357+
358+
it('an override of crm_account:crm_close declared on crm_lead names both objects and keeps objectName: crm_account', async () => {
359+
const { res, saved, ledger } = await install(pairOf({ name: 'crm_close', objectName: 'crm_lead', label: 'Ours', overrides }), [{ id: BASE }], holdings);
360+
expect(res.status, JSON.stringify(res.payload)).toBe(403);
361+
expect(res.payload?.error?.code).toBe('NOT_OVERRIDABLE');
362+
expect(refusedOf(res)).toEqual([['action', 'crm_close', 'declared_package_ships_no_such_item']]);
363+
expect(messageOf(res).split('. ')[0]).toBe(
364+
`'action/crm_close' declares an override of '${BASE}' on object 'crm_lead', but '${BASE}' ships its action 'crm_close' on object 'crm_account'`);
365+
expect(messageOf(res)).toContain("keep the base action's `objectName: 'crm_account'` in the override");
366+
expect([saved, ledger]).toEqual([[], []]);
367+
});
368+
369+
it('CONTROL: a name the base does not ship at all keeps the name sentence', async () => {
370+
const { res } = await install(pairOf({ name: 'crm_reopen', objectName: 'crm_account', label: 'Ours', overrides }), [{ id: BASE }], holdings);
371+
expect(res.status, JSON.stringify(res.payload)).toBe(403);
372+
expect(refusedOf(res)).toEqual([['action', 'crm_reopen', 'declared_package_ships_no_such_item']]);
373+
expect(messageOf(res).split('. ')[0]).toBe(`'action/crm_reopen' declares an override of '${BASE}', which ships no action named 'crm_reopen'`);
374+
expect(messageOf(res)).toContain('or correct the name');
375+
});
376+
});

‎packages/cloud-connection/src/marketplace-install-local-plugin.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3717,16 +3717,21 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
37173717
// [#30065] …and each shipper's own item, what a declared
37183718
// action override is judged against.
37193719
const shipped = new Map<string, unknown>();
3720+
// [#30623] …and each base's items of this type, what the
3721+
// ships-no-such-item refusal names an action's object from.
3722+
const listed = new Map<string, unknown[]>();
37203723
for (const base of bases) {
3721-
const held = (await heldBy(item.type, base)).get(item.identity);
3724+
const byIdentity = await heldBy(item.type, base);
3725+
listed.set(base, [...byIdentity.values()]);
3726+
const held = byIdentity.get(item.identity);
37223727
if (held !== undefined) {
37233728
shippers.push(base);
37243729
shipped.set(base, (delivered.has(base) ? undefined : artifactOf(item, base)) ?? held);
37253730
}
37263731
}
37273732
const verdict = judgeOverrideWrite(
37283733
{ type: item.type, name: item.name, packageId, item: item.item },
3729-
{ dependencies: closure, shippers, baseItemOf: (base) => shipped.get(base) },
3734+
{ dependencies: closure, shippers, baseItemOf: (base) => shipped.get(base), baseItemsOf: (base) => listed.get(base) ?? [] },
37303735
);
37313736
if (verdict.verdict === 'refuse') {
37323737
refusals.push({

‎packages/metadata-protocol/src/protocol-packaged-base-refusal.ts‎

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,17 @@ export interface OverrideWriteClosure {
816816
* holds it.
817817
*/
818818
readonly baseItemOf?: (basePackageId: string) => unknown;
819+
/**
820+
* [#30623] The items of this type a base ships, at any object — read by
821+
* the door that holds them (the protocol's artifact lookup, the artifact
822+
* `os validate` walks, the install door's listing). The gate reads it for
823+
* one sentence only: when a declared `action` override names a base that
824+
* ships no action of that name on the override's object, the
825+
* `declared_package_ships_no_such_item` refusal names the object(s) the
826+
* base does ship it on ({@link shipsNoSuchItemMessage}). Absent, or
827+
* answering none of that name, the refusal reads as before.
828+
*/
829+
readonly baseItemsOf?: (basePackageId: string) => readonly unknown[];
819830
}
820831

821832
/** [#28834] The item door an `object_extension` is saved through (#28574), as the refusals name it. */
@@ -944,6 +955,49 @@ function excludedTypeRemedy(type: string, base: string | undefined): string {
944955
return `Remove \`overrides\`: ${articleFor(type)} ${type} is defined only in package source, so it changes only with a new version of ${shipper}.`;
945956
}
946957

958+
/**
959+
* [#30623] The `declared_package_ships_no_such_item` sentence, the one every
960+
* door renders (they all ask {@link judgeOverrideWrite}). An `action` is keyed
961+
* `<objectName>:<name>`, so when `base` ships an action of that name on
962+
* ANOTHER object (`baseItems`, {@link OverrideWriteClosure.baseItemsOf}) the
963+
* name is right and the object is what is wrong: the sentence names both
964+
* objects and tells the author to keep the base action's `objectName`.
965+
* Otherwise — another type, no such name at all, or no items in hand — it
966+
* says the base ships no such item and to correct the name.
967+
*/
968+
function shipsNoSuchItemMessage(
969+
type: string,
970+
name: string,
971+
base: string,
972+
packageId: string | undefined,
973+
overrideItem: unknown,
974+
baseItems: readonly unknown[] | undefined,
975+
): string {
976+
const item = `'${type}/${name}'`;
977+
if (type === 'action') {
978+
const bare = (n: unknown): unknown => (typeof n === 'string' ? n.slice(n.lastIndexOf(':') + 1) : n);
979+
const object = overrideWriteActionObjectOf(overrideItem);
980+
const others = Array.from(new Set((baseItems ?? [])
981+
.filter((a) => isPlainRecord(a) && bare(a.name) === bare(name))
982+
.map(overrideWriteActionObjectOf)
983+
.filter((o) => o !== object))).sort();
984+
if (others.length > 0) {
985+
const objectPhrase = (o: string): string => (o === '' ? 'no object' : `object '${o}'`);
986+
const fix = others.length === 1 && others[0] !== ''
987+
? `keep the base action's \`objectName: '${others[0]}'\` in the override`
988+
: 'keep the `objectName` of the base action it replaces';
989+
return `${item} declares an override of '${base}' on ${objectPhrase(object)}, but '${base}' ships its action `
990+
+ `'${bare(name)}' on ${others.map(objectPhrase).join(' and ')}. An override replaces the base's action on that `
991+
+ `action's own object: ${fix}, or remove \`overrides\` to keep it as '${packageId}''s own item (ADR-0146 D2).`;
992+
}
993+
}
994+
// [#28746] A pair-identified type names the member it addressed.
995+
const member = overrideWriteMemberOf(type, overrideItem);
996+
const ofMember = member === undefined ? '' : ` with ${ITEM_KEY_DISCRIMINATORS[type]?.field} '${member}'`;
997+
return `${item} declares an override of '${base}', which ships no ${type} named '${name}'${ofMember}. `
998+
+ `Remove \`overrides\` to keep it as '${packageId}''s own item, or correct the name (ADR-0146 D2).`;
999+
}
1000+
9471001
/** The indefinite article for a type name, by its first letter: "an api", "a permission". */
9481002
function articleFor(type: string): 'a' | 'an' {
9491003
return /^[aeiou]/.test(type) ? 'an' : 'a';
@@ -1034,12 +1088,8 @@ export function judgeOverrideWrite(
10341088
+ `Declare a dependency on '${base}' in the \`dependencies\` of '${packageId}''s manifest, or remove \`overrides\` (ADR-0146 D2).`);
10351089
}
10361090
if (!closure.shippers.includes(base)) {
1037-
// [#28746] A pair-identified type names the member it addressed.
1038-
const member = overrideWriteMemberOf(type, subject.item);
1039-
const ofMember = member === undefined ? '' : ` with ${ITEM_KEY_DISCRIMINATORS[type]?.field} '${member}'`;
10401091
return refuse('declared_package_ships_no_such_item',
1041-
`${item} declares an override of '${base}', which ships no ${type} named '${name}'${ofMember}. `
1042-
+ `Remove \`overrides\` to keep it as '${packageId}''s own item, or correct the name (ADR-0146 D2).`);
1092+
shipsNoSuchItemMessage(type, name, base, packageId, subject.item, closure.baseItemsOf?.(base)));
10431093
}
10441094
// [#30065] A declared action override replaces the whole action, so
10451095
// it could drop the gates that decide who may run it — the base's
@@ -1208,7 +1258,12 @@ export function overrideWriteVerdict(
12081258
const baseItemOf = (basePackageId: string): unknown => this.shippedArtifactsOf(request.type, request.name, undefined, member)
12091259
.find((a) => overrideWriteMemberOf(request.type, a) === member && onItsObject(a)
12101260
&& (a as { _packageId?: unknown } | null | undefined)?._packageId === basePackageId);
1211-
return judgeOverrideWrite(request, { dependencies, shippers, baseItemOf });
1261+
// [#30623] The base's items of this name at every object: what the
1262+
// ships-no-such-item refusal names when the override moved the action.
1263+
const baseItemsOf = (basePackageId: string): unknown[] => this.shippedArtifactsOf(request.type, request.name, undefined, member)
1264+
.filter((a) => overrideWriteMemberOf(request.type, a) === member
1265+
&& (a as { _packageId?: unknown } | null | undefined)?._packageId === basePackageId);
1266+
return judgeOverrideWrite(request, { dependencies, shippers, baseItemOf, baseItemsOf });
12121267
}
12131268

12141269

‎packages/objectql/src/protocol-flow-custom-override.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,29 @@ describe('[#30065] a declared .custom action override is the packaged action, at
524524
.toMatchObject({ verdict: 'admit' });
525525
});
526526

527+
it('[#30623] the moved override\'s refusal names both objects and tells the author to keep the base action\'s objectName', async () => {
528+
const { protocol } = await boot();
529+
await install(protocol, PAIR);
530+
const moved = actionBody('MOVED', { ...declared, objectName: 'showcase_other' });
531+
const refusal = await protocol.saveMetaItem({
532+
type: 'action', name: ACTION, item: moved, mode: 'draft', packageId: PAIR,
533+
} as any).then(() => undefined, (e: any) => e);
534+
expect(refusal).toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 });
535+
expect(refusal.message).toBe(
536+
`'action/${ACTION}' declares an override of '${BASE}' on object 'showcase_other', but '${BASE}' ships its action `
537+
+ `'${ACTION}' on object 'showcase_task'. An override replaces the base's action on that action's own object: `
538+
+ `keep the base action's \`objectName: 'showcase_task'\` in the override, or remove \`overrides\` to keep it as `
539+
+ `'${PAIR}''s own item (ADR-0146 D2).`);
540+
// CONTROL: a name the base does not ship at all keeps the name sentence.
541+
const unknown = { ...actionBody('NONE', declared), name: 'showcase_task_nope' };
542+
expect((protocol as any).overrideWriteVerdict({ type: 'action', name: 'showcase_task_nope', packageId: PAIR, item: unknown }))
543+
.toEqual({
544+
verdict: 'refuse', rule: 'declared_package_ships_no_such_item',
545+
message: `'action/showcase_task_nope' declares an override of '${BASE}', which ships no action named 'showcase_task_nope'. `
546+
+ `Remove \`overrides\` to keep it as '${PAIR}''s own item, or correct the name (ADR-0146 D2).`,
547+
});
548+
});
549+
527550
it('the pair\'s undeclared save is refused, and the remedy names the declaration, never a clone', async () => {
528551
const { protocol } = await boot();
529552
await install(protocol, PAIR);

0 commit comments

Comments
 (0)