Automate external plugin adapter code generation with Kinobi renderer - #297
Open
nhanphan wants to merge 2 commits into
Open
Automate external plugin adapter code generation with Kinobi renderer#297nhanphan wants to merge 2 commits into
nhanphan wants to merge 2 commits into
Conversation
The ergonomic external-plugin-adapter layer (per-adapter `type`-discriminated
types, option-unwrapping `fromBase`/`initToBase`/`updateToBase`, manifests, and
the registry/dispatch) used to be hand-written — roughly one ~90-line file per
adapter plus coordinated edits across ~6 files for every new adapter, which is
how drift like the DataSection init-args mislabel and the irregular
`agentIdentities` plural crept in.
This adds a custom Kinobi renderer (configs/kinobiExternalPluginAdapters.cjs)
that derives the whole layer from the IDL node tree:
- the adapter set comes from the `externalPluginAdapter` enum,
- per-field transforms come from inspecting each `base*` defined type
(option-wrapping, substitutable leaf links, lifecycle-check tuples),
- a small OVERRIDES table captures only what the IDL cannot express
(which adapters carry inline `data`, the DataSection `dataAuthority`
derivation, redundant plugin-key fields, DataSection being non-updatable).
The renderer runs from configs/kinobi.cjs after the JS render and emits into
clients/js/src/generated/plugins. The hand-written src/plugins adapter files
become thin re-export shims that keep only genuinely custom logic
(findOracleAccount, deserializeOracleValidation, findExtraAccounts). Adding a
regular external plugin adapter now needs no JS changes at all — add the Rust
type and run `pnpm generate`.
Verification: whole-package typecheck, build and eslint pass; a runtime
old-vs-new diff over a fixture matrix shows every transformer and the registry
dispatch are byte-identical, and re-running the pipeline reproduces the output
exactly.
Behavioral note: the generator uses nullish coalescing (`?? null`) uniformly
for optional `schema`. The old LinkedLifecycleHook.initToBase used a truthy
check (`l.schema ? l.schema : null`) that silently dropped
`schema: Binary` (enum value 0) to null; the generated code now preserves it.
This is the only behavioral difference from the previous implementation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uxe3mDBDA2GiQdWvXpvnQE
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Summary by CodeRabbit
WalkthroughThe PR adds a Kinobi renderer for generated external plugin adapter modules, integrates it into JavaScript generation, and changes handwritten plugin files to re-export generated types, converters, manifests, and registry functions. ChangesExternal plugin adapter generation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant KinobiConfig
participant AdapterGenerator
participant GeneratedPlugins
participant PluginEntryPoints
KinobiConfig->>AdapterGenerator: pass IDL root and JavaScript output settings
AdapterGenerator->>GeneratedPlugins: generate and format adapter modules
GeneratedPlugins-->>KinobiConfig: return generated plugin paths
PluginEntryPoints->>GeneratedPlugins: re-export generated adapter APIs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: a925972 | Previous: 2181404 | Ratio |
|---|---|---|---|
CU: create a new, empty asset |
8200 Compute Units |
8200 Compute Units |
1 |
Space: create a new, empty asset |
91 Bytes |
91 Bytes |
1 |
CU: create a new, empty asset with empty collection |
17084 Compute Units |
17084 Compute Units |
1 |
Space: create a new, empty asset with empty collection |
91 Bytes |
91 Bytes |
1 |
CU: create a new asset with plugins |
26913 Compute Units |
26913 Compute Units |
1 |
Space: create a new asset with plugins |
194 Bytes |
194 Bytes |
1 |
CU: create a new asset with plugins and empty collection |
32366 Compute Units |
32366 Compute Units |
1 |
Space: create a new asset with plugins and empty collection |
194 Bytes |
194 Bytes |
1 |
CU: list an asset |
20269 Compute Units |
20269 Compute Units |
1 |
CU: sell an asset |
25146 Compute Units |
25146 Compute Units |
1 |
CU: list an asset with empty collection |
25177 Compute Units |
25177 Compute Units |
1 |
CU: sell an asset with empty collection |
33191 Compute Units |
33191 Compute Units |
1 |
CU: list an asset with collection royalties |
24294 Compute Units |
24294 Compute Units |
1 |
CU: sell an asset with collection royalties |
35962 Compute Units |
35962 Compute Units |
1 |
CU: transfer an empty asset |
3958 Compute Units |
3958 Compute Units |
1 |
CU: transfer an empty asset with empty collection |
5684 Compute Units |
5684 Compute Units |
1 |
CU: transfer an asset with plugins |
7271 Compute Units |
7271 Compute Units |
1 |
CU: transfer an asset with plugins and empty collection |
8997 Compute Units |
8997 Compute Units |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@configs/kinobiExternalPluginAdapters.cjs`:
- Line 562: Update the generated isExternalPluginAdapterType check to test
whether plugin.type is an own property of externalPluginAdapterManifests, rather
than using the prototype-inclusive in operator. Preserve true results for
supported manifest keys while returning false for inherited names such as
constructor, toString, and __proto__.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d9cabb36-5b58-418f-8848-cccf4863d0d9
⛔ Files ignored due to path filters (10)
clients/js/src/generated/plugins/agentIdentity.tsis excluded by!**/generated/**clients/js/src/generated/plugins/appData.tsis excluded by!**/generated/**clients/js/src/generated/plugins/base.tsis excluded by!**/generated/**clients/js/src/generated/plugins/dataSection.tsis excluded by!**/generated/**clients/js/src/generated/plugins/index.tsis excluded by!**/generated/**clients/js/src/generated/plugins/lifecycleHook.tsis excluded by!**/generated/**clients/js/src/generated/plugins/linkedAppData.tsis excluded by!**/generated/**clients/js/src/generated/plugins/linkedLifecycleHook.tsis excluded by!**/generated/**clients/js/src/generated/plugins/oracle.tsis excluded by!**/generated/**clients/js/src/generated/plugins/registry.tsis excluded by!**/generated/**
📒 Files selected for processing (10)
clients/js/src/plugins/agentIdentity.tsclients/js/src/plugins/appData.tsclients/js/src/plugins/dataSection.tsclients/js/src/plugins/externalPluginAdapters.tsclients/js/src/plugins/lifecycleHook.tsclients/js/src/plugins/linkedAppData.tsclients/js/src/plugins/linkedLifecycleHook.tsclients/js/src/plugins/oracle.tsconfigs/kinobi.cjsconfigs/kinobiExternalPluginAdapters.cjs
| ) | ||
| .join('\n')}\n};`; | ||
|
|
||
| const isType = `export const isExternalPluginAdapterType = (plugin: { type: string }) =>\n plugin.type in externalPluginAdapterManifests;`; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use an own-property check for adapter detection.
in accepts inherited names such as constructor, toString, and __proto__, so callers can receive true for unsupported adapter types. (tc39.es)
Proposed fix
- const isType = `export const isExternalPluginAdapterType = (plugin: { type: string }) =>\n plugin.type in externalPluginAdapterManifests;`;
+ const isType = `export const isExternalPluginAdapterType = (plugin: { type: string }) =>\n Object.prototype.hasOwnProperty.call(\n externalPluginAdapterManifests,\n plugin.type\n );`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const isType = `export const isExternalPluginAdapterType = (plugin: { type: string }) =>\n plugin.type in externalPluginAdapterManifests;`; | |
| const isType = `export const isExternalPluginAdapterType = (plugin: { type: string }) =>\n Object.prototype.hasOwnProperty.call( | |
| externalPluginAdapterManifests, | |
| plugin.type | |
| );`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@configs/kinobiExternalPluginAdapters.cjs` at line 562, Update the generated
isExternalPluginAdapterType check to test whether plugin.type is an own property
of externalPluginAdapterManifests, rather than using the prototype-inclusive in
operator. Preserve true results for supported manifest keys while returning
false for inherited names such as constructor, toString, and __proto__.
…manifest
The internal-plugin section of clients/js/src/plugins/types.ts (the `*Plugin`
aliases, the owner/authority/create-only V2 argument unions, and the
`*PluginsList` types) was hand-maintained: adding an internal plugin meant
editing it in ~6 places, which is how ordering drift crept in.
This generates that layer:
- The plugin set and whether each carries inline data come from the IDL
`Plugin` enum.
- The owner-managed / authority-managed / create-only split is derived from
the program itself. A new Rust example (`dump_plugin_manifest`) emits
configs/plugin-manifest.json from `PluginType::manager()` and the
`PERMANENT_DELEGATES` set, so the JS unions cannot drift from on-chain
creation rules. `pnpm generate:plugin-manifest` regenerates it and it is
wired into `pnpm generate`.
- The asset/collection/common list scope stays a small documented table in
the renderer, because the SDK's fetch-side lists are intentionally more
permissive than the program's create-time rules (e.g. owner-managed plugins
are listed as common).
types.ts becomes a thin shim that re-exports the generated `internal.ts` and
keeps only the deprecated v1 `CreatePluginArgs` and the foundational
`BasePlugin`. lib.ts is left as-is: its `createPluginV2`/`mapPlugin` default
branches already handle new plugins without edits, so it does not grow per
plugin.
Verified: the generated unions/lists are mutually assignable with the previous
hand-written types (temporary `Eq<>` assertion), and typecheck, build and
eslint pass across the package.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uxe3mDBDA2GiQdWvXpvnQE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a custom Kinobi renderer that automatically generates the ergonomic external plugin adapter wrapper layer, eliminating the need for hand-written per-adapter files. Previously, adding a new external plugin adapter required coordinated edits across ~6 files (~90 lines each). Now, regular adapters require no manual changes—just add the Rust type and re-run
pnpm generate.Key Changes
New Kinobi Renderer (
configs/kinobiExternalPluginAdapters.cjs): A 730-line custom renderer that:externalPluginAdapterenum in the IDLbase*defined type to determine per-field transformsOVERRIDEStable to capture IDL-inexpressible details (inline data fields, redundant plugin-key fields, DataSectiondataAuthorityderivation, non-updatable adapters)Generated Plugin Files (moved to
clients/js/src/generated/plugins/):base.ts: Base types and interfaces for all adaptersregistry.ts: Central registry with adapter manifests and conversion functionslifecycleHook.ts,oracle.ts,appData.ts,linkedLifecycleHook.ts,linkedAppData.ts,dataSection.ts,agentIdentity.ts*ToBase/*FromBasetransform functionsSimplified Hand-Written Files (
clients/js/src/plugins/):lifecycleHook.ts,oracle.ts,appData.ts,linkedLifecycleHook.ts,linkedAppData.ts,dataSection.ts,agentIdentity.ts: Now contain only runtime logic (e.g., Oracle validation serialization, data parsing)externalPluginAdapters.ts: Drastically simplified to import from generated registry and provide high-level utilitiesKinobi Configuration (
configs/kinobi.cjs): Added custom renderer invocation to generate the adapter layer during the standardpnpm generateworkflowImplementation Details
Benefits
https://claude.ai/code/session_01Uxe3mDBDA2GiQdWvXpvnQE