Skip to content

Commit e4dbede

Browse files
dmealingclaude
andcommitted
fix(metadata): the promised @forge* migration did nothing
Three shipped places — the 1.0 migration guide, the changelog and the provider's own docblock — told adopters `meta upgrade --apply` strips the `@forge*` attributes. No entry existed in `RETIRED_VOCABULARY`, so on a project carrying nine such nodes the command reported "nothing to rewrite (1 file(s) checked)" and exited 0. A migration instruction that silently does nothing is worse than one that refuses: the adopter believes the migration finished. **The promise was also wrong, so it is not what shipped.** That vocabulary still LOADS for a project that opts the provider back in — the chartered ADR-0023 path the same guide recommends two bullets later. An automated strip would destroy valid metadata for exactly the adopters who made that choice deliberately. So the entries carry no `rewrite`, which is the map's existing way of saying the fix is judgment (`@status: abandoned` is the precedent): `meta upgrade` now names every occurrence with its line, the reason and both options, and exits 1. `type: "*"` is new on the scope axis, and a retired COMMON attribute is what needs it — one registered on every node type. Without it the entry needs a row per top-level type, and the row for whichever type nobody thought of is the one that goes silently unmatched. **The refusal message dropped its own content.** It read `Retired in <since>. See <guide>` — printing `why` only when a guide was ABSENT, which is almost never, and `replacedBy` not at all. For these entries that removed the only actionable sentence there is. It now prints the reason and the same suggestion list every other retirement door prints, through `retirementSuggestions` rather than a second rendering that can drift from it. "Retired in" also became "no longer accepted as of", which is true of an authored `<type>.base` too — that one was never authorable, so announcing its retirement told the reader they used to have a working feature. **Two more false statements, from the same review.** "Metadata carrying `@forgeConfidence` now fails to load" is true of the node half only: the TypeScript loader is lax on unknown attributes, so `meta verify` rejects them and `meta gen` silently ignores them. And two docs still described the old default composition a commit after it changed — `extending-with-providers.md` said `loadMemory` composes the provider, `ports/typescript.md` still listed it in the default set, and `forge-types.ts` contradicted itself inside one docblock ("NOT in any default composition" above "forge is included by default"). Gate proved by removing the entries: all four cases red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTcEKXTQMYt84fAjuw5A2M
1 parent cce45f8 commit e4dbede

8 files changed

Lines changed: 194 additions & 28 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,12 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
99

1010
### BREAKING — the default provider composition registered vocabulary no other port has
1111

12-
`forgeTypesProvider` leaves `defaultLoadMemoryProviders` and `meta types`. Metadata carrying
13-
`@forgeConfidence` — or a `decision` / `principle` / `convention` / `glossary` / `failure`
14-
node — now fails to load unless the project opts the provider in.
12+
`forgeTypesProvider` leaves `defaultLoadMemoryProviders` and `meta types`. A `decision` /
13+
`principle` / `convention` / `glossary` / `failure` **node** now fails to load unless the
14+
project opts the provider in. An `@forge*` **attribute** is rejected by `meta verify`
15+
(`ERR_UNKNOWN_ATTR`) and silently IGNORED by `meta gen` — the TypeScript loader is lax on
16+
unknown attributes and only `verify` is strict, a distinction this entry originally flattened
17+
into "fails to load", which is true of the node half only.
1518

1619
That vocabulary was registered in **TypeScript and nowhere else**: no C#, Python, Java or
1720
Kotlin registry declares a line of it, and `expected-registry.json` — the manifest all five
@@ -25,7 +28,10 @@ agrees with it.
2528

2629
The provider is still exported and unchanged: `loadMemory(root, { providers:
2730
[forgeTypesProvider] })` is the chartered ADR-0023 opt-in, and a project taking it knows it
28-
has a surface the other four ports do not. `meta upgrade --apply` strips the attributes.
31+
has a surface the other four ports do not. `meta upgrade` names every occurrence with its
32+
line, reason and guide and deliberately does NOT rewrite it — the vocabulary still loads for a
33+
project that opts the provider back in, so an automatic strip would destroy valid metadata for
34+
the adopters who chose that on purpose.
2935
Migration: [`0.x-to-1.0.md` §12](docs/features/migrations/0.x-to-1.0.md).
3036

3137
**The gate closes the door, not the instance.** `metadata`'s registry-conformance test pins

‎docs/features/extending-with-providers.md‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -223,18 +223,23 @@ Every port's loader defaults to `[...coreProviders, ...callerProviders]` when th
223223
caller supplies `providers`. Pass `{ replaceDefaults: true }` (TS) or the per-port
224224
equivalent to skip the defaults entirely — rare, but supported.
225225

226-
**`forgeTypesProvider` is TypeScript-only, and this page used to say otherwise.** It
227-
was listed here as part of the default composition for "the TS / C# / Python loaders";
228-
in fact only TypeScript has it — `@metaobjectsdev/sdk`'s `loadMemory` composes it, and
229-
no C#, Python, Java or Kotlin registry registers any `@forge*` attribute or `decision` /
230-
`principle` / `convention` / `glossary` / `failure` type at all. Nor does
231-
`expected-registry.json`, which is the manifest all five ports byte-match, so the
232-
cross-port gate never had jurisdiction over it.
233-
234-
The consequence is worth stating plainly rather than leaving to be discovered: a
235-
document carrying `@forgeConfidence` **loads on TypeScript and fails
236-
`ERR_UNKNOWN_ATTR` on the other four ports** — one document, two verdicts, decided by
237-
which toolchain reads it.
226+
**`forgeTypesProvider` is TypeScript-only, and is no longer in any default
227+
composition.** It was listed here as part of the default composition for "the TS / C# /
228+
Python loaders"; in fact only TypeScript ever had it, and no C#, Python, Java or Kotlin
229+
registry registers any `@forge*` attribute or `decision` / `principle` / `convention` /
230+
`glossary` / `failure` type at all. Nor does `expected-registry.json`, the manifest all
231+
five ports byte-match, so the cross-port gate never had jurisdiction over it — one
232+
document had two verdicts, decided by which toolchain read it, and 1.0 would have frozen
233+
that in.
234+
235+
At the 1.0 cut it left `defaultLoadMemoryProviders` and `meta types`. It is still
236+
exported and unchanged, as the chartered opt-in:
237+
238+
```ts
239+
const root = await loadMemory("./", { providers: [forgeTypesProvider] });
240+
```
241+
242+
A project taking it knows it has a surface the other four ports do not.
238243

239244
## Stable error codes
240245

‎docs/features/migrations/0.x-to-1.0.md‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,13 @@ longer registered by default**. Metadata carrying any of them fails the load wit
339339
One document had two verdicts depending on which toolchain read it. That is the defect the
340340
`0.25.0` line was spent on, and 1.0 would have frozen it into a permanent public surface.
341341

342-
- **Migration, if you do not want the vocabulary:** `meta upgrade --apply` strips the
343-
attributes. Delete any `decision` / `principle` / `convention` / `glossary` / `failure`
344-
node, or move what it recorded into `notes` on a node that survives.
342+
- **Migration, if you do not want the vocabulary:** `meta upgrade` names every `@forge*`
343+
attribute it finds — with its line, the reason and this guide — and deliberately does
344+
**not** rewrite them, even with `--apply`. There is no safe automatic edit: the
345+
vocabulary still loads for a project that opts the provider back in, so a strip would
346+
destroy valid metadata for exactly the adopters who chose that deliberately. Delete them
347+
yourself, along with any `decision` / `principle` / `convention` / `glossary` / `failure`
348+
node, or move what they recorded into `notes` on a node that survives.
345349

346350
- **Migration, if you DO want it:** opt the provider in — the chartered ADR-0023
347351
consumer-provider path. It is still exported and unchanged:

‎docs/ports/typescript.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,11 @@ import { loadMemory } from "@metaobjectsdev/sdk";
179179
const root = await loadMemory("./", { providers: [yourProvider] });
180180
```
181181

182-
Default composition is `[...coreProviders, forgeTypesProvider,
183-
...callerProviders]`. Pass `{ replaceDefaults: true }` to skip the core
184-
bundle entirely (rare — usually only useful in tests). See
182+
Default composition is `[...coreProviders, ...callerProviders]`. Pass
183+
`{ replaceDefaults: true }` to skip the core bundle entirely (rare — usually
184+
only useful in tests). `forgeTypesProvider` left the default set at the 1.0
185+
cut: it is TypeScript-only vocabulary no other port registers, so pass it
186+
explicitly if you want it. See
185187
[`../features/extending-with-providers.md`](../features/extending-with-providers.md)
186188
for the full contract and
187189
[`../recipes/extending-metaobjects-with-providers.md`](../recipes/extending-metaobjects-with-providers.md)

‎server/typescript/packages/cli/src/commands/upgrade.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
composeRegistry,
2727
coreProviders,
2828
SUBTYPE_BASE,
29+
retirementSuggestions,
2930
} from "@metaobjectsdev/metadata";
3031
import { log } from "../lib/log.js";
3132

@@ -147,10 +148,22 @@ export async function upgradeCommand(args: string[], cwd: string): Promise<numbe
147148
log.info(`\n${rel}`);
148149
for (const c of r.changes) log.info(` ${c.line}: @${c.from} → ${c.to}`);
149150
for (const f of r.refusals) {
151+
// The WHY and the replacement both print, and the guide is not an alternative to
152+
// them. This used to read `Retired in <since>. See <guide>` — dropping `why`
153+
// whenever a guide existed, which is almost always, and dropping `replacedBy`
154+
// unconditionally. For the `@forge*` entries that removed the only actionable
155+
// sentence there is ("opt the provider in explicitly"), and for an authored
156+
// `<type>.base` it announced a retirement of something that was never authorable —
157+
// telling the reader they used to have a working feature. Hence "no longer accepted
158+
// as of", which is true of both, with the carefully-written `why` beside it.
150159
log.warn(
151-
` ${f.line}: ${f.subject}${f.value !== undefined ? `: ${f.value}` : ""} — needs a decision. ` +
152-
`Retired in ${f.since}. ${f.migration !== undefined ? `See ${f.migration}` : f.why}`,
160+
` ${f.line}: ${f.subject}${f.value !== undefined ? `: ${f.value}` : ""} — needs a decision ` +
161+
`(no longer accepted as of ${f.since}).`,
153162
);
163+
log.warn(` ${f.why}`);
164+
// The same suggestion list every OTHER door prints for a retirement, rather than a
165+
// second rendering of the same note that can drift from it.
166+
for (const line of retirementSuggestions(f)) log.warn(` ${line}`);
154167
}
155168

156169
totalChanges += r.changes.length;
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// The 1.0 migration promised `meta upgrade --apply` would strip the `@forge*` attributes.
2+
// It did nothing: no entry existed in RETIRED_VOCABULARY, so the command reported
3+
// "nothing to rewrite (1 file(s) checked)", exit 0, over a project carrying nine such
4+
// nodes. A migration instruction that silently does nothing is worse than one that
5+
// refuses — the adopter believes the migration finished.
6+
//
7+
// The fix is NOT an automated strip. That vocabulary still loads for a project that opts
8+
// the provider back in (the chartered ADR-0023 path), so a rewrite would destroy valid
9+
// metadata for exactly the adopters who chose it deliberately. So the entries carry no
10+
// `rewrite`, which is the map's existing way of saying the fix is judgment: the command
11+
// names every occurrence, prints the reason and both options, and exits non-zero.
12+
13+
import { describe, test, expect, afterAll } from "bun:test";
14+
import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
15+
import { tmpdir } from "node:os";
16+
import { join } from "node:path";
17+
import { run } from "../src/index.js";
18+
19+
const dirs: string[] = [];
20+
afterAll(async () => { for (const d of dirs) await rm(d, { recursive: true, force: true }); });
21+
22+
const META = {
23+
"metadata.root": {
24+
package: "acme",
25+
children: [{
26+
"object.entity": {
27+
name: "Order", "@forgeConfidence": "high", "@forgeSource": "an interview",
28+
children: [
29+
{ "source.rdb": { "@table": "orders" } },
30+
{ "field.long": { name: "id", "@forgeRationale": "surrogate" } },
31+
{ "identity.primary": { "@fields": ["id"], "@generation": "increment" } },
32+
],
33+
},
34+
}],
35+
},
36+
};
37+
38+
async function upgrade(args: string[]): Promise<{ exit: number; err: string }> {
39+
const dir = await mkdtemp(join(tmpdir(), "forge-retire-"));
40+
dirs.push(dir);
41+
await mkdir(join(dir, "metaobjects"), { recursive: true });
42+
await writeFile(join(dir, "metaobjects", "meta.json"), JSON.stringify(META), "utf8");
43+
const lines: string[] = [];
44+
const oe = console.error, ol = console.log;
45+
console.error = (...a: unknown[]) => { lines.push(a.map(String).join(" ")); };
46+
console.log = (...a: unknown[]) => { lines.push(a.map(String).join(" ")); };
47+
try { return { exit: await run(["upgrade", "--cwd", dir, ...args]), err: lines.join("\n") }; }
48+
finally { console.error = oe; console.log = ol; }
49+
}
50+
51+
describe("meta upgrade and the deregistered @forge* vocabulary", () => {
52+
test("names every occurrence and exits non-zero — not 'nothing to rewrite'", async () => {
53+
const { exit, err } = await upgrade([]);
54+
expect(exit).toBe(1);
55+
for (const attr of ["@forgeConfidence", "@forgeSource", "@forgeRationale"]) {
56+
expect(err).toContain(attr);
57+
}
58+
expect(err).not.toContain("nothing to rewrite (1 file(s) checked)");
59+
});
60+
61+
test("--apply does not rewrite them — the strip would destroy valid metadata", async () => {
62+
const { exit, err } = await upgrade(["--apply"]);
63+
expect(exit).toBe(1);
64+
expect(err).toContain("needs a decision");
65+
});
66+
67+
test("the message carries the reason AND both options, not just a guide link", async () => {
68+
// It used to print `Retired in <since>. See <guide>`, dropping `why` whenever a guide
69+
// existed — which is almost always — and dropping `replacedBy` unconditionally. For
70+
// this entry that removed the only actionable sentence there is.
71+
const { err } = await upgrade([]);
72+
expect(err).toContain("expected-registry.json"); // the why
73+
expect(err).toContain("forgeTypesProvider"); // keep them
74+
expect(err).toContain("or delete them"); // or don't
75+
expect(err).toContain("0.x-to-1.0.md"); // the guide
76+
// And it does not announce the retirement of something that still works elsewhere.
77+
expect(err).toContain("no longer accepted as of");
78+
});
79+
80+
test("the retirement is scoped to EVERY type, because it was a common attr", async () => {
81+
// `@forgeConfidence` sits on an object here and `@forgeRationale` on a field. A
82+
// per-type entry list would cover whichever types someone thought of; the type
83+
// wildcard is what makes the row match the way the registration did.
84+
const { err } = await upgrade([]);
85+
expect(err).toContain("@forgeConfidence"); // on object.entity
86+
expect(err).toContain("@forgeRationale"); // on field.long
87+
});
88+
});

‎server/typescript/packages/metadata/src/retired-vocabulary.ts‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ export type VocabularyRewrite =
8989
readonly otherwise: "drop" | "refuse";
9090
};
9191

92-
/** One retirement. `subType: "*"` means every subtype of `type`. */
92+
/** One retirement. `subType: "*"` means every subtype of `type`; `type: "*"` means
93+
* every type, which is what a retired COMMON attribute needs. */
9394
export interface RetiredEntry extends RetirementNote {
9495
readonly type: string;
9596
/** `*` for every subtype of `type`, else the exact subtype. */
@@ -109,6 +110,44 @@ const REQUIREMENT_MIGRATION = "docs/features/migrations/verified-by-retirement.m
109110
const RETIRED_STATUS_MIGRATION = "docs/features/migrations/retired-status-restore.md";
110111
const EMIT_ATTR_MIGRATION = "docs/features/migrations/emit-attrs-to-generator-config.md";
111112

113+
const FORGE_MIGRATION = "docs/features/migrations/0.x-to-1.0.md";
114+
115+
/**
116+
* The `@forge*` common attributes, deregistered from the DEFAULT provider composition at
117+
* the 1.0 cut (`forgeTypesProvider` left `defaultLoadMemoryProviders`).
118+
*
119+
* They are here so `meta upgrade` NAMES them and points at the migration — and
120+
* deliberately WITHOUT a `rewrite`, which is the map's way of saying the fix is judgment.
121+
* That is not conservatism: this vocabulary still LOADS for a project that opts the
122+
* provider back in (`loadMemory(root, { providers: [forgeTypesProvider] })`, the chartered
123+
* ADR-0023 escape hatch). An automated strip would therefore destroy valid metadata for
124+
* exactly the adopters who made that choice deliberately, while the two real options —
125+
* opt the provider in, or delete the attributes — are a decision only they can make.
126+
*
127+
* Before this, three shipped places promised `meta upgrade --apply` would strip them and
128+
* the tool reported "nothing to rewrite, exit 0" on a project carrying nine such nodes.
129+
* A migration instruction that silently does nothing is worse than one that refuses.
130+
*/
131+
const FORGE_ATTRS = [
132+
"forgeAlternatives", "forgeAppliesTo", "forgeCapturedAt", "forgeCodeAnchors",
133+
"forgeConfidence", "forgeCounterExamples", "forgeDefinition", "forgeEnforcement",
134+
"forgeExamples", "forgeLastValidatedCommit", "forgeOccurrences",
135+
"forgePatternDescription", "forgePrimaryLocation", "forgeRationale", "forgeScope",
136+
"forgeSeeAlso", "forgeSource", "forgeStatement", "forgeSynonyms", "forgeTerm",
137+
"forgeWhatWasTried", "forgeWhyItFailed",
138+
] as const;
139+
140+
const FORGE_ENTRIES: readonly RetiredEntry[] = FORGE_ATTRS.map((attr) => ({
141+
type: "*", subType: "*", attr,
142+
since: "1.0.0",
143+
why: "it was registered by a TypeScript-only provider and appears nowhere in " +
144+
"expected-registry.json, so one document had two verdicts depending on which " +
145+
"port read it — and 1.0 would have frozen that",
146+
replacedBy: "`forgeTypesProvider`, opted in explicitly — " +
147+
"loadMemory(root, { providers: [forgeTypesProvider] }) — or delete them",
148+
migration: FORGE_MIGRATION,
149+
}));
150+
112151
export const RETIRED_VOCABULARY: readonly RetiredEntry[] = [
113152
// ── 0.24.0: `@violation` is renamed `@counterexample` ──
114153
//
@@ -326,6 +365,9 @@ export const RETIRED_VOCABULARY: readonly RetiredEntry[] = [
326365
migration: EMIT_ATTR_MIGRATION,
327366
rewrite: { kind: "dropAttr" },
328367
},
368+
369+
// Deregistered from the default composition at the 1.0 cut — see FORGE_ENTRIES.
370+
...FORGE_ENTRIES,
329371
];
330372

331373

@@ -339,7 +381,11 @@ export const RETIRED_VOCABULARY: readonly RetiredEntry[] = [
339381
export function scopeMatches(entry: RetiredEntry, typeKey: string): boolean {
340382
const dot = typeKey.indexOf(".");
341383
if (dot < 0) return false;
342-
if (entry.type !== typeKey.slice(0, dot)) return false;
384+
// `type: "*"` is for an attribute that was registered as a COMMON attr — valid on
385+
// EVERY node, so its retirement is scoped the way its registration was. Without it
386+
// such an entry needs one row per top-level type, and the row for whichever type
387+
// nobody thought of is the one that goes silently unmatched.
388+
if (entry.type !== "*" && entry.type !== typeKey.slice(0, dot)) return false;
343389
return entry.subType === "*" || entry.subType === typeKey.slice(dot + 1);
344390
}
345391

‎server/typescript/packages/sdk/src/forge-types.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,10 @@ function def(
171171
* `@forgeConfidence` loaded on TS and failed `ERR_UNKNOWN_ATTR` everywhere else. It is
172172
* opt-in now, the chartered ADR-0023 consumer-provider way.
173173
*
174-
* Use via `composeRegistry([...coreProviders, forgeTypesProvider])` or via
175-
* `loadMemory()`'s default bundle (forge is included by default).
174+
* Use via `composeRegistry([...coreProviders, forgeTypesProvider])` or
175+
* `loadMemory(root, { providers: [forgeTypesProvider] })`. It is NOT in any default
176+
* composition — this line used to say "forge is included by default", contradicting the
177+
* paragraph above it inside one docblock.
176178
*/
177179
export const forgeTypesProvider: MetaDataTypeProvider = {
178180
id: "metaobjects-forge",

0 commit comments

Comments
 (0)