Skip to content

Commit 735fc9d

Browse files
committed
fix(spec): stop the i18n index pointing at five schemas a translation bundle cannot reach
The `objectstack-i18n` core list is correct; the damage was downstream, in the transitive closure. Eight pointers shipped in that index and seven arrived through a single edge: `shared/strict-object.ts` imports `shared/suggestions.zod.ts` for its "did you mean?" text, which imports `data/field.zod.ts`, which drags in filter, expression, field-value, identifiers and value-domain. That is a schema-building helper's implementation, not the authorable shape of a translation bundle -- which addresses everything by name string. The largest of them, the Unified Query DSL, is a different skill's whole subject, shipped into every i18n session with an instruction to read it. The feasibility question the finding asked -- a general reachability rule, or a per-package list -- is answered first, and against the general rule. Cutting traversal through non-shipping helpers is the precise version of that rule, and it removes five of the five pointers named; it also removes `shared/identifiers.zod.ts`, which must STAY (bundle keys are exactly those `snake_case` identifiers, and the SKILL.md spends a table and a "Critical:" note on it, while nothing imports the file), and it keeps `kernel/metadata-protection.zod.ts`, which must go (a first-class direct import). A depth-4 pointer reached through a helper belongs on the keep side and a depth-1 pointer reached through a schema edge on the drop side: no predicate over the import graph orders those that way, because the fact that separates them is not in the graph. So: a per-package allowlist beside the map, opt-in, with a guard that refuses a package name the map does not have, a file the closure never reaches, a file that is already core, and a repeat. An allowlist rather than a denylist because `shared/value-domain.zod.ts` joined this index recently and unnoticed, when a new import edge appeared several files away -- a denylist misses every new arrival by construction. `data/field.zod.ts` is kept deliberately: `FieldTranslationSchema.options` is keyed by select-option value, and `SelectOptionSchema` is the declaration those keys must match. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H2oQebDDxYKfWZusyd8GXk
1 parent 5607ed5 commit 735fc9d

4 files changed

Lines changed: 207 additions & 7 deletions

File tree

packages/spec/scripts/build-skill-references.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ import { findModuleDocBlock } from './lib/file-description';
3030
import { createSink, type Owns } from './lib/generated-output';
3131
import {
3232
SHARED_CORE_SCHEMAS,
33+
TRANSITIVE_ALLOWLIST,
3334
checkCoreEntryShape,
3435
checkSingleOwner,
36+
checkTransitiveAllowlist,
3537
} from './lib/skill-map-guards';
3638

3739
// ── Paths ────────────────────────────────────────────────────────────────────
@@ -396,6 +398,14 @@ function main() {
396398
];
397399
let totalSkills = 0;
398400

401+
// The allowlist guard needs each package's closure, so the closures are
402+
// resolved once, up front, and reused by the emit loop below.
403+
const closures: Record<string, string[]> = {};
404+
for (const [skillName, coreFiles] of Object.entries(SKILL_MAP)) {
405+
closures[skillName] = resolveAll(coreFiles).files;
406+
}
407+
problems.push(...checkTransitiveAllowlist(SKILL_MAP, TRANSITIVE_ALLOWLIST, closures));
408+
399409
for (const [skillName, coreFiles] of Object.entries(SKILL_MAP)) {
400410
const skillDir = path.resolve(SKILLS_DIR, skillName);
401411
if (!fs.existsSync(skillDir)) {
@@ -404,8 +414,19 @@ function main() {
404414
}
405415

406416
console.log(`📦 ${skillName}`);
407-
const { files: allFiles, missing } = resolveAll(coreFiles);
417+
const { files: resolved, missing } = resolveAll(coreFiles);
408418
for (const m of missing) problems.push(`${skillName}${m} (no such file under packages/spec/src)`);
419+
420+
// A package that declares a transitive allowlist publishes its core files
421+
// plus exactly those pointers; one that declares none publishes the whole
422+
// closure, as before. See TRANSITIVE_ALLOWLIST for why the constraint is a
423+
// hand-authored list and not a rule over the import graph.
424+
const allowed = TRANSITIVE_ALLOWLIST[skillName];
425+
const coreSet = new Set(coreFiles);
426+
const allFiles =
427+
allowed === undefined
428+
? resolved
429+
: resolved.filter((f) => coreSet.has(f) || allowed.includes(f));
409430
console.log(` ${coreFiles.length} core + ${allFiles.length - coreFiles.length} deps`);
410431

411432
const refsDir = path.resolve(skillDir, 'references');

packages/spec/scripts/lib/skill-map-guards.ts

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,118 @@ export function checkSingleOwner(map: SkillCoreMap, shared: Record<string, strin
134134
return problems;
135135
}
136136

137+
/**
138+
* Which transitive pointers a package publishes, when the closure is wrong.
139+
*
140+
* ## The feasibility question this answers, decided before anything was built
141+
*
142+
* The closure walks every local `import ... from` edge out of a package's core
143+
* files. Two routes were on the table for constraining it: (1) a REACHABILITY
144+
* RULE -- follow only imports the package's authorable face can reach, which
145+
* would generalise to every package; (2) a per-package list beside the map,
146+
* which fixes one package at a time. Route 1 is the better shape IF it can be
147+
* made precise. It cannot, and the measurement is specific rather than
148+
* hand-wavy:
149+
*
150+
* - `objectstack-i18n` publishes eight transitive pointers, and SEVEN of them
151+
* arrive through one edge -- `shared/strict-object.ts` imports
152+
* `shared/suggestions.zod.ts` for its "did you mean?" text, which imports
153+
* `data/field.zod.ts`, which drags in filter, expression, field-value,
154+
* identifiers and value-domain. That is a schema-building HELPER's
155+
* implementation, not the authorable shape of a translation bundle. Cutting
156+
* traversal through non-shipping helpers is the obvious precise rule, and it
157+
* removes five of the five pointers the finding names.
158+
* - It also removes `shared/identifiers.zod.ts`, which MUST STAY: a bundle's
159+
* object and field keys are the `snake_case` identifiers that file defines,
160+
* and the SKILL.md spends a table and a "Critical:" note on exactly that.
161+
* No import edge expresses it -- `system/translation.zod.ts` does not import
162+
* the file at all, because a bundle addresses everything by NAME STRING.
163+
* - And it KEEPS `kernel/metadata-protection.zod.ts`, which must go: that one
164+
* is a first-class direct `.zod.ts` import of `translation.zod.ts`.
165+
*
166+
* So the required outcome puts a depth-4 pointer reached through a helper on
167+
* the KEEP side and a depth-1 pointer reached through a schema edge on the DROP
168+
* side. No predicate over the import graph orders those two that way, because
169+
* the fact that separates them -- what a translation bundle can address -- is
170+
* not in the graph. Route 1 is therefore not merely unbuilt here; it is
171+
* unbuildable from this input, and route 2 is what ships.
172+
*
173+
* The list is an ALLOWLIST, not a denylist, and that is the half that keeps it
174+
* from rotting the way the closure did: `shared/value-domain.zod.ts` joined the
175+
* i18n index recently, unnoticed, when a new import edge appeared several files
176+
* away. An allowlist cannot silently gain a row; a denylist silently misses
177+
* every new arrival.
178+
*
179+
* A package with NO entry here publishes its full closure, unchanged. Declaring
180+
* a list is a claim about that package's authorable face, and only a package
181+
* whose face someone has actually read should carry one.
182+
*/
183+
export const TRANSITIVE_ALLOWLIST: Record<string, readonly string[]> = {
184+
// Everything else the closure reaches here is `strictObject()`'s error-message
185+
// machinery and what that drags behind it -- the Unified Query DSL among them,
186+
// a different skill's whole subject, shipped into every i18n session with an
187+
// instruction to read it.
188+
'objectstack-i18n': [
189+
// Bundle keys ARE these identifiers: `objects.<name>.fields.<name>` must
190+
// match the `snake_case` names the object and field schemas declare, which
191+
// the SKILL.md states as a "Critical:" rule with its own table.
192+
'shared/identifiers.zod.ts',
193+
// `FieldTranslationSchema.options` is keyed by select-option VALUE, and the
194+
// SKILL.md teaches that keying by example. `SelectOptionSchema` -- the
195+
// declaration those keys must match -- lives here.
196+
'data/field.zod.ts',
197+
],
198+
};
199+
200+
/**
201+
* A declared transitive allowlist must name a real package and reachable files.
202+
*
203+
* The list is hand-authored, and a hand-authored list that can quietly say
204+
* nothing is the same defect one layer up: a typo'd package name would leave
205+
* the over-eager closure fully published while the map LOOKS constrained, and a
206+
* file the closure never reaches would read as a pointer that is being kept
207+
* when it was never there to keep.
208+
*/
209+
export function checkTransitiveAllowlist(
210+
map: SkillCoreMap,
211+
allowlist: Record<string, readonly string[]>,
212+
closures: Record<string, readonly string[]>,
213+
): string[] {
214+
const problems: string[] = [];
215+
for (const [skillName, allowed] of Object.entries(allowlist)) {
216+
const coreFiles = map[skillName];
217+
if (coreFiles === undefined) {
218+
problems.push(
219+
`TRANSITIVE_ALLOWLIST names ${skillName}, which is not a SKILL_MAP package — ` +
220+
`the list would constrain nothing. Fix the name or delete the entry.`,
221+
);
222+
continue;
223+
}
224+
const core = new Set(coreFiles);
225+
const closure = new Set(closures[skillName] ?? []);
226+
const seen = new Set<string>();
227+
for (const rel of allowed) {
228+
if (seen.has(rel)) {
229+
problems.push(`${skillName}${rel} is listed twice in TRANSITIVE_ALLOWLIST.`);
230+
continue;
231+
}
232+
seen.add(rel);
233+
if (core.has(rel)) {
234+
problems.push(
235+
`${skillName}${rel} is already a core entry; listing it as a transitive ` +
236+
`pointer says it is both, and the index would name it once regardless.`,
237+
);
238+
} else if (!closure.has(rel)) {
239+
problems.push(
240+
`${skillName}${rel} is in TRANSITIVE_ALLOWLIST but nothing in the package's ` +
241+
`core closure imports it — this row keeps a pointer that does not exist.`,
242+
);
243+
}
244+
}
245+
}
246+
return problems;
247+
}
248+
137249
/**
138250
* Every core entry must be a path this generator can actually publish.
139251
*

packages/spec/scripts/skill-map-guards.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@ import { describe, expect, it } from 'vitest';
3434

3535
import {
3636
SHARED_CORE_SCHEMAS,
37+
TRANSITIVE_ALLOWLIST,
3738
checkCoreEntryShape,
3839
checkSingleOwner,
40+
checkTransitiveAllowlist,
3941
type SkillCoreMap,
4042
} from './lib/skill-map-guards';
4143

@@ -115,6 +117,70 @@ describe('checkSingleOwner — one schema file, one owning package', () => {
115117
});
116118
});
117119

120+
describe('checkTransitiveAllowlist — a constraint that constrains nothing is refused', () => {
121+
const map: SkillCoreMap = { 'objectstack-i18n': ['system/translation.zod.ts'] };
122+
const closures = {
123+
'objectstack-i18n': ['system/translation.zod.ts', 'shared/identifiers.zod.ts'],
124+
};
125+
126+
it('accepts a list naming a file the closure really reaches', () => {
127+
expect(
128+
checkTransitiveAllowlist(map, { 'objectstack-i18n': ['shared/identifiers.zod.ts'] }, closures),
129+
).toEqual([]);
130+
});
131+
132+
it('accepts an empty list — publishing no transitive pointer is a real answer', () => {
133+
expect(checkTransitiveAllowlist(map, { 'objectstack-i18n': [] }, closures)).toEqual([]);
134+
});
135+
136+
it('refuses a package name that is not in the map', () => {
137+
// The failure this exists for: a typo leaves the over-eager closure fully
138+
// published while the map LOOKS constrained.
139+
const problems = checkTransitiveAllowlist(map, { 'objectstack-i18nn': [] }, closures);
140+
expect(problems).toHaveLength(1);
141+
expect(problems[0]).toContain('not a SKILL_MAP package');
142+
});
143+
144+
it('refuses a file the closure never reaches', () => {
145+
const problems = checkTransitiveAllowlist(
146+
map,
147+
{ 'objectstack-i18n': ['data/query.zod.ts'] },
148+
closures,
149+
);
150+
expect(problems).toHaveLength(1);
151+
expect(problems[0]).toContain('does not exist');
152+
});
153+
154+
it('refuses a file that is already a core entry', () => {
155+
const problems = checkTransitiveAllowlist(
156+
map,
157+
{ 'objectstack-i18n': ['system/translation.zod.ts'] },
158+
closures,
159+
);
160+
expect(problems).toHaveLength(1);
161+
expect(problems[0]).toContain('already a core entry');
162+
});
163+
164+
it('refuses a file listed twice', () => {
165+
const problems = checkTransitiveAllowlist(
166+
map,
167+
{ 'objectstack-i18n': ['shared/identifiers.zod.ts', 'shared/identifiers.zod.ts'] },
168+
closures,
169+
);
170+
expect(problems).toHaveLength(1);
171+
expect(problems[0]).toContain('listed twice');
172+
});
173+
174+
it('every shipped list names a package the map has', () => {
175+
// The one fact about the real list this file can assert without re-running
176+
// the generator; reachability of each row is `check:skill-refs`'s job,
177+
// because only it has the closure.
178+
for (const skillName of Object.keys(TRANSITIVE_ALLOWLIST)) {
179+
expect(skillName).toMatch(/^objectstack-/);
180+
}
181+
});
182+
});
183+
118184
describe('the generator wires the guards in', () => {
119185
const source = (): string => fs.readFileSync(GENERATOR, 'utf-8');
120186

@@ -131,4 +197,11 @@ describe('the generator wires the guards in', () => {
131197
it('calls checkSingleOwner on SKILL_MAP and the declared ledger', () => {
132198
expect(source()).toContain('checkSingleOwner(SKILL_MAP, SHARED_CORE_SCHEMAS)');
133199
});
200+
201+
it('calls checkTransitiveAllowlist, and filters the emitted set by the list', () => {
202+
// Both halves matter: the guard alone would validate a list the emit path
203+
// never reads, which is the shape of a constraint that constrains nothing.
204+
expect(source()).toContain('checkTransitiveAllowlist(SKILL_MAP, TRANSITIVE_ALLOWLIST, closures)');
205+
expect(source()).toContain('allowed.includes(f)');
206+
});
134207
});

skills/objectstack-i18n/references/_index.md

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,8 @@ from `node_modules` — there is no local copy in the skill bundle.
1414

1515
## Transitive dependencies
1616

17-
- `node_modules/@objectstack/spec/src/data/field-value.zod.ts` — Field runtime VALUE-shape contract (ADR-0104 D1).
1817
- `node_modules/@objectstack/spec/src/data/field.zod.ts` — Exports: FieldType, SelectOptionSchema, LocationCoordinatesSchema, CurrencyConfigSchema, CurrencyValueSchema
19-
- `node_modules/@objectstack/spec/src/data/filter.zod.ts` — Unified Query DSL Specification
20-
- `node_modules/@objectstack/spec/src/kernel/metadata-protection.zod.ts` — Metadata Protection Model — Phase 1 (ADR-0010)
21-
- `node_modules/@objectstack/spec/src/shared/expression.zod.ts` — Expression Protocol
2218
- `node_modules/@objectstack/spec/src/shared/identifiers.zod.ts` — Exports: SystemIdentifierSchema, SnakeCaseIdentifierSchema, MetadataItemNameSchema
23-
- `node_modules/@objectstack/spec/src/shared/suggestions.zod.ts` — "Did you mean?" Suggestion Utilities
24-
- `node_modules/@objectstack/spec/src/shared/value-domain.zod.ts` — Standard value domains: one closed vocabulary and one membership predicate for settings and fields.
2519

2620
## How to read these
2721

0 commit comments

Comments
 (0)