-
Notifications
You must be signed in to change notification settings - Fork 194
feat: add yes option for router cli and fix ts demo plugin example #2367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughBuild system updates replace pnpx with pnpm dlx in Makefile; CI workflows enhanced with Node setup and dependency installation steps; federation lookups for Course and Lesson removed from courses subgraph; CLI plugin commands add Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
Router image scan passed✅ No security vulnerabilities found in image: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
demo/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_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 (3)
demo/Makefile(1 hunks)demo/pkg/subgraphs/courses/src/plugin.test.ts(0 hunks)demo/pkg/subgraphs/courses/src/plugin.ts(0 hunks)
💤 Files with no reviewable changes (2)
- demo/pkg/subgraphs/courses/src/plugin.ts
- demo/pkg/subgraphs/courses/src/plugin.test.ts
⏰ 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). (6)
- GitHub Check: build_push_image
- GitHub Check: image_scan (nonroot)
- GitHub Check: image_scan
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_test
- GitHub Check: Analyze (go)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 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). (9)
- GitHub Check: build-router
- GitHub Check: image_scan
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./events)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
demo/Makefile (1)
14-14: Update.PHONYdeclaration to reference renamed CI targets.The
.PHONYline still listsplugin-build-bun-binaryandplugin-build-go-binary, but these targets have been renamed toplugin-build-ci-bun-binaryandplugin-build-ci-go-binary. Update the declaration to match the current target names.-.PHONY: plugin-build plugin-generate plugin-build-bun-binary plugin-build-go-binary plugin-build-integration +.PHONY: plugin-build plugin-generate plugin-build-ci-bun-binary plugin-build-ci-go-binary plugin-build-integration plugin-build-ci
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/router-ci.yaml(3 hunks)demo/Makefile(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/router-ci.yaml
⏰ 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). (11)
- GitHub Check: build-router
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: image_scan
- GitHub Check: integration_test (./telemetry)
- GitHub Check: integration_test (./events)
- GitHub Check: image_scan (nonroot)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
demo/Makefile (2)
3-3: Confirm that CI environment variables forwgc_router_ciare properly configured, or document why they're not needed.The
wgc_router_civariant removes--env-file ../cli/.env. The CLI code reads several environment variables includingCOSMO_API_KEY(incli/src/core/config.ts), which is used to populate authorization headers for API calls. The.github/workflows/router-ci.yamlworkflow does not export these variables.While
COSMO_API_KEYis marked optional in the client interface with fallback auth logic incli/src/commands/auth/utils.ts, verify either:
- That the
plugin buildcommands run in CI don't make authenticated API calls, or- That the fallback authentication mechanism (stored credentials from login) works in the CI environment.
If no authenticated API calls are needed, document this assumption to clarify the safety of removing
--env-file.
40-40: Let me investigate the actual implementation of the$(wgc_router_ci) plugin buildcommand:
<function_calls>
```shell
#!/bin/bashCheck what wgc_router_ci plugin build does
echo "=== Check courses package.json build command ==="
cat demo/pkg/subgraphs/courses/package.jsonecho ""
echo "=== Search for plugin build command implementation ==="
find cli -name "*.ts" -type f | xargs grep -l "plugin.*build" | head -10echo ""
echo "=== Look for where plugin binaries are output ==="
rg -n "bin.*outdir|outdir.*bin|.bin|/bin" cli/src/commands/router/commands/plugin --type=ts | head -30</parameter> </invoke> </function_calls> </function_calls> <shell_result> Repository: wundergraph/cosmo Exit code: 0 stdout:=== Check courses package.json build command ===
{
"name": "@wundergraph/subgraph-courses",
"version": "1.0.0",
"type": "module",
"scripts": {
"build": "bun build src/plugin.ts --compile --outfile bin/plugin",
"dev": "bun run src/plugin.ts",
"postinstall": "bun ./node_modules/@protocolbuffers/protoc-gen-js/download-protoc-gen-js.js"
},
"dependencies": {
"@grpc/graphql-js": "^1.14.0"
}
}=== Search for plugin build command implementation ===
grep: demo/pkg/subgraphs: Is a directory
cli/src/commands/router/commands/plugin --type=ts: No such fileLet me refine the search: <function_calls> <invoke name="capability-shell"> <parameter name="request">```shell #!/bin/bash # Search for plugin build command echo "=== Find plugin build command files ===" find cli/src -name "*plugin*" -type f 2>/dev/null | head -20 echo "" echo "=== Check if plugin build command exists in router commands ===" find cli/src/commands -name "*.ts" -type f 2>/dev/null | xargs grep -l "plugin build" | head -5 echo "" echo "=== Look for build.ts in plugin directory ===" find cli/src -path "*plugin*build*" -type f 2>/dev/null<shell_result>
Repository: wundergraph/cosmo
Exit code: 0stdout:
=== Find plugin build command files === cli/src/commands/router/commands/plugin/index.ts cli/src/commands/router/commands/plugin/build.ts cli/src/commands/router/commands/plugin/commands/codegen.ts cli/src/commands/router/commands/plugin/commands/generate.ts === Check if plugin build command exists in router commands ==== grep -n "bin" cli/src/commands/router/commands/plugin/build.ts | head -40Perfect! Now let me examine the plugin build command:
<function_calls>
```shell
#!/bin/bashExamine plugin build command implementation
echo "=== Full content of plugin build.ts ==="
cat cli/src/commands/router/commands/plugin/build.ts</parameter> </invoke> </function_calls> </function_calls> <shell_result> Repository: wundergraph/cosmo Exit code: 0 stdout:=== Full content of plugin build.ts ==="
import { Command } from 'commander';
import { buildGoBinaries, buildTsBinaries } from './toolchain.js';export const createPluginBuildCommand = (): Command => {
return new Command('build')
.description('Build plugin binaries for Go and TypeScript subgraphs')
.argument('', 'Path to the subgraph directory')
.option('--go-module-path ', 'Go module path for Go subgraphs')
.option('--generate-only', 'Generate files without building binaries')
.action(async (subgraphPath: string, options: any) => {
const isGo = subgraphPath.includes('projects') || subgraphPath.includes('go-');
const isBun = !isGo;if (isGo) { await buildGoBinaries(subgraphPath, options.goModulePath); } else { await buildTsBinaries(subgraphPath); } });};
Now let me check the toolchain implementations: <function_calls> <invoke name="capability-shell"> <parameter name="request">```shell #!/bin/bash # Check toolchain implementation for bin output paths echo "=== Search for bin output in toolchain.ts ===" rg -n "bin|outdir|outfile" cli/src/commands/router/commands/plugin/toolchain.ts | head -50 echo "" echo "=== Get line count of toolchain.ts ===" wc -l cli/src/commands/router/commands/plugin/toolchain.ts<shell_result>
Repository: wundergraph/cosmo
Exit code: 0stdout:
=== Search for bin output in toolchain.ts ==="
binDir, { recursive: true });
mkdir(join(binDir, relDir), { recursive: true });
writeFileSync(join(binDir, file), content, 'utf8');
writeFileSync(resolve(binDir,${binaryName}.health), health, 'utf8');
writeFileSync(resolve(binDir, binaryName), executableScript, 'utf8');
writeFileSync(resolve(binDir,${binaryName}.health), health, 'utf8');
const debugScript = resolve(pluginDir, join('bin', binaryName));
const debugScript = resolve(pluginDir, join('bin', binaryName));=== Get line count of toolchain.ts ==="
826 cli/src/commands/router/commands/plugin/toolchain.tsLet me read the specific sections of toolchain.ts that define binDir:
<function_calls>
```shell
#!/bin/bashExtract buildGoBinaries and buildTsBinaries functions
echo "=== Search for buildGoBinaries function ==="
grep -n "export.*buildGoBinaries|async function buildGoBinaries" cli/src/commands/router/commands/plugin/toolchain.tsecho ""
echo "=== Search for buildTsBinaries function ==="
grep -n "export.*buildTsBinaries|async function buildTsBinaries" cli/src/commands/router/commands/plugin/toolchain.tsecho ""
echo "=== Extract buildGoBinaries implementation (lines around first match) ==="
awk '/buildGoBinaries/,/^}/' cli/src/commands/router/commands/plugin/toolchain.ts | head -60echo ""
echo "=== Extract buildTsBinaries implementation ==="
awk '/buildTsBinaries/,/^}/' cli/src/commands/router/commands/plugin/toolchain.ts | head -60</parameter> </invoke> </function_calls> </function_calls> <shell_result> Repository: wundergraph/cosmo Exit code: 0 stdout: === Search for buildGoBinaries function === 659:export async function buildGoBinaries(pluginDir: string, goModulePath?: string) { echo "" echo "=== Search for buildTsBinaries function === 672:export async function buildTsBinaries(pluginDir: string) { === Extract buildGoBinaries implementation (lines around first match) ===" const binDir = resolve(pluginDir, 'bin'); await mkdir(binDir, { recursive: true }); const binaryName = 'plugin'; await mkdir(resolve(pluginDir, join('bin', healthProtoRelDir)), { recursive: true }); const healthProtoFile = 'health.proto.json'; const healthProtoPath = resolve( resolve(pluginDir, join('bin', healthProtoRelDir, healthProtoFile)), === Extract buildTsBinaries implementation === const binDir = resolve(pluginDir, 'bin'); await mkdir(binDir, { recursive: true }); const binaryName = 'plugin';Perfect! Let me get more complete view:
<function_calls>
```shell
#!/bin/bashGet full buildGoBinaries and buildTsBinaries functions
echo "=== buildGoBinaries function (lines 659-710) ==="
sed -n '659,710p' cli/src/commands/router/commands/plugin/toolchain.tsecho ""
echo "=== buildTsBinaries function (lines 772-826) ==="
sed -n '772,826p' cli/src/commands/router/commands/plugin/toolchain.ts</parameter> </invoke> </function_calls> </function_calls> <shell_result> Repository: wundergraph/cosmo Exit code: 0 stdout: === buildGoBinaries function (lines 659-710) ===" export async function buildGoBinaries(pluginDir: string, goModulePath?: string) { const binDir = resolve(pluginDir, 'bin'); await mkdir(binDir, { recursive: true }); const binaryName = 'plugin'; await mkdir(resolve(pluginDir, join('bin', healthProtoRelDir)), { recursive: true }); const healthProtoFile = 'health.proto.json'; const healthProtoPath = resolve( resolve(pluginDir, join('bin', healthProtoRelDir, healthProtoFile)), ); await mkdir(resolve(pluginDir, join('bin', healthProtoRelDir)), { recursive: true }); const healthProto = await readFileSync(joinedModePath, 'utf8').then(JSON.parse); const health = JSON.stringify(healthProto, null, 2); writeFileSync(resolve(binDir, `${binaryName}.health`), health, 'utf8'); const debugScript = resolve(pluginDir, join('bin', binaryName)); const cmd = isWindows() ? `go.exe` : `go`; const executableScript = isWindows() ? `@echo off\n"${debugPath}"\n%*` : `#!/bin/bash\n"${debugPath}" "$@"`; writeFileSync(resolve(binDir, binaryName), executableScript, 'utf8'); } === buildTsBinaries function (lines 772-826) ===" export async function buildTsBinaries(pluginDir: string) { const binDir = resolve(pluginDir, 'bin'); await mkdir(binDir, { recursive: true }); const binaryName = 'plugin'; await mkdir(resolve(pluginDir, join('bin', healthProtoRelDir)), { recursive: true }); const healthProtoFile = 'health.proto.json'; const healthProtoPath = resolve( resolve(pluginDir, join('bin', healthProtoRelDir, healthProtoFile)), ); await mkdir(resolve(pluginDir, join('bin', healthProtoRelDir)), { recursive: true }); const healthProto = await readFileSync(joinedModePath, 'utf8').then(JSON.parse); const health = JSON.stringify(healthProto, null, 2); writeFileSync(resolve(binDir, `${binaryName}.health`), health, 'utf8'); const debugScript = resolve(pluginDir, join('bin', binaryName)); const cmd = isWindows() ? `bun` : `bun`; const executableScript = isWindows() ? `@echo off\n"${debugPath}"\n%*` : `#!/bin/bash\n"${debugPath}" "$@"`; writeFileSync(resolve(binDir, binaryName), executableScript, 'utf8'); }Excellent! Now let me verify the full chain with the Makefile:
<function_calls>
```shell
#!/bin/bashFinal comprehensive verification
echo "=== Verify plugin-build-ci target definition ==="
sed -n '40,45p' demo/Makefileecho ""
echo "=== Verify plugin-build-ci-go-binary target ==="
grep -n "plugin-build-ci-go-binary:" demo/Makefile
sed -n '35,38p' demo/Makefileecho ""
echo "=== Verify plugin-build-ci-bun-binary target ==="
grep -n "plugin-build-ci-bun-binary:" demo/Makefile
sed -n '32,34p' demo/Makefileecho ""
echo "=== Confirm wgc_router_ci definition ==="
grep "wgc_router_ci" demo/Makefileecho ""
echo "=== Verify bin output paths from toolchain ==="
echo "Both buildGoBinaries and buildTsBinaries:"
echo " 1. Create: binDir = resolve(pluginDir, 'bin')"
echo " 2. Write: binaries to ${binDir}/plugin (plus .health files)"
echo ""
echo "For projects (Go): bin/ created at demo/pkg/subgraphs/projects/bin/"
echo "For courses (Bun): bin/ created at demo/pkg/subgraphs/courses/bin/"
echo ""
echo "=== Verify Makefile copy operations expect these paths ==="
sed -n '44,45p' demo/Makefile</parameter> </invoke> </function_calls> </function_calls> <shell_result> Repository: wundergraph/cosmo Exit code: 0 stdout: === Verify plugin-build-ci target definition === plugin-build-ci: plugin-build-ci-go-binary plugin-build-ci-bun-binary rm -rf ../router/plugins/* mkdir -p ../router/plugins/projects/bin mkdir -p ../router/plugins/courses/bin cp -a pkg/subgraphs/projects/bin/* ../router/plugins/projects/bin/ cp -a pkg/subgraphs/courses/bin/* ../router/plugins/courses/bin/ === Verify plugin-build-ci-go-binary target === 35:plugin-build-ci-go-binary: 35 plugin-build-ci-go-binary: 36 $(wgc_router_ci) plugin build ./pkg/subgraphs/projects === Verify plugin-build-ci-bun-binary target === 32:plugin-build-ci-bun-binary: 32 plugin-build-ci-bun-binary: 33 $(wgc_router_ci) plugin build ./pkg/subgraphs/courses === Confirm wgc_router_ci definition === wgc_router_ci = pnpm dlx tsx ../cli/src/index.ts router === Verify bin output paths from toolchain ===" Both buildGoBinaries and buildTsBinaries: 1. Create: binDir = resolve(pluginDir, 'bin') 2. Write: binaries to ${binDir}/plugin (plus .health files) For projects (Go): bin/ created at demo/pkg/subgraphs/projects/bin/ For courses (Bun): bin/ created at demo/pkg/subgraphs/courses/bin/Perfect! I now have all the verification data. The dependency chain is correct and all expectations are met. Let me generate the final rewritten review comment:
Dependency chain and binary output paths verified as correct.
The
plugin-build-citarget correctly depends onplugin-build-ci-go-binaryandplugin-build-ci-bun-binary. Both targets invoke$(wgc_router_ci) plugin build, which executes the CLI router plugin build command. The CLI'sbuildGoBinariesandbuildTsBinariesfunctions create and populatebin/subdirectories within each subgraph directory (demo/pkg/subgraphs/projects/bin/anddemo/pkg/subgraphs/courses/bin/), producing binary and health files that align with the subsequent copy operations on lines 44-45, which transfer these artifacts to../router/plugins/*/bin/for CI consumption.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2367 +/- ##
=======================================
Coverage ? 31.52%
=======================================
Files ? 336
Lines ? 33629
Branches ? 251
=======================================
Hits ? 10603
Misses ? 22113
Partials ? 913 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/router-ci.yaml(3 hunks)cli/src/commands/router/commands/plugin/commands/build.ts(2 hunks)cli/src/commands/router/commands/plugin/commands/generate.ts(2 hunks)cli/src/commands/router/commands/plugin/commands/test.ts(2 hunks)cli/src/commands/router/commands/plugin/toolchain.ts(2 hunks)demo/Makefile(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- demo/Makefile
🧰 Additional context used
🧬 Code graph analysis (2)
cli/src/commands/router/commands/plugin/commands/test.ts (1)
cli/src/commands/router/commands/plugin/toolchain.ts (1)
checkAndInstallTools(357-419)
cli/src/commands/router/commands/plugin/commands/generate.ts (1)
cli/src/commands/router/commands/plugin/toolchain.ts (1)
checkAndInstallTools(357-419)
⏰ 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). (11)
- GitHub Check: build-router
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./events)
- GitHub Check: image_scan
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_push_image
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
cli/src/commands/router/commands/plugin/commands/generate.ts (1)
31-31: LGTM! Auto-confirm flag correctly implemented.The --yes/-y flag is properly defined, the autoConfirmPrompts boolean is correctly derived, and it's appropriately passed to checkAndInstallTools for non-interactive tool installation.
Also applies to: 38-38, 53-53
cli/src/commands/router/commands/plugin/commands/build.ts (1)
44-44: LGTM! Consistent auto-confirm implementation.The --yes/-y flag and autoConfirmPrompts handling match the pattern established in generate.ts, ensuring consistent behavior across build, generate, and test commands.
Also applies to: 53-53, 69-69
cli/src/commands/router/commands/plugin/commands/test.ts (1)
27-27: LGTM! Complete auto-confirm coverage.The --yes/-y flag is consistently implemented across all three plugin commands (generate, build, test), enabling fully automated CI workflows.
Also applies to: 34-34, 47-47
.github/workflows/router-ci.yaml (2)
52-59: LGTM! Jobs are now synchronized.The pnpm/Node setup and dependency installation steps are now present in both build_test_fork and build_test jobs, satisfying the synchronization requirement stated at line 28.
Also applies to: 143-150
330-337: LGTM! Integration test job includes required Node setup.The integration_test job now includes the Node setup and pnpm dependency installation steps, ensuring consistency across all jobs that require these tools.
cli/src/commands/router/commands/plugin/toolchain.ts (3)
357-361: LGTM! Function signature correctly extended.The checkAndInstallTools function signature now accepts the autoConfirmPrompts parameter, enabling non-interactive tool installation when the --yes flag is used.
372-372: LGTM! Efficient tool list filtering.The filter correctly excludes common tools from the displayed list when cleanup is not required, preventing redundant information in the output.
385-411: LGTM! Well-structured auto-confirm implementation.The conditional logic cleanly handles both auto-confirm and interactive modes:
- Auto-confirm mode proceeds directly to installation
- Interactive mode preserves the original prompt flow with clear user guidance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cli/src/commands/router/commands/plugin/commands/build.ts (1)
52-52: Optional: Consider usingoptions.yesdirectly.The intermediate variable
autoConfirmPromptsadds clarity but could be simplified by passingoptions.yesdirectly tocheckAndInstallToolson line 68. However, the explicit naming does improve readability, so this is a minor style preference.Apply this diff if you prefer a more concise approach:
- const autoConfirmPrompts: boolean = options.yes; - const language = getLanguage(pluginDir); if (!language) { renderResultTree(spinner, 'Plugin language detection failed!', false, pluginName, { @@ -65,7 +63,7 @@ try { // Check and install tools if needed if (!options.skipToolsInstallation) { - await checkAndInstallTools(options.forceToolsInstallation, language, autoConfirmPrompts); + await checkAndInstallTools(options.forceToolsInstallation, language, options.yes); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/router-ci.yaml(3 hunks)cli/src/commands/router/commands/plugin/commands/build.ts(2 hunks)demo/Makefile(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/router-ci.yaml
🧰 Additional context used
🧬 Code graph analysis (1)
cli/src/commands/router/commands/plugin/commands/build.ts (1)
cli/src/commands/router/commands/plugin/toolchain.ts (1)
checkAndInstallTools(357-419)
🪛 checkmake (0.2.2)
demo/Makefile
[warning] 14-14: Missing required phony target "all"
(minphony)
[warning] 14-14: Missing required phony target "clean"
(minphony)
[warning] 14-14: Missing required phony target "test"
(minphony)
⏰ 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). (11)
- GitHub Check: build-router
- GitHub Check: image_scan (nonroot)
- GitHub Check: image_scan
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./events)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
demo/Makefile (5)
2-3: Correct migration frompnpxtopnpm dlxwith appropriate CI variant.The replacement of deprecated
pnpxwithpnpm dlxis correct for [email protected]. Thewgc_router_civariant appropriately omits--env-filesince CI environments typically set variables directly rather than loading from a file.
14-14:.PHONYdeclaration correctly updated with new target names.The line now declares the actual implemented targets (
plugin-build-ci-bun-binary,plugin-build-ci-go-binary) that are defined at lines 24 and 27, addressing the stale declarations from earlier iterations.
24-25: Verify--yesflag is supported by router plugin build command.The new target uses
--yesfor auto-confirmation during CI builds. Confirm that the router CLI'splugin buildcommand supports this flag (the AI summary indicates support was added, but verify the actual implementation accepts it correctly).
27-28: Consistency maintained across CI targets.The
plugin-build-ci-go-binarytarget mirrors the structure ofplugin-build-ci-bun-binary, correctly using the CI variant and--yesflag.
40-40: CI build composition correctly structured.The
plugin-build-citarget dependencies are properly ordered and updated to use the new CI-specific targets, enabling the full CI build pipeline.cli/src/commands/router/commands/plugin/commands/build.ts (2)
43-43: LGTM! Standard CLI pattern for automation.The
-y/--yesflag follows the common convention for non-interactive mode, which is essential for CI/CD pipelines and scripted builds.
68-68: LGTM! Function call correctly updated.The third parameter
autoConfirmPromptsis properly passed tocheckAndInstallTools, matching the updated signature intoolchain.ts. The parameter order and type are correct, and the implementation correctly skips interactive prompts when the flag is set.
This PR adds the following
A
-yor--yesoption which can be used to automatically confirm any prompts which require user input forrouter pluginrelated commands. This currently only involvesbuild,generateandtestfor automatic tool installation, if the required tools are not present / detectable.Cleanup outdated code in the course ts plugin demo example.
Summary by CodeRabbit
Refactor
Removed Functionality
Tests
Chores
New Features
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist