[POC] document v1-on-v2 workspace creation port#3204
Draft
Kitenite wants to merge 14 commits into
Draft
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConsolidates the dashboard “new workspace” modal into a single prompt-driven form with expanded draft state and attachments in the renderer, and adds a new host-service TRPC router Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer
participant HostService as HostService
participant GitFS as Git/Filesystem
participant CloudAPI as CloudAPI
participant DB as Database
Renderer->>HostService: workspaceCreation.create(payload)
HostService->>DB: ensure project row, check existing workspaces
HostService->>GitFS: inspect branches & worktree path
alt existing DB workspace
HostService->>DB: return opened_existing_workspace
HostService-->>Renderer: outcome: opened_existing_workspace + workspace info
else worktree exists on disk
HostService->>DB: is worktree tracked?
alt tracked
HostService->>CloudAPI: create workspace (with device metadata)
HostService->>DB: insert local workspace row
HostService-->>Renderer: outcome: opened_worktree
else not tracked
HostService->>CloudAPI: create workspace (adopt)
HostService->>DB: insert local workspace row
HostService-->>Renderer: outcome: adopted_external_worktree
end
else no worktree
HostService->>GitFS: git worktree add (maybe -b with compareBaseBranch)
HostService->>CloudAPI: create workspace
alt cloud create success
HostService->>DB: insert local workspace row
HostService-->>Renderer: outcome: created_workspace
else cloud create failure
HostService->>GitFS: remove worktree (rollback)
HostService-->>Renderer: error + rollback status
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
…event wiring - Forward full semantic create payload from renderer (linkedContext internalIssueIds/githubIssueUrls, launch.agentId/autoRun, behavior flags) - Implement opened_worktree outcome by distinguishing tracked vs external worktrees in workspaceCreation.create - Add initPhase/initProgress columns to host-service workspaces table so workspace.getInitState returns real hydrated state instead of hardcoded "ready" - Extend useWorkspaceEvent with workspace:init:changed overload so renderer can subscribe to init/setup progress
![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
♻️ Duplicate comments (1)
packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts (1)
448-459:⚠️ Potential issue | 🟡 MinorOnly fall back to
-bwhen the branch is actually missing.The bare
catchstill treats everygit worktree addfailure as “branch doesn't exist”. Path, permission, or repo-health errors get masked, and the retry can create a new branch fromcompareBaseBranchafter an unrelated failure. Check branch existence explicitly and rethrow unexpected errors.Safer fallback
- } catch { + } catch (err) { + const branchExists = ( + await git.raw(["show-ref", "--verify", `refs/heads/${branchName}`]).catch( + () => "", + ) + ).trim().length > 0; + if (branchExists) { + throw err; + } const baseBranch = input.composer.compareBaseBranch || "HEAD"; await git.raw([ "worktree",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts` around lines 448 - 459, The current try/catch around git.raw(["worktree","add", worktreePath, branchName]) treats any error as “branch missing”; instead, after a failed worktree add, explicitly check whether the branch exists (e.g., use git.raw(["rev-parse","--verify", branchName]) or git.raw(["show-ref","--verify", `refs/heads/${branchName}`])) and only run the fallback git.raw(["worktree","add","-b", branchName, worktreePath, baseBranch]) when that check indicates the branch is absent; for any other error from the initial worktree add or the branch-existence check, rethrow/propagate the original error so path/permission/repo-health failures are not masked (update the block around git.raw, worktreePath, branchName, and input.composer.compareBaseBranch accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`:
- Around line 298-300: The initial clone uses simpleGit().clone which bypasses
the authenticated git factory; add an authenticated clone method to the git
factory (e.g., createGitFactory returns an object with clone: (url, path) =>
Promise<void>) that requests initial credentials from the GitCredentialProvider
and sets them (env or auth) on simpleGit before cloning, then replace direct
simpleGit().clone(...) calls in workspace-creation.ts (the clone call near
repoPath) and workspace/workspace.ts (the call around line 65) with
ctx.git.clone(cloudProject.repoCloneUrl, repoPath) to ensure the initial clone
is performed with the provider-supplied credentials.
- Around line 273-276: The branchName/workspaceName assignment allows
unvalidated names (branchName, workspaceName) to flow into worktreePath and
git.raw calls; reject any names that contain path traversal (".."), absolute
paths, path separators, or other unsafe characters and return an error; after
constructing worktreePath use path.resolve and verify it is inside the expected
.worktrees root (ensure resolved.startsWith(rootResolved)) before storing the
path or calling git.raw(["worktree", "add", worktreePath, branchName]) in
workspace-creation.ts, and likewise validate/resolve any path passed to
git.raw(["worktree", "remove", ...]) in workspace.ts; additionally, verify the
resolved path is an actual git worktree for this repository (e.g., run a git
check or inspect .git/worktrees) before persisting or deleting.
---
Duplicate comments:
In
`@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`:
- Around line 448-459: The current try/catch around git.raw(["worktree","add",
worktreePath, branchName]) treats any error as “branch missing”; instead, after
a failed worktree add, explicitly check whether the branch exists (e.g., use
git.raw(["rev-parse","--verify", branchName]) or git.raw(["show-ref","--verify",
`refs/heads/${branchName}`])) and only run the fallback
git.raw(["worktree","add","-b", branchName, worktreePath, baseBranch]) when that
check indicates the branch is absent; for any other error from the initial
worktree add or the branch-existence check, rethrow/propagate the original error
so path/permission/repo-health failures are not masked (update the block around
git.raw, worktreePath, branchName, and input.composer.compareBaseBranch
accordingly).
🪄 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: 6756f70e-7e1b-40c4-b782-c43dcac6afd6
📒 Files selected for processing (2)
apps/desktop/plans/20260405-1945-v1-workspace-ux-into-v2-modal.mdpackages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/plans/20260405-1945-v1-workspace-ux-into-v2-modal.md
Comment on lines
+298
to
+300
| if (!existsSync(repoPath)) { | ||
| mkdirSync(dirname(repoPath), { recursive: true }); | ||
| await simpleGit().clone(cloudProject.repoCloneUrl, repoPath); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
rg -n -C3 'createGitFactory|ctx\.git\(|simpleGit\(\)\.clone|\.clone\(' packages -g '*.ts'Repository: superset-sh/superset
Length of output: 12866
🏁 Script executed:
#!/bin/bash
set -e
# Check GitFactory type definition
echo "=== GitFactory type definition ==="
find packages/host-service/src/runtime/git -name '*.ts' -exec cat -n {} +
echo -e "\n=== Check how ctx is structured ==="
rg -n 'type.*Context|interface.*Context' packages/host-service/src/trpc -A 10
echo -e "\n=== Check app.ts ctx creation ==="
head -100 packages/host-service/src/app.ts | tail -50Repository: superset-sh/superset
Length of output: 5053
🏁 Script executed:
#!/bin/bash
set -e
# Find HostServiceContext definition
echo "=== HostServiceContext type definition ==="
rg -n 'type HostServiceContext|interface HostServiceContext' packages/host-service/src -A 15
echo -e "\n=== Check if git is directly available in ctx ==="
rg -n 'git\s*=' packages/host-service/src/types -A 3 -B 3
</parameter>Repository: superset-sh/superset
Length of output: 1012
🏁 Script executed:
#!/bin/bash
set -e
# Check the credentials provider and how it's used
echo "=== Credentials provider usage in clone context ==="
rg -n 'credentials\.' packages/host-service/src -B 2 -A 2 | grep -A 5 -B 5 'clone\|getCredentials'
# Check PullRequestRuntimeManager which gets git passed in
echo -e "\n=== PullRequestRuntimeManager to see how git is used ==="
rg -n 'class PullRequestRuntimeManager|git\(' packages/host-service/src/runtime/pull-requests -A 5 | head -80
# Check if there are any authenticated clone methods elsewhere
echo -e "\n=== Search for alternative clone patterns ==="
rg -n 'clone' packages/host-service/src --type ts -B 3 -A 3Repository: superset-sh/superset
Length of output: 115
🏁 Script executed:
#!/bin/bash
set -e
# Look at app.ts more completely to see how context is built
echo "=== Full context creation in app.ts ==="
rg -n 'createContext|context:' packages/host-service/src/app.ts -A 20 -B 3
# Check if credentials are stored anywhere accessible
echo -e "\n=== Search for credentials in context setup ==="
rg -n 'credentials|provider' packages/host-service/src/app.ts -B 2 -A 2 | head -80
# Check what env variables are being set
echo -e "\n=== Check credential env variables ==="
find packages/host-service/src/runtime -name '*.ts' -exec grep -n 'env\|GIT_' {} +Repository: superset-sh/superset
Length of output: 2662
🏁 Script executed:
#!/bin/bash
set -e
# Find LocalGitCredentialProvider
echo "=== LocalGitCredentialProvider implementation ==="
rg -n 'class LocalGitCredentialProvider|export.*LocalGitCredentialProvider' packages/host-service/src -A 30
# Check if there's any authenticated clone method anywhere
echo -e "\n=== Search for authenticated clone or init patterns ==="
rg -n 'clone|init' packages/host-service/src/runtime -B 2 -A 3 --type ts
# Check what getCredentials returns for null remoteUrl (initial clone case)
echo -e "\n=== Full credentials provider interface ==="
rg -n 'GitCredentialProvider' packages/host-service/src -B 5 -A 15Repository: superset-sh/superset
Length of output: 22221
🏁 Script executed:
#!/bin/bash
set -e
# Check if there are other clone patterns or if CloudGitCredentialProvider has special handling
echo "=== CloudGitCredentialProvider getCredentials ==="
rg -n 'getCredentials' packages/host-service/src/providers/git/CloudGitCredentialProvider -A 15
# Check if credentials are ever passed through context or used for clone elsewhere
echo -e "\n=== Check all clone usages context ==="
rg -n 'simpleGit.*clone|clone.*repoCloneUrl' packages/host-service/src -B 5 -A 5Repository: superset-sh/superset
Length of output: 4575
Use authenticated git factory for initial clone.
simpleGit().clone() bypasses the credentialed git factory created in packages/host-service/src/app.ts. The git factory requires an existing repository to determine credentials, so initial clones of private repositories cannot be authenticated with the configured provider. This affects both workspace-creation.ts (line 300) and workspace/workspace.ts (line 65).
The fix requires exposing authenticated clone capability. Consider adding a dedicated clone method to the git factory that applies initial credentials before the repository exists:
// In packages/host-service/src/runtime/git/git.ts
export function createGitFactory(provider: GitCredentialProvider): GitFactory & { clone: (url: string, path: string) => Promise<void> } {
const clone = async (url: string, path: string) => {
const initialCredentials = await provider.getCredentials(null);
await simpleGit().env(initialCredentials.env).clone(url, path);
};
return Object.assign(
async (repoPath: string) => { /* existing factory code */ },
{ clone }
);
}Then use ctx.git.clone(cloudProject.repoCloneUrl, repoPath) instead of simpleGit().clone(...).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`
around lines 298 - 300, The initial clone uses simpleGit().clone which bypasses
the authenticated git factory; add an authenticated clone method to the git
factory (e.g., createGitFactory returns an object with clone: (url, path) =>
Promise<void>) that requests initial credentials from the GitCredentialProvider
and sets them (env or auth) on simpleGit before cloning, then replace direct
simpleGit().clone(...) calls in workspace-creation.ts (the clone call near
repoPath) and workspace/workspace.ts (the call around line 65) with
ctx.git.clone(cloudProject.repoCloneUrl, repoPath) to ensure the initial clone
is performed with the provider-supplied credentials.
- Validate worktreePath stays inside repoPath/.worktrees (prevents path traversal via ../.. in branch names) - opened_worktree path no longer calls v2Workspace.create — the local row's id *is* the cloud workspace id, so the cloud row already exists (removes duplicate cloud create) - Remove unused launch.agentId/autoRun from schema and client; agent handoff will be wired in a later phase - Throw INTERNAL_SERVER_ERROR if v2Workspace.create returns no row (prevents silent partial failure with null workspace) - Rollback worktree if cloud create returns null - Bottom-bar ProjectSelector now uses handleSelectProject so switching projects from either picker consistently resets compareBaseBranch - Log original error in worktree-add fallback catch block and AI branch-name generation catch (previously silently swallowed)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/DashboardNewWorkspaceForm.tsx (1)
462-465:⚠️ Potential issue | 🟡 MinorAvoid manufacturing a fake GitHub issue number.
If a GitHub-linked issue ever arrives without
number,issue.number ?? 0renders a misleading#0pill. Guard the GitHub-pill branch withissue.number != nullor make the prop optional instead of synthesizing0.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/DashboardNewWorkspaceForm.tsx` around lines 462 - 465, The GitHub-pill branch in DashboardNewWorkspaceForm renders a misleading "#0" by passing a synthesized 0 via issue.number ?? 0 to LinkedGitHubIssuePill; instead, gate the branch on a real number (e.g., issue.number != null) or change LinkedGitHubIssuePill's prop to optional and only render it when issue.number is present, ensuring you reference the LinkedGitHubIssuePill usage and the issue.number field when making the change.
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts (1)
507-515: **Consider: cloud create succeeds but local insert fails leaves orphan.**The cloud workspace creation (line 487) succeeds before the local DB insert (lines 507-515). "A transaction in its entirety can commit to a database as a single logical unit or rollback (become undone) as a single logical unit." However, since the cloud API call is external, the local insert and cloud create cannot be in the same DB transaction. If the local insert fails after cloud create succeeds, there's an orphaned cloud workspace with no local tracking.This is an inherent challenge with distributed operations. Consider wrapping the local insert in a try/catch that attempts to delete the cloud workspace on failure, or accepting eventual consistency with a cleanup job.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts` around lines 507 - 515, The cloud workspace is created before the local DB insert (see the cloud creation call that returns cloudRow and the subsequent ctx.db.insert(workspaces).values(...).run()), so if the local insert fails we get an orphaned cloud workspace; wrap the local insert in a try/catch around the ctx.db.insert(...) call and on error call the cloud deletion API using the created cloudRow.id (or appropriate delete function used elsewhere) to rollback the external resource, logging both the DB error and the cloud delete result; if delete can fail, ensure the error is surfaced or queued for retry (or document switching to eventual consistency/cleanup job).
🤖 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/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/DashboardNewWorkspaceForm.tsx`:
- Around line 294-309: The modal currently calls closeAndResetDraft() before the
async createWorkspace(...) completes, which discards draft, prompt, linked
context and detachedFiles on failure; change the flow so detachedFiles =
attachments.takeFiles() is still captured, then call createWorkspace(...) and
only invoke closeAndResetDraft() after createWorkspace resolves successfully
(i.e., in the success path of the returned Promise from createWorkspace), and in
the error path restore the draft and reattach detachedFiles (or requeue them) so
the user can retry; update code referencing draft.selectedProjectId,
workspaceNameEdited/branchNameEdited, compareBaseBranch, runSetupScript,
linkedPR, linkedIssues, and attachmentFiles to follow this new flow.
- Around line 344-368: The current addLinkedGitHubIssue builds a non-unique slug
(`slug: \`#${issueNumber}\``) causing duplicate keys and accidental removals in
addLinkedGitHubIssue/removeLinkedIssue; change addLinkedGitHubIssue to set slug
to a repo-qualified identifier (for example the issue URL or a canonical form
like `owner/repo#${issueNumber}`) when calling updateDraft, and update
removeLinkedIssue to match that same repo-qualified slug logic (or use the url
field directly) so filtering by i.slug correctly removes only the intended
issue; ensure any React keys rendered from linkedIssues also use this new unique
slug.
- Around line 601-603: When the DevicePicker's hostTarget changes, the draft
retains the old compareBaseBranch causing invalid branch submission; update the
onSelectHostTarget handler (the one that calls updateDraft when DevicePicker
emits hostTarget) to also clear/reset compareBaseBranch (e.g., set it to
undefined/null) so the branch is refetched via useBranchContext for the new host
and cannot be submitted for the wrong target.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/DashboardNewWorkspaceForm.tsx`:
- Around line 462-465: The GitHub-pill branch in DashboardNewWorkspaceForm
renders a misleading "#0" by passing a synthesized 0 via issue.number ?? 0 to
LinkedGitHubIssuePill; instead, gate the branch on a real number (e.g.,
issue.number != null) or change LinkedGitHubIssuePill's prop to optional and
only render it when issue.number is present, ensuring you reference the
LinkedGitHubIssuePill usage and the issue.number field when making the change.
---
Nitpick comments:
In
`@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`:
- Around line 507-515: The cloud workspace is created before the local DB insert
(see the cloud creation call that returns cloudRow and the subsequent
ctx.db.insert(workspaces).values(...).run()), so if the local insert fails we
get an orphaned cloud workspace; wrap the local insert in a try/catch around the
ctx.db.insert(...) call and on error call the cloud deletion API using the
created cloudRow.id (or appropriate delete function used elsewhere) to rollback
the external resource, logging both the DB error and the cloud delete result; if
delete can fail, ensure the error is surfaced or queued for retry (or document
switching to eventual consistency/cleanup job).
🪄 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: 306c1fbf-3095-4299-a28c-d29a42d549db
📒 Files selected for processing (3)
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/DashboardNewWorkspaceForm.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useCreateDashboardWorkspace/useCreateDashboardWorkspace.tspackages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useCreateDashboardWorkspace/useCreateDashboardWorkspace.ts
Comment on lines
+294
to
+309
| const detachedFiles = attachments.takeFiles(); | ||
| closeAndResetDraft(); | ||
|
|
||
| void createWorkspace({ | ||
| projectId: draft.selectedProjectId, | ||
| hostTarget, | ||
| prompt, | ||
| workspaceName: workspaceNameEdited ? workspaceName : undefined, | ||
| branchName, | ||
| branchNameEdited, | ||
| compareBaseBranch: compareBaseBranch || undefined, | ||
| runSetupScript, | ||
| linkedPR, | ||
| linkedIssues, | ||
| attachmentFiles: detachedFiles, | ||
| }); |
Contributor
There was a problem hiding this comment.
Keep the draft alive until creation actually succeeds.
Line 295 clears the modal before the async create finishes. If createWorkspace(...) fails, the user loses the prompt, linked context, and detached attachments with no retry path. Move the reset/close into the success path, or restore the draft on error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/DashboardNewWorkspaceForm.tsx`
around lines 294 - 309, The modal currently calls closeAndResetDraft() before
the async createWorkspace(...) completes, which discards draft, prompt, linked
context and detachedFiles on failure; change the flow so detachedFiles =
attachments.takeFiles() is still captured, then call createWorkspace(...) and
only invoke closeAndResetDraft() after createWorkspace resolves successfully
(i.e., in the success path of the returned Promise from createWorkspace), and in
the error path restore the draft and reattach detachedFiles (or requeue them) so
the user can retry; update code referencing draft.selectedProjectId,
workspaceNameEdited/branchNameEdited, compareBaseBranch, runSetupScript,
linkedPR, linkedIssues, and attachmentFiles to follow this new flow.
Comment on lines
+344
to
+368
| const addLinkedGitHubIssue = ( | ||
| issueNumber: number, | ||
| title: string, | ||
| url: string, | ||
| state: string, | ||
| ) => { | ||
| if (linkedIssues.some((i) => i.url === url)) return; | ||
| updateDraft({ | ||
| linkedIssues: [ | ||
| ...linkedIssues, | ||
| { | ||
| slug: `#${issueNumber}`, | ||
| title, | ||
| source: "github" as const, | ||
| url, | ||
| number: issueNumber, | ||
| state: state.toLowerCase() === "closed" ? "closed" : "open", | ||
| }, | ||
| ], | ||
| }); | ||
| }; | ||
|
|
||
| const removeLinkedIssue = (slug: string) => | ||
| updateDraft({ | ||
| linkedIssues: linkedIssues.filter((i) => i.slug !== slug), |
Contributor
There was a problem hiding this comment.
Use a repo-qualified identifier for GitHub issues.
slug: \#${issueNumber}`is not unique across repositories. If a user linksrepo-a#123andrepo-b#123`, Line 456 gets duplicate React keys and Line 368 removes both entries at once. Use the canonical URL or another repo-qualified id for the stored key/removal token.
💡 Minimal fix
const addLinkedGitHubIssue = (
issueNumber: number,
title: string,
url: string,
state: string,
) => {
if (linkedIssues.some((i) => i.url === url)) return;
updateDraft({
linkedIssues: [
...linkedIssues,
{
- slug: `#${issueNumber}`,
+ slug: url,
title,
source: "github" as const,
url,
number: issueNumber,
state: state.toLowerCase() === "closed" ? "closed" : "open",
},
],
});
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/DashboardNewWorkspaceForm.tsx`
around lines 344 - 368, The current addLinkedGitHubIssue builds a non-unique
slug (`slug: \`#${issueNumber}\``) causing duplicate keys and accidental
removals in addLinkedGitHubIssue/removeLinkedIssue; change addLinkedGitHubIssue
to set slug to a repo-qualified identifier (for example the issue URL or a
canonical form like `owner/repo#${issueNumber}`) when calling updateDraft, and
update removeLinkedIssue to match that same repo-qualified slug logic (or use
the url field directly) so filtering by i.slug correctly removes only the
intended issue; ensure any React keys rendered from linkedIssues also use this
new unique slug.
Comment on lines
+601
to
+603
| <DevicePicker | ||
| hostTarget={hostTarget} | ||
| onSelectHostTarget={(t) => updateDraft({ hostTarget: t })} |
Contributor
There was a problem hiding this comment.
Clear the selected compare-base branch when the host changes.
useBranchContext(...) refetches branches for the new host, but the previously chosen compareBaseBranch survives in draft state and is still submitted at Line 304. Switching devices can therefore send a branch name that does not exist on the new target. Mirror the project-change path and reset it here too.
💡 Minimal fix
<DevicePicker
hostTarget={hostTarget}
- onSelectHostTarget={(t) => updateDraft({ hostTarget: t })}
+ onSelectHostTarget={(t) =>
+ updateDraft({ hostTarget: t, compareBaseBranch: null })
+ }
/>📝 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
| <DevicePicker | |
| hostTarget={hostTarget} | |
| onSelectHostTarget={(t) => updateDraft({ hostTarget: t })} | |
| <DevicePicker | |
| hostTarget={hostTarget} | |
| onSelectHostTarget={(t) => | |
| updateDraft({ hostTarget: t, compareBaseBranch: null }) | |
| } | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/DashboardNewWorkspaceForm.tsx`
around lines 601 - 603, When the DevicePicker's hostTarget changes, the draft
retains the old compareBaseBranch causing invalid branch submission; update the
onSelectHostTarget handler (the one that calls updateDraft when DevicePicker
emits hostTarget) to also clear/reset compareBaseBranch (e.g., set it to
undefined/null) so the branch is refetched via useBranchContext for the new host
and cannot be submitted for the wrong target.
# Conflicts: # apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/BranchesGroup/BranchesGroup.tsx # apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/DashboardNewWorkspaceFormHeader/DashboardNewWorkspaceFormHeader.tsx # apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/DashboardNewWorkspaceListTabContent/DashboardNewWorkspaceListTabContent.tsx # apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/DashboardNewWorkspacePromptTabContent/DashboardNewWorkspacePromptTabContent.tsx # apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/IssuesGroup/IssuesGroup.tsx # apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/PromptGroup/PromptGroup.tsx # apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/PullRequestsGroup/PullRequestsGroup.tsx # apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useCreateDashboardWorkspace/useCreateDashboardWorkspace.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts (1)
311-313:⚠️ Potential issue | 🟠 MajorUse authenticated git factory for initial clone.
The
simpleGit().clone()call bypasses the credentialed git factory (ctx.git). For private repositories, this clone will fail because no authentication is configured. This was flagged in a previous review and remains unaddressed.Consider exposing an authenticated clone method from the git factory, or using the credentials provider to configure git before cloning:
// Option: Use credentials provider directly const credentials = await ctx.credentialProvider.getCredentials(null); await simpleGit().env(credentials.env).clone(cloudProject.repoCloneUrl, repoPath);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts` around lines 311 - 313, The clone uses unauthenticated simpleGit() which will fail for private repos; replace it to use the authenticated git factory or credentials provider: obtain credentials via ctx.credentialProvider.getCredentials(...) or call an exposed authenticated clone on ctx.git, then perform the clone of cloudProject.repoCloneUrl into repoPath using the credentialed git client (referencing simpleGit, ctx.git, ctx.credentialProvider, cloudProject.repoCloneUrl, and repoPath) so the initial clone uses the repository credentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`:
- Around line 311-313: The clone uses unauthenticated simpleGit() which will
fail for private repos; replace it to use the authenticated git factory or
credentials provider: obtain credentials via
ctx.credentialProvider.getCredentials(...) or call an exposed authenticated
clone on ctx.git, then perform the clone of cloudProject.repoCloneUrl into
repoPath using the credentialed git client (referencing simpleGit, ctx.git,
ctx.credentialProvider, cloudProject.repoCloneUrl, and repoPath) so the initial
clone uses the repository credentials.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 054966a9-c454-41a9-b1ec-0b11454cbc2d
📒 Files selected for processing (5)
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceDraftContext.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/hooks/useBranchContext/useBranchContext.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useCreateDashboardWorkspace/useCreateDashboardWorkspace.tspackages/host-service/src/trpc/router/router.tspackages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/hooks/useBranchContext/useBranchContext.ts
- apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceDraftContext.tsx
9 tasks
Kitenite
changed the title
Collaborator
Author
|
Superseded by #3302 which takes the clone V1, rewire boundaries approach instead of the hand-rewrite in this branch. Keeping this open as POC / reference for the design iteration that got us there. |
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.
What changed
This PR adds two planning docs for the workspace-modal migration work:
Why
The current V2 modal UX diverges materially from V1, while the implementation goal is now a full port of the V1 experience rather than a hybrid. These docs lock that direction and make the API discussion concrete enough to review.
Impact
Validation
Summary by cubic
Ports the V1 create‑workspace flow to V2 with a single composer and a new host‑service
workspaceCreationAPI. Creation now blocks until the worktree and workspace are ready, so no init events or polling are needed.New Features
workspaceCreation.getContext|searchBranches|createwith V1 semantics; branch data is fetched from host viauseBranchContext;createreturnscreated_workspace|opened_existing_workspace|opened_worktree|adopted_external_worktreeand blocks until done.Bug Fixes
repo/.worktreesto prevent path traversal.opened_worktree(distinguish tracked vs external worktrees), rollback worktree if cloud create fails, throw on null cloud create, and log fallback/AI branch‑name errors.Written for commit 672c5ac. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Refactor