Move managed workspace controls to sidebar - #7
Conversation
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthrough
ChangesManaged Dev PC status
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:57bfad5215
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <SidebarFooter className="p-2"> | ||
| <SidebarProviderUpdatePill /> | ||
| <SidebarUpdatePill /> | ||
| <ManagedDevPcStatus /> |
There was a problem hiding this comment.
When a managed workspace has moved away from its bootstrap ready state, navigating between a thread and Settings replaces SidebarChromeFooter with SettingsSidebarNav, unmounting this status instance and mounting a new one. Because ManagedDevPcStatus initializes from the one-time window.__DEVPC_MANAGED_BOOTSTRAP__ value and does not refresh until the first 10-second interval, each such transition temporarily reports the workspace as Online and re-enables Restart even if polling had already learned that it is restarting, stopped, or errored. Keep the polled state above these mutually exclusive sidebar variants, or refresh immediately on mount.
Useful? React with 👍 / 👎.
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 `@apps/web/src/components/ManagedDevPcStatus.test.tsx`:
- Around line 9-29: Add focused tests for ManagedDevPcStatus covering
VITE_DEVPC_MANAGED disabled, asserting the component renders no markup, and
starting/error workspace states, asserting the Restart Dev PC control is
disabled. Reuse the existing bootstrap setup and import pattern while keeping
the current ready-state assertions 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6716b0b9-ad3f-4228-9c87-307eb56e7347
📒 Files selected for processing (6)
apps/web/src/AppRoot.test.tsxapps/web/src/AppRoot.tsxapps/web/src/components/ManagedDevPcStatus.test.tsxapps/web/src/components/ManagedDevPcStatus.tsxapps/web/src/components/settings/SettingsSidebarNav.tsxapps/web/src/components/sidebar/SidebarChrome.tsx
💤 Files with no reviewable changes (1)
- apps/web/src/AppRoot.tsx
| describe("ManagedDevPcStatus", () => { | ||
| it("renders workspace state and restart as sidebar utility controls", async () => { | ||
| vi.stubEnv("VITE_DEVPC_MANAGED", "1"); | ||
| vi.resetModules(); | ||
| vi.stubGlobal("window", { | ||
| __DEVPC_MANAGED_BOOTSTRAP__: { | ||
| managed: true, | ||
| state: "ready", | ||
| ready: true, | ||
| previewUrlTemplate: "https://{port}.preview.example.test/", | ||
| }, | ||
| }); | ||
| const { ManagedDevPcStatus } = await import("./ManagedDevPcStatus"); | ||
| const markup = renderToStaticMarkup(<ManagedDevPcStatus />); | ||
| expect(markup).toContain('data-devpc-workspace-status="ready"'); | ||
| expect(markup).toContain("Dev PC · Online"); | ||
| expect(markup).toContain('aria-label="Restart Dev PC"'); | ||
| expect(markup).not.toContain("fixed"); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Cover the managed-off and non-ready states.
This test only exercises VITE_DEVPC_MANAGED=1 with a ready workspace. Add focused cases asserting that the component renders nothing when managed mode is disabled and that the restart button is disabled for starting/error states.
🤖 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 `@apps/web/src/components/ManagedDevPcStatus.test.tsx` around lines 9 - 29, Add
focused tests for ManagedDevPcStatus covering VITE_DEVPC_MANAGED disabled,
asserting the component renders no markup, and starting/error workspace states,
asserting the Restart Dev PC control is disabled. Reuse the existing bootstrap
setup and import pattern while keeping the current ready-state assertions
unchanged.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Verification
vp test run src/AppRoot.test.tsx src/components/ManagedDevPcStatus.test.tsx --project unit(2 passed)vp run --filter @t3tools/web typecheck(passed)vp lint --report-unused-disable-directiveson all changed files (passed)vp run --filter @t3tools/web build(passed)git diff --check(passed)Impact
VITE_DEVPC_MANAGEDis disabled.Summary by CodeRabbit
New Features
Bug Fixes
Tests