feat(desktop): persist last project + base branch in v2 new-workspace modal#3844
Conversation
Remember the user's most recent project and per-project base branch in localStorage so the new-workspace modal pre-fills them on next open. Uses the same plain localStorage pattern as useAgentLaunchPreferences.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a persisted defaults store for V2 workspace creation and updates the modal and PromptGroup to rehydrate and persist per-project base branch, last project, and last host target selections instead of always clearing them. Changes
Sequence DiagramsequenceDiagram
actor User
participant Modal as DashboardNewWorkspaceModal
participant Store as useV2WorkspaceCreateDefaultsStore
participant PromptGroup as PromptGroup
User->>Modal: Open modal
Modal->>Store: get lastProjectId, lastHostTarget, baseBranchesByProjectId
Store-->>Modal: return persisted defaults
Modal->>Modal: rehydrate draft (once per open)
User->>Modal: Select project
Modal->>Store: setLastProjectId(projectId)
Store-->>Store: persist lastProjectId
Modal->>Modal: update draft.selectedProjectId
User->>PromptGroup: Select host target
PromptGroup->>Store: setLastHostTarget(hostTarget)
Store-->>Store: persist lastHostTarget
PromptGroup->>PromptGroup: update draft.hostTarget
User->>PromptGroup: Change base branch
PromptGroup->>Store: setBaseBranchDefault(projectId, branch, source) / clearBaseBranchDefault
Store-->>Store: persist or clear per-project base branch
PromptGroup->>PromptGroup: update draft.baseBranch
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 4/8 reviews remaining, refill in 26 minutes and 18 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib/v2-workspace-create-defaults.ts`:
- Around line 25-47: readBaseBranches() currently trusts JSON.parse and casts to
Record<string, V2WorkspaceCreateBaseBranchDefault>, which can let corrupt or
malicious localStorage values leak into state; update readBaseBranches to
validate and sanitize the parsed value (ensure it's an object, iterate its keys
and that each value has the expected shape and types for
V2WorkspaceCreateBaseBranchDefault — e.g., branchName is a string, source is a
string and optionally matches allowed values — and skip or coerce invalid
entries) before returning a Record; keep getBaseBranchDefault behavior but rely
on the sanitized output from readBaseBranches (use BASE_BRANCHES_KEY,
readBaseBranches, V2WorkspaceCreateBaseBranchDefault in your changes).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a5375be-5998-4684-9c36-8c2cc6c90fb5
📒 Files selected for processing (3)
apps/desktop/src/renderer/lib/v2-workspace-create-defaults.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceModalContent/DashboardNewWorkspaceModalContent.tsx
| function readBaseBranches(): Record< | ||
| string, | ||
| V2WorkspaceCreateBaseBranchDefault | ||
| > { | ||
| if (typeof window === "undefined") return {}; | ||
| const raw = window.localStorage.getItem(BASE_BRANCHES_KEY); | ||
| if (!raw) return {}; | ||
| try { | ||
| return JSON.parse(raw) as Record< | ||
| string, | ||
| V2WorkspaceCreateBaseBranchDefault | ||
| >; | ||
| } catch { | ||
| return {}; | ||
| } | ||
| } | ||
|
|
||
| export function getBaseBranchDefault( | ||
| projectId: string | null, | ||
| ): V2WorkspaceCreateBaseBranchDefault | null { | ||
| if (!projectId) return null; | ||
| return readBaseBranches()[projectId] ?? null; | ||
| } |
There was a problem hiding this comment.
Validate parsed localStorage data before returning typed defaults.
readBaseBranches() trusts JSON.parse via cast only. Corrupt or manually edited storage can inject invalid branchName/source, which then propagates into state as if it were valid.
Suggested fix
+function isBaseBranchSource(value: unknown): value is V2WorkspaceCreateBaseBranchSource {
+ return value === "local" || value === "remote-tracking";
+}
+
+function toBaseBranchDefault(
+ value: unknown,
+): V2WorkspaceCreateBaseBranchDefault | null {
+ if (!value || typeof value !== "object") return null;
+ const entry = value as { branchName?: unknown; source?: unknown };
+ if (typeof entry.branchName !== "string") return null;
+ const branchName = entry.branchName.trim();
+ if (!branchName || !isBaseBranchSource(entry.source)) return null;
+ return { branchName, source: entry.source };
+}
+
function readBaseBranches(): Record<
string,
V2WorkspaceCreateBaseBranchDefault
> {
if (typeof window === "undefined") return {};
const raw = window.localStorage.getItem(BASE_BRANCHES_KEY);
if (!raw) return {};
try {
- return JSON.parse(raw) as Record<
- string,
- V2WorkspaceCreateBaseBranchDefault
- >;
+ const parsed: unknown = JSON.parse(raw);
+ if (!parsed || typeof parsed !== "object") return {};
+ const safe: Record<string, V2WorkspaceCreateBaseBranchDefault> = {};
+ for (const [projectId, value] of Object.entries(
+ parsed as Record<string, unknown>,
+ )) {
+ const normalized = toBaseBranchDefault(value);
+ if (normalized) safe[projectId] = normalized;
+ }
+ return safe;
} catch {
return {};
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/lib/v2-workspace-create-defaults.ts` around lines
25 - 47, readBaseBranches() currently trusts JSON.parse and casts to
Record<string, V2WorkspaceCreateBaseBranchDefault>, which can let corrupt or
malicious localStorage values leak into state; update readBaseBranches to
validate and sanitize the parsed value (ensure it's an object, iterate its keys
and that each value has the expected shape and types for
V2WorkspaceCreateBaseBranchDefault — e.g., branchName is a string, source is a
string and optionally matches allowed values — and skip or coerce invalid
entries) before returning a Record; keep getBaseBranchDefault behavior but rely
on the sanitized output from readBaseBranches (use BASE_BRANCHES_KEY,
readBaseBranches, V2WorkspaceCreateBaseBranchDefault in your changes).
There was a problem hiding this comment.
1 issue found across 3 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/lib/v2-workspace-create-defaults.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/v2-workspace-create-defaults.ts:33">
P2: Validate parsed localStorage data shape before treating it as a branch-default map; non-object JSON values can cause runtime errors.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR adds Confidence Score: 5/5Safe to merge; the one finding is a minor UX edge case (stale persisted branch after a clear) that does not block the primary workflow. All findings are P2 style/UX suggestions. Core happy-path behavior (persist → pre-fill on reopen) is correct, persisted project selection properly validates against the live project list, and the utility handles SSR, JSON parse errors, and empty strings. No data-loss or security concerns. PromptGroup.tsx — the onBaseBranchChange handler should also clear the persisted entry when branch is null.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/lib/v2-workspace-create-defaults.ts | New localStorage utility for persisting last project ID and per-project base branch defaults; clean implementation with SSR guard, JSON parse error handling, and trim validation — no delete path for clearing a project's branch entry. |
| apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/PromptGroup.tsx | Reset-on-project-change effect now pre-fills persisted base branch; write site only persists when branch is truthy, meaning an explicit clear leaves a stale default in localStorage. |
| apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceModalContent/DashboardNewWorkspaceModalContent.tsx | Project-selection effect now prefers persisted last project over list position 0; manual project picks correctly write to localStorage via onSelectProject. |
Sequence Diagram
sequenceDiagram
actor User
participant Modal as DashboardNewWorkspaceModalContent
participant LS as localStorage
participant PG as PromptGroup
User->>Modal: Open modal
Modal->>LS: getLastProjectId()
LS-->>Modal: projectA (or null)
Modal->>Modal: updateDraft({ selectedProjectId: projectA ?? recentProjects[0] })
Modal->>PG: render(projectId=projectA)
PG->>PG: useEffect — projectId changed (null→projectA)
PG->>LS: getBaseBranchDefault(projectA)
LS-->>PG: { branchName, source } (or null)
PG->>PG: updateDraft({ baseBranch, baseBranchSource })
User->>Modal: Select project B
Modal->>LS: setLastProjectId(projectB)
Modal->>PG: render(projectId=projectB)
PG->>PG: useEffect — projectId changed (A→B)
PG->>LS: getBaseBranchDefault(projectB)
LS-->>PG: null (no prior default)
PG->>PG: updateDraft({ baseBranch: null, baseBranchSource: null })
User->>PG: Pick base branch X
PG->>LS: setBaseBranchDefault(projectB, X, source)
PG->>PG: updateDraft({ baseBranch: X, baseBranchSource: source })
User->>PG: Submit workspace
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/PromptGroup.tsx
Line: 145-150
Comment:
**Clearing branch doesn't remove persisted default**
When `onBaseBranchChange` is called with `branch = null` (user clears the selection), the `if (projectId && branch && source)` guard skips the write, so the old persisted value stays in localStorage. On the next modal open the cleared branch re-appears as the pre-filled default, contradicting the user's intent.
Consider removing the stored default when branch is explicitly cleared:
```suggestion
onBaseBranchChange: (branch, source) => {
if (projectId) {
if (branch && source) {
setBaseBranchDefault(projectId, branch, source);
} else {
removeBaseBranchDefault(projectId);
}
}
updateDraft({ baseBranch: branch, baseBranchSource: source });
},
```
This requires exporting a `removeBaseBranchDefault` helper from `v2-workspace-create-defaults.ts` that deletes the project's entry from the stored map.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(desktop): persist last project + ba..." | Re-trigger Greptile
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/lib/v2-workspace-create-defaults.ts (1)
30-45:⚠️ Potential issue | 🟠 MajorValidate parsed base-branch defaults before returning typed data.
readBaseBranches()trusts a cast fromJSON.parse, so malformed or manually edited localStorage can inject invalidbranchName/sourcevalues into state. Sanitize each entry and drop invalid records before returning the map.Proposed fix
+function isBaseBranchSource( + value: unknown, +): value is V2WorkspaceCreateBaseBranchSource { + return value === "local" || value === "remote-tracking"; +} + +function toBaseBranchDefault( + value: unknown, +): V2WorkspaceCreateBaseBranchDefault | null { + if (!value || typeof value !== "object") return null; + const entry = value as { branchName?: unknown; source?: unknown }; + if (typeof entry.branchName !== "string") return null; + const branchName = entry.branchName.trim(); + if (!branchName) return null; + if (!isBaseBranchSource(entry.source)) return null; + return { branchName, source: entry.source }; +} + function readBaseBranches(): Record< string, V2WorkspaceCreateBaseBranchDefault > { if (typeof window === "undefined") return {}; const raw = window.localStorage.getItem(BASE_BRANCHES_KEY); if (!raw) return {}; try { - return JSON.parse(raw) as Record< - string, - V2WorkspaceCreateBaseBranchDefault - >; + const parsed: unknown = JSON.parse(raw); + if (!parsed || typeof parsed !== "object") return {}; + const safe: Record<string, V2WorkspaceCreateBaseBranchDefault> = {}; + for (const [projectId, value] of Object.entries( + parsed as Record<string, unknown>, + )) { + const normalized = toBaseBranchDefault(value); + if (normalized) safe[projectId] = normalized; + } + return safe; } catch { return {}; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/v2-workspace-create-defaults.ts` around lines 30 - 45, readBaseBranches currently casts JSON.parse output directly to V2WorkspaceCreateBaseBranchDefault allowing malformed localStorage to leak into state; update readBaseBranches to parse raw JSON safely, verify the parsed value is an object/map, then iterate its entries and only keep records where the value is an object with a valid non-empty string branchName and a valid string source (and, if an enum of allowed sources exists, check membership against that set), dropping any invalid entries before returning the sanitized Record; keep references to BASE_BRANCHES_KEY, readBaseBranches, and V2WorkspaceCreateBaseBranchDefault when locating and updating the code.
🤖 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/lib/v2-workspace-create-defaults.ts`:
- Around line 30-45: readBaseBranches currently casts JSON.parse output directly
to V2WorkspaceCreateBaseBranchDefault allowing malformed localStorage to leak
into state; update readBaseBranches to parse raw JSON safely, verify the parsed
value is an object/map, then iterate its entries and only keep records where the
value is an object with a valid non-empty string branchName and a valid string
source (and, if an enum of allowed sources exists, check membership against that
set), dropping any invalid entries before returning the sanitized Record; keep
references to BASE_BRANCHES_KEY, readBaseBranches, and
V2WorkspaceCreateBaseBranchDefault when locating and updating the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf3c5631-34f6-4a19-8ced-824466b39399
📒 Files selected for processing (3)
apps/desktop/src/renderer/lib/v2-workspace-create-defaults.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceModalContent/DashboardNewWorkspaceModalContent.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceModalContent/DashboardNewWorkspaceModalContent.tsx
Switches the persistence layer to a zustand persist store, matching the pattern used by v2-project-local-meta and other stores in renderer/stores. Also clears the per-project base-branch default when the user explicitly clears the picker, so a stale default doesn't re-appear on next open.
There was a problem hiding this comment.
1 issue found across 4 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/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceModalContent/DashboardNewWorkspaceModalContent.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceModalContent/DashboardNewWorkspaceModalContent.tsx:93">
P2: Validate the persisted `lastHostTarget` shape before applying it to draft state; a truthy check alone can accept malformed localStorage data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
localStorageso the v2 new-workspace modal pre-fills them on next open.renderer/lib/v2-workspace-create-defaults.tsfollows the same plain-localStorage pattern asuseAgentLaunchPreferences— no zustand store.Test plan
Summary by cubic
Persist the last selected project, per-project base branch, and device (host target) in the v2 new-workspace modal so it pre-fills on next open and reduces setup steps.
New Features
zustand-persisted store (v2-workspace-create-defaults).Bug Fixes
lastHostTargetshape before applying.Written for commit 82ba7bb. Summary will update on new commits. Review in cubic
Summary by CodeRabbit