feat(tui): add cloud settings screen and docs#607
Conversation
📝 WalkthroughWalkthroughAdds a TUI Cloud sync settings screen with dashboard navigation, submenu controls, shared menu rendering, tests, and documentation for cloud token precedence and fallback behavior. ChangesCloud sync settings
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new Cloud sync settings entry point to the TUI dashboard and introduces a dedicated Cloud Settings menu screen, while updating cloud troubleshooting docs to explicitly document the intentional cloud.json.token fallback behavior.
Changes:
- Added a new
ScreenCloudSettingsscreen with a Cloud Settings menu view and key handling, plus a sharedrenderMenuhelper to reduce duplicated menu rendering logic. - Updated TUI routing and tests to cover the new screen and the dashboard navigation path into/out of Cloud Settings.
- Updated cloud troubleshooting docs to clarify token precedence (
ENGRAM_CLOUD_TOKENovercloud.json.token) and the rationale for the fallback.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tui/model.go | Adds ScreenCloudSettings to the screen enum. |
| internal/tui/model_test.go | Adds coverage to ensure the new screen constant is unique/consistent. |
| internal/tui/update.go | Adds dashboard menu item, Cloud Settings submenu items, and key handling/router integration. |
| internal/tui/update_test.go | Adds navigation/menu tests for the new Cloud Settings screen (with a couple brittle index assumptions). |
| internal/tui/view.go | Adds Cloud Settings view and factors out a reusable renderMenu helper. |
| internal/tui/view_test.go | Extends view-router coverage to include the new screen. |
| docs/engram-cloud/troubleshooting.md | Documents the intentional cloud.json.token fallback and env-var precedence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestCloudSettingsNavigation(t *testing.T) { | ||
| m := New(nil, "") | ||
| m.Cursor = 4 // Cloud sync settings | ||
|
|
| m = New(nil, "") | ||
| m.Screen = ScreenCloudSettings | ||
| m.Cursor = 3 // Back | ||
| updatedModel, cmd := m.handleCloudSettingsKeys("enter") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/tui/update_test.go`:
- Around line 310-321: Add test coverage alongside the existing cloud-settings
key handling test for an unimplemented item such as m.Cursor = 0, verifying both
“enter” and “space” return the unchanged Model and a nil command. Keep the
existing ScreenDashboard and refresh-command assertions for the Back item
unchanged.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f363ddb3-6837-4c6f-9f2c-61f766abca4c
📒 Files selected for processing (7)
docs/engram-cloud/troubleshooting.mdinternal/tui/model.gointernal/tui/model_test.gointernal/tui/update.gointernal/tui/update_test.gointernal/tui/view.gointernal/tui/view_test.go
| m = New(nil, "") | ||
| m.Screen = ScreenCloudSettings | ||
| m.Cursor = 3 // Back | ||
| updatedModel, cmd := m.handleCloudSettingsKeys("enter") | ||
| updated = updatedModel.(Model) | ||
| if updated.Screen != ScreenDashboard { | ||
| t.Fatalf("enter on Back should return to dashboard, got %v", updated.Screen) | ||
| } | ||
| if cmd == nil { | ||
| t.Fatal("enter on Back should refresh stats") | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add coverage for selecting unimplemented menu items.
As per path instructions for **/*_test.go, please verify coverage of edge cases. The test covers pressing "enter" on the "Back" item (m.Cursor = 3), but lacks an assertion for pressing "enter" or "space" on the currently unimplemented items (e.g., m.Cursor = 0). Verify that selecting these items safely acts as a no-op (returning the model unchanged with a nil command) to prevent regressions in future PRs.
🧪 Proposed test addition
if cmd == nil {
t.Fatal("enter on Back should refresh stats")
}
+
+ m = New(nil, "")
+ m.Screen = ScreenCloudSettings
+ m.Cursor = 0 // Configure server
+ updatedModel, cmd = m.handleCloudSettingsKeys("enter")
+ updated = updatedModel.(Model)
+ if updated.Screen != ScreenCloudSettings {
+ t.Fatalf("enter on unimplemented item should not change screen, got %v", updated.Screen)
+ }
+ if cmd != nil {
+ t.Fatal("enter on unimplemented item should return nil command")
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| m = New(nil, "") | |
| m.Screen = ScreenCloudSettings | |
| m.Cursor = 3 // Back | |
| updatedModel, cmd := m.handleCloudSettingsKeys("enter") | |
| updated = updatedModel.(Model) | |
| if updated.Screen != ScreenDashboard { | |
| t.Fatalf("enter on Back should return to dashboard, got %v", updated.Screen) | |
| } | |
| if cmd == nil { | |
| t.Fatal("enter on Back should refresh stats") | |
| } | |
| } | |
| m = New(nil, "") | |
| m.Screen = ScreenCloudSettings | |
| m.Cursor = 3 // Back | |
| updatedModel, cmd := m.handleCloudSettingsKeys("enter") | |
| updated = updatedModel.(Model) | |
| if updated.Screen != ScreenDashboard { | |
| t.Fatalf("enter on Back should return to dashboard, got %v", updated.Screen) | |
| } | |
| if cmd == nil { | |
| t.Fatal("enter on Back should refresh stats") | |
| } | |
| m = New(nil, "") | |
| m.Screen = ScreenCloudSettings | |
| m.Cursor = 0 // Configure server | |
| updatedModel, cmd = m.handleCloudSettingsKeys("enter") | |
| updated = updatedModel.(Model) | |
| if updated.Screen != ScreenCloudSettings { | |
| t.Fatalf("enter on unimplemented item should not change screen, got %v", updated.Screen) | |
| } | |
| if cmd != nil { | |
| t.Fatal("enter on unimplemented item should return nil command") | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tui/update_test.go` around lines 310 - 321, Add test coverage
alongside the existing cloud-settings key handling test for an unimplemented
item such as m.Cursor = 0, verifying both “enter” and “space” return the
unchanged Model and a nil command. Keep the existing ScreenDashboard and
refresh-command assertions for the Back item unchanged.
Source: Path instructions
Summary
Adds the Cloud sync settings entry to the TUI dashboard with a sub-menu (Configure server / View status / Enroll projects / Back), and updates the troubleshooting docs to document the intentional
cloud.json.tokenfallback.Spec
What's included
8ed5031feat(tui): add cloud settings screen and dashboard entry1e48b10docs(cloud): document cloud.json.token fallback policyTest evidence
go test ./...passesinternal/tuigo vet ./...cleanMerge order
This is PR 1 of 6 in the
tui-cloud-configfeature delivery. Suggested order:All 6 PRs target
main. After merging PR N, GitHub's "Update branch" button on PR N+1 will refresh the diff to show only that PR's changes.Issue link
Part of #577
Summary by CodeRabbit
New Features
Documentation