Skip to content

fix(spec,core): PluginSchema requires staticPath/slug for type "ui", and Plugin derives its metadata keys from PluginDefinition (#16334) - #16598

Merged
huangyiirene merged 2 commits into
mainfrom
claude/issue-16334-plugin-schema-ui-required
Sep 7, 2026
Merged

fix(spec,core): PluginSchema requires staticPath/slug for type "ui", and Plugin derives its metadata keys from PluginDefinition (#16334)#16598
huangyiirene merged 2 commits into
mainfrom
claude/issue-16334-plugin-schema-ui-required

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #16334

Spec half of the plugin-contract enforcement ruling (decision batch #58, 2026-09-06). #16049 (the engine half, landed as #16363) is not addressed here beyond what its boot path already surfaces: this PR makes PluginSchema say what its prose already promised, derives core's Plugin metadata keys from the schema, and touches nothing on kernel.use().

Base. Branched from origin/main at 90e7e6de114af2586da89dd417101b24db716d39 (PR #16554, the one-row-per-kind api-surface generator). Verified before generating anything: git merge-base --is-ancestor 90e7e6de114af2586da89dd417101b24db716d39 HEAD exit 0 (HEAD was that very commit). The regenerated shards are therefore in the post-#16554 format; export total 5411 before.

What changed

1. packages/spec/src/kernel/plugin.zod.tsPluginSchema.superRefine

  • staticPath and slug were described as (Required for type="ui") and declared .optional(). A superRefine now refuses a type: 'ui' plugin that omits either, or spells it undefined: one custom issue per missing key, path naming the key.
  • Stable code: new export PLUGIN_UI_REQUIRED_KEY_MISSING (the only api-surface row this PR adds). Carried in two places — at the HEAD of the issue message, the channel the boot path surfaces today (PluginLoader.validatePluginContract re-emits the first issue's path and message, and ObjectKernel.use() keeps only the message), and on the issue's params.code with params.key, zod's structured slot, which no reader consumes yet.
  • Scope is exact and pinned: every other declared type, and a plugin with no type, parses with neither key; a PRESENT value is judged by its own declaration (slug regex unchanged; staticPath still any string, empty string included — unchanged, not widened).
  • .describe() strings are byte-identical, so check:docs reads the same prose, which is now true.

2. packages/core/src/types.tsPlugin extends PluginDefinition

  • The nine PluginSchema keys (id, type, staticPath, slug, default, version, description, author, homepage) are inherited from PluginDefinition (z.input of PluginSchema), not restated; type and version are no longer spelled on the interface. Runtime-only members (name, the ADR-0116 ordering declarations, init / start / destroy) stay on the interface. PluginType is unchanged as an exported alias.
  • Pinned in packages/rest/src/plugin-type-closed-set.pin.test.ts, which compiles against core's built .d.ts: the inherited Plugin['type'], non-nullable, is type-equal to PluginType — added beside the existing parity pins; all four green.
  • PluginMetadata extends Plugin { version: string } still narrows the inherited optional version; core, rest and hono typecheck green (below).

3. The types.ts comment the card cites (:127, "refuses it at parse")

Fixture triage — the tightened schema now bites at kernel.use()

  • packages/core/src/plugin-contract-enforcement.test.ts: the ui fixtures (modern-ui, bad-slug / good-slug, the legacy twin) now carry staticPath + slug, so each refusal stays attributable to its own key. New group F pins the boot path SURFACING the spec code (at 'staticPath' / at 'slug' plus PLUGIN_UI_REQUIRED_KEY_MISSING inside the PLUGIN_CONTRACT_VIOLATION envelope), the calibration twin, the standard scope control, and that the two keys are members of Plugin. The Fixture widening alias is now a plain Plugin.
  • packages/core/src/plugin-type-closed-set.test.ts: the { type } fixture gets staticPath + slug for ui only (minimalLegal), so the pin keeps asking for a FULL parse per member — it guards the value set — without being re-read as a pin on the two keys.
  • packages/plugins/plugin-hono-server/src/ui-plugin-auto-discovery.pin.test.ts: two pins pinned exactly the branches the contract closes — "derives the slug when none is declared" and "a ui type with no staticPath mounts nothing". Both inverted into refusal pins at kernel.use() (the object never reaches kernel.plugins); the NON_UI_TYPES control still proves the harness can produce an empty result. hono-plugin.ts itself is untouched; its slug || name.split('/').pop() fallback is now unreachable through the kernel — an observation, not fixed here.

Restatements kept true — docblock and table only, no behaviour

  • packages/core/src/plugin-loader.ts, the validatePluginContract docblock: its "what this refuses" enumeration asks to be restated everywhere it is restated; one paragraph added for the conditional ui requirement. No code change on the boot path.
  • packages/runtime/src/dispatcher-error-vocabulary.ts: one row, PLUGIN_UI_REQUIRED_KEY_MISSING, door: 'none' / boot-refusal, shape objlitconst — the scanner sees the params: { code } site in plugin.zod.ts and requires a verdict. check:dispatcher-error-vocabulary: 74 sites found, 74 classified.

The default clause — deliberately not refined

The card allows a per-object refinement only if cheap; the documented semantics ("only one ui plugin can be default") are cross-object and stay the kernel's job. The only per-object reading available (default: true implies type: 'ui') is not what the prose promises, so nothing was added. The hono block reads default inside its ui branch only.

Acceptance

  • a ui plugin without staticPath or slug fails PluginSchema.safeParsepackages/spec/src/kernel/plugin-ui-required-keys.test.ts group A (5 cases) plus boot-path group F in core; negative control below.
  • Plugin metadata keys are derived, not restated — Plugin extends PluginDefinition; the type / version declarations are gone from the interface; the rest pin that the inherited type equals PluginType is green.
  • api-surface / declaration-map baselines regenerated with the repo tooling — pnpm --filter @objectstack/spec check:generated --fix regenerated exactly the two it proved stale (api-surface/kernel.json, export-origins/kernel.json); check:declaration-map reported current before and after, so there was nothing to regenerate there; clean re-run: "All 15 generated artifacts are up to date."
  • changeset present — .changeset/plugin-schema-ui-required-keys.md, spec and core minor (clause-② card: no patch on a package that grew), with the ADR-0087 not-required (no-migration-prescription) disposition; check:adr-0087-registration, check:changeset-no-major, check:empty-changeset green.

api-surface diff shape

  • ./kernel: +1 row, PLUGIN_UI_REQUIRED_KEY_MISSING (const); 0 removed, 0 modified. Every other entry point: 0 / 0 / 0.
  • Export total 5411 → 5412.
  • export-origins/kernel.json: +1 entry for the same symbol. No existing row removed or modified anywhere.

Evidence

Every build and test run went through scripts/pm/os-verify-lock.sh; each reading below is the wrapper's own VERDICT command-exit line or the gate's own printed verdict, captured by redirect-then-$?.

  • Build: pnpm --filter '@objectstack/core...' build (spec, its deps, core) — VERDICT command-exit 0. Rest and hono closures via turbo build --filter='@objectstack/rest^...' --filter='@objectstack/plugin-hono-server^...' — 24 / 24 successful.
  • Vitest, targeted with --maxWorkers=2: spec 4 files / 31 passed (the new pin file: 16 cases); core 3 files / 48 passed (plugin-contract-enforcement, plugin-type-closed-set, plugin-loader); hono ui-plugin-auto-discovery.pin 14 passed, 1 todo; rest plugin-type-closed-set.pin 4 passed.
  • Typecheck (pnpm --filter X typecheck, each including check:test-typecheck): spec, core, rest, plugin-hono-server — all exit 0. Direction: core is the producer of the widened Plugin; rest and hono are DOWNSTREAM consumers whose pins read core's built .d.ts or source. The remaining downstream consumers of Plugin (turbo lists 76 affected packages off the base) are declared to CI's required TypeScript Type Check job — the repo-wide sweep is CI's run, not reproduced locally.
  • Negative control (ablation), measured: with ONLY the superRefine guard deleted from the working copy (export kept, so the pin file still links) — mutation proven on disk by .superRefine((plugin, ctx) count 1 → 0, ctx.addIssue 1 → 0, blob 5e581efdf4bcbb, not HEAD's — the spec pin file reports exactly 5 failed | 11 passed: every group-A refusal case red, every B / C calibration and scope case green. Restore by git checkout HEAD -- ABSOLUTE_PATH under an EXIT / INT / TERM trap, proven by git diff HEAD --quiet exit 0, blob hash back to 5e581ef, porcelain empty. No build ran in the mutation window: the pin file reads ./plugin.zod from source, so the authorable-surface/api.json confound never applied. A first attempt of this leg was VOID by my own anchor (I counted the word superRefine, which the docblock also contains); the script refused to read a result and restored; re-anchored on the code spelling and re-run as above.
  • Gates run locally, each by its own verdict line: check:generated (15 / 15 up to date), check:dispatcher-error-vocabulary, check:error-code-casing (5757 files clean), check:nul-bytes plus a control-byte scan of every touched file (no hits), check:test-source-alias, check:cross-package-test-inputs, check:type-check-coverage (76 / 80 OK), check:closing-keyword-parity, spec check:dual-source-exports and check:exported-any, docs-audit check-affected-docs and check-drift-comment — all green.
  • NOT MEASURED locally: check:type-check-debt — exit 3 PREREQUISITE NOT MET (seven ledgered packages' deps are unbuilt here; CI builds the full closure first). Not a pass, not a finding.
  • The remaining families from scripts/pm/dispatch-gates.mjs --commands (repo-wide sweeps such as check:dts-closure, check:published-files, the census gates) are declared to CI's Lint & Repo Gates.

Open question for the reviewer — not blocking

params.code is the structured channel; the boot path today surfaces the code only through the message head. Whether the loader should read params.code and stamp a more specific err.code than PLUGIN_CONTRACT_VIOLATION is a decision on the boot path's side that I could not verify from here; the message-head contract works unchanged either way.

Observations, not fixed here — filed and linked from the report comment on #16334

#16554, #16363, #15638 and #16050 are context only; none is addressed here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno


Generated by Claude Code

…derives from PluginDefinition

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno
…N_UI_REQUIRED_KEY_MISSING

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/core, @objectstack/runtime, @objectstack/spec, touching 7 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/spec/api-surface/kernel.json, packages/spec/export-origins/kernel.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/getting-started/quick-reference.mdx (via PluginSchema (symbol, a top-level const))
  • content/docs/plugins/anatomy.mdx (via staticPath (literal, a string literal in PLUGIN_UI_REQUIRED_KEYS; a string literal in PluginLoader; a string literal in PluginSchema; a string literal on a changed line))
What this run could not see
  • 2 changed file(s) yielded no anchor (packages/spec/api-surface/kernel.json, packages/spec/export-origins/kernel.json) — pages documenting those are invisible to this run
  • 6 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 140 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ac9376a7428996bbfa9807cea823ae52c1879a1epackageMentionDocs.

Which tree this was computed on

This run read content/docs from cbb111ad171987ae625f61181f5d31647a06ed3b — the merge of head 8dc4a33040aeff2b2e173f318225f284af7a9a77 into base ac9376a7428996bbfa9807cea823ae52c1879a1e, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin cbb111ad171987ae625f61181f5d31647a06ed3b && git checkout cbb111ad171987ae625f61181f5d31647a06ed3b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ac9376a7428996bbfa9807cea823ae52c1879a1e 8dc4a33040aeff2b2e173f318225f284af7a9a77 && git checkout -B drift-repro ac9376a7428996bbfa9807cea823ae52c1879a1e && git merge --no-ff 8dc4a33040aeff2b2e173f318225f284af7a9a77

node scripts/docs-audit/affected-docs.mjs --json ac9376a7428996bbfa9807cea823ae52c1879a1e

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs ac9376a7428996bbfa9807cea823ae52c1879a1e → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

Contract review at CONTRACT_REVIEW_TIER (claude-fable-5-1) — PASS, no blocking findings

Anchors: head 8dc4a33040, base 90e7e6de11 (#16554merge-base --is-ancestor exit 0 and the merge-base with current origin/main is exactly that commit, measured). All measurement from a detached worktree; the shared primary checkout was never edited.

The four questions I could not decide without this review

1. The ADR-0087 disposition is honest in substance and vacuous in mechanism — and that distinction is the finding.

The changeset's **BREAKING** form is the recognised one: breakingDeclaration returns {breaking: true, signals: ["BREAKING"]}, with a bare BREAKING lit as a negative control. So the banner is right, and this is now the third PR today where that mattered.

But findMigrationPrescription returns null on the body — zero REWRITE_RE hits, no FROM/TO, no arrows. Control: the same "The fix for an affected plugin…" paragraph rewritten as **Migration:** \type: 'ui'` → …**is** caught. ⇒ theno-migration-prescription` green is a detector miss on an arrow-free instruction, the #8277 shape.

It is nonetheless the correct disposition, and this is why it is an advisory rather than a blocker: the claim it rests on is measured true — PluginSchema/plugin.zod appears nowhere in authorable-surface/api.json, its only runtime caller is plugin-loader.ts:522, no manifest in packages/apps/examples carries type: ui, and neither ADR-0087 registry references it. ADR-0087 D8 says in terms that the detector "cannot tell a prescription for a metadata upgrader from a prescription for a source-code consumer, and only the first is the ledger's business." This is the second kind. The other two categories are closed to it by their own conditions.

On the grade: with the carrier present, the gate itself forces minor — measured by driving judgeLevel/judge on the real changeset: as-is clean; spec or core at patch → exit 1; major → launch-window enforce. So minor is correct and now independently confirmed for the third time today.

2. The hono pin inversions pin the ruled behaviour, and they are lit. This was the highest-risk edit and it holds. Mutating the spec source to type !== 'app' — reaching the real kernel.use(), since hono's vitest aliases @objectstack/core and @objectstack/spec/* to source — makes exactly the two inverted pins fail with "expected kernel.use() to refuse the plugin, but it loaded" (2 failed / 12 passed / 1 todo), while all seven NON_UI_TYPES controls still pass. Pin 2 uses staticPath: undefined and asserts at 'staticPath', so it drives the explicit-undefined branch through the loader too. Changed because the code changed, not because they were failing.

3. Plugin extends PluginDefinition holds for consumers. The interface body is now only name, the four ordering arrays and init/start/destroy; the nine schema keys are the schema's. A compile probe passes 8 positives clean — including a ui Plugin literal carrying staticPath/slug, a class implements Plugin carrying them, and PluginDefinition assignable from a Plugin — and errors on exactly the 6 negatives, with both drifted Equal pins raising TS2322, so the helper is lit rather than decorative. The rest probe compiles against core's built dist/index.d.ts (interface Plugin extends PluginDefinition at :644), which is what a real consumer sees.

4. The card's piece 3 was genuinely stale — confirmed, with the removing commit named. git log -S"refuses it at parse" -- packages/core/src/types.ts returns exactly two commits: d8024f050e (#14608, introduced) and 2025b1f7ec (#16363, removed), and git show on the latter carries the - line. Lit control: the same grep at base still finds the string in rest/src/plugin-type-closed-set.pin.test.ts:7, so the search could match and the absence is a reading. The author edited nothing on that basis — correct.

Also measured

Scope is exactly as claimed: explicit undefined refused; '' for staticPath accepted; an invalid slug raises only its own invalid_format; null/number only invalid_type; all seven non-ui types and {} accept; one custom issue per key with path naming it. Four independent ablations, each landed by blob hash and restored to 5e581ef: neutralising the guard reproduces the author's 5-failed/11-passed by a different mutation; scoping to the wrong type lights the scope loop; merging the two issues into one fails the per-key pin. default untouched. hono-plugin.ts and kernel.ts diff-empty. api-surface a pure insertion — numstat 1/0 on two kernel shards, nothing else, totals 5411 → 5412, with check:generated 15/15 and tracked porcelain 0 after a real build.

Method note in the reviewer's favour

Its core group-F leg came back 20/20 green under a spec-source mutation — and it discarded that as void, because core's vitest resolves @objectstack/spec/kernel to dist, so the mutation never reached it. It said so and rested no verdict on it. A second run was also voided (a turbo rebuild wiped dist mid-probe) and re-done. That is the void-leg standard met twice, on top of the author's own self-caught void ablation earlier.

Advisories

  1. Acted on — the carrier. The PR carried no needs:contract-review label and its body has no legible Clause-② line, so readClause2Line returned null and CI's LEVEL AXIS was not-measured: the grade is right, but only the reviewer's offline drive said so. I have hung needs:contract-review on the PR so the gate actually judges it. That is the third carrier-legibility defect surfaced today and every one has been on my side of the line, not the devs'.
  2. The ADR-0087 gap is repo-level, not this PR's. There is no positively-verified category for "a Zod schema over never-stored runtime objects." This PR claims the only category open to it and its prose is true; minting a proper category is a follow-up worth filing, and I am recording it here rather than opening a card during wind-down.
  3. The empty-string staticPath promise is unpinned. The changeset and docblock promise '' is accepted, and a future widening that refuses it would leave all 16 spec pins green. One calibration case in group B closes it. ⛔ Not held for this: the promise is true today and measured, unlike the false claim that held feat(spec): declare the operator run-lifecycle verbs cancelRun and restoreConsumedSuspension on IAutomationService (contract half of the #13953 ruling) #16563 — an unpinned true statement is a weaker defect than a stated falsehood, and holding a fully-verified PR for it would be disproportionate.
  4. packages/runtime/src moved with no runtime changeset (vocabulary row); precedent is mixed and feat(core): enforce PluginSchema at kernel.use() (#16049) #16363 did not bump it either. Low.

Arming once CI re-runs Check Changeset with the carrier present — I want that gate to have actually asked.


Generated by Claude Code

@huangyiirene
huangyiirene marked this pull request as ready for review September 7, 2026 12:35
@huangyiirene
huangyiirene added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit 2a3decc Sep 7, 2026
40 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-16334-plugin-schema-ui-required branch September 7, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants