Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .changeset/skill-refs-exports-fallback-ranking.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
---
"@objectstack/spec": patch
---

fix(spec): keep machine constants off the skill-reference `Exports:` fallback

When a `.zod.ts` has no module doc block, `build-skill-references.ts` falls back
to listing its exports. That line is TRUE — an accurate list of what the module
exports, which is why #12094 kept the fallback rather than refusing. What was
wrong is the RANKING: the list was whichever five exports happened to be
DECLARED FIRST, and the extraction had no notion of authorable surface, so any
`export const` qualified — including constants whose own names say they are not
for authoring.

Three of the eleven modules that reach this fallback declare their machine
constants near the top, so three published rows headlined them:

- `automation/approval.zod.ts` named `DEPRECATED_APPROVER_TYPES`,
`NON_AUTHORABLE_APPROVER_TYPES`, `ORG_MEMBERSHIP_LEVELS` and
`APPROVER_EXPRESSION_ROOTS` — four of its five slots
- `kernel/plugin.zod.ts` named `CORE_PLUGIN_TYPES`, `CONSUMER_INSTALLABLE_TYPES`
- `system/translation.zod.ts` named `LEGACY_OBJECT_FIRST_KEYS`

`skills/**` is loaded whole into a customer agent's context window and its job
is to teach that agent what it may author, so a row headlining
`DEPRECATED_APPROVER_TYPES` and `NON_AUTHORABLE_APPROVER_TYPES` pointed an
authoring agent at exactly the vocabulary it must not use, with nothing on the
line marking them as such. No gate could see it: `check:skill-refs` compares the
artifact against the generator, and the generator ranked faithfully.

`SCREAMING_SNAKE` exports are now dropped and source order is kept for what
remains, with the cap of five applied AFTER filtering so the authorable names
waiting behind the constants are promoted rather than the row merely shortened.
A module whose entire export surface is machine constants falls through to no
description at all rather than printing a bare `Exports:`.

Sorting `*Schema` exports first was considered and NOT taken: on the very row
that motivated this it demotes `ApproverType` — the approver-type enum an author
actually writes — below four schema objects, which is worse by this surface's
own standard. The rule moves to `scripts/lib/export-list.ts` so it can be pinned
without running the generator, and `scripts/export-list.test.ts` enforces it
both as unit cases and as a corpus gate over the checked-in artifacts.
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -173,6 +173,11 @@ jobs:
- 'docs/audits/2026-07-unknown-key-strictness-ledger.md'
- 'skills/objectstack-formula/**'
- 'skills/objectstack-automation/SKILL.md'
# @objectstack/spec's scripts/export-list.test.ts corpus gate reads
# the whole published catalog (#12201). Verbatim per the declaration;
# it subsumes the two narrower skills entries above, which are left
# as the packages that declared them spelled them.
- 'skills/**'
- '.github/workflows/scaffold-e2e.yml'
- '.claude/skills/spec-property-retirement/SKILL.md'

Expand Down
16 changes: 10 additions & 6 deletions packages/spec/scripts/build-skill-references.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,7 @@

import fs from 'fs';
import path from 'path';
import { exportListDescription } from './lib/export-list';
import { findModuleDocBlock } from './lib/file-description';
import { createSink, type Owns } from './lib/generated-output';

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

// ── Index generator ──────────────────────────────────────────────────────────
Expand Down
171 changes: 171 additions & 0 deletions packages/spec/scripts/export-list.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* Pin for WHICH exports the skill-reference `Exports: …` fallback publishes —
* #12201.
*
* The fallback ranked by SOURCE ORDER and had no notion of authorable surface,
* so `slice(0, 5)` kept whichever five exports happened to be declared first.
* Three rows in the published catalog therefore headlined machine constants
* whose own names say they are not for authoring —
* `DEPRECATED_APPROVER_TYPES`, `NON_AUTHORABLE_APPROVER_TYPES`,
* `CORE_PLUGIN_TYPES`, `CONSUMER_INSTALLABLE_TYPES`,
* `LEGACY_OBJECT_FIRST_KEYS` — on a surface loaded whole into a customer
* agent's context window to teach it what it may author.
*
* No gate could see it: `check:skill-refs` compares the artifact against the
* generator, and the generator reproduced the ranking faithfully. That is the
* same blind spot #5059 found one layer up, and the answer is the same one —
* the rule is extracted to a pure module and this file IS its enforcement.
*
* MEASURED (reverse verification) — and the two halves of this file fail
* DIFFERENTLY, which is why both exist. Dropping the `MACHINE_CONSTANT` test
* from `exportListDescription` (keeping everything else) turns four of the six
* unit cases below red immediately — the three about which names survive, plus
* the fall-through case, whose `null` exists only because filtering can empty a
* list. The corpus gate meanwhile stays GREEN: it reads the checked-in
* artifacts, and those only move when someone regenerates.
* What turns the corpus gate red is regenerating with the rule dropped — i.e.
* the state this card found, measured before the fix as 7 offenders across the
* 3 rows (`DEPRECATED_APPROVER_TYPES`, `NON_AUTHORABLE_APPROVER_TYPES`,
* `ORG_MEMBERSHIP_LEVELS`, `APPROVER_EXPRESSION_ROOTS`,
* `LEGACY_OBJECT_FIRST_KEYS`, `CORE_PLUGIN_TYPES`,
* `CONSUMER_INSTALLABLE_TYPES`).
*
* So the unit cases catch a rule that was weakened, and the corpus gate catches
* an artifact that was regenerated from one — including from a `.zod.ts` that
* grew a new constant. Neither subsumes the other. The "keeps a lone all-caps
* token", "no exports at all" and cap-of-five cases stay green under that
* ablation either way, because for those inputs the two rules agree; that
* asymmetry is the point, since the defect was invisible on exactly the inputs
* anyone would have thought to check.
*
* The corpus gate at the end is the part that cannot rot. It re-derives the
* verdict from the checked-in `skills/**` artifacts — the bytes a customer
* agent actually loads — so a future `.zod.ts` that declares a new
* `SCREAMING_SNAKE` const above its schemas cannot quietly re-acquire a
* hazardous row.
*/

import fs from 'fs';
import path from 'path';
import url from 'url';

import { describe, expect, it } from 'vitest';

import { exportListDescription } from './lib/export-list';

const HERE = path.dirname(url.fileURLToPath(import.meta.url));
const REPO_ROOT = path.resolve(HERE, '../../..');
const SKILLS_DIR = path.resolve(REPO_ROOT, 'skills');

/** Same convention the filter encodes, restated so the gate is self-contained. */
const SCREAMING_SNAKE = /^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+$/;

describe('exportListDescription — machine constants never headline a pointer row', () => {
it('drops SCREAMING_SNAKE constants and keeps source order for the rest', () => {
// `automation/approval.zod.ts`, reduced. The published row opened
// "Exports: ApproverType, DEPRECATED_APPROVER_TYPES,
// NON_AUTHORABLE_APPROVER_TYPES, ORG_MEMBERSHIP_LEVELS,
// APPROVER_EXPRESSION_ROOTS" — four of five names unusable by an author.
const source = [
"export const ApproverType = z.enum(['user', 'role']);",
'export const DEPRECATED_APPROVER_TYPES = [] as const;',
'export const NON_AUTHORABLE_APPROVER_TYPES = [] as const;',
'export const ORG_MEMBERSHIP_LEVELS = [] as const;',
'export const APPROVER_EXPRESSION_ROOTS = [] as const;',
"export const ApprovalDecision = z.enum(['approve']);",
'export const ApprovalNodeApproverSchema = z.object({});',
].join('\n');

expect(exportListDescription(source)).toBe(
'Exports: ApproverType, ApprovalDecision, ApprovalNodeApproverSchema',
);
});

it('keeps source order — it does NOT sort *Schema exports first', () => {
// Adjudicated on #12201 and pinned here so it is not "improved" later:
// Schema-first ranking demotes `ApproverType`, the enum an author actually
// writes, below the schema objects — worse by this surface's own standard.
const source = [
'export const PluginContextSchema = z.object({});',
'export const CORE_PLUGIN_TYPES = [] as const;',
'export const ApproverType = z.enum([]);',
'export const PluginSchema = z.object({});',
].join('\n');

expect(exportListDescription(source)).toBe(
'Exports: PluginContextSchema, ApproverType, PluginSchema',
);
});

it('applies the cap of five AFTER filtering, so authorable names are promoted', () => {
// Slicing first would let the constants consume the row's five slots and
// then be deleted from it, shortening the row instead of repairing it.
const source = [
'export const A_CONST = 1;',
'export const B_CONST = 1;',
'export const One = 1;',
'export const Two = 1;',
'export const Three = 1;',
'export const Four = 1;',
'export const Five = 1;',
'export const Six = 1;',
].join('\n');

expect(exportListDescription(source)).toBe('Exports: One, Two, Three, Four, Five');
});

it('keeps a lone all-caps token — the boundary is deliberate', () => {
// No export in the eleven-module fallback corpus is a lone all-caps token,
// so the corpus cannot distinguish "all caps" from "all caps with an
// underscore". The narrower rule is chosen; widening it is a decision, and
// this case is where that decision gets made.
expect(exportListDescription('export const URL = 1;')).toBe('Exports: URL');
});

it('falls through (null) when every export is a machine constant', () => {
// Not `Exports:` with nothing after it — the caller prints no description.
const source = ['export const CORE_PLUGIN_TYPES = [];', 'export const OTHER_KEYS = [];'].join('\n');
expect(exportListDescription(source)).toBeNull();
});

it('falls through (null) when the module exports no const at all', () => {
expect(exportListDescription('export function f() {}\n')).toBeNull();
});
});

describe('published catalog — no Exports: row names a machine constant', () => {
/** Every `Exports: …` pointer row in the checked-in skill references. */
const publishedRows = (): { file: string; source: string; names: string[] }[] => {
const rows: { file: string; source: string; names: string[] }[] = [];
for (const skill of fs.readdirSync(SKILLS_DIR)) {
const index = path.resolve(SKILLS_DIR, skill, 'references/_index.md');
if (!fs.existsSync(index)) continue;
for (const line of fs.readFileSync(index, 'utf-8').split('\n')) {
const match = /^- `([^`]+)` — Exports: (.+)$/.exec(line);
if (match) {
rows.push({
file: path.relative(REPO_ROOT, index),
source: match[1],
names: match[2].split(',').map(n => n.trim()),
});
}
}
}
return rows;
};

it('finds the fallback rows at all', () => {
// Nothing parsed means nothing compared, and "no hazardous row" would read
// as green — the same failure mode the generator's own emptiness guard has.
expect(publishedRows().length).toBeGreaterThan(0);
});

it('names no SCREAMING_SNAKE constant on any published row', () => {
const offenders = publishedRows().flatMap(row =>
row.names.filter(name => SCREAMING_SNAKE.test(name)).map(name => `${row.file}: ${row.source} → ${name}`),
);
expect(offenders).toEqual([]);
});
});
115 changes: 115 additions & 0 deletions packages/spec/scripts/lib/export-list.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* The `Exports: …` line a skill-reference row falls back to when its module has
* no doc block of its own — and which exports may appear on it.
*
* Extracted from `build-skill-references.ts` (#12201) for the same reason
* `file-description.ts` (#5059), `format-type.ts` (#4912) and `escape-mdx.ts`
* (#5452) were: the generator is a top-level script that runs `main()` on
* import, so the only way to assert on this list used to be to run the whole
* thing and read the emitted `_index.md`.
*
* ## Why the list is filtered
*
* The line is TRUE either way — an accurate list of what the module exports,
* which is why #12094 kept the fallback instead of refusing (an honest export
* list beats a confidently wrong prose sentence). What is wrong is the
* RANKING. Two properties combined badly:
*
* 1. Rank was SOURCE ORDER — `slice(0, 5)` kept whichever five happened to be
* declared first, which is a fact about file layout, not about importance.
* 2. The extraction has no notion of authorable surface — any `export const`
* qualified, including constants whose own names say they are not for
* authoring.
*
* So a `.zod.ts` that declares its machine constants near the top headlined
* them. Measured on the post-#12094 catalog, three of the eleven modules that
* reach this fallback did exactly that:
*
* - `automation/approval.zod.ts` — `DEPRECATED_APPROVER_TYPES`,
* `NON_AUTHORABLE_APPROVER_TYPES`
* - `kernel/plugin.zod.ts` — `CORE_PLUGIN_TYPES`, `CONSUMER_INSTALLABLE_TYPES`
* - `system/translation.zod.ts` — `LEGACY_OBJECT_FIRST_KEYS`
*
* `skills/**` is loaded WHOLE into a customer agent's context window, and its
* job is to teach that agent what it may author. A row headlining
* `DEPRECATED_APPROVER_TYPES` and `NON_AUTHORABLE_APPROVER_TYPES` points an
* authoring agent at precisely the vocabulary it must not use, with nothing on
* the line marking them as such. Nothing is broken and no gate is wrong — this
* is the "make AI-written metadata hard to get wrong" axis, and it is why the
* repair belongs on this surface rather than in a lint rule about naming.
*
* ## Why filtering, and NOT `*Schema`-first sorting
*
* Machine constants are dropped and source order is kept for everything that
* survives. Sorting `*Schema` exports ahead of the rest was considered and
* deliberately NOT taken: on the very row that motivated this card it demotes
* `ApproverType` — the approver-type enum an author actually writes — below
* four schema objects, which is worse by this surface's own standard. The
* hazard that was measured is machine vocabulary appearing AT ALL, not schemas
* appearing late.
*
* The loud-refusal alternative (require a module doc block on every `.zod.ts`
* reachable from `SKILL_MAP`, and drop this fallback) is also not taken —
* #12094 declined it for this same population and that reasoning stands.
* Authoring the missing module doc blocks remains a separate editorial
* question; it would remove the symptom without any generator change, and this
* filter does not stand in its way.
*/

/**
* A machine constant by naming convention: all caps, with at least one
* underscore.
*
* The underscore is REQUIRED rather than incidental. `SCREAMING_SNAKE` is a
* convention about multi-word constants, and the separator is what makes the
* reading unambiguous — a lone all-caps token (`URL`, `ID`, `MCP`) is as
* plausibly an acronym inside a name as it is a constant. Measured across the
* eleven modules that reach this fallback, the two readings are
* indistinguishable: every all-caps export in the corpus has an underscore, and
* no export is a lone all-caps token. Where the corpus cannot choose, the
* narrower rule wins, and `export-list.test.ts` pins that boundary — so
* widening it later is a decision someone makes on evidence, not a regex
* someone quietly loosens.
*/
const MACHINE_CONSTANT = /^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+$/;

/**
* Every `export const` name, in source order.
*
* Carried over from the generator verbatim, alternation included. The
* `\w+Schema|\w+` branch is redundant — the second alternative subsumes the
* first for every input, since both are anchored by the same trailing
* `\s*[:=]` — but this change is about RANKING, and rewriting the extraction
* at the same time would widen what the diff has to be trusted about.
*/
const EXPORT_CONST = /export\s+const\s+(\w+Schema|\w+)\s*(?:[:=])/g;

/** How many names a pointer row lists before it becomes noise. */
const MAX_NAMES = 5;

/**
* The `Exports: …` description for a module with no doc block of its own, or
* `null` when there is nothing authorable to name.
*
* `null` is the "fall through" answer, and it is distinct from an empty list on
* purpose: the caller prints no description at all rather than a bare
* `Exports:` with nothing after it. A module whose entire public surface is
* machine constants has nothing to say to an authoring agent, and 宁可缺,
* 不要错 — a row with no description is a gap the reader can see.
*
* The cap is applied AFTER filtering, not before. Slicing first would let a
* module's constants consume the row's five slots and then be deleted from it,
* so the fix would merely SHORTEN the hazardous rows instead of promoting the
* authorable names waiting behind them — `system/translation.zod.ts` would
* publish four names where five were available.
*/
export function exportListDescription(source: string): string | null {
const names: string[] = [];
for (const match of source.matchAll(EXPORT_CONST)) {
if (!MACHINE_CONSTANT.test(match[1])) names.push(match[1]);
}
if (names.length === 0) return null;
return `Exports: ${names.slice(0, MAX_NAMES).join(', ')}`;
}
Loading
Loading