Skip to content

refactor(appkit): add defineManifest to remove manifest casts - #485

Open
MarioCadenas wants to merge 1 commit into
mainfrom
refactor/define-manifest
Open

refactor(appkit): add defineManifest to remove manifest casts#485
MarioCadenas wants to merge 1 commit into
mainfrom
refactor/define-manifest

Conversation

@MarioCadenas

@MarioCadenasMarioCadenas commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

What

Introduces defineManifest() in the registry so plugins stop hand-casting their JSON manifests. All 10 plugins now declare:

staticmanifest=defineManifest<"my-plugin">(manifest);

instead of static manifest = manifest as PluginManifest<...> (agents needed the uglier as unknown as PluginManifest).

It's also exported from @databricks/appkit and emitted by appkit plugin create, so external plugins get the same validated, cast-free path.

Why

The cast was unavoidable with a raw .json import:

  • TS widens every JSON field to string, but PluginManifest.resources[].type is the nominal ResourceType enum — a structural string never assigns to it.
  • Multi-resource plugins (only agents today) infer a heterogeneous union for resources, which a plain as also rejects (hence agents' as unknown as).

So the escape hatch wasn't a plugin bug — it was the JSON→type boundary being crossed by assertion in every plugin, with no runtime validation.

How

defineManifest() parses the manifest through the canonical pluginManifestSchema (Zod) and returns the strict type via one audited internal assertion, after parse has confirmed the values are real ResourceType/permission strings. This:

  • removes every per-plugin cast (one bridge assertion, centralized),
  • adds real runtime validation at module load that the old cast never provided — including the camelCase PLUGIN_NAME_PATTERN,
  • preserves the literal plugin name via <TName> (needed — toPlugin derives the typed plugin key from manifest.name; widening to string would collapse the typed registry).

pluginManifestSchema is now exported from shared for the parse.

Public surface

  • defineManifest is exported from the @databricks/appkit root so external plugin authors can use it (previously it lived only in the internal registry barrel).
  • The appkit plugin create scaffold now emits defineManifest<"name">(manifest) instead of manifest as PluginManifest<...>, so newly scaffolded plugins get runtime validation by default.
  • TypeDoc API reference regenerated (new Function.defineManifest page; Variable.agents now shows the restored literal key ToPlugin<..., "agents"> instead of ..., string>).

Verification

  • shared + appkit typecheck — clean
  • Registry + plugins + plugin-CLI suites — 1311 pass, 1 skipped (every real manifest parses at module load)
  • New unit tests: valid pass-through + rejection of unknown resource type, invalid permission, and missing required field
  • Scaffold test updated to assert the defineManifest output
  • oxlint + oxfmt — clean on changed files (no new warnings vs. main)

Split out of the MLflow-tracing PR (pr/agent-evals-1-tracing), which is where this cast friction first surfaced.

@MarioCadenas
MarioCadenas requested a review from a team as a code ownerJuly 20, 2026 16:58
@github-actions

Copy link
Copy Markdown
Contributor

🔬 Run evals on this PR · Go to Evals Monitor →

@github-actions

github-actionsBot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle size report

Compared against bundle-size-baseline.json (main).

@databricks/appkit

npm tarball (packed): 862 KB (+22 KB) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

distrawgzip
JS (runtime)899 KB (+30 KB)311 KB (+8.5 KB)
Type declarations316 KB (+1.2 KB)109 KB (+497 B)
Source maps1.7 MB (+58 KB)580 KB (+14 KB)
Other11 KB3.7 KB
Total2.9 MB (+89 KB)1005 KB (+23 KB)
Per-entry composition (own code — deps external (as shipped))
EntryInitial (gz)Lazy (gz)Total (gz)node_modules (min)Own code (min)
.93 KB (+4.9 KB)2.5 KB96 KB (+4.9 KB)external306 KB (+17 KB)
./beta54 KB (+5.1 KB)457 B54 KB (+5.1 KB)external161 KB (+18 KB)
./type-generator21 KB0 B21 KBexternal61 KB

Chunks:

EntryChunkLoadSize (gz)
.index.jsinitial89 KB
.utils.jsinitial4.0 KB
.remote-tunnel-manager.jslazy2.5 KB
./betabeta.jsinitial38 KB
./betastream-manager.jsinitial5.8 KB
./betawide-event-emitter.jsinitial3.2 KB
./betadatabricks.jsinitial3.0 KB
./betaconfiguration.jsinitial2.1 KB
./betaservice-context.jsinitial1.3 KB
./betaclient.jsinitial434 B
./betaclient-options.jsinitial220 B
./betasupervisor-api.jslazy192 B
./betadatabricks.jslazy142 B
./betaindex.jslazy123 B
./type-generatorindex.jsinitial21 KB

@databricks/appkit-ui

npm tarball (packed): 342 KB (-281 B) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

distrawgzip
JS (runtime)390 KB130 KB (+1 B)
Type declarations228 KB (+32 B)83 KB (+9 B)
Source maps752 KB (-334 B)247 KB (-197 B)
CSS16 KB (-462 B)3.2 KB (-90 B)
Total1.4 MB (-764 B)464 KB (-277 B)
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
EntryInitial (gz)Lazy (gz)Total (gz)node_modules (min)Own code (min)
./js5.3 KB49 KB55 KB208 KB14 KB
./js/beta20 B0 B20 B0 B0 B
./react432 KB (+127 B)49 KB480 KB (+127 B)1.3 MB175 KB
./react/beta1.0 KB0 B1.0 KB0 B1.9 KB

Chunks:

EntryChunkLoadSize (gz)
./jsindex.jsinitial5.2 KB
./jschunkinitial120 B
./jsapache-arrowlazy49 KB
./js/betabeta.jsinitial20 B
./reactindex.jsinitial430 KB
./reacttslibinitial2.1 KB
./reactapache-arrowlazy49 KB
./react/betabeta.jsinitial1.0 KB

@MarioCadenas
MarioCadenas marked this pull request as draft July 21, 2026 08:17
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has had no activity for 23 days and has been marked as stale. It will be closed in 7 days if there is no further activity. Add a comment, push a commit, or apply the no-stale label to keep it open.

@github-actions

github-actionsBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

🤖 AppKit PR bot

🔬 Run evals

Start an eval for this PR from the evals-monitor app: Go to Evals Monitor →

📦 Try this PR's app template

Scaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh auth login — and the Databricks CLI):

gh run download 32349898864 -R databricks/appkit -n appkit-template-0.61.1-pr.495ea7a-refactor-define-manifest-485 -D appkit-pr-485 \
&& unzip -o "appkit-pr-485/appkit-template-0.61.1-pr.495ea7a-refactor-define-manifest-485.zip" -d "appkit-pr-485" \
&& databricks apps init --template "appkit-pr-485"

The template pins @databricks/appkit and @databricks/appkit-ui to tarballs built from this branch, so the scaffolded app runs against this PR's code.

@MarioCadenas
MarioCadenasforce-pushed the refactor/define-manifest branch 2 times, most recently from 219b61c to da0c99fCompareAugust 19, 2026 16:36
@MarioCadenas
MarioCadenas marked this pull request as ready for review August 19, 2026 16:36
Every plugin declared `static manifest = manifest as PluginManifest`
(agents needed `as unknown as`). The cast is unavoidable with a raw JSON
import: TS widens JSON fields to `string`, but PluginManifest.resources[].type
is the nominal ResourceType enum, so the structural shape never assigns —
and multi-resource plugins infer a heterogeneous union that a plain `as`
also rejects.
Add defineManifest() in the registry: it parses the JSON through the
canonical pluginManifestSchema (real runtime validation, which the loader
did not do before) and returns the strict type via one audited internal
assertion. All 9 plugins now use `static manifest = defineManifest<"name">(manifest)`
with no local casts. Export pluginManifestSchema from shared for the parse.
Adds tests covering valid pass-through and rejection of unknown resource
type, invalid permission, and missing required fields.
Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
@MarioCadenas
MarioCadenasforce-pushed the refactor/define-manifest branch from da0c99f to c153608CompareAugust 20, 2026 08:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@MarioCadenas