feat(desktop): unified prompt-first new workspace modal#2391
Merged
AviPeltz merged 13 commits intoMar 15, 2026
Conversation
…mpt-first experience Consolidate the multi-tab modal (Prompt/Issues/PRs/Branches) into a single unified view where all context—project, branch, issue, attachments—lives inside the PromptInput as pill-button pickers. Adds workspace name input, inline base branch picker, linked issue chips, and a plus menu for attachments/issues. Optimizes branch selector responsiveness by showing local branch data immediately and keeping search state local to the popover.
Contributor
📝 WalkthroughWalkthroughAdds PR-listing tRPC, consolidates NewWorkspaceModal UI into a PromptGroup with PR/issue linking and file attachments, removes branch/issue/PR subcomponents and tests, and propagates new initialFiles through chat/agent launch schemas and orchestration for workspace creation flows. Changes
sequenceDiagram
participant User
participant PromptGroup
participant PRPicker as PRLinkCommand
participant tRPC as tRPC(listPullRequests)
participant CLI as GitHub_CLI
participant CreateFromPR as useCreateFromPr.mutateAsyncWithSetup
participant Orchestrator
participant WorkspaceService
User->>PromptGroup: open modal, link PR + attach files
PromptGroup->>PRPicker: open PR selector
PRPicker->>tRPC: call listPullRequests(projectId)
tRPC->>CLI: exec gh pr list --json ... (env via execWithShellEnv)
CLI-->>tRPC: PR JSON
tRPC-->>PRPicker: normalized PR list
PRPicker-->>PromptGroup: selected PR
PromptGroup->>CreateFromPR: call mutateAsyncWithSetup(payload + initialFiles)
CreateFromPR->>Orchestrator: enqueue agentLaunchRequest (includes initialFiles)
Orchestrator->>WorkspaceService: create workspace and start agent
WorkspaceService-->>User: workspace opened/created
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
…u-can-attach-images-and-docum
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…ue support Replace basic LinkedIssueChip with the proper LinkedIssuePill component (LinearIcon logo, title + slug display, hover-to-reveal X). Support multiple linked issues with framer-motion animations. Update PlusMenu to use Linear icon. Fix pill horizontal centering via self-stretch.
Add "Link pull request" option to the + menu in the new workspace modal. Uses gh CLI to list open PRs for the selected project, displays linked PR as a pill with status icon, and creates workspace via createFromPr when a PR is linked.
Move ProjectPickerPill from the prompt footer toolbar to the row below the prompt input, alongside the branch picker.
Replace CommandDialog with Popover+Command pattern for the PR selector, matching the existing ProjectPickerPill and BaseBranchPickerInline components. Show "based off PR #N" where the branch picker appears when a PR is linked.
When a PR is linked in the new workspace modal with a prompt and agent selected, the agent now launches after workspace setup instead of being silently dropped.
…ring Switch PRLinkCommand from children-wrapping to virtualRef anchor pattern. Add popover variant to IssueLinkCommand so it renders inline in the new workspace modal while keeping the dialog mode for existing callers.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 28 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/lib/trpc/routers/projects/projects.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/projects/projects.ts:350">
P2: Do not silently swallow `listPullRequests` failures; log the error context before returning `[]` so CLI/auth/output issues are diagnosable.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts:22">
P2: The new file-aware guard is inconsistent with draft handling: draft launches can pass the guard because `initialFiles` is present, but files are then removed from the launch config. This creates a no-op launch config and drops attachments for that request shape.</violation>
</file>
<file name="apps/desktop/src/renderer/react-query/workspaces/useCreateFromPr.ts">
<violation number="1" location="apps/desktop/src/renderer/react-query/workspaces/useCreateFromPr.ts:64">
P1: `agentLaunchRequest` is stored in a shared ref across async mutation calls, which creates a race condition for concurrent `mutateAsyncWithSetup` invocations.</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:510">
P1: Clear `linkedPR` when `projectId` changes; otherwise a PR selected for the previous project can still drive `createFromPr` for the newly selected project.</violation>
<violation number="2" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx:580">
P2: Handle attachment conversion failures before continuing; a failed fetch/FileReader currently rejects `handleCreate` and bypasses the user-facing error flow.
(Based on your team's feedback about handling async calls with proper await and catch.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/agent-launch.ts (1)
51-63:⚠️ Potential issue | 🟠 MajorCarry
initialFilesthrough legacy launch normalization.
legacyAgentLaunchRequestSchemawill now acceptchatLaunchConfig.initialFiles, butnormalizeLegacyLaunchRequest()still rebuilds thechatpayload without copying that field. During rollout, legacy callers will validate successfully and silently lose attachments.Suggested change
chat: { paneId: chatConfig?.paneId ?? legacy.paneId, sessionId: chatConfig?.sessionId, initialPrompt: chatConfig?.initialPrompt, + initialFiles: chatConfig?.initialFiles, model: chatConfig?.model, retryCount: chatConfig?.retryCount, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/agent-launch.ts` around lines 51 - 63, The normalization function normalizeLegacyLaunchRequest() rebuilds the chat payload but omits chatLaunchConfig.initialFiles, causing attachments to be dropped; update normalizeLegacyLaunchRequest() so when it constructs the chat object (the payload validated by legacyAgentLaunchRequestSchema) it copies over chatLaunchConfig.initialFiles (if present) into the rebuilt chat payload (preserve optional typing and shape defined by chatLaunchConfigSchema), ensuring existing codepaths and validation continue to accept and forward initialFiles through the legacy normalization.
🤖 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/agent-session-orchestrator/adapters/chat-adapter.ts`:
- Around line 17-23: The current checks (using !initialFiles?.length) strip
attachments for draft launches; update the logic that builds the launch payload
so initialFiles is preserved when it contains items and only normalized to
undefined if it's an empty array. Concretely, replace truthy checks that use
!initialFiles?.length with an explicit check (initialFiles === undefined ||
initialFiles.length === 0) when deciding to omit attachments, and ensure the
branch that runs when autoExecute === false does not overwrite non-empty
initialFiles; apply this change to the same conditional logic occurrences
referencing prompt, model, retryCount, taskSlug and initialFiles (the two places
flagged in the comment).
In `@apps/desktop/src/renderer/react-query/workspaces/useCreateFromPr.ts`:
- Around line 20-26: pendingLaunchRequestRef is shared across hook calls and can
be overwritten by concurrent mutate calls; replace it with per-call state by
generating a unique requestId for each mutateAsyncWithSetup invocation, include
that requestId in the mutation variables (or in onMutate context), and store the
AgentLaunchRequest in a Map keyed by requestId; then in the mutation's onSuccess
(electronTrpc.workspaces.createFromPr.useMutation → onSuccess) use the
variables/requestId to look up and remove the correct AgentLaunchRequest from
the Map so concurrent calls don’t clobber each other — apply the same pattern to
the other shared ref usages mentioned (lines 38-40, 59-69).
---
Outside diff comments:
In `@packages/shared/src/agent-launch.ts`:
- Around line 51-63: The normalization function normalizeLegacyLaunchRequest()
rebuilds the chat payload but omits chatLaunchConfig.initialFiles, causing
attachments to be dropped; update normalizeLegacyLaunchRequest() so when it
constructs the chat object (the payload validated by
legacyAgentLaunchRequestSchema) it copies over chatLaunchConfig.initialFiles (if
present) into the rebuilt chat payload (preserve optional typing and shape
defined by chatLaunchConfigSchema), ensuring existing codepaths and validation
continue to accept and forward initialFiles through the legacy normalization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66b3f2c5-9bbd-44c0-8168-c4dda1aa762f
📒 Files selected for processing (28)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModalDraftContext.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.test.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/buildCreateWorkspaceFromBranchInput.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/resolveBranchAction.test.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/resolveBranchAction.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceModalContent/NewWorkspaceModalContent.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/ProjectSelector/ProjectSelector.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/ProjectSelector/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/LinkedPRPill/LinkedPRPill.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/LinkedPRPill/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PromptGroupAdvancedOptions/PromptGroupAdvancedOptions.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/index.tsapps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.tsapps/desktop/src/renderer/react-query/workspaces/useCreateFromPr.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/IssueLinkCommand/IssueLinkCommand.tsxapps/desktop/src/shared/tabs-types.tspackages/shared/src/agent-launch.ts
💤 Files with no reviewable changes (12)
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ProjectSelector/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.test.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ProjectSelector/ProjectSelector.tsx
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/buildCreateWorkspaceFromBranchInput.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/resolveBranchAction.test.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/resolveBranchAction.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx
Comment on lines
+20
to
+26
| const pendingLaunchRequestRef = useRef<AgentLaunchRequest | null>(null); | ||
|
|
||
| return electronTrpc.workspaces.createFromPr.useMutation({ | ||
| const mutation = electronTrpc.workspaces.createFromPr.useMutation({ | ||
| ...options, | ||
| onSuccess: async (data, ...rest) => { | ||
| if (!data.wasExisting && data.initialCommands) { | ||
| const agentLaunchRequest = pendingLaunchRequestRef.current; | ||
| pendingLaunchRequestRef.current = null; |
Contributor
There was a problem hiding this comment.
Shared ref can mix up launch requests across overlapping mutations.
pendingLaunchRequestRef is global to the hook, so a second mutateAsyncWithSetup() call can overwrite the first request before the first mutation's onSuccess reads it. If a user double-submits, the wrong prompt/files can be attached to the wrong workspace. Keep the launch request in per-call state instead of a single ref—for example by keying it off the variables object returned to onSuccess.
Also applies to: 38-40, 59-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/react-query/workspaces/useCreateFromPr.ts` around
lines 20 - 26, pendingLaunchRequestRef is shared across hook calls and can be
overwritten by concurrent mutate calls; replace it with per-call state by
generating a unique requestId for each mutateAsyncWithSetup invocation, include
that requestId in the mutation variables (or in onMutate context), and store the
AgentLaunchRequest in a Map keyed by requestId; then in the mutation's onSuccess
(electronTrpc.workspaces.createFromPr.useMutation → onSuccess) use the
variables/requestId to look up and remove the correct AgentLaunchRequest from
the Map so concurrent calls don’t clobber each other — apply the same pattern to
the other shared ref usages mentioned (lines 38-40, 59-69).
…u-can-attach-images-and-docum # Conflicts: # 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/NewWorkspaceModalContent/NewWorkspaceModalContent.tsx # apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx # apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PromptGroupAdvancedOptions/PromptGroupAdvancedOptions.tsx # apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx (1)
573-582: Consider handling attachment conversion failures gracefully.If
convertBlobUrlToDataUrlfails for any file, the entire workspace creation will fail. Consider filtering out failed conversions or showing a warning while continuing with successful ones.♻️ Suggested resilient conversion
if (attachments.files.length > 0) { - convertedFiles = await Promise.all( - attachments.files.map(async (file) => ({ - data: await convertBlobUrlToDataUrl(file.url), - mediaType: file.mediaType, - filename: file.filename, - })), - ); + const results = await Promise.allSettled( + attachments.files.map(async (file) => ({ + data: await convertBlobUrlToDataUrl(file.url), + mediaType: file.mediaType, + filename: file.filename, + })), + ); + convertedFiles = results + .filter((r): r is PromiseFulfilledResult<ConvertedFile> => r.status === "fulfilled") + .map((r) => r.value); + const failedCount = results.filter((r) => r.status === "rejected").length; + if (failedCount > 0) { + toast.warning(`${failedCount} attachment(s) could not be processed`); + } }🤖 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 573 - 582, When converting attachments in the PromptGroup component, convertBlobUrlToDataUrl can throw and currently one failure aborts the whole Promise.all; update the conversion logic around attachments.files.map so each file conversion is wrapped in a try/catch (or Promise.catch) and returns either a ConvertedFile or null/undefined, then filter out failed entries before assigning convertedFiles; also log or surface a non-blocking warning for failed files (e.g., reference convertBlobUrlToDataUrl, convertedFiles, and attachments.files) so workspace creation can continue with successful conversions.apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PromptGroupAdvancedOptions/PromptGroupAdvancedOptions.tsx (1)
72-72: Consider a more explicit prop for controlling branch visibility.Deriving
showBaseBranchfrom the presence ofonBaseBranchOpenChangeworks but relies on implicit behavior. A dedicatedshowBaseBranch?: booleanprop would be more self-documenting.♻️ Suggested improvement
interface PromptGroupAdvancedOptionsProps { showAdvanced: boolean; onShowAdvancedChange: (open: boolean) => void; + showBaseBranch?: boolean; branchInputValue: string; // ... } export function PromptGroupAdvancedOptions({ + showBaseBranch = false, // ... }: PromptGroupAdvancedOptionsProps) { - const showBaseBranch = onBaseBranchOpenChange != null;🤖 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/components/PromptGroupAdvancedOptions/PromptGroupAdvancedOptions.tsx` at line 72, The component PromptGroupAdvancedOptions currently infers showBaseBranch from the presence of onBaseBranchOpenChange which is implicit; add an explicit optional prop showBaseBranch?: boolean to the component's props (update the prop type/interface for PromptGroupAdvancedOptions) and use that value to control branch visibility, falling back to the existing behavior (i.e., if showBaseBranch is undefined, set showBaseBranch = onBaseBranchOpenChange != null) so existing callers keep working; update any usages of the showBaseBranch variable in the component to read from the new prop and ensure default behavior is preserved.
🤖 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 512-523: The convertBlobUrlToDataUrl useCallback lacks error
handling for fetch and FileReader; update the function so it catches fetch
errors (try/catch or .catch) and rejects/returns a clear failure when
response.ok is false, and attach onerror/onabort handlers to the FileReader to
reject the Promise with the error; ensure convertBlobUrlToDataUrl returns a
rejected Promise or throws on failure so callers can handle it.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PromptGroupAdvancedOptions/PromptGroupAdvancedOptions.tsx`:
- Line 72: The component PromptGroupAdvancedOptions currently infers
showBaseBranch from the presence of onBaseBranchOpenChange which is implicit;
add an explicit optional prop showBaseBranch?: boolean to the component's props
(update the prop type/interface for PromptGroupAdvancedOptions) and use that
value to control branch visibility, falling back to the existing behavior (i.e.,
if showBaseBranch is undefined, set showBaseBranch = onBaseBranchOpenChange !=
null) so existing callers keep working; update any usages of the showBaseBranch
variable in the component to read from the new prop and ensure default behavior
is preserved.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`:
- Around line 573-582: When converting attachments in the PromptGroup component,
convertBlobUrlToDataUrl can throw and currently one failure aborts the whole
Promise.all; update the conversion logic around attachments.files.map so each
file conversion is wrapped in a try/catch (or Promise.catch) and returns either
a ConvertedFile or null/undefined, then filter out failed entries before
assigning convertedFiles; also log or surface a non-blocking warning for failed
files (e.g., reference convertBlobUrlToDataUrl, convertedFiles, and
attachments.files) so workspace creation can continue with successful
conversions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52ffc433-2150-4ab0-a8ec-43e84e2b4411
📒 Files selected for processing (5)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModalDraftContext.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceModalContent/NewWorkspaceModalContent.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PromptGroupAdvancedOptions/PromptGroupAdvancedOptions.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/lib/trpc/routers/projects/projects.ts
…u-can-attach-images-and-docum # Conflicts: # apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx
- Sanitize branch name on blur so the input reflects the actual value that will be submitted (strips invalid git chars like ~, ^, :, ..) - Add error handling to convertBlobUrlToDataUrl (response.ok check, FileReader onerror/onabort) - Wrap attachment conversion in try/catch with toast on failure - Log warning in listPullRequests catch block instead of swallowing
Contributor
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/lib/trpc/routers/projects/projects.ts`:
- Around line 325-353: The filter/type-guard for parsed CLI output (variable
raw) currently only checks number/title/url but the map assumes pr.state and
pr.isDraft exist, causing a potential runtime error; update the guard used in
the Array.filter (or replace it with a zod schema) to also validate that "state"
is a string and "isDraft" is a boolean before mapping, or run a zod.parse on raw
to guarantee all fields, then map using pr.state and pr.isDraft safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f7cda51-d173-4cac-b373-9b2825cc43ce
📒 Files selected for processing (2)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx
Comment on lines
+325
to
+353
| const raw: unknown = JSON.parse(stdout.trim() || "[]"); | ||
| if (!Array.isArray(raw)) return []; | ||
| return raw | ||
| .filter( | ||
| ( | ||
| item: unknown, | ||
| ): item is { | ||
| number: number; | ||
| title: string; | ||
| url: string; | ||
| state: string; | ||
| isDraft: boolean; | ||
| } => | ||
| typeof item === "object" && | ||
| item !== null && | ||
| "number" in item && | ||
| "title" in item && | ||
| "url" in item, | ||
| ) | ||
| .map((pr) => ({ | ||
| prNumber: pr.number, | ||
| title: pr.title, | ||
| url: pr.url, | ||
| state: pr.isDraft | ||
| ? "draft" | ||
| : pr.state === "OPEN" | ||
| ? "open" | ||
| : pr.state.toLowerCase(), | ||
| })); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In GitHub CLI docs for gh pr list --json, are stateandisDraft guaranteed to be present and non-null for each returned PR item?
💡 Result:
GitHub CLI’s gh pr list manual lists state and isDraft as valid --json fields, but it does not explicitly state any guarantee about those fields being “always present and non-null” in the output. [1]
That said, those fields come from GitHub’s PullRequest GraphQL object, where both are defined as non-nullable (isDraft: Boolean! and state: PullRequestState!). So, when gh pr list --json state,isDraft succeeds, each returned PR item should include them with non-null values (otherwise the GraphQL response would error rather than yield null for those fields). [2]
Answer: Not guaranteed by the gh docs themselves, but effectively yes on successful output, because the underlying GraphQL fields are non-nullable. [1][2]
🏁 Script executed:
# Check if zod is a dependency
fd package.json --exec cat {} \; | grep -A 50 '"dependencies"' | grep -i zod || echo "Checking root package.json..."
cat package.json | grep -A 100 '"dependencies"' | grep -i zodRepository: superset-sh/superset
Length of output: 460
🏁 Script executed:
# Search for existing zod usage in the codebase
rg 'from.*zod|import.*z.*from' --type ts --type tsx | head -20Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check the actual file and context around the problematic code
sed -n '320,360p' apps/desktop/src/lib/trpc/routers/projects/projects.tsRepository: superset-sh/superset
Length of output: 1018
🏁 Script executed:
# Search for other gh command usages in projects.ts to see if there's a pattern
rg -B5 -A15 'gh.*pr\|gh.*issue' apps/desktop/src/lib/trpc/routers/projects/projects.ts | head -80Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check for zod schema usage patterns in the desktop app
rg 'z\.object|z\.string|z\.number|z\.boolean' apps/desktop/src --max-count=10 -A2Repository: superset-sh/superset
Length of output: 50376
Harden PR payload validation to match type guard declarations.
The type predicate declares state and isDraft as required fields (lines 331–337), but the filter only validates the presence of number, title, and url. This creates a type safety gap where pr.state could be undefined when .toLowerCase() is called on line 352, despite the type guard suggesting otherwise.
While GitHub's CLI returns non-nullable fields in practice (GraphQL guarantees), the code should validate all declared fields for consistency. Consider using a zod schema to ensure complete validation:
Proposed fix
+const PullRequestItemSchema = z.object({
+ number: z.number().int(),
+ title: z.string(),
+ url: z.string(),
+ state: z.string(),
+ isDraft: z.boolean(),
+});
+
const raw: unknown = JSON.parse(stdout.trim() || "[]");
-if (!Array.isArray(raw)) return [];
-return raw
- .filter(
- (
- item: unknown,
- ): item is {
- number: number;
- title: string;
- url: string;
- state: string;
- isDraft: boolean;
- } =>
- typeof item === "object" &&
- item !== null &&
- "number" in item &&
- "title" in item &&
- "url" in item,
- )
- .map((pr) => ({
+const parsed = z.array(PullRequestItemSchema).safeParse(raw);
+if (!parsed.success) return [];
+return parsed.data.map((pr) => ({
prNumber: pr.number,
title: pr.title,
url: pr.url,
state: pr.isDraft
? "draft"
: pr.state === "OPEN"
? "open"
: pr.state.toLowerCase(),
}));📝 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.
Suggested change
| const raw: unknown = JSON.parse(stdout.trim() || "[]"); | |
| if (!Array.isArray(raw)) return []; | |
| return raw | |
| .filter( | |
| ( | |
| item: unknown, | |
| ): item is { | |
| number: number; | |
| title: string; | |
| url: string; | |
| state: string; | |
| isDraft: boolean; | |
| } => | |
| typeof item === "object" && | |
| item !== null && | |
| "number" in item && | |
| "title" in item && | |
| "url" in item, | |
| ) | |
| .map((pr) => ({ | |
| prNumber: pr.number, | |
| title: pr.title, | |
| url: pr.url, | |
| state: pr.isDraft | |
| ? "draft" | |
| : pr.state === "OPEN" | |
| ? "open" | |
| : pr.state.toLowerCase(), | |
| })); | |
| const PullRequestItemSchema = z.object({ | |
| number: z.number().int(), | |
| title: z.string(), | |
| url: z.string(), | |
| state: z.string(), | |
| isDraft: z.boolean(), | |
| }); | |
| const raw: unknown = JSON.parse(stdout.trim() || "[]"); | |
| const parsed = z.array(PullRequestItemSchema).safeParse(raw); | |
| if (!parsed.success) return []; | |
| return parsed.data.map((pr) => ({ | |
| prNumber: pr.number, | |
| title: pr.title, | |
| url: pr.url, | |
| state: pr.isDraft | |
| ? "draft" | |
| : pr.state === "OPEN" | |
| ? "open" | |
| : pr.state.toLowerCase(), | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts` around lines 325 -
353, The filter/type-guard for parsed CLI output (variable raw) currently only
checks number/title/url but the map assumes pr.state and pr.isDraft exist,
causing a potential runtime error; update the guard used in the Array.filter (or
replace it with a zod schema) to also validate that "state" is a string and
"isDraft" is a boolean before mapping, or run a zod.parse on raw to guarantee
all fields, then map using pr.state and pr.isDraft safely.
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
1 participant
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.
Summary
Test plan
+> "Add attachment" — file chip appears+> "Link issue" — search dialog opens, select issue, chip appears with X to remove⌘+↵— workspace created with all contextbun run typecheck)Summary by cubic
Replaces the tabbed New Workspace modal with a unified, prompt‑first flow. Inlines project selection, base and editable branch, PR/issue linking, and file attachments, and preserves agent chat auto‑launch (including from attached files and when creating from a linked PR).
New Features
projects.listPullRequestsusinggh); shows a status pill, creates from that PR, and passes the agent launch request through.Bug Fixes
convertBlobUrlToDataUrlerror handling; wrap conversions in try/catch and toast on failure.listPullRequestserrors instead of swallowing them.Written for commit 8513536. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor
Tests
Style