Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds interactive CLI documentation browsing/search and guided tutorials with progress persistence.
Changes:
- Adds documentation hierarchy rendering, fuzzy search, browser UI, and command tips.
- Adds tutorial catalog, runner, resume support, and progress storage.
- Registers both namespaces and adds tests, fixtures, and dependencies.
File summaries
| File | Description |
|---|---|
| internal/namespaces/tutorial/tutorials.go | Updated as part of this pull request. |
| internal/namespaces/tutorial/tutorial_test.go | Updated as part of this pull request. |
| internal/namespaces/tutorial/tutorial_disabled.go | Updated as part of this pull request. |
| internal/namespaces/tutorial/testdata/test-tutorial-resume-no-progress-no-progress.golden | Updated as part of this pull request. |
| internal/namespaces/tutorial/testdata/test-tutorial-list-simple.golden | Updated as part of this pull request. |
| internal/namespaces/tutorial/runner.go | Updated as part of this pull request. |
| internal/namespaces/tutorial/progress.go | Updated as part of this pull request. |
| internal/namespaces/tutorial/custom.go | Updated as part of this pull request. |
| internal/namespaces/docs/tips.go | Updated as part of this pull request. |
| internal/namespaces/docs/testdata/test-docs-search-search-server.golden | Updated as part of this pull request. |
| internal/namespaces/docs/testdata/test-docs-search-no-results-no-results.golden | Updated as part of this pull request. |
| internal/namespaces/docs/testdata/test-docs-namespace-not-found-not-found.golden | Updated as part of this pull request. |
| internal/namespaces/docs/testdata/test-docs-list-non-interactive.golden | Updated as part of this pull request. |
| internal/namespaces/docs/testdata/test-docs-command-page-instance-server-list.golden | Updated as part of this pull request. |
| internal/namespaces/docs/search.go | Updated as part of this pull request. |
| internal/namespaces/docs/render.go | Updated as part of this pull request. |
| internal/namespaces/docs/fuzzy.go | Updated as part of this pull request. |
| internal/namespaces/docs/docs_test.go | Updated as part of this pull request. |
| internal/namespaces/docs/custom.go | Updated as part of this pull request. |
| internal/namespaces/docs/browser.go | Updated as part of this pull request. |
| internal/namespaces/docs/browser_disabled.go | Updated as part of this pull request. |
| go.sum | Updated as part of this pull request. |
| go.mod | Updated as part of this pull request. |
| commands/commands.go | Updated as part of this pull request. |
Review details
Suppressed comments (12)
internal/namespaces/docs/custom.go:103
- A namespace-only command is not necessarily a namespace container: commands such as
login,version, anddocshave no subcommands but do have a runnable command page. This branch rendersscw docs login(and similar leaf commands) as an empty namespace page, omitting usage, arguments, examples, and tips; checkcommands.HasSubCommands(cmd)before choosing the namespace renderer.
if cmd.Namespace != "" && cmd.Resource == "" && cmd.Verb == "" {
return RenderNamespacePage(commands, cmd.Namespace), nil
internal/namespaces/docs/custom.go:84
SearchCommandsandBuildHierarchyboth excludeHiddencommands, but direct-path rendering bypasses that policy. A user who knows a hidden command path can therefore expose it throughscw docs; reject hidden commands here as well.
if cmd == nil {
internal/namespaces/docs/fuzzy.go:27
- The target is lowercased before being passed to
isWordBoundary, so the helper can never see an original lower-to-upper transition. The documented camelCase boundary bonus is therefore unreachable and fuzzy ranking is wrong for camelCase text; retain the original target runes for the boundary check while comparing lowercased runes.
targetLower := strings.ToLower(target)
queryLower := strings.ToLower(query)
queryRunes := []rune(queryLower)
targetRunes := []rune(targetLower)
internal/namespaces/docs/tips.go:11
organization-idis a command argument, not a registered global flag, so--organization-idis not a valid token for this command. Users following this tip will receive an argument parsing error.
"Use --organization-id to filter by organization.",
internal/namespaces/docs/tips.go:15
- These Kubernetes examples use the same invalid
--argument syntax. The command-specific forms areversion=<version>andcni=ciliumorcni=calico, otherwise the copied commands fail parsing.
"Use --version to specify the Kubernetes version.",
"Use --cni to specify the container network interface (cilium or calico).",
internal/namespaces/docs/tips.go:25
- This pitfall again uses
--zone=all, butzoneis a command-specific argument and must be passed aszone=all. The current copy-paste example is rejected by the CLI argument parser.
"By default, only servers in the default zone are listed. Use --zone=all to list across all zones.",
internal/namespaces/tutorial/progress.go:44
- The context is discarded and the SDK's process-global
GetScwConfigDiris used for progress storage. This ignores the CLI's contextual environment overrides; in particular,core.Testisolates HOME only throughMeta.OverrideEnv, so these commands can read or write the developer's real~/.config/scw/tutorial-progress.jsonand make resume behavior depend on external state. Derive the path from the command context and isolate the test fixture.
func progressFilePath(_ context.Context) string {
configDir, err := scw.GetScwConfigDir()
if err != nil {
configDir = filepath.Join(os.Getenv("HOME"), ".config", "scw")
}
return filepath.Join(configDir, "tutorial-progress.json")
internal/namespaces/tutorial/runner.go:130
Readlinereturns aninteractive.InterruptErroron Ctrl-C, but this error is discarded here, so cancelling a non-command step still advances and saves that step as completed. Return the error before updatingp.CurrentStep.
_, _ = interactive.Readline(ctx, &interactive.ReadlineConfig{})
internal/namespaces/tutorial/runner.go:235
- Selecting yes here does not run
scw init; it only returns an error telling the user to run it later. Either invoke the init command or change the earlier prompt to say that manual action is required.
if runInit {
return errors.New("please run 'scw init' and then restart the tutorial")
internal/namespaces/tutorial/runner.go:22
- This non-interactive branch discards
startStepand never updates progress becauserunTutorialNonInteractivealways iterates from step zero. Consequentlyscw tutorial resumerepeats completed steps and leaves the tutorial unfinished when no TTY is available. Pass the start index through and either persist progress consistently or explicitly reject non-interactive resume.
func RunTutorial(ctx context.Context, tutorial Tutorial, startStep int) error {
if !interactive.IsInteractive {
return runTutorialNonInteractive(ctx, tutorial)
internal/namespaces/tutorial/runner.go:266
- This prompt is reached before
RunTutorialchecksinteractive.IsInteractive, so a non-interactivescw tutorial <id>orresumewith saved progress from another CLI version attempts to open readline. That makes automation fail or block on input; choose and implement a deterministic non-interactive restart/continue policy before callingPromptBool.
restart, err := interactive.PromptBool(
ctx,
fmt.Sprintf(
"This tutorial was started with CLI version %s but you are now running version %s. Restart from the beginning?",
p.CLIVersion,
currentVersion,
),
true,
)
internal/namespaces/tutorial/tutorials.go:62
- This hint uses
--zone=all, but command-specific arguments usename=valuetokens and the raw parser does not strip--prefixes. Users copying the hint will get an invalid argument error; usezone=allinstead.
ErrorHint: "Make sure you have servers in your default zone. Use --zone=all to list across all zones.",
- Files reviewed: 23/24 changed files
- Comments generated: 12
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+3
to
+7
| scw block volume create - Create a volume | ||
| scw config validate - Validate the config | ||
| scw help output - Get help about how the CLI output works | ||
| 🟩🟩🟩 JSON STDOUT 🟩🟩🟩 | ||
| "scw block volume create - Create a volume\nscw config validate - Validate the config\nscw help output - Get help about how the CLI output works" |
| return nil, err | ||
| } | ||
|
|
||
| return progress, nil |
| { | ||
| Title: "Check Configuration", | ||
| Concept: "Let's verify your CLI is properly configured. This command shows your current settings including credentials, default zone, and project.", | ||
| Command: "scw config show", |
Comment on lines
+208
to
+211
| resCmd := resNode.Command | ||
| if resCmd != nil && resCmd.Run != nil && len(resNode.Verbs) == 0 { | ||
| m.commandPage = RenderCommandPage(m.ctx, resCmd, m.commands) | ||
| m.level = levelCommand |
Comment on lines
+231
to
+232
| case levelCommand: | ||
| return tea.Quit |
Comment on lines
+277
to
+280
| func runTutorialNonInteractive(_ context.Context, tutorial Tutorial) error { | ||
| for i, step := range tutorial.Steps { | ||
| fmt.Printf("Step %d/%d: %s\n\n", i+1, len(tutorial.Steps), step.Title) | ||
| fmt.Println(step.Concept) |
Comment on lines
+5
to
+6
| "Use --boot-type=local to boot from a local image for faster startup.", | ||
| "Use --root-volume=size:50GB to specify the root disk size.", |
Comment on lines
+21
to
+22
| "Forgetting to specify --image or --boot-type will result in a server that cannot boot.", | ||
| "The --root-volume flag must include a size suffix (e.g. 50GB).", |
| ), | ||
| ) | ||
| } | ||
| sb.WriteString("\nRun 'scw tutorial <title>' to start a tutorial.") |
Comment on lines
+52
to
+54
| t.Run("no-progress", core.Test(&core.TestConfig{ | ||
| Commands: commands.GetCommands(), | ||
| Cmd: "scw tutorial resume", |
remyleone
marked this pull request as draft
September 14, 2026 12:41
This branch has not been deployed
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
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.
Community Note
Relates OR Closes #0000