Skip to content

fix(desktop): persist new workspace modal state#2194

Merged
Kitenite merged 3 commits into
superset-sh:mainfrom
Kitenite:kitenite/whip-rosehip
Mar 7, 2026
Merged

fix(desktop): persist new workspace modal state#2194
Kitenite merged 3 commits into
superset-sh:mainfrom
Kitenite:kitenite/whip-rosehip

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Mar 7, 2026

Summary

  • persist new workspace modal UI state across close/reopen, including selected tab, selected project, prompt draft, and list searches
  • move modal state into the shared zustand store instead of component-local state
  • clear modal inputs only after successful workspace create/open actions

Testing

  • bunx biome check apps/desktop/src/renderer/stores/new-workspace-modal.ts apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx
  • bunx tsc -p apps/desktop/tsconfig.json --noEmit (currently fails due to pre-existing missing generated/module files: routeTree.gen and resources/public/file-icons/manifest.json)

Summary by cubic

Persisted New Workspace modal state across open/close and refined the Prompt tab’s advanced options. Users keep their tab, project, prompt, and per‑tab searches; inputs clear only after a successful create or when opening an existing workspace.

  • Bug Fixes

    • Modal no longer resets when closed and reopened.
    • Inputs clear only after workspace creation or when opening an existing workspace, guarded by a draftVersion to avoid wiping new edits.
  • Refactors

    • Centralized modal state in a shared Zustand store: active tab, selected project (auto-select most recent if none or missing), prompt, branch name + edited flag, base branch, run setup, showAdvanced, branch search, and per‑tab list queries.
    • Wired CommandInput to the correct tab-specific query; advanced options are toggled via an inline button next to Create and render only when open.

Written for commit 7fe165c. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Centralized workspace-creation modal with per-tab queries and tab-aware search; unified search input follows the active tab.
    • Prompt area updated: grouped action buttons with an advanced-options toggle, refined branch/base controls, and clearer draft-aware behavior.
    • Creation shows a promise-backed toast while in progress.
  • Bug Fixes

    • Modal inputs reliably cleared after successful creation (draft-aware) and when opening an existing workspace.
    • Recent-project selection only applied if no project is already selected.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 7, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3162fd77-6da0-4a65-98ed-6adf07ce383d

📥 Commits

Reviewing files that changed from the base of the PR and between f2a6120 and 7fe165c.

📒 Files selected for processing (6)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx
  • apps/desktop/src/renderer/stores/new-workspace-modal.ts

📝 Walkthrough

Walkthrough

Modal state and inputs were migrated to a centralized store (active tab, selected project, prompt/branch fields, per-tab queries, setters). Create flows now produce a single creation promise used with toast.promise; modal inputs are cleared via store hooks before navigation or after successful creation.

Changes

Cohort / File(s) Summary
Modal Store Refactor
apps/desktop/src/renderer/stores/new-workspace-modal.ts
Adds NewWorkspaceModalTab; expands modal state with activeTab, selectedProjectId, prompt, branch fields, issuesQuery/pullRequestsQuery/branchesQuery, draftVersion, setters, clearInputs() and clearInputsIfDraftVersion(); updates openModal/closeModal; exposes many new read/set hooks.
Main Modal Component
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
Replaces local state with store-backed hooks for active tab, selected project and per-tab queries; introduces listQuery derived from active tab, handleListQueryChange, COMMAND_CLASS_NAME, and filters recentProjects for truthy IDs; casts Tabs value to NewWorkspaceModalTab.
Create Flow in List Groups
apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx, .../IssuesGroup/IssuesGroup.tsx, .../PullRequestsGroup/PullRequestsGroup.tsx
Add useClearNewWorkspaceModalInputs / useClearNewWorkspaceModalInputsIfDraftVersion / useNewWorkspaceModalDraftVersion; create a single createWorkspacePromise (via mutateAsync), pass it to toast.promise, clear inputs after success, and clear inputs before navigating to existing workspaces.
Prompt Group — Inputs to Store & UI Changes
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx
Moves prompt/branch/base/showAdvanced/runSetup/branchSearch state into store hooks and setters; constructs createWorkspacePromise, closes modal before showing toast.promise, clears inputs on success; replaces single Create button with a ButtonGroup and conditionally renders advanced options.
Prompt Advanced Options — UI Simplification
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PromptGroupAdvancedOptions/PromptGroupAdvancedOptions.tsx
Removes Collapsible-based UI and the showAdvanced/onShowAdvancedChange props; replaces collapsible with a static advanced options block and flattens base-branch selection layout and inner JSX.

Sequence Diagram

sequenceDiagram
    participant User
    participant Modal as NewWorkspaceModal
    participant Store as NewWorkspaceModalStore
    participant API as WorkspaceAPI
    participant Toast as Toast

    User->>Modal: Click "Create"
    Modal->>Store: read activeTab, selectedProjectId, inputs
    Store-->>Modal: state values
    Modal->>API: call mutateAsync(...) -> createWorkspacePromise
    Modal->>Toast: toast.promise(createWorkspacePromise, ...)
    API->>API: perform creation
    API-->>Modal: success / error
    alt success
        Modal->>Store: clearInputsIfDraftVersion(draftVersion)
        Store-->>Modal: inputs cleared
        Modal->>User: navigate / show success
    else error
        Modal->>User: show error
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hopped through tabs and tucked queries tight,

Stored my prompts and nudged the modal right,
One promise warmed the toast's sweet song,
Cleared the inputs, then we moved along,
Off to workspaces, swift and light.

🚥 Pre-merge checks | ✅ 2 | ❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(desktop): persist new workspace modal state' is concise, specific, and accurately reflects the main change in the pull request.
Description check ✅ Passed The description is comprehensive and covers all major aspects: a clear summary of changes, testing steps performed, and context about the refactoring.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

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

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx">

<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx:165">
P3: Avoid swallowing create-from-PR failures with an empty catch; log the error context in the rejection handler so failures remain observable.</violation>
</file>

<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx">

<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx:72">
P2: Avoid swallowing create-workspace errors in an empty catch; log the rejection context so failures remain observable.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx">

<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx:135">
P2: Avoid swallowing workspace creation failures in an empty catch; log a warning with context instead.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx">

<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx:153">
P3: Avoid using an empty catch here; log the rejected error so create-workspace failures are still diagnosable.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

.then(() => {
clearInputs();
})
.catch(() => undefined);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Avoid swallowing create-workspace errors in an empty catch; log the rejection context so failures remain observable.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx, line 72:

<comment>Avoid swallowing create-workspace errors in an empty catch; log the rejection context so failures remain observable.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) </comment>

<file context>
@@ -52,29 +54,33 @@ export function BranchesGroup({ projectId, onClose }: BranchesGroupProps) {
+				.then(() => {
+					clearInputs();
+				})
+				.catch(() => undefined);
 		},
-		[projectId, onClose, createBranchWorkspace],
</file context>
Suggested change
.catch(() => undefined);
.catch((err) => {
console.warn("[BranchesGroup] failed to create workspace from branch", err);
return undefined;
});
Fix with Cubic

.then(() => {
clearInputs();
})
.catch(() => undefined);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Avoid swallowing workspace creation failures in an empty catch; log a warning with context instead.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx, line 135:

<comment>Avoid swallowing workspace creation failures in an empty catch; log a warning with context instead.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) </comment>

<file context>
@@ -106,23 +113,26 @@ export function PromptGroup({ projectId, onClose }: PromptGroupProps) {
+			.then(() => {
+				clearInputs();
+			})
+			.catch(() => undefined);
 	};
 
</file context>
Suggested change
.catch(() => undefined);
.catch((err) => {
console.warn("[PromptGroup] workspace creation failed", err);
});
Fix with Cubic

.then(() => {
clearInputs();
})
.catch(() => undefined);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Avoid swallowing create-from-PR failures with an empty catch; log the error context in the rejection handler so failures remain observable.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx, line 165:

<comment>Avoid swallowing create-from-PR failures with an empty catch; log the error context in the rejection handler so failures remain observable.</comment>

<file context>
@@ -138,25 +140,29 @@ export function PullRequestsGroup({
+							.then(() => {
+								clearInputs();
+							})
+							.catch(() => undefined);
 					}}
 					className="group h-12"
</file context>
Suggested change
.catch(() => undefined);
.catch((err) => {
console.warn("[PullRequestsGroup] failed to create workspace from PR", err);
});
Fix with Cubic

.then(() => {
clearInputs();
})
.catch(() => undefined);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Avoid using an empty catch here; log the rejected error so create-workspace failures are still diagnosable.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.)

View Feedback: 1 2

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx, line 153:

<comment>Avoid using an empty catch here; log the rejected error so create-workspace failures are still diagnosable.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) </comment>

<file context>
@@ -125,26 +127,30 @@ export function IssuesGroup({ projectId, onClose }: IssuesGroupProps) {
+							.then(() => {
+								clearInputs();
+							})
+							.catch(() => undefined);
 					}}
 					className="group h-12"
</file context>
Suggested change
.catch(() => undefined);
.catch((err) => console.warn("[IssuesGroup] workspace creation failed", err));
Fix with Cubic

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 48-53: When the modal opens, the effect handling selection should
not only set a default when selectedProjectId is falsy but also clear it when
the persisted selectedProjectId is no longer present in recentProjects; update
the useEffect that references isOpen, recentProjects, selectedProjectId, and
setSelectedProjectId so it checks if selectedProjectId exists in recentProjects
(by matching ids) and calls setSelectedProjectId(undefined) if it does not, and
apply the same change to the similar effect at the other location (lines 55-57)
to prevent child components from receiving a stale projectId.

In `@apps/desktop/src/renderer/stores/new-workspace-modal.ts`:
- Around line 57-63: clearInputs() currently mutates shared modal state and
causes races when createWorkspacePromise.then(() => clearInputs()) in
PromptGroup/IssuesGroup/PullRequestsGroup/BranchesGroup clears inputs from a
newly opened modal; fix by making the reset request-scoped: generate a unique
request/session token when starting the create (e.g., workspaceCreateToken),
attach it to the submission, and either (a) have clearInputs accept that token
and only clear when the store's current token still equals the submission token,
or (b) before calling clearInputs from the promise, compare a snapshot of
relevant store fields (prompt, issuesQuery, pullRequestsQuery, branchesQuery) or
the token to ensure they match the submission that finished; update
createWorkspacePromise callers (PromptGroup, IssuesGroup, PullRequestsGroup,
BranchesGroup) to pass the token/snapshot so clearing only happens for the
matching request.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: da61f6c6-7bea-49cf-af55-b0550d2cd3a4

📥 Commits

Reviewing files that changed from the base of the PR and between 639c133 and e1061ba.

📒 Files selected for processing (6)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx
  • apps/desktop/src/renderer/stores/new-workspace-modal.ts

Comment on lines +57 to +63
clearInputs: () =>
set({
prompt: "",
issuesQuery: "",
pullRequestsQuery: "",
branchesQuery: "",
}),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make modal resets request-scoped.

clearInputs() now mutates shared modal state. The new createWorkspacePromise.then(() => clearInputs()) flows in PromptGroup, IssuesGroup, PullRequestsGroup, and BranchesGroup will wipe whatever the user typed after reopening the modal while an earlier create is still in flight. Please gate the reset behind a modal/session token, or only clear when the current store snapshot still matches the submission that just finished.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/stores/new-workspace-modal.ts` around lines 57 -
63, clearInputs() currently mutates shared modal state and causes races when
createWorkspacePromise.then(() => clearInputs()) in
PromptGroup/IssuesGroup/PullRequestsGroup/BranchesGroup clears inputs from a
newly opened modal; fix by making the reset request-scoped: generate a unique
request/session token when starting the create (e.g., workspaceCreateToken),
attach it to the submission, and either (a) have clearInputs accept that token
and only clear when the store's current token still equals the submission token,
or (b) before calling clearInputs from the promise, compare a snapshot of
relevant store fields (prompt, issuesQuery, pullRequestsQuery, branchesQuery) or
the token to ensure they match the submission that finished; update
createWorkspacePromise callers (PromptGroup, IssuesGroup, PullRequestsGroup,
BranchesGroup) to pass the token/snapshot so clearing only happens for the
matching request.

@Kitenite Kitenite force-pushed the kitenite/whip-rosehip branch from e1061ba to 9753fbf Compare March 7, 2026 19:20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx (1)

169-174: ⚠️ Potential issue | 🟠 Major

Skip this reset on the initial mount.

Line 170 runs on every mount, not only when projectId actually changes. If the modal unmounts on close, reopening it will immediately wipe the persisted baseBranch and branchSearch, which breaks the new close/reopen persistence behavior.

Suggested fix
+	const previousProjectIdRef = useRef<string | null>(projectId);
+
-	// biome-ignore lint/correctness/useExhaustiveDependencies: intentionally reset base branch controls when project changes
 	useEffect(() => {
+		if (previousProjectIdRef.current === projectId) {
+			return;
+		}
+
+		previousProjectIdRef.current = projectId;
 		setBaseBranch(null);
 		setBaseBranchOpen(false);
 		setBranchSearch("");
-	}, [projectId]);
+	}, [projectId, setBaseBranch, setBranchSearch]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`
around lines 169 - 174, The effect that resets base branch state (useEffect
using setBaseBranch, setBaseBranchOpen, setBranchSearch with dependency
[projectId]) runs on the initial mount and clears persisted values on modal
reopen; change the effect so it skips its reset on the initial mount and only
runs when projectId actually changes (e.g., track initial mount or previous
projectId with a useRef and only call
setBaseBranch(null)/setBaseBranchOpen(false)/setBranchSearch("") when projectId
!== prevProjectId and not the initial render).
🧹 Nitpick comments (2)
apps/desktop/src/renderer/stores/new-workspace-modal.ts (1)

58-67: Consider whether openModal should reset state for fresh starts.

Currently openModal preserves all existing inputs (prompt, queries, etc.) and only conditionally updates selectedProjectId. This is the intended behavior per PR objectives (persist state across close/reopen). However, if the user explicitly wants a "fresh" modal in certain flows, there's no mechanism for that.

If this is intentional, this is fine. Otherwise, consider adding an optional reset?: boolean parameter.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/stores/new-workspace-modal.ts` around lines 58 -
67, openModal currently preserves existing modal inputs and only updates
selectedProjectId; add an optional boolean parameter (e.g., reset?: boolean) to
openModal to allow clearing modal fields when a fresh start is needed, and
implement logic inside openModal to reset state (prompt, queries, etc.) when
reset is true while still setting isOpen and selectedProjectId; keep closeModal
behavior the same.
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)

36-37: Long class string is acceptable but could be extracted.

This Tailwind class string is quite lengthy. Consider extracting it to a shared styles constant or using a className utility if this pattern repeats elsewhere.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`
around lines 36 - 37, The long Tailwind class string assigned to
COMMAND_CLASS_NAME should be extracted to a shared style constant or a className
utility to improve reuse and readability; create a new exported constant (e.g.,
COMMAND_CLASS_NAME or COMMAND_STYLES) in a shared module (styles/constants or
utils/classNames), replace the inline definition in NewWorkspaceModal's
COMMAND_CLASS_NAME with an import, and update any other components that use the
same pattern to import this shared constant or use the utility to compose
classes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`:
- Around line 226-248: The success handler currently calls clearInputs()
unconditionally, which can wipe a newer draft if an earlier request completes
later; to fix, capture a submission token or snapshot of the inputs when calling
createWorkspace.mutateAsyncWithPendingSetup (e.g., store a local submissionId or
the trimmedPrompt/branchSlug/baseBranch snapshot alongside
createWorkspacePromise) and in the promise .then() only call clearInputs() if
the current modal state still matches that submission token/snapshot (or the
stored submissionId matches the modal's latest submissionId); update the
handlers around createWorkspacePromise, mutateAsyncWithPendingSetup, and
clearInputs to perform this guard so only the originating submission clears the
inputs.

---

Outside diff comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`:
- Around line 169-174: The effect that resets base branch state (useEffect using
setBaseBranch, setBaseBranchOpen, setBranchSearch with dependency [projectId])
runs on the initial mount and clears persisted values on modal reopen; change
the effect so it skips its reset on the initial mount and only runs when
projectId actually changes (e.g., track initial mount or previous projectId with
a useRef and only call
setBaseBranch(null)/setBaseBranchOpen(false)/setBranchSearch("") when projectId
!== prevProjectId and not the initial render).

---

Nitpick comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 36-37: The long Tailwind class string assigned to
COMMAND_CLASS_NAME should be extracted to a shared style constant or a className
utility to improve reuse and readability; create a new exported constant (e.g.,
COMMAND_CLASS_NAME or COMMAND_STYLES) in a shared module (styles/constants or
utils/classNames), replace the inline definition in NewWorkspaceModal's
COMMAND_CLASS_NAME with an import, and update any other components that use the
same pattern to import this shared constant or use the utility to compose
classes.

In `@apps/desktop/src/renderer/stores/new-workspace-modal.ts`:
- Around line 58-67: openModal currently preserves existing modal inputs and
only updates selectedProjectId; add an optional boolean parameter (e.g., reset?:
boolean) to openModal to allow clearing modal fields when a fresh start is
needed, and implement logic inside openModal to reset state (prompt, queries,
etc.) when reset is true while still setting isOpen and selectedProjectId; keep
closeModal behavior the same.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b96dc7a9-185e-4b87-aeb7-5867163beafe

📥 Commits

Reviewing files that changed from the base of the PR and between e1061ba and 9753fbf.

📒 Files selected for processing (6)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx
  • apps/desktop/src/renderer/stores/new-workspace-modal.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx (1)

227-249: ⚠️ Potential issue | 🟠 Major

Guard clearInputs() against late request completions.

Because the modal state now survives close/reopen, the .then(() => clearInputs()) at Line 246 can erase a newer draft if request A finishes after the user has already reopened the modal and started request B. Please tie the clear to the originating submission before wiping the shared store.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`
around lines 227 - 249, The clearInputs() call can wipe a newer draft if an
earlier createWorkspacePromise resolves after a reopened modal; fix by tagging
each submission and only clearing when the resolving promise matches that tag:
when calling createWorkspace.mutateAsyncWithPendingSetup create a local unique
submissionId (e.g. a UUID/timestamp or Symbol) and set a component-level
ref/state like lastSubmissionId = submissionId; keep the createWorkspacePromise
as-is but in its .then handler only call clearInputs() if submissionId ===
lastSubmissionId; reference createWorkspace.mutateAsyncWithPendingSetup,
createWorkspacePromise, lastSubmissionId (ref/state) and clearInputs to locate
and implement the guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`:
- Around line 227-249: The clearInputs() call can wipe a newer draft if an
earlier createWorkspacePromise resolves after a reopened modal; fix by tagging
each submission and only clearing when the resolving promise matches that tag:
when calling createWorkspace.mutateAsyncWithPendingSetup create a local unique
submissionId (e.g. a UUID/timestamp or Symbol) and set a component-level
ref/state like lastSubmissionId = submissionId; keep the createWorkspacePromise
as-is but in its .then handler only call clearInputs() if submissionId ===
lastSubmissionId; reference createWorkspace.mutateAsyncWithPendingSetup,
createWorkspacePromise, lastSubmissionId (ref/state) and clearInputs to locate
and implement the guard.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e1d58d3d-cb97-44c9-8998-6badde517882

📥 Commits

Reviewing files that changed from the base of the PR and between 9753fbf and f2a6120.

📒 Files selected for processing (2)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PromptGroupAdvancedOptions/PromptGroupAdvancedOptions.tsx

@Kitenite Kitenite merged commit 6228e8c into superset-sh:main Mar 7, 2026
6 of 7 checks passed
@Kitenite Kitenite deleted the kitenite/whip-rosehip branch March 7, 2026 20:14
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx">

<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx:63">
P2: This condition resets the selected project while recents are still loading (empty array), which can wipe the persisted project selection on reopen.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment on lines +63 to 65
if (!hasSelectedProject) {
setSelectedProjectId(recentProjects[0]?.id ?? null);
}
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This condition resets the selected project while recents are still loading (empty array), which can wipe the persisted project selection on reopen.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx, line 63:

<comment>This condition resets the selected project while recents are still loading (empty array), which can wipe the persisted project selection on reopen.</comment>

<file context>
@@ -57,8 +57,11 @@ export function NewWorkspaceModal() {
+		const hasSelectedProject = recentProjects.some(
+			(project) => project.id === selectedProjectId,
+		);
+		if (!hasSelectedProject) {
+			setSelectedProjectId(recentProjects[0]?.id ?? null);
 		}
</file context>
Suggested change
if (!hasSelectedProject) {
setSelectedProjectId(recentProjects[0]?.id ?? null);
}
if (recentProjects.length > 0 && !hasSelectedProject) {
setSelectedProjectId(recentProjects[0]?.id ?? null);
}
Fix with Cubic

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