Skip to content

Commit 445a25f

Browse files
os-justinclaude
andauthored
fix(spec): reconcile SKILL_MAP with the SKILL.md bodies that own each surface, and guard it (#15268)
* fix(spec): narrow the objectstack-ai reference index to what its SKILL.md teaches The published `objectstack-ai` index advertised five schemas the body never teaches -- `conversation`, `mcp`, `embedding`, `knowledge-document`, `usage`, three of them with zero consumers outside `packages/spec` -- and omitted `ai/solution-blueprint.zod.ts`, the schema behind the `solution_design` built-in skill the body's own table names. An index entry is a POINTER, so pointing at a schema the body cannot help with sends an agent reading into `node_modules` for guidance that does not exist. The schemas keep existing and stay importable; only the false promise leaves. `embedding` is still published, as a transitive dependency: `knowledge-source.zod.ts` composes `EmbeddingModelSchema`, so that pointer IS reachable from the authorable face -- the test the other four fail. Adds the first map-level guard, in `scripts/lib/` beside `export-list.ts` and `file-description.ts` because the generator self-executes on import: a core entry that is not a `*.zod.ts` path emits no pointer row at all today, with no `missing` row and a green `--check`, because the closure filter drops it silently before the index template ever sees it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H2oQebDDxYKfWZusyd8GXk * fix(spec): stop routing automation authors at the ADR-0020-retired state machine The published `objectstack-automation` index pointed at `automation/state-machine.zod.ts`. ADR-0020 retired that shape AS A RECORD-LIFECYCLE DECLARATION -- the top-level `workflow` metadata type and `object.stateMachines` are both gone -- and a record's legal transitions are now a `state_machine` validation rule in `data/validation.zod.ts`, which was already the last entry of the same list. So the index did not merely over-promise: it pointed at something the platform deliberately removed from this package's surface. The file's one surviving door is `ai/agent.zod.ts`'s `lifecycle`, an objectstack-ai door, and that index reaches it transitively. The other direction, from the same audit: the two per-node-type `config` schemas the body teaches were in no package's list at all. Screen `fields` and the ADR-0031 loop/parallel/try_catch containers reach `automation/builtin-node-config.zod.ts`; `NotifyConfigSchema` and the `http` `timeoutMs` reach `automation/io-node-config.zod.ts`. An agent told to always read the source for exact field shapes had no route to either. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H2oQebDDxYKfWZusyd8GXk * fix(spec): objectstack-query is the sole owner of date macros, and the map now says so `data/date-macros.zod.ts` sat in two core lists, so the published `objectstack-formula` index carried a Date Macro Tokens row while its own SKILL.md routes that surface away: view list filters are not a CEL surface, and the token list lives in objectstack-query's `rules/filters.md`. The catalog's whole contract is "this package owns this surface", and a reader following the formula index landed on a schema that skill will not teach. The duplicate is the whole defect, so the entry leaves the formula list and the generator gains the guard that refuses the next one at generation time. The guard is NOT the flat "at most one owner" rule it was asked for, and the reason is measured rather than argued: `date-macros` was one of FOUR duplicates in the map, and the other three are deliberate -- `data/validation.zod.ts` (data + automation) and `data/datasource.zod.ts` / `data/seed.zod.ts` (data + platform, the surface absorbed from the retired quickstart skill, already carrying that reason as a comment). The flat rule would refuse the map on its first run. So duplicates must now be DECLARED with a reason in `SHARED_CORE_SCHEMAS`, and two further guards keep that ledger from becoming a silent allowlist: a row with no reason is refused, and so is a row whose sharing has gone. The deviation from the letter of the instruction is recorded in the ledger's own header and in the PR body. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H2oQebDDxYKfWZusyd8GXk * 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 * fix(spec): keep internal tracker ids out of the published skill catalog `check:doc-authoring` went red the moment `automation/io-node-config.zod.ts` joined a package list: the generator publishes the first sentence of a module's doc block, and that file's opens with a bare tracker citation. The gate has no per-passage exemption, by design, and its argument is the audience -- `skills` ships to customer projects and is loaded WHOLE into customer context windows, where a tracker id resolves to nothing for the people paying for the tokens. The gate prescribes stripping the id AT THE SOURCE and regenerating. That remedy is not taken here, and the reasons are specific rather than convenient: the source is a `.zod.ts` this card is forbidden to edit; it is a file the package PUBLISHES, so editing it would change what `@objectstack/spec` ships and reopen the changeset decision this PR settled from precedent; and the same sentence is also projected to `content/docs/references/automation`, which would drag a second generated tree into a diff whose surface is the skill catalog. Three surfaces to remove one token. The decisive measurement is that the gate does NOT flag that identical sentence on the docs page: the rule is about the skill catalog specifically. So the strip is applied at the boundary INTO that catalog, where the rule lives. Every future pointer row is covered, rather than this one being corrected once. The criterion is the gate's own, restated with a pin over the shapes that must and must not match -- an ordinal, a hex colour, an over-long number and a doubled hash all survive untouched. This is a deviation from the gate's stated remedy and is flagged as such in the PR body for a reviewer to overrule. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H2oQebDDxYKfWZusyd8GXk --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0e24b0c commit 445a25f

7 files changed

Lines changed: 639 additions & 24 deletions

File tree

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

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ import path from 'path';
2828
import { exportListDescription } from './lib/export-list';
2929
import { findModuleDocBlock } from './lib/file-description';
3030
import { createSink, type Owns } from './lib/generated-output';
31+
import {
32+
SHARED_CORE_SCHEMAS,
33+
TRANSITIVE_ALLOWLIST,
34+
checkCoreEntryShape,
35+
checkSingleOwner,
36+
checkTransitiveAllowlist,
37+
stripInternalIssueIds,
38+
} from './lib/skill-map-guards';
3139

3240
// ── Paths ────────────────────────────────────────────────────────────────────
3341

@@ -100,12 +108,18 @@ const SKILL_MAP: Record<string, string[]> = {
100108
'ai/tool.zod.ts',
101109
'ai/skill.zod.ts',
102110
'ai/model-registry.zod.ts',
103-
'ai/conversation.zod.ts',
104-
'ai/mcp.zod.ts',
105-
'ai/embedding.zod.ts',
106111
'ai/knowledge-source.zod.ts',
107-
'ai/knowledge-document.zod.ts',
108-
'ai/usage.zod.ts',
112+
// The schema behind the `solution_design` built-in skill the body's
113+
// built-in-skills table names. Taught, never advertised until now.
114+
'ai/solution-blueprint.zod.ts',
115+
// `conversation`, `mcp`, `embedding`, `knowledge-document` and `usage`
116+
// left this list: the body teaches none of them, and three had zero
117+
// consumers outside packages/spec. An index entry is a POINTER, and
118+
// pointing at a schema the body cannot help with is the defect — the
119+
// schemas keep existing and stay importable. `embedding` is still
120+
// published here as a transitive dep, because `knowledge-source.zod.ts`
121+
// composes `EmbeddingModelSchema`: that pointer is reachable from the
122+
// authorable face, which is exactly the test the other four fail.
109123
],
110124
'objectstack-api': [
111125
'api/endpoint.zod.ts',
@@ -121,10 +135,23 @@ const SKILL_MAP: Record<string, string[]> = {
121135
'automation/flow.zod.ts',
122136
'automation/time-relative-trigger.zod.ts',
123137
'automation/approval.zod.ts',
124-
'automation/state-machine.zod.ts',
138+
// `automation/state-machine.zod.ts` left this list: ADR-0020 retired that
139+
// shape AS A RECORD-LIFECYCLE DECLARATION (the top-level `workflow` type
140+
// and `object.stateMachines` are both gone), and a record's legal
141+
// transitions are now a `state_machine` VALIDATION RULE — `data/validation`
142+
// below, already the correct destination. The file's one surviving door is
143+
// `ai/agent.zod.ts`'s `lifecycle`, an objectstack-ai door, and that index
144+
// reaches it transitively. Advertising it here pointed automation authors
145+
// at a shape the platform deliberately removed from their surface.
125146
'automation/execution.zod.ts',
126147
'automation/webhook.zod.ts',
127148
'automation/node-executor.zod.ts',
149+
// The per-node-type `config` shapes the body teaches and the index did not
150+
// name: screen `fields` and the ADR-0031 loop/parallel/try_catch containers
151+
// reach `builtin-node-config`, `NotifyConfigSchema` and the `http`
152+
// `timeoutMs` reach `io-node-config`.
153+
'automation/builtin-node-config.zod.ts',
154+
'automation/io-node-config.zod.ts',
128155
'data/validation.zod.ts',
129156
],
130157
'objectstack-ui': [
@@ -162,7 +189,11 @@ const SKILL_MAP: Record<string, string[]> = {
162189
],
163190
'objectstack-formula': [
164191
'shared/expression.zod.ts',
165-
'data/date-macros.zod.ts',
192+
// `data/date-macros.zod.ts` left this list: it is objectstack-query's, and
193+
// both bodies say so — view list filters are not a CEL surface, and the
194+
// token list lives in objectstack-query's `rules/filters.md`. One schema
195+
// file, one owning package; see SHARED_CORE_SCHEMAS for the three the map
196+
// deliberately shares and why.
166197
],
167198
};
168199

@@ -271,11 +302,11 @@ function extractDescription(filePath: string): string {
271302
const firstLine = lines[0];
272303
if (firstLine && firstLine.length > 5) {
273304
const clean = firstLine.replace(/^#+\s*/, '');
274-
const sentence = clean.split(/\.\s/)[0];
305+
const sentence = stripInternalIssueIds(clean.split(/\.\s/)[0]);
275306
return sentence.length > 120 ? sentence.slice(0, 117) + '...' : sentence;
276307
}
277308
}
278-
return exportListDescription(content) ?? '';
309+
return stripInternalIssueIds(exportListDescription(content) ?? '');
279310
}
280311

281312
// ── Index generator ──────────────────────────────────────────────────────────
@@ -359,9 +390,23 @@ function ownsReferenceEntry(refsDir: string): Owns {
359390

360391
function main() {
361392
console.log('🔗 Building skill schema reference indexes...\n');
362-
const problems: string[] = [];
393+
// Map-level guards run before any file is read: they ask questions of the
394+
// authored config that the artifact-vs-generator comparison structurally
395+
// cannot (see lib/skill-map-guards.ts).
396+
const problems: string[] = [
397+
...checkCoreEntryShape(SKILL_MAP),
398+
...checkSingleOwner(SKILL_MAP, SHARED_CORE_SCHEMAS),
399+
];
363400
let totalSkills = 0;
364401

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

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

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

0 commit comments

Comments
 (0)