Conversation
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
k11kirky
added
the
Create release
Graphite App
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/code/src/renderer/features/sessions/service/service.test.ts:2662-2689
**Prefer parameterised test for pre-boot failure cases**
The two `it` blocks cover the same code path (pre-boot `cloudStatus === "failed"` with `status !== "connected"`) and differ only in the presence of `cloudErrorMessage`. This is a textbook candidate for `it.each`, keeping the fixture and assertion logic in one place and making it easier to add further error-message variants later.
Reviews (1): Last reviewed commit: "feat(code): don't show cloud as option i..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+2662
to
+2689
| it("refuses to resume when the previous run failed before the agent booted", async () => { | ||
| const service = getSessionService(); | ||
| mockPreBootFailedSession({ | ||
| cloudErrorMessage: "Sandbox could not be provisioned", | ||
| }); | ||
|
|
||
| await expect(service.sendPrompt("task-123", "retry?")).rejects.toThrow( | ||
| "Sandbox could not be provisioned", | ||
| ); | ||
| expect(mockAuthenticatedClient.runTaskInCloud).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("falls back to a generic message when the failed run has no error", async () => { | ||
| const service = getSessionService(); | ||
| mockPreBootFailedSession(); | ||
|
|
||
| await expect(service.sendPrompt("task-123", "retry?")).rejects.toThrow( | ||
| /Cloud run couldn't start/, | ||
| ); | ||
| expect(mockAuthenticatedClient.runTaskInCloud).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("still resumes when a previously running agent failed mid-execution", async () => { | ||
| const service = getSessionService(); | ||
| mockSessionStoreSetters.getSessionByTaskId.mockReturnValue( | ||
| createMockSession({ | ||
| isCloud: true, | ||
| cloudStatus: "failed", |
Contributor
There was a problem hiding this comment.
The two it blocks cover the same code path (pre-boot cloudStatus === "failed" with status !== "connected") and differ only in the presence of cloudErrorMessage. This is a textbook candidate for it.each, keeping the fixture and assertion logic in one place and making it easier to add further error-message variants later.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/sessions/service/service.test.ts
Line: 2662-2689
Comment:
**Prefer parameterised test for pre-boot failure cases**
The two `it` blocks cover the same code path (pre-boot `cloudStatus === "failed"` with `status !== "connected"`) and differ only in the presence of `cloudErrorMessage`. This is a textbook candidate for `it.each`, keeping the fixture and assertion logic in one place and making it easier to add further error-message variants later.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
joshsny
approved these changes
May 5, 2026
k11kirky
deleted the
05-05-feat_code_don_t_show_cloud_as_option_if_git_integration_not_existing
branch
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.
Problem
Two related cloud session bugs needed fixing:
When a cloud run fails before the agent ever boots (e.g. sandbox provisioning failure), sending a new prompt would attempt to resume the run, spinning up another sandbox that hits the same failure in a loop. The error was never surfaced to the user.
When a user without a GitHub integration had previously used cloud mode, the workspace mode selector would briefly show "cloud" as selected on load before snapping to a local mode once the integration check resolved, causing a visible flicker.
Changes
Pre-boot cloud failure guard:
sendPromptnow checks whether a failed cloud session never reached a connected state. If so, it throws immediately with the stored error message (or a generic fallback) rather than attempting to resume, preventing the provisioning failure loop.Workspace mode flicker fix: The
workspaceModevalue is now derived after the GitHub integration check resolves. While the integration list is still loading, the UI stays optimistic and keeps the current selection. Once loaded, if cloud is unavailable and the last-used mode was"cloud", it falls back to the last-used local workspace mode. The cloud option inWorkspaceModeSelectis also hidden whencloudAvailableisfalse.