fix(desktop): auto-name workspaces after setup completes#2107
Merged
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review 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:
📝 WalkthroughWalkthroughAdds an optional AI naming prompt to workspace creation, propagates it through initialization, tries an asynchronous provider-driven auto-rename (with explicit skip reasons and warnings), surfaces warnings in init progress, and shows a one-time UI toast when auto-rename is skipped. Changes
Sequence DiagramsequenceDiagram
participant User as NewWorkspaceModal (User)
participant Server as TRPC Server (create.ts)
participant WInit as Workspace Init (workspace-init.ts)
participant AI as AI Naming (ai-name.ts)
participant DB as Workspace DB
participant UI as Layout/Toast (layout.tsx)
User->>Server: createWorkspace(projectId, name, prompt?)
Server->>WInit: initializeWorkspaceWorktree(params + namingPrompt)
WInit->>WInit: create or reuse worktree/workspace
WInit->>AI: attemptWorkspaceAutoRenameFromPrompt(workspaceId, prompt)
AI->>AI: iterate TITLE_PROVIDERS -> generateWorkspaceNameFromPrompt(prompt)
AI->>DB: read workspace state (getWorkspaceAutoRenameDecision)
AI->>AI: decide rename or skip (with reason/warning)
alt renamed
AI->>DB: update workspace name
AI-->>WInit: { kind: "renamed", name }
else skipped
AI-->>WInit: { kind: "skip", reason, warning? }
end
WInit->>Server: updateProgress(step, msg, warning?)
Server->>UI: emit init progress event
UI->>UI: if progress.warning and not shown -> toast.warning(warning)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 11 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/workspaces/procedures/create.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts:416">
P2: Unhandled promise rejection risk: `void attemptWorkspaceAutoRenameFromPrompt(...)` discards the promise without catching errors. If the DB query or rename logic throws, this becomes an unhandled rejection that can crash the Electron main process. Wrap the call with `.catch()` to match the error-safe pattern used in the `initializeWorkspaceWorktree` path.
(Based on your team's feedback about handling async calls with proper await and catch.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts:147">
P1: Auto-rename is not wrapped in its own try/catch, so an unexpected error (e.g. DB failure in `ai-name.ts`) will fail the entire workspace init and **delete the already-created worktree**. Since the PR intends naming to be non-fatal, wrap this in a try/catch that falls back to reporting "ready" without a rename.</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: 3
🧹 Nitpick comments (3)
apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-auto-rename.ts (1)
1-6: Consider exporting the interfaces for reuse.The
WorkspaceAutoRenameStateandResolveWorkspaceAutoRenameParamsinterfaces are not exported, which may limit reusability if other modules need to construct these types. If these are intentionally internal, this is fine.♻️ Optional: Export interfaces for external use
-interface WorkspaceAutoRenameState { +export interface WorkspaceAutoRenameState { branch: string; name: string; isUnnamed: boolean | null; deletingAt?: number | null; } -interface ResolveWorkspaceAutoRenameParams { +export interface ResolveWorkspaceAutoRenameParams { workspace: WorkspaceAutoRenameState | null; generatedName: string | null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-auto-rename.ts` around lines 1 - 6, The interfaces WorkspaceAutoRenameState and ResolveWorkspaceAutoRenameParams are declared but not exported, limiting reuse; export them by adding the export keyword to their declarations (e.g., export interface WorkspaceAutoRenameState and export interface ResolveWorkspaceAutoRenameParams) so other modules can import these types; ensure any local usages/imports still compile and update any files that should import these interfaces accordingly.apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts (1)
175-184: DB query correctly filters out deleting workspaces.The query includes
isNull(workspaces.deletingAt)which ensures workspaces marked for deletion are not renamed. However, this creates a potential inconsistency: thegetWorkspaceAutoRenameDecisionfunction also checksdeletingAt, but if the workspace is already filtered out by the query, it will returnnulland trigger "missing-workspace" instead of "workspace-deleting".This is functionally correct (rename is skipped either way) but the logged skip reason may be misleading.
♻️ Optional: Remove deletingAt filter from query for accurate skip reason
const workspace = localDb .select({ branch: workspaces.branch, name: workspaces.name, isUnnamed: workspaces.isUnnamed, deletingAt: workspaces.deletingAt, }) .from(workspaces) - .where(and(eq(workspaces.id, workspaceId), isNull(workspaces.deletingAt))) + .where(eq(workspaces.id, workspaceId)) .get();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts` around lines 175 - 184, The select query that fetches the workspace currently includes isNull(workspaces.deletingAt) which causes a null result and hides the fact the workspace is deleting, leading getWorkspaceAutoRenameDecision to report "missing-workspace" instead of "workspace-deleting"; remove the isNull(workspaces.deletingAt) predicate from the localDb.select/.where call (or alternatively keep the predicate but after fetching check the workspace.deletingAt field) so that getWorkspaceAutoRenameDecision can read deletingAt and return the accurate "workspace-deleting" skip reason (refer to the workspace query using localDb.select(...) .from(workspaces) .where(eq(workspaces.id, workspaceId)) and the getWorkspaceAutoRenameDecision function).apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-auto-rename.test.ts (1)
7-79: Good test coverage for core auto-rename scenarios.The tests cover the essential paths: unnamed workspace with valid name, already-named workspace, placeholder-changed scenario, and whitespace handling. The test for
getWorkspaceAutoRenameDecisionverifies the skip reason structure.Consider adding tests for the remaining skip reasons (
"missing-workspace","workspace-deleting","workspace-name-changed") to ensure complete coverage of the decision logic.🧪 Optional: Additional test cases for complete coverage
test("reports skip reason for missing workspace", () => { expect( getWorkspaceAutoRenameDecision({ workspace: null, generatedName: "Fix auth flow", }), ).toEqual({ kind: "skip", reason: "missing-workspace", }); }); test("reports skip reason for deleting workspace", () => { expect( getWorkspaceAutoRenameDecision({ workspace: { branch: "feat/test-branch", name: "feat/test-branch", isUnnamed: true, deletingAt: Date.now(), }, generatedName: "Fix auth flow", }), ).toEqual({ kind: "skip", reason: "workspace-deleting", }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-auto-rename.test.ts` around lines 7 - 79, Add unit tests to cover the remaining skip reasons returned by getWorkspaceAutoRenameDecision: add one test calling getWorkspaceAutoRenameDecision with workspace: null and generatedName "Fix auth flow" expecting { kind: "skip", reason: "missing-workspace" }; add one test with workspace.deletingAt set (e.g., Date.now()) and isUnnamed true expecting { kind: "skip", reason: "workspace-deleting" }; and add one test exercising the "workspace-name-changed" branch by providing a workspace whose branch and name no longer match the original placeholder (e.g., branch: "feat/test-branch", name: "Running setup", isUnnamed: true) and assert the result is { kind: "skip", reason: "workspace-name-changed" } so getWorkspaceAutoRenameDecision is fully covered.
🤖 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/workspaces/procedures/create.ts`:
- Around line 416-419: The call to attemptWorkspaceAutoRenameFromPrompt({
workspaceId: workspace.id, prompt: input.prompt }) is being fire-and-forget
which causes unhandled rejections and drops any returned warning/result; change
this to await the promise and wrap it in try/catch so rejections are handled,
then propagate any non-fatal naming warning into the mutation response (or
forward the outcome through the same init-progress mechanism used elsewhere) so
callers receive the warning/toast instead of it being lost.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts`:
- Line 72: The modelId value "gpt-4.1" is invalid and will cause OpenAI API
errors—update the modelId in the AI name request configuration (the modelId
property in this file/function) to a valid OpenAI model string such as "gpt-4",
"gpt-4-turbo", or "gpt-4o" (or a dated snapshot like "gpt-4o-2024-11-20"), then
run/fix any related callers of this config to match the chosen model id.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts`:
- Around line 147-163: attemptWorkspaceAutoRenameFromPrompt can throw and
currently its await is inside the outer init try so failures will bubble to the
outer catch and cause removeWorktree() to run after
manager.markWorktreeCreated(workspaceId) — convert rename failures into warnings
by wrapping the await in a local try/catch (or extract a helper like
safeAttemptWorkspaceAutoRenameFromPrompt), on error set autoRenameResult = {
status: "skipped", warning: <error message> } (or similar) and then call
manager.updateProgress(workspaceId, "ready", "Ready", undefined,
autoRenameResult.status === "skipped" ? autoRenameResult.warning : undefined);
apply the same change to the other occurrence (lines ~466-482) so both branches
remain in sync and rename errors no longer trigger removeWorktree().
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts`:
- Around line 175-184: The select query that fetches the workspace currently
includes isNull(workspaces.deletingAt) which causes a null result and hides the
fact the workspace is deleting, leading getWorkspaceAutoRenameDecision to report
"missing-workspace" instead of "workspace-deleting"; remove the
isNull(workspaces.deletingAt) predicate from the localDb.select/.where call (or
alternatively keep the predicate but after fetching check the
workspace.deletingAt field) so that getWorkspaceAutoRenameDecision can read
deletingAt and return the accurate "workspace-deleting" skip reason (refer to
the workspace query using localDb.select(...) .from(workspaces)
.where(eq(workspaces.id, workspaceId)) and the getWorkspaceAutoRenameDecision
function).
In
`@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-auto-rename.test.ts`:
- Around line 7-79: Add unit tests to cover the remaining skip reasons returned
by getWorkspaceAutoRenameDecision: add one test calling
getWorkspaceAutoRenameDecision with workspace: null and generatedName "Fix auth
flow" expecting { kind: "skip", reason: "missing-workspace" }; add one test with
workspace.deletingAt set (e.g., Date.now()) and isUnnamed true expecting { kind:
"skip", reason: "workspace-deleting" }; and add one test exercising the
"workspace-name-changed" branch by providing a workspace whose branch and name
no longer match the original placeholder (e.g., branch: "feat/test-branch",
name: "Running setup", isUnnamed: true) and assert the result is { kind: "skip",
reason: "workspace-name-changed" } so getWorkspaceAutoRenameDecision is fully
covered.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-auto-rename.ts`:
- Around line 1-6: The interfaces WorkspaceAutoRenameState and
ResolveWorkspaceAutoRenameParams are declared but not exported, limiting reuse;
export them by adding the export keyword to their declarations (e.g., export
interface WorkspaceAutoRenameState and export interface
ResolveWorkspaceAutoRenameParams) so other modules can import these types;
ensure any local usages/imports still compile and update any files that should
import these interfaces accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7de30076-2f6f-4aad-ac39-a69244450b41
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
apps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/workspaces/procedures/create.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-auto-rename.test.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-auto-rename.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.tsapps/desktop/src/main/lib/workspace-init-manager.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/routes/_authenticated/layout.tsxapps/desktop/src/shared/types/workspace-init.ts
Contributor
There was a problem hiding this comment.
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/lib/trpc/routers/workspaces/utils/ai-name.ts`:
- Around line 97-100: The logs in ai-name.ts are printing user-authored content
(prompt, promptPreview, generatedName, workspace names/state) which may contain
sensitive info; update all console.log calls (those referencing prompt,
prompt.length/prompt.slice, promptPreview, generatedName, workspace.name,
workspace.state) to avoid verbatim content by either removing the raw strings or
replacing them with redacted values or metadata only (e.g., log prompt length,
provider name, and reason), and ensure any preview uses a deterministic
redaction like "<redacted>" or a hash instead of the actual text; apply this
change to the console.log sites around the prompt logging block and the other
occurrences noted (lines ~124-127, ~157-161, ~197-202, ~216-220).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9eecbade-f3ee-4f24-9980-92c4497e3c8a
📒 Files selected for processing (2)
apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts (1)
147-159:⚠️ Potential issue | 🔴 CriticalKeep auto-naming failures out of the init cleanup path.
These awaits still sit inside the outer
tryaftermanager.markWorktreeCreated(workspaceId). IfattemptWorkspaceAutoRenameFromPrompt()rejects, the catch below will remove an otherwise healthy worktree and mark init as failed even though setup already succeeded. Catch rename errors locally, turn them into a warning, and re-check cancellation beforeupdateProgress("ready")so a late delete doesn't resurrect the workspace.Suggested direction
+ const completeReadyState = async () => { + let warning: string | undefined; + try { + const autoRenameResult = await attemptWorkspaceAutoRenameFromPrompt({ + workspaceId, + prompt: namingPrompt, + }); + warning = + autoRenameResult.status === "skipped" + ? autoRenameResult.warning + : undefined; + } catch (error) { + console.warn("[workspace-init] Auto naming failed", { + workspaceId, + error: error instanceof Error ? error.message : String(error), + }); + warning = "Couldn't auto-name this workspace."; + } + + if (manager.isCancellationRequested(workspaceId)) { + return; + } + + manager.updateProgress(workspaceId, "ready", "Ready", undefined, warning); + }; ... - const autoRenameResult = await attemptWorkspaceAutoRenameFromPrompt({ - workspaceId, - prompt: namingPrompt, - }); - manager.updateProgress( - workspaceId, - "ready", - "Ready", - undefined, - autoRenameResult.status === "skipped" - ? autoRenameResult.warning - : undefined, - ); + await completeReadyState();Also applies to: 462-474
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts` around lines 147 - 159, The auto-rename call (attemptWorkspaceAutoRenameFromPrompt) is inside the outer try that, if it throws, will trigger the catch that removes the already-created worktree; change this by wrapping the attemptWorkspaceAutoRenameFromPrompt call in its own try/catch so any rejection is caught locally and converted into a warning string (e.g., autoRenameWarning) rather than thrown, then re-check cancellation/state after markWorktreeCreated(workspaceId) and before calling manager.updateProgress(workspaceId, "ready", ...) so a late cancellation can still abort without resurrecting the workspace; finally pass the warning into manager.updateProgress when auto-rename was skipped/failed instead of letting the outer catch run.apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts (1)
143-170:⚠️ Potential issue | 🔴 CriticalRe-check the rename preconditions in the
UPDATE.Line 151 filters deleting rows out of the read entirely, which bypasses the
workspace-deletingbranch ingetWorkspaceAutoRenameDecision(), and Line 169 updates byidonly. A manual rename or delete that lands between those statements can still be overwritten while this path returns"renamed". Fetch byidonly, then carrydeletingAt IS NULL,isUnnamed = true, andname === branchinto theUPDATE, and only report"renamed"if that guarded write actually matches a row.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts` around lines 143 - 170, The code reads the workspace filtered by deletingAt IS NULL, then unconditionally runs an update by id which can race and produce a false "renamed" result; change the read to fetch by id only (use workspaceId and not filter out deletingAt) before calling getWorkspaceAutoRenameDecision(workspace: workspace ?? null, generatedName), then perform the UPDATE against workspaces with a WHERE that includes eq(workspaces.id, workspaceId) AND isNull(workspaces.deletingAt) (or NOT deleting) AND eq(workspaces.isUnnamed, true) AND eq(workspaces.name, workspaces.branch) (to ensure name === branch) and set name, isUnnamed=false, updatedAt; finally, inspect the update result/affected count and only return {status: "renamed"} if a row was actually updated, otherwise return {status: "skipped", reason: "precondition failed"} (or forward decision.reason).
🤖 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/workspaces/utils/ai-name.ts`:
- Around line 1-11: Imports in ai-name.ts are not ordered per Biome's
import-order rule; reorder the import statements so groups follow project
convention (external packages first, then internal aliases, then relative
imports) and alphabetize within groups — specifically reorder the symbols like
Agent, createAnthropic, createOpenAI, workspaces,
getAnthropicCredentialsFromAnySource, getOpenAICredentialsFromAnySource,
and/eq/isNull, localDb, and getWorkspaceAutoRenameDecision into the correct
group order; after reordering, run the formatter/linter auto-fix (bun run
lint:fix) to apply Biome's import-order and formatting rules.
---
Duplicate comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts`:
- Around line 143-170: The code reads the workspace filtered by deletingAt IS
NULL, then unconditionally runs an update by id which can race and produce a
false "renamed" result; change the read to fetch by id only (use workspaceId and
not filter out deletingAt) before calling
getWorkspaceAutoRenameDecision(workspace: workspace ?? null, generatedName),
then perform the UPDATE against workspaces with a WHERE that includes
eq(workspaces.id, workspaceId) AND isNull(workspaces.deletingAt) (or NOT
deleting) AND eq(workspaces.isUnnamed, true) AND eq(workspaces.name,
workspaces.branch) (to ensure name === branch) and set name, isUnnamed=false,
updatedAt; finally, inspect the update result/affected count and only return
{status: "renamed"} if a row was actually updated, otherwise return {status:
"skipped", reason: "precondition failed"} (or forward decision.reason).
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts`:
- Around line 147-159: The auto-rename call
(attemptWorkspaceAutoRenameFromPrompt) is inside the outer try that, if it
throws, will trigger the catch that removes the already-created worktree; change
this by wrapping the attemptWorkspaceAutoRenameFromPrompt call in its own
try/catch so any rejection is caught locally and converted into a warning string
(e.g., autoRenameWarning) rather than thrown, then re-check cancellation/state
after markWorktreeCreated(workspaceId) and before calling
manager.updateProgress(workspaceId, "ready", ...) so a late cancellation can
still abort without resurrecting the workspace; finally pass the warning into
manager.updateProgress when auto-rename was skipped/failed instead of letting
the outer catch run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a314332-be80-4cf0-b4ad-27ad7e5e0b58
📒 Files selected for processing (2)
apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts
Contributor
There was a problem hiding this comment.
1 issue found across 5 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/lib/trpc/routers/workspaces/utils/ai-name.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts:148">
P3: This `if (!workspace)` block is unreachable because `getWorkspaceAutoRenameDecision` already returns `kind: "skip"` when workspace is null. After the skip return, workspace must be non-null, so this block is dead code. Remove it to avoid misleading control flow.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (decision.kind === "skip") { | ||
| return { status: "skipped", reason: decision.reason }; | ||
| } | ||
| if (!workspace) { |
Contributor
There was a problem hiding this comment.
P3: This if (!workspace) block is unreachable because getWorkspaceAutoRenameDecision already returns kind: "skip" when workspace is null. After the skip return, workspace must be non-null, so this block is dead code. Remove it to avoid misleading control flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts, line 148:
<comment>This `if (!workspace)` block is unreachable because `getWorkspaceAutoRenameDecision` already returns `kind: "skip"` when workspace is null. After the skip return, workspace must be non-null, so this block is dead code. Remove it to avoid misleading control flow.</comment>
<file context>
@@ -128,32 +128,68 @@ export async function attemptWorkspaceAutoRenameFromPrompt({
+ if (decision.kind === "skip") {
return { status: "skipped", reason: decision.reason };
}
+ if (!workspace) {
+ return { status: "skipped", reason: "missing-workspace" };
+ }
</file context>
Contributor
There was a problem hiding this comment.
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 (2)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (1)
409-425: 🛠️ Refactor suggestion | 🟠 MajorMake
isUnnamedexplicit for the orphaned-worktree insert.This new immediate rename path only works when the freshly inserted workspace is still marked unnamed, but
createWorkspaceFromWorktree()leaves that column implicit. On theinput.name ?? branchpath, that makes auto-renaming depend on theworkspaces.isUnnameddefault instead of the create flow, so this branch can silently skip naming. ThreadisUnnamed: !input.namethrough the helper so it matches the other workspace insert paths.Suggested direction
const workspace = createWorkspaceFromWorktree({ projectId: input.projectId, worktreeId: orphanedWorktree.id, branch, name: input.name ?? branch, + isUnnamed: !input.name, });
createWorkspaceFromWorktree()should then persistisUnnamedon the insert.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts` around lines 409 - 425, The create path relies on the new workspace being marked unnamed so the immediate rename flow runs, but createWorkspaceFromWorktree currently omits isUnnamed so the DB default controls behavior; update the call site to pass isUnnamed: !input.name into createWorkspaceFromWorktree (alongside projectId, worktreeId, branch, name) and then update the createWorkspaceFromWorktree implementation to persist that isUnnamed value on insert (so workspaces.isUnnamed is set explicitly), ensuring the logic around attemptWorkspaceAutoRenameFromPrompt and autoRenameResult behaves deterministically.apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts (1)
23-30:⚠️ Potential issue | 🟠 MajorPersist the naming prompt across init retries.
Because naming moved to ready-time,
namingPromptnow has to survive any failed first init. Right now it only lives on this in-memory init call, while the retry path inapps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts(Lines 164-176) requeuesinitializeWorkspaceWorktree()without it. A workspace that only reachesreadyon retry will therefore never be auto-named. Persist the prompt, or persist a precomputed title request, with the workspace/init job instead of threading it only through the first call.Also applies to: 50-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts` around lines 23 - 30, The namingPrompt currently only lives in the transient WorkspaceInitParams passed to initializeWorkspaceWorktree and is dropped when init.ts requeues the job; persist it by adding namingPrompt (or a precomputed title request string) to the long-lived workspace/init job payload and to the WorkspaceInitParams interface so requeue paths (the code that calls initializeWorkspaceWorktree in apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts) include it when enqueuing/re-enqueuing; update all call sites of initializeWorkspaceWorktree and the job serialization/deserialization to read/write namingPrompt (or titleRequest) so a retry that reaches ready still has the prompt available for auto-naming.
🤖 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/workspaces/utils/ai-name.ts`:
- Around line 115-146: The function attemptWorkspaceAutoRenameFromPrompt
currently calls generateWorkspaceNameFromPrompt before loading the workspace,
which can consume provider calls and return wrong skip reasons; move the localDb
select that loads the workspace (using workspaceId and
localDb/select/from/where) to run before generateWorkspaceNameFromPrompt, run
getWorkspaceAutoRenameDecision on the loaded workspace to short-circuit and
return non-"empty-generated-name" skip cases early, and only call
generateWorkspaceNameFromPrompt and the guarded UPDATE if the decision allows
renaming, keeping the existing post-generation race check that uses
generatedName and getWorkspaceAutoRenameDecision.
---
Outside diff comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts`:
- Around line 409-425: The create path relies on the new workspace being marked
unnamed so the immediate rename flow runs, but createWorkspaceFromWorktree
currently omits isUnnamed so the DB default controls behavior; update the call
site to pass isUnnamed: !input.name into createWorkspaceFromWorktree (alongside
projectId, worktreeId, branch, name) and then update the
createWorkspaceFromWorktree implementation to persist that isUnnamed value on
insert (so workspaces.isUnnamed is set explicitly), ensuring the logic around
attemptWorkspaceAutoRenameFromPrompt and autoRenameResult behaves
deterministically.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts`:
- Around line 23-30: The namingPrompt currently only lives in the transient
WorkspaceInitParams passed to initializeWorkspaceWorktree and is dropped when
init.ts requeues the job; persist it by adding namingPrompt (or a precomputed
title request string) to the long-lived workspace/init job payload and to the
WorkspaceInitParams interface so requeue paths (the code that calls
initializeWorkspaceWorktree in
apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts) include it when
enqueuing/re-enqueuing; update all call sites of initializeWorkspaceWorktree and
the job serialization/deserialization to read/write namingPrompt (or
titleRequest) so a retry that reaches ready still has the prompt available for
auto-naming.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 820539ae-af5c-4c3a-be01-86b012feca5f
📒 Files selected for processing (6)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.tsapps/desktop/src/renderer/routes/_authenticated/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/layout.tsx
Comment on lines
+115
to
+146
| const generatedName = await generateWorkspaceNameFromPrompt(cleanedPrompt); | ||
| if (!generatedName) { | ||
| const hasCredentials = | ||
| getAnthropicCredentialsFromAnySource() !== null || | ||
| getOpenAICredentialsFromAnySource() !== null; | ||
| return { | ||
| status: "skipped", | ||
| reason: hasCredentials ? "generation-failed" : "missing-credentials", | ||
| warning: hasCredentials | ||
| ? "Couldn't auto-name this workspace." | ||
| : "Couldn't auto-name this workspace because chat credentials aren't configured.", | ||
| }; | ||
| } | ||
|
|
||
| const workspace = localDb | ||
| .select({ | ||
| id: workspaces.id, | ||
| branch: workspaces.branch, | ||
| name: workspaces.name, | ||
| isUnnamed: workspaces.isUnnamed, | ||
| deletingAt: workspaces.deletingAt, | ||
| }) | ||
| .from(workspaces) | ||
| .where(eq(workspaces.id, workspaceId)) | ||
| .get(); | ||
|
|
||
| const decision = getWorkspaceAutoRenameDecision({ | ||
| workspace: workspace ?? null, | ||
| generatedName, | ||
| }); | ||
| if (decision.kind === "skip") { | ||
| return { status: "skipped", reason: decision.reason }; |
Contributor
There was a problem hiding this comment.
Short-circuit skip cases before generating a title.
attemptWorkspaceAutoRenameFromPrompt() calls the provider before loading the workspace. If the workspace is already missing, deleting, or no longer eligible for rename, this still burns a provider call and can return missing-credentials / generation-failed instead of the real skip reason. Load the workspace first, return the non-empty-generated-name skip cases early, then keep the guarded UPDATE as the post-generation race check.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts` around lines
115 - 146, The function attemptWorkspaceAutoRenameFromPrompt currently calls
generateWorkspaceNameFromPrompt before loading the workspace, which can consume
provider calls and return wrong skip reasons; move the localDb select that loads
the workspace (using workspaceId and localDb/select/from/where) to run before
generateWorkspaceNameFromPrompt, run getWorkspaceAutoRenameDecision on the
loaded workspace to short-circuit and return non-"empty-generated-name" skip
cases early, and only call generateWorkspaceNameFromPrompt and the guarded
UPDATE if the decision allows renaming, keeping the existing post-generation
race check that uses generatedName and getWorkspaceAutoRenameDecision.
This was referenced Mar 7, 2026
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
Testing
Summary by cubic
Auto-names workspaces after setup completes with Anthropic-first and OpenAI fallback, using shared title generation and fixed Anthropic auth (API key or OAuth). Shows a one-time warning when auto-naming is skipped or fails.
Bug Fixes
Refactors
Written for commit fa39f69. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests