Skip to content

fix(cli): explain restart after config changes - #400

Open
acari-git wants to merge 1 commit into
Waishnav:mainfrom
acari-git:fix/config-restart-notice
Open

acari-git wants to merge 1 commit into
Waishnav:mainfrom
acari-git:fix/config-restart-notice

Conversation

@acari-git

@acari-git acari-git commented Oct 12, 2026 •

Copy link
Copy Markdown

DevSpace reads its server configuration at startup, but devspace config set publicBaseUrl previously only printed that the file was updated. This can make a running server appear to ignore a correct config change, as described in #364.

This adds a restart reminder to that command and clarifies in the configuration reference that manual config.jsonc edits, including workspaces.allowedRoots, do not take effect until the server restarts. It does not add hot reload or change configuration behavior.

Verified on macOS with Node 22: the new CLI regression test passes; the full test suite passed (148 passed, 1 skipped); TypeScript typecheck and the Vite/TypeScript build passed.

Summary by CodeRabbit

  • Documentation
    • Clarified that configuration changes require restarting devspace serve to take effect.
  • User Guidance
    • The CLI now reminds you to restart devspace serve after changing publicBaseUrl.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6670eea4-8a24-44c2-98aa-2a8959940cc6

📥 Commits

Reviewing files that changed from the base of the PR and between 531d3f9 and 54fae3f.


📒 Files selected for processing (3)
  • docs/configuration.md
  • src/cli-config.test.ts
  • src/cli.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.



📝 Walkthrough

Walkthrough

The CLI now instructs users to restart devspace serve after setting publicBaseUrl. The integration test checks the message and updated value. The configuration guide explains that server configuration changes take effect after a restart.

Changes

Server Configuration Restart Guidance

Layer / File(s) Summary
CLI restart guidance and validation
src/cli.ts, src/cli-config.test.ts, docs/configuration.md
The CLI prints a restart instruction after setting publicBaseUrl. The integration test checks the instruction and loaded value. The guide states that server configuration changes apply after restarting the server.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other

Suggested reviewers: waishnav


Merge Risk: ⚪ Minimal · up to 54fae

The supported CLI setting takes effect in a running server after restart, and the CLI and guide now explain that requirement. No actionable user-facing regression is established.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main CLI change: explaining that configuration changes require a server restart.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit reads the config with care,
Then sets a new address to share.
“Restart the server,” the CLI will say,
The guide explains when changes take their way.
The test checks the value is right,
And thumps its paws beneath the moonlight.

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Low impact] Safe to merge. The reminder follows a successful save, and configuration persistence and error handling remain unchanged.

T-Rex evidence

Command output from the check

  • Ran the requested existing CLI test on head and captured its command and output; the single test passed.

Evidence from the check

  • Executed this runner to capture the focused test and compare an isolated base checkout with head; it preserves the exact commands and observed output.

Evidence from the check

  • Executed this identical assertion harness against base and head using disposable configuration directories; it checks successful saves, resets, fresh reads, and failure behavior.

Evidence from the check

  • Loaded this hook in CLI subprocesses to fail only the isolated config temporary-file write; it exercises the real save-error path without touching user configuration.

Command output from the check

  • Ran the isolated harness against the specified base revision and captured every CLI response and fresh read; persistence and error checks passed without restart guidance.

Command output from the check

  • Ran the same isolated harness against head and captured every CLI response and fresh read; persistence remained correct and restart guidance appeared only after successful saves.

Command output from the check

  • Ran the requested existing CLI test on head and captured its command and output; the single test passed.

Evidence from the check

  • Executed this runner to capture the focused test and compare an isolated base checkout with head; it preserves the exact commands and observed output.

Evidence from the check

  • Executed this identical assertion harness against base and head using disposable configuration directories; it checks successful saves, resets, fresh reads, and failure behavior.

Evidence from the check

  • Loaded this hook in CLI subprocesses to fail only the isolated config temporary-file write; it exercises the real save-error path without touching user configuration.

Command output from the check

  • Ran the isolated harness against the specified base revision and captured every CLI response and fresh read; persistence and error checks passed without restart guidance.

Command output from the check

  • Ran the same isolated harness against head and captured every CLI response and fresh read; persistence remained correct and restart guidance appeared only after successful saves.

View artifacts

Summary

This PR explains that saved configuration changes require a server restart.

  • Config changes now come with a reminder to restart the server.

The focused CLI regression test passed. Isolated checks against both revisions confirmed that URL updates and null resets remain correctly saved and are read by a fresh configuration load. Invalid input and failed writes leave the configuration unchanged and do not print success or restart messages.

No actionable issues were found. This change is safe to merge.

Reviews (1) · Last reviewed commit: "fix(cli): explain restart after config c..." · Reviewed by Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant