feat: onboarding wizard #5#2750
Merged
comatory merged 15 commits intoondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-topicfrom Apr 29, 2026
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds a new interactive Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-04
branch
from
31d2073 to
b2f5c39
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-05
branch
from
5855726 to
4e88f5b
Compare
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (24.82%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-topic #2750 +/- ##
=======================================================================================================
- Coverage 46.28% 37.59% -8.70%
=======================================================================================================
Files 1045 780 -265
Lines 139773 103288 -36485
Branches 8768 5186 -3582
=======================================================================================================
- Hits 64695 38827 -25868
+ Misses 73326 62717 -10609
+ Partials 1752 1744 -8
🚀 New features to boost your workflow:
|
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-04
branch
from
b2f5c39 to
02e794c
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-05
branch
2 times, most recently
from
1a5c570 to
8bfd103
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-05
branch
4 times, most recently
from
addc513 to
bfdd07f
Compare
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-04
branch
from
02e794c to
9648dc5
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-05
branch
2 times, most recently
from
9629758 to
cefd0e0
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-04
branch
from
9648dc5 to
578d732
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-05
branch
2 times, most recently
from
7867a28 to
5eb613b
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-04
branch
from
578d732 to
6b106ed
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-05
branch
from
5eb613b to
e629243
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-04
branch
3 times, most recently
from
6b106ed to
2a88e40
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-05
branch
from
e629243 to
0a365b8
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-04
branch
from
2a88e40 to
81d369b
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-05
branch
from
f4018ed to
09f1524
Compare
This was referenced Apr 20, 2026
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
cli/src/core/plugin-publish.ts (1)
52-93: Duplicate platform list definitions.
supportedPlatformsinsidegetDefaultPlatforms()duplicates the exportedSUPPORTED_PLATFORMSconstant. Consider reusing the constant to avoid maintenance burden.♻️ Proposed refactor to reuse constant
+export const SUPPORTED_PLATFORMS = ['linux/amd64', 'linux/arm64', 'darwin/amd64', 'darwin/arm64', 'windows/amd64']; + export function getDefaultPlatforms(): string[] { - const supportedPlatforms = ['linux/amd64', 'linux/arm64', 'darwin/amd64', 'darwin/arm64', 'windows/amd64']; const defaultPlatforms = ['linux/amd64']; // ... rest of function ... - if (dockerPlatform && supportedPlatforms.includes(dockerPlatform) && !defaultPlatforms.includes(dockerPlatform)) { + if (dockerPlatform && SUPPORTED_PLATFORMS.includes(dockerPlatform) && !defaultPlatforms.includes(dockerPlatform)) { defaultPlatforms.push(dockerPlatform); } return defaultPlatforms; } - -export const SUPPORTED_PLATFORMS = ['linux/amd64', 'linux/arm64', 'darwin/amd64', 'darwin/arm64', 'windows/amd64'];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/core/plugin-publish.ts` around lines 52 - 93, getDefaultPlatforms currently declares a local supportedPlatforms array that duplicates the exported SUPPORTED_PLATFORMS; remove the local supportedPlatforms declaration and references and reuse the exported SUPPORTED_PLATFORMS constant instead (keep the local defaultPlatforms logic and dockerPlatform calculation as-is), i.e., replace checks like supportedPlatforms.includes(dockerPlatform) with SUPPORTED_PLATFORMS.includes(dockerPlatform) and delete the duplicated array inside getDefaultPlatforms so SUPPORTED_PLATFORMS is the single source of truth.docs-website/cli/demo.mdx (1)
29-31: Break this behavior summary into a short list.This paragraph bundles several distinct actions and outcomes into one long sentence, which makes the page harder to scan as reference docs. A short bullet list for graph creation/reuse, plugin publishing, router startup, and log-path output would fit the docs style better.
As per coding guidelines: "Prefer short, declarative sentences. If a sentence has more than one comma-separated clause, consider splitting it. Use structured lists when presenting multiple distinct items. Do not pack them into a single paragraph."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs-website/cli/demo.mdx` around lines 29 - 31, Split the long paragraph under the "What The Command Does" heading into a short bullet list of distinct actions: (1) create or reuse a demo federated graph in the `default` namespace, (2) configure and publish required demo plugins/components, (3) start a local router at http://localhost:3002, (4) print the path to the local log file for router/publishing output, and also add a brief separate sentence noting that an existing demo can be reused or deleted to start over; update the "What The Command Does" section accordingly so each action is its own short, declarative list item.cli/src/commands/demo/util.ts (3)
483-510: Missing error handling aroundreadPluginFiles.If
readPluginFilesthrows an exception (e.g., plugin directory doesn't exist, file read error), the spinner will continue running and the error will propagate unhandled. Consider wrapping the inner loop body in try/catch to stop the spinner and return a structured error.Proposed fix
for (let i = 0; i < pluginNames.length; i++) { const pluginName = pluginNames[i]; const pluginDir = path.join(supportDir, 'plugins', pluginName); const spinner = demoSpinner(`Publishing plugin ${pc.bold(pluginName)} (${i + 1}/${pluginNames.length})…`).start(); + let files; + try { + files = await readPluginFiles(pluginDir); + } catch (err) { + spinner.fail(`Failed to read plugin ${pc.bold(pluginName)}: ${err instanceof Error ? err.message : String(err)}`); + return { error: err instanceof Error ? err : new Error(String(err)) }; + } - const files = await readPluginFiles(pluginDir); const result = await publishPluginPipeline({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/commands/demo/util.ts` around lines 483 - 510, The loop calling readPluginFiles lacks try/catch so thrown errors leave demoSpinner running and propagate; wrap the inner per-plugin logic (where readPluginFiles, publishPluginPipeline, pipeToLog are called and spinner is created) in a try/catch that calls spinner.fail with a descriptive message, ends logStream, and returns a structured error object (same shape as publishPluginPipeline result) so the caller receives { error: <Error> }; ensure you still call logStream.end() in the finally path and preserve existing behavior when publishPluginPipeline returns result.error.
129-181: Consider handling GitHub API rate limits.The
prepareSupportingDatafunction fetches from the GitHub API without authentication. Unauthenticated requests are limited to 60 requests/hour, which could be exhausted quickly during development or repeated runs. Consider documenting this limitation or optionally supporting aGITHUB_TOKENenvironment variable for authenticated requests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/commands/demo/util.ts` around lines 129 - 181, prepareSupportingData currently uses unauthenticated fetches to GitHub (treeResponse and individual rawUrl fetches) which can hit the 60 requests/hour rate limit; update the function to read a GITHUB_TOKEN env var (or config) and, when present, add an Authorization: Bearer <token> header to the GitHub API request for the tree (and to any API requests if you switch to api.github.com for file contents), falling back to unauthenticated requests if absent; ensure the header is applied to the fetch calls that use config.demoOnboardingRepositoryName/config.demoOnboardingRepositoryBranch (i.e., where treeResponse is fetched) and include a brief error message noting rate-limit status when treeResponse.ok is false so the user can be guided to set GITHUB_TOKEN.
456-456: Unreachable code after try/catch/finally.Line 456 (
return { error: null }) appears to be unreachable. All execution paths within the try block either return explicitly (lines 434, 440) or fall through toawait proc(line 443) which will either complete normally (caught by finally and then exit), throw (caught by catch), or be canceled (caught by catch). The catch block returns on all paths (lines 447, 450).Consider removing this line or restructuring to make control flow clearer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/commands/demo/util.ts` at line 456, The final "return { error: null }" is unreachable because all paths in the try/catch already return or rethrow after awaiting "proc" and the catch block returns on all branches; remove the unreachable return or refactor the function so all branches set a single result variable and perform one return at the end (e.g., replace direct returns in the try/catch with assignments to a local "result" and return "result" after the try/catch/finally). Update references to "proc" and the existing catch branches to use the chosen pattern so control flow is explicit and no dead return remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/commands/demo/api.ts`:
- Line 30: Fix the misspelling "occured" → "occurred" in the three error message
constructions that create new Error objects (e.g., the expression error: new
Error(response.response?.details ?? 'An unknown error occured') and the
analogous messages at the other two occurrences). Update the string literal in
each place so the fallback message reads 'An unknown error occurred' (preserving
the existing conditional/response usage and surrounding code such as the
response.response?.details checks).
- Around line 9-34: The RPC calls in fetchUserInfo (and similarly in
checkExistingOnboarding, fetchFederatedGraphByName, cleanUpFederatedGraph,
createFederatedGraph) can throw transport-level ConnectError and must be wrapped
in try/catch; update each function to wrap the await client.platform.* call in a
try block, catch exceptions (specifically handle ConnectError) and return the
same structured response shape (userInfo/other payload as null and error set to
a new Error with the caught error's message or details) so transport failures
become handled errors rather than unhandled rejections.
In `@cli/src/commands/demo/command.ts`:
- Around line 106-120: The current flow sets deleted = true and prints success
messages unconditionally even when cleanupFederatedGraph()/cleanupFn() fails;
change the logic so that spinner.succeed and setting deleted = true only happen
after a confirmed successful deletion (i.e., when deleteResponse has no error
and cleanupFederatedGraph()/cleanupFn() returned success). Move
spinner.succeed(graph removed) and deleted = true into the success branch, keep
spinner.fail and the waitForKeyPress retry flow inside the error branch, and
ensure cleanupFederatedGraph()/cleanupFn() returns or throws a clear status so
the calling code can reliably decide success versus failure.
- Around line 297-299: Remove the unconditional token echo: stop printing
createResult.token to stdout by deleting or guarding the console.log(`
${pc.bold(createResult.token)}`) call; the token is already passed to
runRouterContainer(), so either remove that line entirely or only print the
token when an explicit, opt-in flag (e.g. --show-token) is provided and
documented, ensuring spinner.succeed('Router token generated.') remains but the
secret token is not echoed by default.
- Around line 64-73: The error branches that call waitForKeyPress (e.g., the
getFederatedGraphResponse.error branch referencing retryFn and waitForKeyPress)
currently await a retry keypress but then fall through returning undefined;
change these to propagate the retry outcome back to the caller by returning the
retry result (i.e., call and return the result of retryFn or repeat the
operation in a loop until success/cancel) instead of just returning undefined.
Apply the same fix to the other similar blocks mentioned (around lines handling
graph creation/plugin publish and onboarding-status retry) so each branch either
returns the retryFn result or uses a loop that only exits on success or cancel,
ensuring the outer flow receives the retried value.
In `@cli/src/core/plugin-publish.ts`:
- Around line 141-158: The call to client.platform.validateAndFetchPluginData
inside publishPluginPipeline can throw transport-level ConnectError but is not
wrapped; add a try-catch around that RPC invocation (the block that constructs
the request with name/namespace/labels and calls
client.platform.validateAndFetchPluginData) and on catch return a
PluginPublishResult with the caught error (preserving error details) so callers
always receive a structured result; ensure you only change the error-handling
around validateAndFetchPluginData and keep the subsequent response.code check
as-is.
- Around line 209-228: Wrap the call to client.platform.publishFederatedSubgraph
in a try-catch that specifically handles transport errors (ConnectError) and
general errors: catch errors around the publishFederatedSubgraph(...) invocation
and on ConnectError log a clear message (including error details) using the same
logger, perform any necessary rollback/cleanup (e.g., mark or remove the pushed
Docker image or set a failure flag), and rethrow or return a controlled failure
so the pipeline does not continue as if publish succeeded; ensure the catch
surrounds the exact call to client.platform.publishFederatedSubgraph and
preserves existing headers from getBaseHeaders() and fields like
name/pluginName, namespace, proto, SubgraphType.GRPC_PLUGIN, and newVersion.
In `@cli/src/core/router-token.ts`:
- Around line 35-50: The await calls to
client.platform.createFederatedGraphToken (in createRouterToken) and the
corresponding delete call (in deleteRouterToken) must be wrapped in try/catch so
transport-level rejections are converted to the promised { error: Error | null }
shape; surround the await of client.platform.createFederatedGraphToken(...) and
the deleteFederatedGraphToken(...) call with try/catch and on catch return {
error: new ConnectError(caughtError.message ?? 'RPC error', /* preserve or map
status if available */) } (or wrap the original error) instead of letting the
rejection escape, keeping the existing success path that returns { error: null,
token } or { error: null } intact.
In `@cli/src/utils.ts`:
- Around line 320-342: The waitForKeyPress function calls
process.stdin.setRawMode(true) unguarded which throws when stdin is not a TTY;
update waitForKeyPress to check process.stdin.isTTY before calling setRawMode
and process.stdin.resume (or wrap setRawMode in try/catch), and ensure cleanup
logic (resetting raw mode and pausing) only runs if raw mode was enabled;
reference the waitForKeyPress function and the onData handler so you add the
isTTY guard around the setRawMode/resume calls and conditionally call
process.stdin.setRawMode(false)/pause in the Ctrl+C and resolve paths.
---
Nitpick comments:
In `@cli/src/commands/demo/util.ts`:
- Around line 483-510: The loop calling readPluginFiles lacks try/catch so
thrown errors leave demoSpinner running and propagate; wrap the inner per-plugin
logic (where readPluginFiles, publishPluginPipeline, pipeToLog are called and
spinner is created) in a try/catch that calls spinner.fail with a descriptive
message, ends logStream, and returns a structured error object (same shape as
publishPluginPipeline result) so the caller receives { error: <Error> }; ensure
you still call logStream.end() in the finally path and preserve existing
behavior when publishPluginPipeline returns result.error.
- Around line 129-181: prepareSupportingData currently uses unauthenticated
fetches to GitHub (treeResponse and individual rawUrl fetches) which can hit the
60 requests/hour rate limit; update the function to read a GITHUB_TOKEN env var
(or config) and, when present, add an Authorization: Bearer <token> header to
the GitHub API request for the tree (and to any API requests if you switch to
api.github.com for file contents), falling back to unauthenticated requests if
absent; ensure the header is applied to the fetch calls that use
config.demoOnboardingRepositoryName/config.demoOnboardingRepositoryBranch (i.e.,
where treeResponse is fetched) and include a brief error message noting
rate-limit status when treeResponse.ok is false so the user can be guided to set
GITHUB_TOKEN.
- Line 456: The final "return { error: null }" is unreachable because all paths
in the try/catch already return or rethrow after awaiting "proc" and the catch
block returns on all branches; remove the unreachable return or refactor the
function so all branches set a single result variable and perform one return at
the end (e.g., replace direct returns in the try/catch with assignments to a
local "result" and return "result" after the try/catch/finally). Update
references to "proc" and the existing catch branches to use the chosen pattern
so control flow is explicit and no dead return remains.
In `@cli/src/core/plugin-publish.ts`:
- Around line 52-93: getDefaultPlatforms currently declares a local
supportedPlatforms array that duplicates the exported SUPPORTED_PLATFORMS;
remove the local supportedPlatforms declaration and references and reuse the
exported SUPPORTED_PLATFORMS constant instead (keep the local defaultPlatforms
logic and dockerPlatform calculation as-is), i.e., replace checks like
supportedPlatforms.includes(dockerPlatform) with
SUPPORTED_PLATFORMS.includes(dockerPlatform) and delete the duplicated array
inside getDefaultPlatforms so SUPPORTED_PLATFORMS is the single source of truth.
In `@docs-website/cli/demo.mdx`:
- Around line 29-31: Split the long paragraph under the "What The Command Does"
heading into a short bullet list of distinct actions: (1) create or reuse a demo
federated graph in the `default` namespace, (2) configure and publish required
demo plugins/components, (3) start a local router at http://localhost:3002, (4)
print the path to the local log file for router/publishing output, and also add
a brief separate sentence noting that an existing demo can be reused or deleted
to start over; update the "What The Command Does" section accordingly so each
action is its own short, declarative list item.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3a5ee44-9b1d-4903-b9dc-b413c4a5305c
📒 Files selected for processing (18)
cli/.env.examplecli/src/commands/demo/api.tscli/src/commands/demo/command.tscli/src/commands/demo/index.tscli/src/commands/demo/types.tscli/src/commands/demo/util.tscli/src/commands/index.tscli/src/commands/router/commands/plugin/commands/publish.tscli/src/commands/router/commands/token/commands/create.tscli/src/commands/router/commands/token/commands/delete.tscli/src/core/config.tscli/src/core/plugin-publish.tscli/src/core/router-token.tscli/src/utils.tscli/test/demo/command.test.tscli/test/demo/util.test.tsdocs-website/cli/demo.mdxdocs-website/docs.json
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-topic #2750 +/- ##
========================================================================================================
- Coverage 40.33% 39.27% -1.07%
========================================================================================================
Files 1014 136 -878
Lines 126712 12802 -113910
Branches 5725 631 -5094
========================================================================================================
- Hits 51112 5028 -46084
+ Misses 73882 7772 -66110
+ Partials 1718 2 -1716
🚀 New features to boost your workflow:
|
comatory
commented
Apr 27, 2026
comatory
commented
Apr 27, 2026
comatory
commented
Apr 27, 2026
comatory
commented
Apr 27, 2026
comatory
commented
Apr 27, 2026
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
wgc democommand for automated local federated graph setup, including plugin publishing, router token generation, and router initialization.Documentation
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.
PR (5): for onboarding wizard - CLI integration
Parent: #2736