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
15 changes: 15 additions & 0 deletions .changeset/react-declaration-parity-cannot-run-is-not-a-skip.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@objectstack/spec': patch
---

`check:react-declaration-parity` had no path on which it could go red — a missing manifest now fails instead of skipping, and the ledger says "cannot run here" instead of "deliberately not run".

Two things stacked. The gate was in **no workflow** (`grep -rl check:react-declaration-parity .github/workflows/` returned nothing), and a manual run without `MANIFEST` printed `⚠ manifest unavailable … — skipping.` and **exited 0**. So the one gate whose history is written up in AGENTS.md as worth keeping — `spec-only` / `registry-only` / `missing` are real signals — could not fail for anybody, ever. Two other issues had already started citing it as the negative example of a gate that cannot be shown to catch anything (#4804 / #4777).

**Where the manifest comes from, measured before anything was wired.** The right-hand side is objectui's `sdui.manifest.json`, and this repository cannot produce one: its only producer drives a real browser at objectui's built console and reads `window.__MANIFEST` (the registry pulls browser-only deps, so nothing enumerates it from Node). `packages/console/dist/` is gitignored, `scripts/build-console.sh` deliberately does not dump one — it must not drag a browser into the console build — and the published `@objectstack/console` tarball contains no `sdui.manifest.json` either (16.1.0: 513 files, zero `sdui` matches; its `dist/manifest.json` is the PWA manifest, so even the CLI's `@objectstack/console/dist/sdui.manifest.json` fallback resolves to nothing). Wiring a step into `lint.yml` would therefore have wired a **permanently skipping** step — the same defect with CI decoration.

- **"Could not run" is now a failure, not a skip.** No `MANIFEST`, a path that does not exist, malformed JSON, or a dump declaring zero components each exit **1** with a prescription that names the producer (`pnpm sdui:manifest`, `OBJECTUI_ROOT=../objectui pnpm objectui:build`) and the browser it needs. Deliberately independent of `--strict`: that flag prices a *divergence*, and this is the other thing entirely — no comparison happened. The empty-dump case is new coverage in the same family; objectui's dumper already refuses to *write* one, and this refuses to *read* one instead of reporting every block as missing.
- **The ledger stops implying someone runs it.** `check:generated` moves it out of `NO_GENERATOR` ("runnable, deliberately not run here") into a new `EXTERNAL_INPUT_REQUIRED` bucket that records the missing input and its producer, and prints `cannot run here: check:react-declaration-parity — needs MANIFEST=…; runs in scripts/gen-sdui-manifest.sh`. Following `EXPLICIT_GENERATORS` (#5358/#5807), the classification carries an enforced claim rather than a label: the reconciliation fails if `scripts/gen-sdui-manifest.sh` stops **invoking** the gate on a non-comment line — a gate filed as "runs elsewhere" while running nowhere is the exact hole this bucket exists to expose.
- **The gate is now demonstrably able to fail.** `check-react-blocks-declaration-parity.test.ts` asserts exit codes, not just report text: a fabricated registry-only input and a vanished block each exit non-zero **naming themselves**, all four "could not run" paths exit non-zero, and an accepted state still exits 0 so the red is discriminating. The pre-existing helper swallowed exit codes by design, so every earlier test would have passed against a script that always exited 0 — which is what this gate was.

Where the manifest should come from in CI (an objectui clone plus a browser in this repo's workflows, a published manifest artifact, or a Node-side dump in objectui) stays an open provenance decision, filed separately. Until it is answered, the honest state is a gate that runs at `pnpm sdui:manifest` and refuses to pretend otherwise.
20 changes: 18 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,8 @@ believe it, and before you file a bug about `main` being red. (Two phantom "brea
removals" this way while writing this section; `check:generated` now prints this caveat
inline when that gate is the one failing.)

`check:liveness`, `check:empty-state`, `check:skill-examples`,
`check:react-declaration-parity`, `check:exported-any` and `check:dual-source-exports` are
`check:liveness`, `check:empty-state`, `check:skill-examples`, `check:exported-any` and
`check:dual-source-exports` are
pure checks with no generator — a failure there is a real finding to fix, not an artifact
to regenerate. `check:generated` names them as deliberately not run, so its "all up to
date" never reads as "everything passed". The last one asks the third question about the
Expand All @@ -436,6 +436,22 @@ and #4413 shipped four dead blocks straight through a green run of it. Renamed a
re-scoped in #4472. The gate is still worth having (`spec-only`, `registry-only` and
`missing` are real signals) — just don't read it as proof anything renders.

⚠️ **It is also the one gate `check:generated` cannot run at all**, and it says so in its
own bucket (`EXTERNAL_INPUT_REQUIRED`, "cannot run here") rather than beside the source
audits that are merely *deliberately* not run. Its right-hand side is objectui's
`sdui.manifest.json`, and nothing here can produce one: the registry is a browser app, so
the manifest exists only after `pnpm sdui:manifest` builds objectui at `.objectui-sha` and
enumerates it in a real browser — `packages/console/dist/` is gitignored, the console
build deliberately does not dump one, and the published `@objectstack/console` carries
none either. Until #4690 that combined with a manual run that printed `⚠ manifest
unavailable` and **exited 0**, so no path existed on which this gate could go red; it now
**exits 1** when it has no usable manifest, because "could not run" is a failure, not a
skip (Route & surface ownership §3, *Absence must be loud*). Run it the one way that
works: `pnpm sdui:manifest` (or `OBJECTUI_ROOT=../objectui pnpm objectui:build` first),
which dumps the manifest and runs the ratchet against it. Where the manifest *should* come
from in CI is an open provenance question, tracked separately — do not "fix" the red by
re-adding a skip.

`check:exported-any` is the one of those that also reads the built `dist/*.d.ts`, so the
stale-`dist` caveat above applies to it too. It asks the other half of the
`api-surface.json` question: that snapshot records an export *exists*, never what it
Expand Down
21 changes: 21 additions & 0 deletions packages/spec/scripts/check-generated-ledger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,27 @@ describe('check:generated --reconcile-only', () => {
expect(output).toContain('1 explicit manual-only generators');
});

it('files the gate whose input this repo cannot produce as EXTERNAL_INPUT_REQUIRED (#4690)', () => {
// NO_GENERATOR would say "runnable, deliberately not run here" — which is what
// `check:react-declaration-parity` said while it was wired into no workflow and
// skipping by default, i.e. while running nowhere at all. The classification has
// to carry the two facts a reader needs instead: WHICH input is missing, and WHO
// supplies it.
const { status, output } = runReconcile();
expect(status, output).toBe(0);
expect(output).toContain('1 needing an external input');
expect(output).toContain('cannot run here: check:react-declaration-parity');
expect(output).toContain('MANIFEST');
expect(output).toContain('scripts/gen-sdui-manifest.sh');
// And the claim is not free: `runBy` must still invoke the gate. The
// reconciliation fails otherwise (a gate classified as "runs elsewhere" while
// running nowhere is the hole this category exists to make visible), so this
// asserts the same fact where the failure message is legible.
const runner = path.resolve(SPEC, '..', '..', 'scripts/gen-sdui-manifest.sh');
expect(fs.existsSync(runner)).toBe(true);
expect(fs.readFileSync(runner, 'utf8')).toContain('check:react-declaration-parity');
});

it('covers the test-layer typecheck gate and its writer (#5286)', () => {
// The specific pair that failed CI on this branch. Named here so a later
// change that drops either script also has to come back through this file.
Expand Down
97 changes: 89 additions & 8 deletions packages/spec/scripts/check-generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
*/

import { execSync } from 'node:child_process';
import { readFileSync } from 'node:fs';
import { existsSync, readFileSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';

Expand Down Expand Up @@ -116,10 +116,6 @@ const GATED: ReadonlyArray<{
const NO_GENERATOR: ReadonlyArray<{ check: string; why: string }> = [
{ check: 'check:liveness', why: 'audits whether declared spec properties have a reader — no artifact' },
{ check: 'check:empty-state', why: 'audits empty-state coverage — no artifact' },
{
check: 'check:react-declaration-parity',
why: 'compares the spec schema props against the registry-declared inputs — two declarations, no artifact (and no renderer: #4472)',
},
{ check: 'check:skill-examples', why: 'validates skill examples parse — no artifact' },
// Landed in #4177 while this ledger landed in #4183 — neither PR could see the
// other, so `main` carried an unclassified script and this reconciliation was
Expand Down Expand Up @@ -152,6 +148,47 @@ const NO_GENERATOR: ReadonlyArray<{ check: string; why: string }> = [
},
];

/**
* Source audits that CANNOT RUN from this repository at all, because the input
* they compare against does not exist here and cannot be produced here.
*
* A separate bucket from `NO_GENERATOR` because the two say different things to a
* reader, and #4690 is what conflating them cost. `NO_GENERATOR` means "runnable,
* deliberately not run in this aggregate — run it yourself and it will answer".
* This one means "you cannot run it here at all, and here is the input it wants
* and who produces it". Sitting in the first list, `check:react-declaration-parity`
* read as the former for the entire time it was the latter: it was wired into no
* workflow, and a manual run without `MANIFEST` printed a `⚠` and exited 0, so no
* path existed on which the gate could go red. Whoever read "deliberately not run"
* reasonably assumed someone, somewhere, was running it.
*
* Encoding WHY in the ledger follows EXPLICIT_GENERATORS (#5807/#5358): a
* classification that records only a name is a classification the next reader has
* to re-derive. `runBy` is what keeps this bucket honest rather than an escape
* hatch — it names the in-repo entry point that DOES run the gate with its input,
* and `reconcileLedger` fails if that file has stopped naming the check. "Cannot
* run here" is a statement about this aggregate; "runs nowhere" would be the defect
* this category is supposed to make visible, not hide.
*/
const EXTERNAL_INPUT_REQUIRED: ReadonlyArray<{
check: string;
input: string;
runBy: string;
why: string;
}> = [
{
check: 'check:react-declaration-parity',
input: 'MANIFEST=<sdui.manifest.json> — objectui\'s registry-inputs dump',
runBy: 'scripts/gen-sdui-manifest.sh',
why:
'compares the spec schema props against the registry-declared inputs (two declarations, no renderer: #4472). ' +
'The registry is a browser app, so its manifest exists only after objectui is built at .objectui-sha and ' +
'enumerated in a real browser — nothing in this repo (console dist is gitignored, the published console ships ' +
'no sdui.manifest.json) can hand it one. `pnpm sdui:manifest` produces it and runs the ratchet; without it the ' +
'gate now exits 1 rather than skipping (#4690)',
},
];

/**
* Generators whose output NOTHING verifies. Recorded rather than ignored: each
* one is an artifact that can silently drift from its source, which is the class
Expand Down Expand Up @@ -203,7 +240,11 @@ const SELF = 'check:generated';
*/
function reconcileLedger(scripts: Record<string, string>): void {
const problems: string[] = [];
const declaredChecks = new Set([...GATED.map((g) => g.check), ...NO_GENERATOR.map((n) => n.check)]);
const declaredChecks = new Set([
...GATED.map((g) => g.check),
...NO_GENERATOR.map((n) => n.check),
...EXTERNAL_INPUT_REQUIRED.map((e) => e.check),
]);
const declaredGens = new Set([
...GATED.map((g) => g.gen),
...UNGATED_GENERATORS.map((u) => u.gen),
Expand All @@ -213,8 +254,9 @@ function reconcileLedger(scripts: Record<string, string>): void {
for (const name of Object.keys(scripts)) {
if (name === SELF) continue;
if (name.startsWith('check:') && !declaredChecks.has(name)) {
problems.push(` \`${name}\` exists in package.json but is in neither GATED nor NO_GENERATOR.\n` +
` Classify it: does it compare a checked-in artifact against a generator, or audit source?`);
problems.push(` \`${name}\` exists in package.json but is in neither GATED nor NO_GENERATOR (nor EXTERNAL_INPUT_REQUIRED).\n` +
` Classify it: does it compare a checked-in artifact against a generator, audit source,\n` +
` or audit source against an input this repo cannot produce (name where it DOES run)?`);
}
if (name.startsWith('gen:') && !declaredGens.has(name)) {
problems.push(` \`${name}\` exists in package.json but no GATED entry names it and it is not in UNGATED_GENERATORS.\n` +
Expand All @@ -226,6 +268,30 @@ function reconcileLedger(scripts: Record<string, string>): void {
for (const { check } of GATED) if (!scripts[check]) problems.push(` GATED names \`${check}\`, which package.json no longer has.`);
for (const { gen } of GATED) if (!scripts[gen]) problems.push(` GATED names \`${gen}\`, which package.json no longer has.`);
for (const { check } of NO_GENERATOR) if (!scripts[check]) problems.push(` NO_GENERATOR names \`${check}\`, which package.json no longer has.`);
for (const { check, runBy } of EXTERNAL_INPUT_REQUIRED) {
if (!scripts[check]) problems.push(` EXTERNAL_INPUT_REQUIRED names \`${check}\`, which package.json no longer has.`);
// The claim that makes this category honest rather than an escape hatch: the
// gate cannot run HERE, but it does run SOMEWHERE, and that somewhere is a file
// in this repo that still invokes it. A `runBy` that has stopped naming the
// check is #4690 all over again — a gate classified as "runs elsewhere" while
// running nowhere.
const runner = join(pkgRoot, '..', '..', runBy);
// Named on a line that RUNS it, not merely one that talks about it: these
// runners are shell scripts whose comments discuss the gate at length, and a
// surviving comment is exactly the evidence a deleted invocation leaves behind.
const invokes = existsSync(runner) &&
readFileSync(runner, 'utf8')
.split('\n')
.some((line) => line.includes(check) && !line.trim().startsWith('#'));
if (!existsSync(runner)) {
problems.push(` EXTERNAL_INPUT_REQUIRED says \`${check}\` runs via \`${runBy}\`, which does not exist.`);
} else if (!invokes) {
problems.push(
` EXTERNAL_INPUT_REQUIRED says \`${check}\` runs via \`${runBy}\`, which no longer invokes it.\n` +
` Either restore the call or reclassify: a gate that runs nowhere is the hole this category records (#4690).`,
);
}
}
for (const { gen } of UNGATED_GENERATORS) if (!scripts[gen]) problems.push(` UNGATED_GENERATORS names \`${gen}\`, which package.json no longer has.`);
for (const { gen, gatedBy } of EXPLICIT_GENERATORS) {
if (!scripts[gen]) problems.push(` EXPLICIT_GENERATORS names \`${gen}\`, which package.json no longer has.`);
Expand Down Expand Up @@ -277,8 +343,14 @@ if (reconcileOnly) {
console.log(
`✓ check:generated ledger reconciles with package.json: ${checks} check: + ${gens} gen: scripts, ` +
`all classified (${GATED.length} gated, ${NO_GENERATOR.length} source audits, ` +
`${EXTERNAL_INPUT_REQUIRED.length} needing an external input, ` +
`${UNGATED_GENERATORS.length} ungated generators, ${EXPLICIT_GENERATORS.length} explicit ` +
`manual-only generators, 1 aggregate).\n` +
// Named, not just counted: this bucket's whole reason for existing is that a
// bare count is what let #4690 read as "someone runs it".
EXTERNAL_INPUT_REQUIRED.map(
(e) => ` ⚠ cannot run here: ${e.check} — needs ${e.input}; runs in ${e.runBy}.\n`,
).join('') +
` --reconcile-only: no gates were run — this verifies coverage, not artifacts.`,
);
process.exit(0);
Expand Down Expand Up @@ -306,6 +378,15 @@ for (const entry of GATED) {
// Narrowing is never silent: say what was deliberately not run.
console.log(`\nNot run here (${NO_GENERATOR.length} source audits with no artifact to regenerate): ` +
NO_GENERATOR.map((n) => n.check).join(', '));
// Narrowing is never silent, part three — and this one is a different sentence:
// "deliberately not run" invites the reader to run it, which for these is not an
// option from this repo. Say what the missing input is and who supplies it.
if (EXTERNAL_INPUT_REQUIRED.length) {
console.log(`Cannot run here (${EXTERNAL_INPUT_REQUIRED.length} source audit(s) whose input this repo cannot produce):`);
for (const e of EXTERNAL_INPUT_REQUIRED) {
console.log(` ${e.check} — needs ${e.input}\n runs in ${e.runBy}; ${e.why}`);
}
}
if (UNGATED_GENERATORS.length) {
console.log(`Generated but ungated (${UNGATED_GENERATORS.length}): ` +
UNGATED_GENERATORS.map((u) => u.gen).join(', ') + ' — nothing verifies these are current.');
Expand Down
Loading
Loading