Skip to content

Commit cdbd920

Browse files
os-zhuangclaude
andauthored
Keep machine constants off the skill-reference Exports: fallback (#12391)
* fix(spec): keep machine constants off the skill-reference `Exports:` fallback The `Exports: …` line a pointer row falls back to when its module has no doc block ranked by SOURCE ORDER and had no notion of authorable surface, so `slice(0, 5)` kept whichever five exports were declared first. Three of the eleven modules reaching that fallback declare machine constants near the top, so three published rows headlined them — `DEPRECATED_APPROVER_TYPES` and `NON_AUTHORABLE_APPROVER_TYPES` among them, on a surface loaded whole into a customer agent's context window to teach it what it may author. SCREAMING_SNAKE exports are now dropped and source order kept for the rest, with the cap of five applied AFTER filtering so the authorable names behind the constants are promoted rather than the row merely shortened. A module whose whole export surface is machine constants falls through to no description rather than a bare `Exports:`. Schema-first sorting was considered and NOT taken: on the row that motivated this it demotes `ApproverType`, the enum an author actually writes. The rule moves to `scripts/lib/export-list.ts` so it can be pinned without running the generator (`main()` runs on import), and `export-list.test.ts` enforces it as unit cases plus a corpus gate over the checked-in artifacts. Closes #12201 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMTpSRF5CjMmQBFfPtPCwJ * record #12201's skills/** root in the parity gate's rollback pin (13 -> 14, by name) The pin is judged over the live declaration table by design; the skills/** declaration this PR adds moves the rollback count, so it is recorded by name per the pin's own convention. Both legs green: self-test 40 assertions, main check 105/84 globs covered. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent fe3d74f commit cdbd920

11 files changed

Lines changed: 377 additions & 19 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): keep machine constants off the skill-reference `Exports:` fallback
6+
7+
When a `.zod.ts` has no module doc block, `build-skill-references.ts` falls back
8+
to listing its exports. That line is TRUE — an accurate list of what the module
9+
exports, which is why #12094 kept the fallback rather than refusing. What was
10+
wrong is the RANKING: the list was whichever five exports happened to be
11+
DECLARED FIRST, and the extraction had no notion of authorable surface, so any
12+
`export const` qualified — including constants whose own names say they are not
13+
for authoring.
14+
15+
Three of the eleven modules that reach this fallback declare their machine
16+
constants near the top, so three published rows headlined them:
17+
18+
- `automation/approval.zod.ts` named `DEPRECATED_APPROVER_TYPES`,
19+
`NON_AUTHORABLE_APPROVER_TYPES`, `ORG_MEMBERSHIP_LEVELS` and
20+
`APPROVER_EXPRESSION_ROOTS` — four of its five slots
21+
- `kernel/plugin.zod.ts` named `CORE_PLUGIN_TYPES`, `CONSUMER_INSTALLABLE_TYPES`
22+
- `system/translation.zod.ts` named `LEGACY_OBJECT_FIRST_KEYS`
23+
24+
`skills/**` is loaded whole into a customer agent's context window and its job
25+
is to teach that agent what it may author, so a row headlining
26+
`DEPRECATED_APPROVER_TYPES` and `NON_AUTHORABLE_APPROVER_TYPES` pointed an
27+
authoring agent at exactly the vocabulary it must not use, with nothing on the
28+
line marking them as such. No gate could see it: `check:skill-refs` compares the
29+
artifact against the generator, and the generator ranked faithfully.
30+
31+
`SCREAMING_SNAKE` exports are now dropped and source order is kept for what
32+
remains, with the cap of five applied AFTER filtering so the authorable names
33+
waiting behind the constants are promoted rather than the row merely shortened.
34+
A module whose entire export surface is machine constants falls through to no
35+
description at all rather than printing a bare `Exports:`.
36+
37+
Sorting `*Schema` exports first was considered and NOT taken: on the very row
38+
that motivated this it demotes `ApproverType` — the approver-type enum an author
39+
actually writes — below four schema objects, which is worse by this surface's
40+
own standard. The rule moves to `scripts/lib/export-list.ts` so it can be pinned
41+
without running the generator, and `scripts/export-list.test.ts` enforces it
42+
both as unit cases and as a corpus gate over the checked-in artifacts.

.github/workflows/ci.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,11 @@ jobs:
173173
- 'docs/audits/2026-07-unknown-key-strictness-ledger.md'
174174
- 'skills/objectstack-formula/**'
175175
- 'skills/objectstack-automation/SKILL.md'
176+
# @objectstack/spec's scripts/export-list.test.ts corpus gate reads
177+
# the whole published catalog (#12201). Verbatim per the declaration;
178+
# it subsumes the two narrower skills entries above, which are left
179+
# as the packages that declared them spelled them.
180+
- 'skills/**'
176181
- '.github/workflows/scaffold-e2e.yml'
177182
- '.claude/skills/spec-property-retirement/SKILL.md'
178183

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import fs from 'fs';
2727
import path from 'path';
28+
import { exportListDescription } from './lib/export-list';
2829
import { findModuleDocBlock } from './lib/file-description';
2930
import { createSink, type Owns } from './lib/generated-output';
3031

@@ -212,6 +213,14 @@ function resolveAll(entryFiles: string[]): { files: string[]; missing: string[]
212213
* A module with no doc block of its own falls through to the export list
213214
* rather than refusing: that line states a true fact about the file, where the
214215
* wrong block asserted a false one about its subject.
216+
*
217+
* WHICH exports that line may name is `exportListDescription()`'s rule, and it
218+
* lives beside this one in `lib/` for the same reason (#12201): the list used
219+
* to rank by source order with no notion of authorable surface, so three rows
220+
* headlined `DEPRECATED_APPROVER_TYPES`, `CORE_PLUGIN_TYPES` and friends —
221+
* machine vocabulary, published to a surface whose job is to teach an agent
222+
* what it may author. `check:skill-refs` could not see that either; it compares
223+
* the artifact against this generator, which ranked faithfully.
215224
*/
216225
function extractDescription(filePath: string): string {
217226
const content = fs.readFileSync(filePath, 'utf-8');
@@ -228,12 +237,7 @@ function extractDescription(filePath: string): string {
228237
return sentence.length > 120 ? sentence.slice(0, 117) + '...' : sentence;
229238
}
230239
}
231-
const exports: string[] = [];
232-
const re = /export\s+const\s+(\w+Schema|\w+)\s*(?:[:=])/g;
233-
let m: RegExpExecArray | null;
234-
while ((m = re.exec(content)) !== null) exports.push(m[1]);
235-
if (exports.length > 0) return `Exports: ${exports.slice(0, 5).join(', ')}`;
236-
return '';
240+
return exportListDescription(content) ?? '';
237241
}
238242

239243
// ── Index generator ──────────────────────────────────────────────────────────
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Pin for WHICH exports the skill-reference `Exports: …` fallback publishes —
5+
* #12201.
6+
*
7+
* The fallback ranked by SOURCE ORDER and had no notion of authorable surface,
8+
* so `slice(0, 5)` kept whichever five exports happened to be declared first.
9+
* Three rows in the published catalog therefore headlined machine constants
10+
* whose own names say they are not for authoring —
11+
* `DEPRECATED_APPROVER_TYPES`, `NON_AUTHORABLE_APPROVER_TYPES`,
12+
* `CORE_PLUGIN_TYPES`, `CONSUMER_INSTALLABLE_TYPES`,
13+
* `LEGACY_OBJECT_FIRST_KEYS` — on a surface loaded whole into a customer
14+
* agent's context window to teach it what it may author.
15+
*
16+
* No gate could see it: `check:skill-refs` compares the artifact against the
17+
* generator, and the generator reproduced the ranking faithfully. That is the
18+
* same blind spot #5059 found one layer up, and the answer is the same one —
19+
* the rule is extracted to a pure module and this file IS its enforcement.
20+
*
21+
* MEASURED (reverse verification) — and the two halves of this file fail
22+
* DIFFERENTLY, which is why both exist. Dropping the `MACHINE_CONSTANT` test
23+
* from `exportListDescription` (keeping everything else) turns four of the six
24+
* unit cases below red immediately — the three about which names survive, plus
25+
* the fall-through case, whose `null` exists only because filtering can empty a
26+
* list. The corpus gate meanwhile stays GREEN: it reads the checked-in
27+
* artifacts, and those only move when someone regenerates.
28+
* What turns the corpus gate red is regenerating with the rule dropped — i.e.
29+
* the state this card found, measured before the fix as 7 offenders across the
30+
* 3 rows (`DEPRECATED_APPROVER_TYPES`, `NON_AUTHORABLE_APPROVER_TYPES`,
31+
* `ORG_MEMBERSHIP_LEVELS`, `APPROVER_EXPRESSION_ROOTS`,
32+
* `LEGACY_OBJECT_FIRST_KEYS`, `CORE_PLUGIN_TYPES`,
33+
* `CONSUMER_INSTALLABLE_TYPES`).
34+
*
35+
* So the unit cases catch a rule that was weakened, and the corpus gate catches
36+
* an artifact that was regenerated from one — including from a `.zod.ts` that
37+
* grew a new constant. Neither subsumes the other. The "keeps a lone all-caps
38+
* token", "no exports at all" and cap-of-five cases stay green under that
39+
* ablation either way, because for those inputs the two rules agree; that
40+
* asymmetry is the point, since the defect was invisible on exactly the inputs
41+
* anyone would have thought to check.
42+
*
43+
* The corpus gate at the end is the part that cannot rot. It re-derives the
44+
* verdict from the checked-in `skills/**` artifacts — the bytes a customer
45+
* agent actually loads — so a future `.zod.ts` that declares a new
46+
* `SCREAMING_SNAKE` const above its schemas cannot quietly re-acquire a
47+
* hazardous row.
48+
*/
49+
50+
import fs from 'fs';
51+
import path from 'path';
52+
import url from 'url';
53+
54+
import { describe, expect, it } from 'vitest';
55+
56+
import { exportListDescription } from './lib/export-list';
57+
58+
const HERE = path.dirname(url.fileURLToPath(import.meta.url));
59+
const REPO_ROOT = path.resolve(HERE, '../../..');
60+
const SKILLS_DIR = path.resolve(REPO_ROOT, 'skills');
61+
62+
/** Same convention the filter encodes, restated so the gate is self-contained. */
63+
const SCREAMING_SNAKE = /^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+$/;
64+
65+
describe('exportListDescription — machine constants never headline a pointer row', () => {
66+
it('drops SCREAMING_SNAKE constants and keeps source order for the rest', () => {
67+
// `automation/approval.zod.ts`, reduced. The published row opened
68+
// "Exports: ApproverType, DEPRECATED_APPROVER_TYPES,
69+
// NON_AUTHORABLE_APPROVER_TYPES, ORG_MEMBERSHIP_LEVELS,
70+
// APPROVER_EXPRESSION_ROOTS" — four of five names unusable by an author.
71+
const source = [
72+
"export const ApproverType = z.enum(['user', 'role']);",
73+
'export const DEPRECATED_APPROVER_TYPES = [] as const;',
74+
'export const NON_AUTHORABLE_APPROVER_TYPES = [] as const;',
75+
'export const ORG_MEMBERSHIP_LEVELS = [] as const;',
76+
'export const APPROVER_EXPRESSION_ROOTS = [] as const;',
77+
"export const ApprovalDecision = z.enum(['approve']);",
78+
'export const ApprovalNodeApproverSchema = z.object({});',
79+
].join('\n');
80+
81+
expect(exportListDescription(source)).toBe(
82+
'Exports: ApproverType, ApprovalDecision, ApprovalNodeApproverSchema',
83+
);
84+
});
85+
86+
it('keeps source order — it does NOT sort *Schema exports first', () => {
87+
// Adjudicated on #12201 and pinned here so it is not "improved" later:
88+
// Schema-first ranking demotes `ApproverType`, the enum an author actually
89+
// writes, below the schema objects — worse by this surface's own standard.
90+
const source = [
91+
'export const PluginContextSchema = z.object({});',
92+
'export const CORE_PLUGIN_TYPES = [] as const;',
93+
'export const ApproverType = z.enum([]);',
94+
'export const PluginSchema = z.object({});',
95+
].join('\n');
96+
97+
expect(exportListDescription(source)).toBe(
98+
'Exports: PluginContextSchema, ApproverType, PluginSchema',
99+
);
100+
});
101+
102+
it('applies the cap of five AFTER filtering, so authorable names are promoted', () => {
103+
// Slicing first would let the constants consume the row's five slots and
104+
// then be deleted from it, shortening the row instead of repairing it.
105+
const source = [
106+
'export const A_CONST = 1;',
107+
'export const B_CONST = 1;',
108+
'export const One = 1;',
109+
'export const Two = 1;',
110+
'export const Three = 1;',
111+
'export const Four = 1;',
112+
'export const Five = 1;',
113+
'export const Six = 1;',
114+
].join('\n');
115+
116+
expect(exportListDescription(source)).toBe('Exports: One, Two, Three, Four, Five');
117+
});
118+
119+
it('keeps a lone all-caps token — the boundary is deliberate', () => {
120+
// No export in the eleven-module fallback corpus is a lone all-caps token,
121+
// so the corpus cannot distinguish "all caps" from "all caps with an
122+
// underscore". The narrower rule is chosen; widening it is a decision, and
123+
// this case is where that decision gets made.
124+
expect(exportListDescription('export const URL = 1;')).toBe('Exports: URL');
125+
});
126+
127+
it('falls through (null) when every export is a machine constant', () => {
128+
// Not `Exports:` with nothing after it — the caller prints no description.
129+
const source = ['export const CORE_PLUGIN_TYPES = [];', 'export const OTHER_KEYS = [];'].join('\n');
130+
expect(exportListDescription(source)).toBeNull();
131+
});
132+
133+
it('falls through (null) when the module exports no const at all', () => {
134+
expect(exportListDescription('export function f() {}\n')).toBeNull();
135+
});
136+
});
137+
138+
describe('published catalog — no Exports: row names a machine constant', () => {
139+
/** Every `Exports: …` pointer row in the checked-in skill references. */
140+
const publishedRows = (): { file: string; source: string; names: string[] }[] => {
141+
const rows: { file: string; source: string; names: string[] }[] = [];
142+
for (const skill of fs.readdirSync(SKILLS_DIR)) {
143+
const index = path.resolve(SKILLS_DIR, skill, 'references/_index.md');
144+
if (!fs.existsSync(index)) continue;
145+
for (const line of fs.readFileSync(index, 'utf-8').split('\n')) {
146+
const match = /^- `([^`]+)` Exports: (.+)$/.exec(line);
147+
if (match) {
148+
rows.push({
149+
file: path.relative(REPO_ROOT, index),
150+
source: match[1],
151+
names: match[2].split(',').map(n => n.trim()),
152+
});
153+
}
154+
}
155+
}
156+
return rows;
157+
};
158+
159+
it('finds the fallback rows at all', () => {
160+
// Nothing parsed means nothing compared, and "no hazardous row" would read
161+
// as green — the same failure mode the generator's own emptiness guard has.
162+
expect(publishedRows().length).toBeGreaterThan(0);
163+
});
164+
165+
it('names no SCREAMING_SNAKE constant on any published row', () => {
166+
const offenders = publishedRows().flatMap(row =>
167+
row.names.filter(name => SCREAMING_SNAKE.test(name)).map(name => `${row.file}: ${row.source}${name}`),
168+
);
169+
expect(offenders).toEqual([]);
170+
});
171+
});
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The `Exports: …` line a skill-reference row falls back to when its module has
5+
* no doc block of its own — and which exports may appear on it.
6+
*
7+
* Extracted from `build-skill-references.ts` (#12201) for the same reason
8+
* `file-description.ts` (#5059), `format-type.ts` (#4912) and `escape-mdx.ts`
9+
* (#5452) were: the generator is a top-level script that runs `main()` on
10+
* import, so the only way to assert on this list used to be to run the whole
11+
* thing and read the emitted `_index.md`.
12+
*
13+
* ## Why the list is filtered
14+
*
15+
* The line is TRUE either way — an accurate list of what the module exports,
16+
* which is why #12094 kept the fallback instead of refusing (an honest export
17+
* list beats a confidently wrong prose sentence). What is wrong is the
18+
* RANKING. Two properties combined badly:
19+
*
20+
* 1. Rank was SOURCE ORDER — `slice(0, 5)` kept whichever five happened to be
21+
* declared first, which is a fact about file layout, not about importance.
22+
* 2. The extraction has no notion of authorable surface — any `export const`
23+
* qualified, including constants whose own names say they are not for
24+
* authoring.
25+
*
26+
* So a `.zod.ts` that declares its machine constants near the top headlined
27+
* them. Measured on the post-#12094 catalog, three of the eleven modules that
28+
* reach this fallback did exactly that:
29+
*
30+
* - `automation/approval.zod.ts` — `DEPRECATED_APPROVER_TYPES`,
31+
* `NON_AUTHORABLE_APPROVER_TYPES`
32+
* - `kernel/plugin.zod.ts` — `CORE_PLUGIN_TYPES`, `CONSUMER_INSTALLABLE_TYPES`
33+
* - `system/translation.zod.ts` — `LEGACY_OBJECT_FIRST_KEYS`
34+
*
35+
* `skills/**` is loaded WHOLE into a customer agent's context window, and its
36+
* job is to teach that agent what it may author. A row headlining
37+
* `DEPRECATED_APPROVER_TYPES` and `NON_AUTHORABLE_APPROVER_TYPES` points an
38+
* authoring agent at precisely the vocabulary it must not use, with nothing on
39+
* the line marking them as such. Nothing is broken and no gate is wrong — this
40+
* is the "make AI-written metadata hard to get wrong" axis, and it is why the
41+
* repair belongs on this surface rather than in a lint rule about naming.
42+
*
43+
* ## Why filtering, and NOT `*Schema`-first sorting
44+
*
45+
* Machine constants are dropped and source order is kept for everything that
46+
* survives. Sorting `*Schema` exports ahead of the rest was considered and
47+
* deliberately NOT taken: on the very row that motivated this card it demotes
48+
* `ApproverType` — the approver-type enum an author actually writes — below
49+
* four schema objects, which is worse by this surface's own standard. The
50+
* hazard that was measured is machine vocabulary appearing AT ALL, not schemas
51+
* appearing late.
52+
*
53+
* The loud-refusal alternative (require a module doc block on every `.zod.ts`
54+
* reachable from `SKILL_MAP`, and drop this fallback) is also not taken —
55+
* #12094 declined it for this same population and that reasoning stands.
56+
* Authoring the missing module doc blocks remains a separate editorial
57+
* question; it would remove the symptom without any generator change, and this
58+
* filter does not stand in its way.
59+
*/
60+
61+
/**
62+
* A machine constant by naming convention: all caps, with at least one
63+
* underscore.
64+
*
65+
* The underscore is REQUIRED rather than incidental. `SCREAMING_SNAKE` is a
66+
* convention about multi-word constants, and the separator is what makes the
67+
* reading unambiguous — a lone all-caps token (`URL`, `ID`, `MCP`) is as
68+
* plausibly an acronym inside a name as it is a constant. Measured across the
69+
* eleven modules that reach this fallback, the two readings are
70+
* indistinguishable: every all-caps export in the corpus has an underscore, and
71+
* no export is a lone all-caps token. Where the corpus cannot choose, the
72+
* narrower rule wins, and `export-list.test.ts` pins that boundary — so
73+
* widening it later is a decision someone makes on evidence, not a regex
74+
* someone quietly loosens.
75+
*/
76+
const MACHINE_CONSTANT = /^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+$/;
77+
78+
/**
79+
* Every `export const` name, in source order.
80+
*
81+
* Carried over from the generator verbatim, alternation included. The
82+
* `\w+Schema|\w+` branch is redundant — the second alternative subsumes the
83+
* first for every input, since both are anchored by the same trailing
84+
* `\s*[:=]` — but this change is about RANKING, and rewriting the extraction
85+
* at the same time would widen what the diff has to be trusted about.
86+
*/
87+
const EXPORT_CONST = /export\s+const\s+(\w+Schema|\w+)\s*(?:[:=])/g;
88+
89+
/** How many names a pointer row lists before it becomes noise. */
90+
const MAX_NAMES = 5;
91+
92+
/**
93+
* The `Exports: …` description for a module with no doc block of its own, or
94+
* `null` when there is nothing authorable to name.
95+
*
96+
* `null` is the "fall through" answer, and it is distinct from an empty list on
97+
* purpose: the caller prints no description at all rather than a bare
98+
* `Exports:` with nothing after it. A module whose entire public surface is
99+
* machine constants has nothing to say to an authoring agent, and 宁可缺,
100+
* 不要错 — a row with no description is a gap the reader can see.
101+
*
102+
* The cap is applied AFTER filtering, not before. Slicing first would let a
103+
* module's constants consume the row's five slots and then be deleted from it,
104+
* so the fix would merely SHORTEN the hazardous rows instead of promoting the
105+
* authorable names waiting behind them — `system/translation.zod.ts` would
106+
* publish four names where five were available.
107+
*/
108+
export function exportListDescription(source: string): string | null {
109+
const names: string[] = [];
110+
for (const match of source.matchAll(EXPORT_CONST)) {
111+
if (!MACHINE_CONSTANT.test(match[1])) names.push(match[1]);
112+
}
113+
if (names.length === 0) return null;
114+
return `Exports: ${names.slice(0, MAX_NAMES).join(', ')}`;
115+
}

0 commit comments

Comments
 (0)