feat: support grpc plugins with ts and bun #2293
Conversation
SkArchon
requested review from
a team,
JivusAyrus,
Noroth,
StarpTech,
devsergiy,
endigma and
jensneuse
as code owners
WalkthroughAdds TypeScript/Bun plugin support and language-aware CLI/toolchain flows: detects plugin language, splits dependency install/test/build and gRPC codegen per language, separates Go and TS templates, adds a template compiler, updates platform/tool mappings for Bun, and updates CI to install Bun and compile templates. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas needing extra attention:
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Router image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/commands/router/commands/plugin/commands/init.ts (1)
147-151: Apply language fallback when unsupported language is detectedThe guard at line 65 logs a fallback message but does not actually update
options.language. Add the assignment after the condition:if (options.language !== 'go' || options.language !== 'ts') { spinner.fail(pc.yellow(`Language '${options.language}' is not supported yet. Using 'go' instead.`)); + options.language = 'go'; }This ensures both the switch statement template selection (line 87) and the result display (line 147) reflect the fallback language.
🧹 Nitpick comments (20)
demo/pkg/subgraphs/student/.cursor/rules/plugin-development.mdc (1)
43-43: Clarify guidance on build verification.Line 43 states "Don't verify if the binary was created. The build command will take care of that," but this is ambiguous. Developers may be uncertain whether this means:
- The build will silently succeed even if the binary is missing
- Errors will be visible in build output
- They should trust the exit code
Consider clarifying the intent:
-6. Your job is done after successfully building the plugin. Don't verify if the binary was created. The build command will take care of that. +6. Your job is done after the build completes successfully. The build command will fail with an error if the binary cannot be created, so you can trust the exit code.cli/student/.cursor/rules/plugin-development.mdc (1)
1-85: Consider consolidating duplicate documentation.Both
.cursor/rules/plugin-development.mdcfiles (this one anddemo/pkg/subgraphs/student/.cursor/rules/plugin-development.mdc) contain identical content except for the directory paths. This duplication creates a maintenance burden: if the guidance needs to be updated, both files must be modified in sync to avoid divergence.Consider options:
- Move to a shared location: Store a single source-of-truth guide in a central docs directory and reference it from both
.cursor/rules/locations via symlink or inclusion.- Auto-generate: If these files are generated by a CLI command, ensure the generation logic produces consistent output across multiple plugin directories.
- Document the policy: If duplication is intentional (e.g., each plugin directory is self-contained), add a comment at the top of each file stating "This file is intentionally duplicated in [other locations] for local IDE guidance. Keep in sync when updating."
cli/student/Makefile (1)
2-2: Add standard phony targets "all" and "clean", and avoid shadowing "make" target.The Makefile should declare the standard phony targets "all" and "clean" per convention. Additionally, renaming the "make" target (line 7) to something like "default" or "setup" would avoid shadowing the built-in Make command. Consider:
-.PHONY: build test generate install-wgc +.PHONY: all build test generate install-wgc clean +all: build + +clean: + rm -rf dist/ node_modules/ + -make: build +default: buildcli/student/README.md (1)
23-34: Address markdown lint and align docs with TS/Bun scaffolding.
- Add a language to fenced code (MD040).
- Paths/instructions appear Go‑only; include TS/Bun equivalents and correct folder to
cli/student/if applicable.- ``` + ```text plugins/student/ @@ - │ ├── main.go # Main plugin implementation - │ ├── main_test.go # Tests for the plugin - │ └── schema.graphql # GraphQL schema defining the API + │ ├── main.go / plugin.ts # Go or TS implementation + │ ├── *_test.go / *.spec.ts # Tests + │ └── schema.graphql # GraphQL schema @@ - └── bin/ # Compiled binaries (created during build) - └── plugin # The compiled plugin binary + └── bin/ # Build output + └── plugin / dist # Go binary or TS build artifactsAlso extend “Customizing” with TS/Bun:
- TS:
bun install,bun run build, run client/server examples.- Go:
make generate && make test && make build.Also applies to: 38-41
demo/pkg/subgraphs/student/src/schema.graphql (1)
1-17: Confirm composition intent for Query.If this schema composes with other subgraphs, prefer
extend type Queryto avoid redefining the root. If it’s a standalone demo, current form is fine. Consider adding federation directives later ifWorldbecomes an entity.demo/pkg/subgraphs/student/src/plugin.ts (2)
3-6: Remove unused imports
pathandfileURLToPatharen’t used.-import * as path from 'path'; -import { fileURLToPath } from 'url';
63-64: Nit: prefer template stringsReadability/consistency.
- world.setId(`world-`+counter); - world.setName(`Hello There, `+ name); + world.setId(`world-${counter}`); + world.setName(`Hello There, ${name}`);demo/pkg/subgraphs/student/Makefile (1)
2-19: Make the default target build, add all/clean, and drop the “make” aliasCurrent first target (
install-wgc) runs on plainmake. Add anallalias, set the default goal, provideclean, and avoid global installs where possible.-.PHONY: build test generate install-wgc +.PHONY: all build test generate publish install-wgc clean +.DEFAULT_GOAL := build install-wgc: - @which wgc > /dev/null 2>&1 || npm install -g wgc@latest + @command -v wgc >/dev/null 2>&1 || npx -y wgc@latest --version >/dev/null -make: build +all: build test: install-wgc wgc router plugin test . generate: install-wgc wgc router plugin generate . -publish: generate +publish: build wgc router plugin publish . build: install-wgc wgc router plugin build . --debug + +clean: + @rm -rf dist .wgc || trueRun checkmake again to confirm warnings are resolved.
cli/student/src/plugin.ts (6)
3-6: Clean up unused imports and derived variables
path,fileURLToPath,__filename,__dirnameare unused; removing them also satisfies the import-order lint failures.-import * as grpc from '@grpc/grpc-js'; -import * as path from 'path'; -import { fileURLToPath } from 'url'; +import * as grpc from '@grpc/grpc-js'; @@ -// Get the directory of the current module -const __filename = fileURLToPath(import.meta.url); -const __dirname = path.dirname(__filename);Also applies to: 18-21
26-41: Logger doesn’t log — write to stderr to avoid polluting stdout handshakeEnsure logs go to stderr; keep stdout reserved for the go-plugin handshake line.
class Logger { log(level: string, message: string): void { const timestamp = new Date().toISOString(); + // Emit to stderr to not interfere with go-plugin stdout handshake + console.error(`[${timestamp}] [${level}] ${message}`); }
55-56: Remove placeholder text in greetingLooks accidental.
- world.setName(`Hello Thereeeee 17 17 17 17 17 17 17 17 1777 `+ name); + world.setName(`Hello There, ${name}`);
96-99: Fix typo and surface startup errors
s/serverL/server/and ensure the message is visible.- logger.error(`Failed to start serverL `+ error.message); + logger.error(`Failed to start server: ${error.message}`);
3-3: Satisfy lint: prefer node: specifier and import ordering (if you keep these imports)If you decide to keep
path/url, follow the lints: usenode:pathdefault import and place@grpc/grpc-jsafterurl.-import * as grpc from '@grpc/grpc-js'; -import * as path from 'path'; -import { fileURLToPath } from 'url'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; +import * as grpc from '@grpc/grpc-js';Also applies to: 4-4
60-62: Use template strings for readability- logger.info("Returning world: id="+world.getId()+", name="+world.getName()+", counter="+counter); + logger.info(`Returning world: id=${world.getId()}, name=${world.getName()}, counter=${counter}`);cli/src/commands/router/commands/plugin/templates/goplugin.ts (1)
15-19: Prefer Register{serviceName}Server for consistency with tests and idiomatic gRPCUse the generated registrar helper instead of
RegisterServicewithServiceDesc.- pl, err := routerplugin.NewRouterPlugin(func(s *grpc.Server) { - s.RegisterService(&service.{serviceName}_ServiceDesc, &{serviceName}{ - nextID: 1, - }) - }, routerplugin.WithTracing()) + pl, err := routerplugin.NewRouterPlugin(func(s *grpc.Server) { + service.Register{serviceName}Server(s, &{serviceName}{ + nextID: 1, + }) + }, routerplugin.WithTracing())cli/src/commands/router/commands/plugin/commands/init.ts (1)
85-97: Minor: add missing break and keep cases symmetricalNot strictly required (last case), but keeps future edits safe.
case 'ts': { await writeFile(resolve(srcDir, 'plugin.ts'), pupa(TsTemplates.pluginTs, { serviceName })); await writeFile(resolve(srcDir, 'client.ts'), pupa(TsTemplates.clientTs, { serviceName })); await writeFile(resolve(tempDir, 'package.json'), pupa(TsTemplates.packageJson, { serviceName })); + break; }cli/src/commands/router/commands/plugin/templates/tsplugin.ts (2)
97-104: Template polish: overly noisy greeting.The hardcoded “Hello Thereeeee 17…” looks accidental. Keep the template clean and predictable.
- world.setName(`Hello Thereeeee 17 17 17 17 17 17 17 17 1777 `+ name); + world.setName(`Hello ${name}`);
146-167: package.json template: versions reasonable; consider pinning @types/bun.To improve reproducibility, avoid "latest" on @types/bun.
- "@types/bun": "latest", + "@types/bun": "^1.1.10",cli/src/commands/router/commands/plugin/toolchain.ts (2)
248-296: Tool install checks always include Go; make language-aware to avoid unnecessary installs for TS-only builds.TS path only needs protoc; prompting to install Go increases friction.
- Accept a language argument in checkAndInstallTools and conditionally verify/install Go toolchain only when language === 'go'.
- Alternatively, split into checkAndInstallGoTools() and checkAndInstallCommonTools().
456-485: TS codegen path: robust; minor Windows chmod caveat.Good validation and plugin wiring. Note chmod on Windows is a no-op; you can guard by process.platform !== 'win32' to avoid confusing logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
cli/student/bun.lockis excluded by!**/*.lockcli/student/generated/mapping.jsonis excluded by!**/generated/**cli/student/generated/service.protois excluded by!**/generated/**cli/student/generated/service.proto.lock.jsonis excluded by!**/generated/**cli/student/generated/service_grpc_pb.d.tsis excluded by!**/generated/**cli/student/generated/service_grpc_pb.jsis excluded by!**/generated/**cli/student/generated/service_pb.d.tsis excluded by!**/generated/**cli/student/generated/service_pb.jsis excluded by!**/generated/**demo/pkg/subgraphs/student/bun.lockis excluded by!**/*.lockdemo/pkg/subgraphs/student/generated/mapping.jsonis excluded by!**/generated/**demo/pkg/subgraphs/student/generated/service.protois excluded by!**/generated/**demo/pkg/subgraphs/student/generated/service.proto.lock.jsonis excluded by!**/generated/**demo/pkg/subgraphs/student/generated/service_grpc_pb.d.tsis excluded by!**/generated/**demo/pkg/subgraphs/student/generated/service_grpc_pb.jsis excluded by!**/generated/**demo/pkg/subgraphs/student/generated/service_pb.d.tsis excluded by!**/generated/**demo/pkg/subgraphs/student/generated/service_pb.jsis excluded by!**/generated/**
📒 Files selected for processing (29)
cli/src/commands/router/commands/plugin/commands/build.ts(3 hunks)cli/src/commands/router/commands/plugin/commands/generate.ts(2 hunks)cli/src/commands/router/commands/plugin/commands/init.ts(2 hunks)cli/src/commands/router/commands/plugin/templates/goplugin.ts(1 hunks)cli/src/commands/router/commands/plugin/templates/plugin.ts(0 hunks)cli/src/commands/router/commands/plugin/templates/tsplugin.ts(1 hunks)cli/src/commands/router/commands/plugin/toolchain.ts(5 hunks)cli/student/.cursor/rules/plugin-development.mdc(1 hunks)cli/student/.cursorignore(1 hunks)cli/student/.gitignore(1 hunks)cli/student/Dockerfile(1 hunks)cli/student/Makefile(1 hunks)cli/student/README.md(1 hunks)cli/student/package.json(1 hunks)cli/student/src/client.ts(1 hunks)cli/student/src/plugin.ts(1 hunks)cli/student/src/schema.graphql(1 hunks)demo/Makefile(2 hunks)demo/graph-with-plugin.yaml(1 hunks)demo/pkg/subgraphs/student/.cursor/rules/plugin-development.mdc(1 hunks)demo/pkg/subgraphs/student/.cursorignore(1 hunks)demo/pkg/subgraphs/student/.gitignore(1 hunks)demo/pkg/subgraphs/student/Dockerfile(1 hunks)demo/pkg/subgraphs/student/Makefile(1 hunks)demo/pkg/subgraphs/student/README.md(1 hunks)demo/pkg/subgraphs/student/package.json(1 hunks)demo/pkg/subgraphs/student/src/plugin.ts(1 hunks)demo/pkg/subgraphs/student/src/schema.graphql(1 hunks)router/demo.config.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- cli/src/commands/router/commands/plugin/templates/plugin.ts
🧰 Additional context used
🧬 Code graph analysis (6)
cli/student/src/client.ts (2)
cli/student/generated/service_grpc_pb.d.ts (1)
StudentServiceClient(36-41)cli/student/generated/service_pb.d.ts (1)
QueryHelloRequest(9-21)
cli/src/commands/router/commands/plugin/commands/generate.ts (1)
cli/src/commands/router/commands/plugin/toolchain.ts (5)
getLanguage(668-682)installTsDependencies(531-545)generateProtoAndMapping(382-427)generateGRPCCode(432-489)installGoDependencies(517-529)
cli/src/commands/router/commands/plugin/commands/init.ts (1)
protographic/src/index.ts (2)
compileGraphQLToMapping(16-32)compileGraphQLToProto(53-79)
cli/student/src/plugin.ts (2)
cli/student/generated/service_grpc_pb.d.ts (2)
IStudentServiceServer(26-28)StudentServiceService(24-24)cli/student/generated/service_pb.d.ts (3)
QueryHelloRequest(9-21)QueryHelloResponse(29-44)World(52-66)
demo/pkg/subgraphs/student/src/plugin.ts (2)
demo/pkg/subgraphs/student/generated/service_grpc_pb.d.ts (2)
StudentServiceService(24-24)IStudentServiceServer(26-28)demo/pkg/subgraphs/student/generated/service_pb.d.ts (3)
QueryHelloRequest(9-21)QueryHelloResponse(29-44)World(52-66)
cli/src/commands/router/commands/plugin/commands/build.ts (1)
cli/src/commands/router/commands/plugin/toolchain.ts (7)
getLanguage(668-682)installTsDependencies(531-545)normalizePlatforms(646-666)generateGRPCCode(432-489)installGoDependencies(517-529)buildGoBinaries(598-641)buildTsBinaries(550-593)
🪛 checkmake (0.2.2)
demo/pkg/subgraphs/student/Makefile
[warning] 2-2: Missing required phony target "all"
(minphony)
[warning] 2-2: Missing required phony target "clean"
(minphony)
[warning] 7-7: Target "make" should be declared PHONY.
(phonydeclared)
cli/student/Makefile
[warning] 2-2: Missing required phony target "all"
(minphony)
[warning] 2-2: Missing required phony target "clean"
(minphony)
[warning] 7-7: Target "make" should be declared PHONY.
(phonydeclared)
🪛 GitHub Actions: wgc CI
cli/src/commands/router/commands/plugin/toolchain.ts
[error] 550-550: ESLint: Block must not be padded by blank lines. (padded-blocks). Lint step failed during 'eslint --cache --ext .ts,.mjs,.cjs .'
🪛 GitHub Check: build_test_node_matrix (20.x)
cli/student/src/client.ts
[failure] 38-38:
Prefer top-level await over using a promise chain
[failure] 9-9:
Async function 'run' has no 'await' expression
[failure] 7-7:
Relative imports and exports must end with .js
[failure] 6-6:
Relative imports and exports must end with .js
cli/src/commands/router/commands/plugin/toolchain.ts
[failure] 550-550:
Block must not be padded by blank lines
cli/student/src/plugin.ts
[failure] 4-4:
Prefer node:path over path
[failure] 4-4:
Use default import for module path
[failure] 3-3:
@grpc/grpc-js import should occur after import of url
🪛 GitHub Check: build_test_node_matrix (22.x)
cli/student/src/client.ts
[failure] 38-38:
Prefer top-level await over using a promise chain
[failure] 9-9:
Async function 'run' has no 'await' expression
[failure] 7-7:
Relative imports and exports must end with .js
[failure] 6-6:
Relative imports and exports must end with .js
cli/src/commands/router/commands/plugin/toolchain.ts
[failure] 550-550:
Block must not be padded by blank lines
cli/student/src/plugin.ts
[failure] 4-4:
Prefer node:path over path
[failure] 4-4:
Use default import for module path
[failure] 3-3:
@grpc/grpc-js import should occur after import of url
🪛 markdownlint-cli2 (0.18.1)
demo/pkg/subgraphs/student/README.md
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
cli/student/README.md
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: build-router
- GitHub Check: build_push_image
- GitHub Check: integration_test (./events)
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: image_scan
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
router/demo.config.yaml (1)
22-22: YAML list indentation corrected.The indentation fix aligns the Redis URL with proper YAML list item formatting under the
urlskey. The change is syntactically correct and improves consistency.cli/student/.cursorignore (1)
1-7: Verify that ignore patterns are also in.gitignore.The
.cursorignorefile is specific to the Cursor IDE and won't be respected by other tools (Git, other IDEs, CI/CD systems). Ensure that these ignore patterns are also present in the appropriate.gitignorefile so that generated artifacts and binaries are properly excluded from version control and other workflows.demo/pkg/subgraphs/student/.gitignore (1)
1-2: Consistent with cli/student/.gitignore pattern.The ignore rule is correct and mirrors the pattern in the parallel CLI file, suggesting a consistent build artifact structure across the plugin scaffolding.
cli/student/Dockerfile (1)
1-22: Dockerfile structure is sound, but clarify multi-language coexistence.The Dockerfile is well-structured for cross-platform Go builds with appropriate use of build cache and scratch image. However, its placement in cli/student/ alongside the Bun package.json (lines 1–22) creates ambiguity. The package.json and Dockerfile both output to
dist/pluginbut use different build systems. Ensure the intended build flow is documented and that both configurations don't conflict or cause confusion about which language/tool to use.demo/graph-with-plugin.yaml (1)
47-50: Configuration looks good.The new student subgraph declaration follows the existing pattern and is properly formatted. Version and path are consistent with the demo/pkg/subgraphs/student/ structure.
cli/student/src/schema.graphql (1)
1-17: Schema design is appropriate for the example.The GraphQL schema is well-structured with clear field documentation. The World type and Query.hello field provide a clean demo implementation. No issues identified.
demo/Makefile (1)
11-11: Integration of student subgraph follows appropriate patterns.The additions to plugin-build (line 11), plugin-generate (line 15), and plugin-build-integration (line 27) correctly mirror the projects subgraph workflow. The absence of
--go-module-pathfor the student subgraph is appropriate since it targets Bun/TypeScript rather than Go. The changes are consistent with the multi-language plugin support introduced in this PR.Also applies to: 15-15, 27-27
cli/src/commands/router/commands/plugin/templates/tsplugin.ts (1)
2-43: Client template: OK to ship.Flows, error handling, and shutdown look fine for a starter template.
cli/src/commands/router/commands/plugin/commands/build.ts (1)
47-62: Language-aware flow looks good; minor cleanups verified as valid.Branching per language, TS deps before codegen, and separate build paths are sound. Verification confirms both optional suggestions:
getHostPlatformis imported at line 13 but never used in build.ts—safe to removeoptions.platformfrom commander's--platform [platforms...]with default[]is alwaysstring[](line 65 passes it tonormalizePlatforms(platforms: string[], ...)), so type safety is guaranteed
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/commands/router/commands/plugin/templates/go.ts (1)
15-63: Fix generated package import to use the module pathBoth
mainGoandmainTestGohard-codeservice "github.com/wundergraph/cosmo/plugin/generated", but the scaffold setsmodule {modulePath}dynamically. Any plugin whose module path differs from that fixed string fails to build because the import cannot be resolved. Please make the import depend on{modulePath}in both places so the generated project always compiles.- service "github.com/wundergraph/cosmo/plugin/generated" + service "{modulePath}/generated" @@ - service "github.com/wundergraph/cosmo/plugin/generated" + service "{modulePath}/generated"Also applies to: 101-198
🧹 Nitpick comments (4)
cli/src/commands/router/commands/plugin/templates/typescript/package.json.template (1)
17-17: Inconsistent version pinning strategy.The
@protocolbuffers/protoc-gen-jsdependency is pinned to an exact version (4.0.0), while other dependencies use caret ranges (^). Consider using a consistent versioning strategy across all dependencies, or document why specific packages require exact pinning.cli/src/commands/router/commands/plugin/templates/project/Makefile.template (1)
1-23: LGTM! Consider adding error handling for the default target.The Makefile structure is sound with proper target dependencies. The conditional logic in the download target correctly prevents redundant downloads.
For improved robustness, consider adding
.ONESHELL:and explicit error handling to the default target so failures in intermediate steps halt the chain:+.ONESHELL: .PHONY: install-wgc build download start compose -make: install-wgc download build compose start +make: install-wgc download build compose start || { echo "Build failed"; exit 1; }cli/src/commands/router/commands/plugin/toolchain.ts (1)
472-487: Consider consolidating protoc plugin path resolution.The TypeScript protoc plugin path checks are repetitive. Consider extracting a helper function to reduce duplication:
function getProtocPluginPath(pluginDir: string, pluginName: string): string { const pluginPath = resolve(pluginDir, `node_modules/.bin/${pluginName}`); if (!existsSync(pluginPath)) { throw new Error(`${pluginName} not found at ${pluginPath}.`); } return pluginPath; } // Usage: const protocGenTsPath = getProtocPluginPath(pluginDir, 'protoc-gen-ts'); const protocGenGrpcPath = getProtocPluginPath(pluginDir, 'grpc_tools_node_protoc_plugin'); const protoGenJsPath = getProtocPluginPath(pluginDir, 'protoc-gen-js');demo/pkg/subgraphs/courses/src/schema.graphql (1)
13-14: Clarify purpose of test/utility query fields.The
killCoursesServiceandthrowErrorCoursesfields appear to be demo/test utilities. For a demo schema this is acceptable, but consider adding a comment or grouping them to clarify their intent if they will be part of the federated graph beyond development.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
demo/pkg/subgraphs/courses/bun.lockis excluded by!**/*.lockdemo/pkg/subgraphs/courses/generated/mapping.jsonis excluded by!**/generated/**demo/pkg/subgraphs/courses/generated/service.protois excluded by!**/generated/**demo/pkg/subgraphs/courses/generated/service.proto.lock.jsonis excluded by!**/generated/**demo/pkg/subgraphs/courses/generated/service_grpc_pb.d.tsis excluded by!**/generated/**demo/pkg/subgraphs/courses/generated/service_grpc_pb.jsis excluded by!**/generated/**demo/pkg/subgraphs/courses/generated/service_pb.d.tsis excluded by!**/generated/**demo/pkg/subgraphs/courses/generated/service_pb.jsis excluded by!**/generated/**
📒 Files selected for processing (42)
cli/courses/src/plugin.test.ts(1 hunks)cli/package.json(1 hunks)cli/src/commands/router/commands/plugin/commands/init.ts(3 hunks)cli/src/commands/router/commands/plugin/commands/test.ts(3 hunks)cli/src/commands/router/commands/plugin/templates/go.ts(1 hunks)cli/src/commands/router/commands/plugin/templates/go/Dockerfile.template(1 hunks)cli/src/commands/router/commands/plugin/templates/go/cursor_rules.template(1 hunks)cli/src/commands/router/commands/plugin/templates/go/go.mod.template(1 hunks)cli/src/commands/router/commands/plugin/templates/go/main.go.template(1 hunks)cli/src/commands/router/commands/plugin/templates/go/main_test.go.template(1 hunks)cli/src/commands/router/commands/plugin/templates/go/readme.partial.md.template(1 hunks)cli/src/commands/router/commands/plugin/templates/plugin.ts(1 hunks)cli/src/commands/router/commands/plugin/templates/plugin/.gitignore.template(1 hunks)cli/src/commands/router/commands/plugin/templates/plugin/Makefile.template(1 hunks)cli/src/commands/router/commands/plugin/templates/plugin/Readme.plugin.md.template(1 hunks)cli/src/commands/router/commands/plugin/templates/plugin/cursorignore.template(1 hunks)cli/src/commands/router/commands/plugin/templates/plugin/schema.graphql.template(1 hunks)cli/src/commands/router/commands/plugin/templates/project.ts(1 hunks)cli/src/commands/router/commands/plugin/templates/project/.gitignore.template(1 hunks)cli/src/commands/router/commands/plugin/templates/project/Makefile.template(1 hunks)cli/src/commands/router/commands/plugin/templates/project/Readme.plugin.md.template(1 hunks)cli/src/commands/router/commands/plugin/templates/project/Readme.project.md.template(1 hunks)cli/src/commands/router/commands/plugin/templates/project/graph.yaml.template(1 hunks)cli/src/commands/router/commands/plugin/templates/project/router_config.yaml.template(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript.ts(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/Dockerfile.template(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/cursor_rules.template(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/fs-polyfill.ts.template(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/package.json.template(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/plugin.test.ts.template(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/plugin.ts.template(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/readme.partial.md.template(1 hunks)cli/src/commands/router/commands/plugin/toolchain.ts(8 hunks)demo/Makefile(2 hunks)demo/graph-with-plugin.yaml(1 hunks)demo/pkg/subgraphs/courses/package.json(1 hunks)demo/pkg/subgraphs/courses/src/fs-polyfill.ts(1 hunks)demo/pkg/subgraphs/courses/src/plugin-server.ts(1 hunks)demo/pkg/subgraphs/courses/src/plugin.test.ts(1 hunks)demo/pkg/subgraphs/courses/src/plugin.ts(1 hunks)demo/pkg/subgraphs/courses/src/schema.graphql(1 hunks)demo/pkg/subgraphs/courses/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- demo/pkg/subgraphs/courses/package.json
- cli/src/commands/router/commands/plugin/templates/project/.gitignore.template
🚧 Files skipped from review as they are similar to previous changes (4)
- cli/src/commands/router/commands/plugin/templates/plugin/Makefile.template
- cli/package.json
- cli/src/commands/router/commands/plugin/templates/typescript/plugin.ts.template
- cli/src/commands/router/commands/plugin/templates/go/main.go.template
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-20T22:13:25.222Z
Learnt from: StarpTech
Repo: wundergraph/cosmo PR: 2157
File: router-tests/go.mod:16-16
Timestamp: 2025-08-20T22:13:25.222Z
Learning: github.com/mark3labs/mcp-go v0.38.0 has regressions and should not be used in the wundergraph/cosmo project. v0.36.0 is the stable version that should be used across router-tests and other modules.
Applied to files:
cli/src/commands/router/commands/plugin/templates/go/go.mod.template
📚 Learning: 2025-09-24T12:54:00.765Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2222
File: router-tests/websocket_test.go:2238-2302
Timestamp: 2025-09-24T12:54:00.765Z
Learning: The wundergraph/cosmo project uses Go 1.25 (Go 1.23+ minimum), so fmt.Appendf and other newer Go standard library functions are available and can be used without compatibility concerns.
Applied to files:
cli/src/commands/router/commands/plugin/templates/go/go.mod.template
📚 Learning: 2025-09-24T12:54:00.765Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2222
File: router-tests/websocket_test.go:2238-2302
Timestamp: 2025-09-24T12:54:00.765Z
Learning: The wundergraph/cosmo project uses Go 1.25 (Go 1.25 minimum), so fmt.Appendf and other newer Go standard library functions are available and can be used without compatibility concerns.
Applied to files:
cli/src/commands/router/commands/plugin/templates/go/go.mod.template
📚 Learning: 2025-08-20T21:05:58.731Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2158
File: router/pkg/grpcconnector/grpccommon/grpc_plugin_client.go:149-153
Timestamp: 2025-08-20T21:05:58.731Z
Learning: In the Cosmo codebase, the GRPCPluginClient struct fields `tracer` and `getTraceAttributes` are always properly initialized during construction and will never be nil, so defensive nil checks are not needed.
Applied to files:
cli/src/commands/router/commands/plugin/templates/go/go.mod.template
📚 Learning: 2025-08-28T09:17:49.477Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2141
File: router-tests/http_subscriptions_test.go:17-55
Timestamp: 2025-08-28T09:17:49.477Z
Learning: The Cosmo router uses a custom, intentionally rigid multipart implementation for GraphQL subscriptions. The multipart parsing in test files should remain strict and not be made more tolerant, as this rigidity is by design.
Applied to files:
cli/src/commands/router/commands/plugin/templates/project/Readme.project.md.templatecli/src/commands/router/commands/plugin/templates/project/Readme.plugin.md.template
📚 Learning: 2025-10-01T20:39:16.113Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2252
File: router-tests/telemetry/telemetry_test.go:9684-9693
Timestamp: 2025-10-01T20:39:16.113Z
Learning: Repo preference: In router-tests/telemetry/telemetry_test.go, keep strict > 0 assertions for request.operation.*Time (parsingTime, normalizationTime, validationTime, planningTime) in telemetry-related tests; do not relax to >= 0 unless CI flakiness is observed.
Applied to files:
cli/src/commands/router/commands/plugin/templates/go/main_test.go.template
📚 Learning: 2025-09-08T20:57:07.946Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Applied to files:
demo/pkg/subgraphs/courses/tsconfig.json
🧬 Code graph analysis (5)
demo/pkg/subgraphs/courses/src/plugin.test.ts (1)
demo/pkg/subgraphs/courses/generated/service_pb.d.ts (20)
QueryCoursesRequest(202-212)QueryCoursesResponse(219-233)QueryCourseRequest(241-253)QueryCourseResponse(261-276)QueryLessonsRequest(284-296)QueryLessonsResponse(304-318)QueryKillCoursesServiceRequest(326-336)QueryKillCoursesServiceResponse(343-355)QueryThrowErrorCoursesRequest(363-373)QueryThrowErrorCoursesResponse(380-392)MutationAddCourseRequest(400-414)MutationAddCourseResponse(423-438)MutationAddLessonRequest(446-462)MutationAddLessonResponse(472-487)LookupCourseByIdRequest(30-44)LookupCourseByIdRequestKey(10-22)LookupCourseByIdResponse(52-66)LookupEmployeeByIdRequest(158-172)LookupEmployeeByIdRequestKey(138-150)LookupEmployeeByIdResponse(180-194)
cli/courses/src/plugin.test.ts (1)
demo/pkg/subgraphs/courses/generated/service_grpc_pb.d.ts (1)
CoursesServiceClient(163-195)
cli/src/commands/router/commands/plugin/commands/test.ts (1)
cli/src/commands/router/commands/plugin/toolchain.ts (6)
getLanguage(723-737)checkAndInstallTools(265-312)installGoDependencies(551-563)installTsDependencies(565-579)runGoTests(512-530)runTsTests(532-546)
cli/src/commands/router/commands/plugin/commands/init.ts (1)
protographic/src/index.ts (2)
compileGraphQLToMapping(16-32)compileGraphQLToProto(53-79)
demo/pkg/subgraphs/courses/src/plugin.ts (2)
demo/pkg/subgraphs/courses/generated/service_pb.d.ts (23)
Course(495-523)Employee(576-592)Lesson(535-563)QueryCoursesRequest(202-212)QueryCoursesResponse(219-233)QueryCourseRequest(241-253)QueryCourseResponse(261-276)QueryLessonsRequest(284-296)QueryLessonsResponse(304-318)QueryKillCoursesServiceRequest(326-336)QueryKillCoursesServiceResponse(343-355)QueryThrowErrorCoursesRequest(363-373)QueryThrowErrorCoursesResponse(380-392)MutationAddCourseRequest(400-414)MutationAddCourseResponse(423-438)MutationAddLessonRequest(446-462)MutationAddLessonResponse(472-487)LookupCourseByIdRequest(30-44)LookupCourseByIdResponse(52-66)LookupLessonByIdRequest(94-108)LookupLessonByIdResponse(116-130)LookupEmployeeByIdRequest(158-172)LookupEmployeeByIdResponse(180-194)demo/pkg/subgraphs/courses/src/plugin-server.ts (1)
PluginServer(13-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: build-router
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./events)
- GitHub Check: build_push_image
- GitHub Check: image_scan
- GitHub Check: build_test
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (35)
cli/src/commands/router/commands/plugin/templates/project/Readme.plugin.md.template (1)
1-35: Template implementation is correct; all placeholders and documentation links verified.All original concerns have been successfully validated:
- Placeholder consistency — The template uses
{name},{readmeText}, and{mainFile}, which match exactly the keys injected by thepupa()call ininit.ts(asname,readmeText, andmainFile: mainFileName).- Documentation links — Both
https://cosmo-docs.wundergraph.com/andhttps://cosmo-docs.wundergraph.com/connect/pluginsare publicly accessible and valid.- Language-specific guidance — The
{mainFile}placeholder is correctly handled by the template injection logic, which dynamically provides the appropriate file name (e.g.,.tsfor TypeScript,.gofor Go) from the calling code.The template file exists at the correct path and its content matches the review snippet exactly.
cli/src/commands/router/commands/plugin/templates/typescript/package.json.template (2)
9-9: Postinstall script executes code from node_modules—verify safety.The postinstall script runs
bun ./node_modules/@protocolbuffers/protoc-gen-js/download-protoc-gen-js.js, which executes code from a dependency. While this is the intended usage pattern for this package, ensure that the dependency is from a trusted source and consider the supply chain security implications.
14-14: No known security vulnerabilities found for grpc-health-check 2.0.0.Verification using the GitHub security API confirmed no registered vulnerabilities for this package. While newer versions (2.0.1, 2.0.2, 2.1.0) are available, updating is not required for security reasons. The current pinning to 2.0.0 is acceptable from a security standpoint.
cli/src/commands/router/commands/plugin/commands/test.ts (1)
37-74: LGTM! Language-aware test workflow implemented correctly.The implementation properly detects the plugin language and routes to the appropriate test runner (Go vs TypeScript/Bun). The control flow is clear, error handling is consistent, and the result propagation works correctly.
cli/src/commands/router/commands/plugin/templates/typescript/Dockerfile.template (1)
1-29: LGTM! Multi-platform Bun build implementation is sound.The Dockerfile correctly implements multi-platform builds with proper TARGETOS/TARGETARCH handling. The dynamic BUN_TARGET computation appropriately maps amd64 to x64 for Bun's naming conventions. The use of scratch for the final image minimizes the attack surface.
cli/src/commands/router/commands/plugin/commands/init.ts (1)
64-127: LGTM! Language-aware plugin initialization implemented correctly.The refactored initialization flow properly:
- Validates and normalizes language selection with appropriate fallback
- Generates shared artifacts (schema, mapping, proto) before language-specific code
- Uses clean switch-based branching for Go vs TypeScript file generation
- Correctly references template properties (e.g.,
cursorRulesnotgoCursorRules)The previous issues flagged in past reviews have been addressed.
cli/src/commands/router/commands/plugin/toolchain.ts (4)
20-26: LGTM! Host platform detection correctly handles language differences.The
getHostPlatformfunction appropriately prefixes Bun platforms withbun-for TypeScript builds and returns plain platform strings for Go. This aligns with the platform mapping strategies used throughout the file.
449-507: LGTM! Language-aware GRPC code generation implemented correctly.The switch-based language branching properly handles:
- Go: Standard protoc with go and go-grpc plugins
- TypeScript: Multiple protoc plugins (ts, grpc, js) with proper path validation
The TypeScript path defensively checks for plugin existence before invoking protoc, which prevents cryptic errors.
602-649: LGTM! TypeScript binary build implementation is correct.The Bun-based build flow properly:
- Maps Bun platform names to Go-style platform-arch for router compatibility
- Uses Bun's
--targetflag for cross-compilation- Handles errors with the correct variable reference (
originalPlatformArch)- Supports debug mode with sourcemaps
Previous issues flagged in past reviews have been resolved.
723-737: LGTM! Language detection strategy is pragmatic.The synchronous file existence checks for
go.modvspackage.jsonprovide a simple, reliable language detection mechanism. The TODO to make this async is noted but not critical for the current use case since these checks are fast.demo/graph-with-plugin.yaml (1)
47-50: LGTM! New courses subgraph follows established pattern.The new subgraph entry correctly mirrors the structure of the existing "projects" plugin-based subgraph with appropriate versioning and path configuration.
cli/src/commands/router/commands/plugin/templates/go/go.mod.template (1)
1-11: LGTM! Go module template uses correct toolchain version.The template correctly specifies Go 1.25.1, which aligns with the project's minimum Go version requirements (Go 1.25+). The router-plugin dependency appropriately uses a commit hash with a semantic version comment for traceability.
Based on learnings
cli/src/commands/router/commands/plugin/templates/plugin/.gitignore.template (1)
1-2: LGTM!Simple and appropriate.
cli/src/commands/router/commands/plugin/templates/go/readme.partial.md.template (1)
1-16: LGTM!Clear, well-structured documentation of the Go plugin layout. Conventions match standard practice.
cli/src/commands/router/commands/plugin/templates/project/graph.yaml.template (1)
1-6: LGTM!Correct YAML structure with appropriate placeholder for plugin name substitution.
demo/pkg/subgraphs/courses/tsconfig.json (1)
1-12: LGTM!TypeScript config is well-suited for Bun: uses
"moduleResolution": "Bundler"(correct for Bun), ESNext modules, strict type checking, and includes Bun typings. Good configuration.demo/Makefile (2)
15-21: Verify courses build works without language-specific flags.Lines 17 and 21 build the courses subgraph without the
--go-module-pathflag used for projects (line 16, 20). Confirm the language-aware toolchain correctly detects courses as TypeScript and handles it appropriately.
30-33: LGTM!The plugin-build-integration target correctly mirrors the projects pattern for the new courses subgraph. Clean and consistent.
cli/src/commands/router/commands/plugin/templates/plugin/cursorignore.template (1)
1-7: LGTM!Appropriate ignore rules for generated artifacts and binaries. Well-commented.
cli/src/commands/router/commands/plugin/templates/typescript/fs-polyfill.ts.template (1)
1-34: LGTM!Well-designed polyfill with robust error handling. Dual-path patching (globalThis and require) and graceful degradation with optional debug logging are appropriate for this workaround. Comments clearly explain the Bun/protobufjs issue being addressed.
Confirm that this polyfill resolves the protobufjs null reference error you mentioned in the PR when bundled with Bun.
cli/src/commands/router/commands/plugin/templates/plugin/Readme.plugin.md.template (1)
1-35: LGTM!Well-written documentation template with clear explanation of the GraphQL + gRPC plugin pattern. Placeholders (
{name},{readmeText},{mainFile}) are appropriately positioned for language-aware substitution.Verify that template expansion correctly replaces all three placeholders (
{name},{readmeText},{mainFile}) in the code generation pipeline.cli/src/commands/router/commands/plugin/templates/typescript/readme.partial.md.template (1)
1-17: LGTM: Clear and well-structured plugin documentation.The TypeScript plugin structure documentation is clear and comprehensive, accurately describing the expected directory layout with all necessary components (package.json, source files, generated code, and binaries).
cli/src/commands/router/commands/plugin/templates/project/router_config.yaml.template (1)
1-15: LGTM: Appropriate router configuration defaults.The router configuration template provides sensible defaults for plugin development (localhost:3010, dev mode enabled, plugins path configured). The yaml-language-server directive is a nice touch for IDE support.
cli/src/commands/router/commands/plugin/templates/go/Dockerfile.template (2)
3-22: LGTM: Well-structured multi-platform build.The multi-platform build setup with
TARGETOSandTARGETARCHis correct, and the use ofCGO_ENABLED=0with a scratch final image produces a minimal, portable binary. The build cache mount on Line 16 is a good optimization.
1-1: No issues found. Go 1.25 is now available.Go 1.25.1 (released September 3, 2025) is the latest stable version as of November 2025, making the Dockerfile's
golang:1.25-alpinespecification correct and current. The original concern about Go 1.25 not being released is no longer valid.cli/src/commands/router/commands/plugin/templates/go/cursor_rules.template (1)
1-85: LGTM: Comprehensive Go plugin development guide.This cursor rules template provides excellent guidance for Go plugin development with clear structure, workflow steps, and practical examples. The inclusion of httpclient usage patterns (Lines 59-85) and warnings about not editing generated files (Line 45) are particularly helpful.
demo/pkg/subgraphs/courses/src/plugin.test.ts (4)
28-53: LGTM: Well-designed test helpers.The
createMockCallandcreateMockCallbackhelpers provide a clean abstraction for testing gRPC handlers with promise-based async/await patterns. The promise-based callback wrapper (Lines 35-53) elegantly handles both success and error cases.
57-140: LGTM: Comprehensive query test coverage.The query tests thoroughly validate:
- Listing all courses with field-level assertions
- Fetching single courses by ID with nested relationships (instructor, lessons)
- Lesson queries with proper filtering
- Edge cases (killCoursesService with mocked process.exit, error throwing)
The use of
vi.spyOnto mockprocess.exit(Line 121) prevents actual test termination while validating the behavior.
144-181: LGTM: Mutation tests validate data creation.Both mutation tests (
addCourseandaddLesson) properly verify:
- Created entities have expected field values
- ID generation (Line 158 confirms generated ID follows expected pattern)
- Relationships are established correctly (lesson.getCourse() at Line 180)
185-230: LGTM: Lookup tests validate batch operations.The lookup tests properly exercise batch entity resolution:
- Multiple course lookups by ID return correct results in order
- Employee lookups include federated relationships (taughtCourses)
This validates the federation lookup pattern expected by the router.
cli/src/commands/router/commands/plugin/templates/project/Readme.project.md.template (1)
1-91: LGTM: Excellent project documentation template.This README template provides comprehensive onboarding for plugin developers with:
- Clear feature highlights (GraphQL + gRPC, embedded subgraphs, type safety)
- Visual project structure (Lines 16-25)
- Well-documented Makefile commands (Lines 34-44)
- Practical quick-start example (Lines 46-59)
- Step-by-step customization guide (Lines 73-82)
The documentation is clear, actionable, and user-friendly.
cli/src/commands/router/commands/plugin/templates/plugin/schema.graphql.template (1)
1-17: LGTM: Clean starter schema.The GraphQL schema template provides a simple, well-documented starting point with:
- A basic
Worldtype with required fields- A
helloquery demonstrating parameter passing- Proper field descriptions using GraphQL description syntax
This serves as an appropriate minimal example for plugin template generation.
cli/src/commands/router/commands/plugin/templates/typescript/cursor_rules.template (1)
1-114: LGTM: Comprehensive TypeScript plugin development guide.This cursor rules template provides excellent guidance for TypeScript plugin development with:
- Clear plugin structure and workflow (Lines 14-46)
- Practical HTTP integration examples using fetch API (Lines 62-90)
- gRPC service implementation patterns with generated types (Lines 96-113)
- Appropriate warnings about not editing generated files (Line 48)
The template properly uses placeholders that will be substituted during generation (the {serviceName} placeholder issue noted in past reviews was already addressed).
demo/pkg/subgraphs/courses/src/schema.graphql (2)
1-2: Well-structured Apollo Federation 2.5 schema setup.The federation link and key directives are correctly configured. All entity types properly include
@keydirectives, enabling proper federation routing.
45-48: Verify ID type consistency across federated entities.
Employee.idis typed asInt!, whileCourse.idandLesson.idareID!. IfEmployeeis extended from another subgraph (suggested by the comment "Fields resolved by this subgraph"), this inconsistency may be intentional. However, confirm this is the desired design, especially sinceaddCoursemutation referencesinstructorId: Int!which aligns withEmployee.id.
cli/src/commands/router/commands/plugin/templates/go/main_test.go.template
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cli/src/commands/router/commands/plugin/templates/typescript.ts (1)
14-15: Critical path resolution bug in grpcHealthCheckFilePatch template.The
grpcHealthCheckFilePatchconstant contains the same path resolution bug identified in the source template filegrpc_health_check_file_patch.template: the leading slashes in theprotoPathresolution will cause paths to resolve from the filesystem root instead of relative to the executable directory.Since this file is auto-generated, the fix must be applied to the source template at:
cli/src/commands/router/commands/plugin/templates/typescript/grpc_health_check_file_patch.templateAfter fixing the source template, regenerate this file using the template compilation script.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cli/src/commands/router/commands/plugin/commands/init.ts(3 hunks)cli/src/commands/router/commands/plugin/templates/typescript.ts(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/Dockerfile.template(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/grpc_health_check_file_patch.template(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/package.json.template(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cli/src/commands/router/commands/plugin/templates/typescript/Dockerfile.template
- cli/src/commands/router/commands/plugin/templates/typescript/package.json.template
🧰 Additional context used
🧬 Code graph analysis (1)
cli/src/commands/router/commands/plugin/commands/init.ts (1)
protographic/src/index.ts (2)
compileGraphQLToMapping(16-32)compileGraphQLToProto(53-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: build-router
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./events)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: image_scan
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_test
🔇 Additional comments (6)
cli/src/commands/router/commands/plugin/templates/typescript.ts (1)
1-3: Note: Auto-generated file - do not edit manually.This file is auto-generated (line 2), so any changes should be made to the source template files under
cli/src/commands/router/commands/plugin/templates/typescript/. The build process will regenerate this file from those templates.cli/src/commands/router/commands/plugin/commands/init.ts (5)
15-16: LGTM!The template imports are correctly added to support multi-language plugin generation.
64-68: LGTM!The language validation logic correctly uses
&&and sets the fallback to'go'when an unsupported language is provided. The past issue has been properly addressed.
96-132: LGTM!The language-specific generation is well-structured with clear separation between Go and TypeScript paths. The TypeScript case comprehensively includes all necessary files (plugin files, package.json, tsconfig, Dockerfile, tests, patches). The past cursor rules property name issue has been properly resolved.
134-169: LGTM!The project vs non-project mode handling is well-organized. The use of different README templates (
ProjectTemplates.readmePluginMdvsPluginTemplates.readmePluginMd) appropriately distinguishes between the two modes, and both paths now consistently use the same parameters (mainFile,readmeText), which improves maintainability.
74-88: ****The
PluginTemplates.schemaGraphqltemplate is a static GraphQL schema with no placeholders. The template content is:type World { ... } type Query { hello(name: String!): World! }Since the schema template contains no interpolation placeholders, the
pupa()call at line 74 is harmless—it simply returns the template unchanged. There is no inconsistency between the written schema file and the compiled mapping/proto artifacts. All three usages (lines 74, 75, 82) operate on identical schema content, making the suggested refactoring unnecessary.Likely an incorrect or invalid review comment.
...c/commands/router/commands/plugin/templates/typescript/grpc_health_check_file_patch.template
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cli/src/commands/router/commands/plugin/toolchain.ts (2)
20-26: Critical: Windows platform mismatch still present.
os.platform()returns'win32'on Windows, butALL_BUN_PLATFORM_MAPPINGS(line 45) expects'bun-windows-x64'. This causesbuildTsBinaries()(line 622) to fail the lookup and throw an error. This issue was flagged in a previous review but remains unresolved.Apply this fix to normalize the platform name:
export function getHostPlatform(language: string) { - const basePlatform = `${os.platform()}-${getOSArch(language)}`; + const nodePlatform = os.platform(); + const normalizedOS = nodePlatform === 'win32' ? 'windows' : nodePlatform; + const basePlatform = `${normalizedOS}-${getOSArch(language)}`; if (language === 'ts') { return `bun-${basePlatform}`; } return basePlatform; }
602-658: Major: Missing Windows .exe extension and unsafe path construction.Two issues remain from the previous review:
- Line 633: Windows binaries require a
.exeextension, which is currently missing.- Line 641: Path construction uses string concatenation instead of
join(binDir, binaryName), which is unsafe across platforms.Apply this fix:
- const binaryName = `${platform}_${arch}`; + const binaryName = `${platform}_${arch}${platform === 'windows' ? '.exe' : ''}`; spinner.text = `Building ${originalPlatformArch}...`; const flags = [ 'build', 'src/plugin.ts', '--compile', '--outfile', - `bin/${binaryName}`, + join(binDir, binaryName), `--target=${originalPlatformArch}`, ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cli/src/commands/router/commands/plugin/toolchain.ts(9 hunks)
🔇 Additional comments (9)
cli/src/commands/router/commands/plugin/toolchain.ts (9)
28-48: LGTM: Platform constants and mappings are well-structured.The Bun platform mappings comprehensively cover all target platforms and the musl preference is correctly documented.
50-92: LGTM: Tool configuration and architecture mapping.The language-aware architecture resolution correctly handles Node.js's
x64vs Go'samd64naming difference. The hardcoded commit hash in the install script URL is appropriate for ensuring build stability.
449-507: LGTM: Language-aware gRPC code generation.The switch-based approach cleanly separates Go and TypeScript protoc invocations. The TypeScript case appropriately validates plugin existence before proceeding, and all paths use
resolve()for safety.
581-597: LGTM: TypeScript type-checking implementation.The function correctly uses
bun tsc --noEmitto validate types without generating output. Note: line 590's comment mentions "bun x tsc" but the actual command is just "bun tsc" (line 591), which works because bun automatically resolves TypeScript.
660-706: LGTM: Go binary building is correctly implemented.The function properly uses
join()for path construction, sets appropriate cross-compilation environment variables, and handles platform/architecture parsing safely. Note that Go's compiler automatically adds.exefor Windows, so explicit extension handling is not needed here.
711-730: LGTM: Platform normalization logic is correct.The function correctly handles default platform selection via
getHostPlatform()and merges language-specific platform sets whenallPlatformsis true. The previous review's shadowing concern was invalid—the parameterallPlatforms: booleanis not shadowed byALL_BUN_PLATFORMS, which is a separate constant.
732-746: LGTM: Language detection is straightforward and functional.The function correctly detects language by checking for
go.modorpackage.json. The TODO comment about making it async is noted, but the current synchronous approach is acceptable for this use case since it's a simple file existence check during CLI operations.
611-618: LGTM: Health proto workaround correctly implemented.This addresses the protobuf.js/Bun bundling issue mentioned in the PR objectives. By copying the health proto file from
node_modulesto thebindirectory, it ensures runtime availability when the proto is dynamically loaded. The implementation safely handles nested directory creation and uses proper path utilities.
154-205: LGTM: Version checking correctly validates all tools including Bun.The Bun version check (lines 194-198) correctly uses
bunVersionin the comparison at line 195. The previously flagged bug wheregoVersionwas mistakenly checked has been resolved.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cli/src/commands/router/commands/plugin/templates/typescript.ts(1 hunks)cli/src/commands/router/commands/plugin/templates/typescript/grpc_health_check_file_patch.template(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/src/commands/router/commands/plugin/templates/typescript.ts
...c/commands/router/commands/plugin/templates/typescript/grpc_health_check_file_patch.template
Outdated
Show resolved
Hide resolved
...c/commands/router/commands/plugin/templates/typescript/grpc_health_check_file_patch.template
Outdated
Show resolved
Hide resolved
…ort-grpc-plugins-with-bun-typescript
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
cli/src/commands/router/commands/plugin/toolchain.ts (2)
600-656:buildTsBinarieslogic looks correct; consider usingbinDirfor--outfile.The Bun build flow (platform mapping, health proto copy,
--targetusage, and debug sourcemap flag) looks correct, and the error message now correctly referencesoriginalPlatformArch.Minor nit: you compute
binDir = resolve(pluginDir, 'bin'), but still hardcode the outfile path asbin/${binaryName}. For consistency with the Go build path handling and to avoid repeating the"bin"literal, consider:- const binaryName = `${platform}_${arch}`; + const binaryName = `${platform}_${arch}`; // ... - '--outfile', - `bin/${binaryName}`, + '--outfile', + join('bin', binaryName),This is purely a readability/maintainability improvement; behavior is unchanged given
cwd: pluginDir.
20-26: Fix Windows host platform normalization ingetHostPlatform.On Windows,
os.platform()returns"win32", but your Go and Bun platform catalogs use"windows-*"(ALL_GO_PLATFORMSandALL_BUN_PLATFORM_MAPPINGS). As written,getHostPlatform('go')will returnwin32-amd64andgetHostPlatform('ts')will returnbun-win32-x64, which will not match your mappings and will break default platform resolution and builds on Windows hosts.Normalize the OS name before composing
basePlatform:-export function getHostPlatform(language: string) { - const basePlatform = `${os.platform()}-${getOSArch(language)}`; +export function getHostPlatform(language: string) { + const nodePlatform = os.platform(); + const normalizedOS = nodePlatform === 'win32' ? 'windows' : nodePlatform; + const basePlatform = `${normalizedOS}-${getOSArch(language)}`; if (language === 'ts') { return `bun-${basePlatform}`; } return basePlatform; }This makes host values like
windows-amd64andbun-windows-x64consistent withALL_GO_PLATFORMSandALL_BUN_PLATFORM_MAPPINGS.
🧹 Nitpick comments (4)
cli/src/commands/router/commands/plugin/toolchain.ts (4)
66-72: Bun tool version wiring looks correct; clarify whether Bun is required for Go-only plugins.The new
bunentry inTOOL_VERSIONSand the Bun version check inareToolsInstalledOnHostare wired correctly and use the right variable (bunVersion) for semver comparison.One design question: with this change, a missing or incompatible Bun on the host will force
checkAndInstallTools()to reinstall the toolchain even for Go-only plugins. If the intent is that Bun is now a hard requirement for all router plugin development, this is fine. If not, consider gating the Bun check based on the plugin language at the call site so Go-only users aren’t forced through a Bun install unnecessarily.Also applies to: 194-198
449-506: Handle unsupported languages explicitly ingenerateGRPCCode.The Go and TS branches look solid, and the TS branch’s checks for required protoc plugins are good. However, if
languageis something other than'go'or'ts', the function silently does nothing after printing “Generating gRPC code…”.For consistency with
normalizePlatformsandgetLanguage, consider adding an explicit default case:switch (language) { case 'go': { // ... break; } case 'ts': { // ... break; } + default: { + throw new Error(`Unsupported language for gRPC generation: ${language}`); + } }This will fail fast instead of silently skipping code generation.
579-595: AligntypeCheckTsimplementation with the comment (bun tsc vs bun x tsc).The comment says “Use
bun x tscto ensure TypeScript is available without requiring it as a dependency”, but the implementation runs:await execa(bunPath, ['tsc', '--noEmit'], ...); // i.e. `bun tsc --noEmit`If you intend to rely on Bun’s
bunx/bun xbehavior to resolve and runtsc(potentially auto-installing it), you likely want something closer to:await execa(bunPath, ['x', 'tsc', '--noEmit'], { ... });Alternatively, if you expect
tscto be available vianode_modulesor globally andbun tscis exactly what you want, update the comment to describe that expectation so it doesn’t mislead future readers.Given Bun CLI behavior evolves, please double-check against your target Bun version before changing this.
730-743: TightengetLanguagereturn type for better call-site safety.The detection logic (
go.mod→'go',package.json→'ts') is straightforward and fine for the current plugin templates.For stronger type safety at call sites (e.g., when switching on language), consider annotating the return type explicitly as a union:
-export function getLanguage(pluginDir: string) { +export function getLanguage(pluginDir: string): 'go' | 'ts' {That will help TS catch typos or missing cases when adding new language-specific branches.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cli/src/commands/router/commands/plugin/toolchain.ts(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: build-router
- GitHub Check: build_test
- GitHub Check: build_push_image (nonroot)
- GitHub Check: image_scan (nonroot)
- GitHub Check: image_scan
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: integration_test (./events)
- GitHub Check: build_test
- GitHub Check: integration_test (./telemetry)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
cli/src/commands/router/commands/plugin/toolchain.ts (5)
28-48: Platform catalogs for Go/Bun look coherent.The
ALL_GO_PLATFORMS,ALL_BUN_PLATFORMS, andALL_BUN_PLATFORM_MAPPINGSsets are internally consistent and cover the targets used later inbuildGoBinariesandbuildTsBinaries. No issues from a correctness standpoint.
81-90: Architecture mapping ingetOSArchmatches Go vs Bun usage.Using
amd64only for Go (language === 'go') and leaving other architectures as returned byos.arch()works well with your Go target strings (linux-amd64,darwin-arm64, etc.) and Bun targets (bun-linux-x64,bun-linux-arm64, ...). No issues here.
532-546: TS test and dependency flows mirror Go equivalents appropriately.
runTsTestsandinstallTsDependenciesmirror the Go flows (runGoTests/installGoDependencies) and correctly reusegetToolsEnv()andgetToolPath('bun'). The commands (bun test,bun install) and spinner messaging are consistent and clear.Also applies to: 565-577
661-704: Go build path remains consistent after split intobuildGoBinaries.The renamed/exported
buildGoBinarieskeeps the existing cross-compilation behavior (GOOS/GOARCH, CGO_DISABLED, bin layout) and now coexists cleanly withbuildTsBinaries. No issues here.
709-728:normalizePlatformslanguage switch is sound; host defaulting is correct.Defaulting to
getHostPlatform(language)whenplatformsis empty, then expanding withALL_GO_PLATFORMSorALL_BUN_PLATFORMSwhenallPlatformsis true, matches the intended behavior. The finalthrowfor unsupported languages is also good for fail-fast behavior.Once
getHostPlatformis fixed for Windows, this function should behave correctly across Go and TS.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/router-ci.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: build-router
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_test
- GitHub Check: image_scan
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./telemetry)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_push_image
- GitHub Check: build_test
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
demo/pkg/subgraphs/courses/patches/[email protected] (1)
39-41: Keep TS/JS in sync; consider minor cleanup for path handlingThe TS changes mirror the JS patch, which is good for keeping the compiled
build/src/health.jsconsistent with the source. A couple of minor, optional improvements:
- To avoid repeating the
process.env.NODE_ENV === 'test'expression, you could factor it into a smallconst isTest = process.env.NODE_ENV === 'test';and reuse it forhealthProtoPathandhealthProtoSuffix.- For
includeDirsandprotoPath, you might preferpath.join/path.resolvefor all segments instead of manual string interpolation (e.g.,path.join(healthProtoPath, healthProtoSuffix)), which is a bit more robust across platforms, even though Node generally accepts mixed separators.These are non-blocking, but if you adjust the TS source, remember to rebuild and update the corresponding
build/src/health.jsin the patch to keep both sides aligned.Also applies to: 48-50, 57-58
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
demo/pkg/subgraphs/courses/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
demo/Makefile(2 hunks)demo/pkg/subgraphs/courses/package.json(1 hunks)demo/pkg/subgraphs/courses/patches/[email protected](1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- demo/pkg/subgraphs/courses/package.json
🧰 Additional context used
🪛 checkmake (0.2.2)
demo/Makefile
[warning] 13-13: Target "PHONY" should be declared PHONY.
(phonydeclared)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: build-router
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: image_scan
- GitHub Check: build_push_image (nonroot)
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./telemetry)
- GitHub Check: integration_test (./events)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_test
🔇 Additional comments (5)
demo/pkg/subgraphs/courses/patches/[email protected] (1)
10-21: Environment-dependent proto path: verify non-test runtime assumptionsThe
healthProtoPath/healthProtoSuffixlogic makesincludeDirsandprotoPathdepend onNODE_ENV:
NODE_ENV === 'test': behaves like before, using__dirname+../../proto.- Any other
NODE_ENV: switches topath.dirname(process.execPath)+grpc-health-check/proto.This is correct for a Bun-compiled binary where
process.execPathpoints at a directory containinggrpc-health-check/proto, but in a plain Node runtime (e.g.,NODE_ENV=developmentor unset)process.execPathwill usually be the Node binary path, which is unlikely to contain that folder and would break proto loading.Please confirm that this patched demo is only executed:
- under tests (
NODE_ENV === 'test'), or- as a compiled Bun binary where you arrange
grpc-health-check/protonext toprocess.execPath,and not as a plain Node process with
NODE_ENV !== 'test'. If such a Node path exists, you may need a third branch (e.g., forNODE_ENV === 'development') that still uses__dirname-relative paths.Also applies to: 28-29
demo/Makefile (4)
42-47: Approve CI integration target structure.The
plugin-build-citarget correctly depends on bothplugin-build-go-binaryandplugin-build-bun-binaryand copies both binaries into separate subdirectories under../router/plugins/. This aligns well with the language-aware build approach and maintains a clean separation between projects (Go) and courses (Bun) plugins.
23-27: Dependency is properly declared—no action needed.The grpc-health-check dependency is already declared in
demo/pkg/subgraphs/courses/package.json(version ^2.0.0) and locked in bun.lock. Thebun installstep will correctly populate node_modules, allowing the subsequentcp -r node_modules/grpc-health-check/prototo succeed.
29-30: Review comment is incorrect; no changes needed.The Go module path is already correctly aligned. The
go build ./srccommand changes todemo/pkg/subgraphs/projects/wherego.modexists with the correct module declaration (github.com/wundergraph/cosmo/demo/pkg/subgraphs/projects), matching the--go-module-pathflag used in the CLI invocation. The build is already standalone and buildable; thego buildcommand doesn't require an explicit module path flag whengo.modis present in the working directory.Likely an incorrect or invalid review comment.
15-21: No issues found. Both subgraph paths are valid and correctly configured.Verification confirms:
demo/pkg/subgraphs/courses/exists with TypeScript plugin structure (src/plugin.ts, src/plugin-server.ts)demo/pkg/subgraphs/projects/exists with Go plugin structure (cmd/service/main.go, go.mod)- Makefile targets correctly differentiate: projects includes
--go-module-path, courses omits it (appropriate for TypeScript)
| ./bump-deps.sh | ||
|
|
||
| PHONY: plugin-build plugin-generate plugin-build-binary plugin-build-integration | ||
| PHONY: plugin-build plugin-generate plugin-build-bun-binary plugin-build-go-binary plugin-build-integration |
There was a problem hiding this comment.
Fix malformed .PHONY declaration.
The .PHONY keyword requires a leading dot; PHONY: is invalid Makefile syntax.
-.PHONY: plugin-build plugin-generate plugin-build-bun-binary plugin-build-go-binary plugin-build-integration
+.PHONY: plugin-build plugin-generate plugin-build-bun-binary plugin-build-go-binary plugin-build-integrationCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 13-13: Target "PHONY" should be declared PHONY.
(phonydeclared)
🤖 Prompt for AI Agents
In demo/Makefile around line 13, the Makefile uses an invalid "PHONY:" target;
replace it with the correct ".PHONY:" declaration and list the phony targets
after it (e.g. .PHONY: plugin-build plugin-generate plugin-build-bun-binary
plugin-build-go-binary plugin-build-integration) so make recognizes these as
phony targets.
This PR implements typescript plugins with bun.
There are no changes in the router, because we build a binary in the same name format it expects
{platform}-{arch}This PR also removes the template TS constants into template files, which are appended into the previous constants dynamically. This makes it easier to manage a growing number of templates, instead of going through TS strings. Includes a CI check to verify that any template changes are generated and committed.
To keep thing simple we didn't add a separate folder for the ts router plugin
router-plugin-ts, this will be done in a follow up pr, after everything is approved.The node health check package dynamically loads fs using an eval causing a null error when bundled with bun, it works when not bundled. @protobufjs/inquire violates default content security policy protobufjs/protobuf.js#997 (comment), adds a workaround for now, to investigate further. We also used
bun patchto override the internal__dirnamebecause it is hardcoded to the directory the binary was compiled at on compile time (this especially breaks cosmo cloud plugins since the docker directory is temporary for building).Adds courses demo
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
Checklist