feat(desktop): auto-create v2 main workspace on host project setup#3632
feat(desktop): auto-create v2 main workspace on host project setup#3632
Conversation
|
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:
📝 WalkthroughWalkthroughIntroduces automated "main" v2 workspace creation per (projectId, hostId) pair after project setup. Adds database schema changes (type enum, pinned_at column, partial unique index), a helper to initialize workspaces via git branch resolution and API calls, integration into clone/import flows, and a background recovery sweep on app startup. Changes
Sequence DiagramsequenceDiagram
participant ProjectSetup as Project Setup
participant HostService as Host Service
participant Git as Git
participant API as Cloud API
participant DB as Database
ProjectSetup->>HostService: ensureMainWorkspace(projectId, repoPath)
HostService->>Git: symbolic-ref --short HEAD
Git-->>HostService: branch name (or fallback)
alt Branch resolved
HostService->>API: device.ensureV2Host.mutate(machineId, organizationId)
API-->>HostService: hostId
HostService->>API: v2Workspace.create.mutate(type: "main", projectId, hostId, branch, pinnedAt)
API->>DB: INSERT v2_workspaces (with partial unique index)
DB-->>API: workspace id (or conflict)
API-->>HostService: workspace id
HostService->>DB: INSERT workspaces row (projectId, worktreePath: repoPath)
DB-->>HostService: row inserted (or onConflictDoNothing)
HostService-->>ProjectSetup: { id: workspace_id }
else Branch unresolved
HostService-->>ProjectSetup: null (with console.warn)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR ports the v1 "main workspace" concept to v2: each Key changes:
Issues found:
Confidence Score: 4/5Safe to merge after fixing the two P1s — the TOCTOU race and the spurious git worktree remove warning on project deletion. The overall design is solid — schema, migration, idempotency via unique index, and startup sweep are all well-structured. The two P1 issues are real but bounded in impact: the TOCTOU race is unlikely in normal usage, and the git worktree remove failure is silently caught. Neither causes data loss or prevents the feature from working in typical flows. packages/trpc/src/router/v2-workspace/v2-workspace.ts (TOCTOU in create) and packages/host-service/src/trpc/router/project/project.ts (git worktree remove on main workspace path)
|
| Filename | Overview |
|---|---|
| packages/trpc/src/router/v2-workspace/v2-workspace.ts | Adds kind field and idempotent main-workspace creation logic, but the check-then-insert pattern has a TOCTOU race: concurrent calls can bypass the findFirst guard and hit a raw unique-index constraint error instead of returning idempotently. |
| packages/host-service/src/trpc/router/project/project.ts | Wires ensureMainWorkspace into clone and import paths correctly, but project.remove now iterates main workspace rows (worktreePath === repoPath) and always fails the git worktree remove call, producing spurious warnings on every project removal. |
| packages/host-service/src/trpc/router/project/utils/ensure-main-workspace.ts | New helper that resolves the current branch, ensures a host row, creates a cloud v2 workspace with kind='main', and inserts a matching local workspaces row. Logic is sound; synchronous SQLite .run() is used correctly without await. |
| packages/host-service/src/runtime/main-workspace-sweep.ts | Startup sweep correctly guards with existsSync, catches per-project errors, and delegates to ensureMainWorkspace. The ctx as HostServiceContext cast is technically safe today but bypasses the type system. |
| packages/db/drizzle/0036_v2_workspaces_kind.sql | Adds kind text column with default 'worktree' and a partial unique index on (project_id, host_id) WHERE kind='main'. Migration is correct and backward-compatible. |
| packages/db/src/schema/schema.ts | Adds kind column and uniqueIndex to v2Workspaces table definition, correctly mirroring the SQL migration. |
| packages/db/src/schema/enums.ts | Adds v2WorkspaceKindValues, v2WorkspaceKindEnum, and V2WorkspaceKind type cleanly alongside existing enum definitions. |
| packages/host-service/src/app.ts | Fires runMainWorkspaceSweep in the background on startup with correct void + catch pattern; does not block server readiness. |
Sequence Diagram
sequenceDiagram
participant App as app.ts (startup)
participant Setup as project.setup
participant EMW as ensureMainWorkspace
participant Git as git (simple-git)
participant Cloud as v2Workspace.create (cloud tRPC)
participant LocalDB as local SQLite
Note over App: On boot — background sweep
App->>+EMW: runMainWorkspaceSweep(ctx)
EMW->>LocalDB: SELECT id, repoPath FROM projects
loop each project
EMW->>Git: symbolic-ref --short HEAD
Git-->>EMW: branch name (or null → skip)
EMW->>Cloud: v2Workspace.create({ kind:'main', ... })
Cloud->>Cloud: findFirst existing row
alt already exists
Cloud-->>EMW: existing row (idempotent)
else new row
Cloud->>Cloud: INSERT v2_workspaces
Cloud-->>EMW: new workspace row
end
EMW->>LocalDB: INSERT workspaces onConflictDoNothing
end
EMW-->>App: done
Note over Setup: project.setup (clone or import)
Setup->>+EMW: ensureMainWorkspace(ctx, projectId, repoPath)
EMW->>Git: symbolic-ref --short HEAD
Git-->>EMW: branch name
EMW->>Cloud: v2Workspace.create({ kind:'main', ... })
Cloud-->>EMW: workspace row
EMW->>LocalDB: INSERT workspaces onConflictDoNothing
EMW-->>Setup: { id }
Comments Outside Diff (1)
-
packages/host-service/src/trpc/router/project/project.ts, line 262-273 (link)git worktree removewill always fail for the main workspaceproject.removeiterates all entries in the localworkspacestable and callsgit worktree remove <ws.worktreePath>for each one. This PR now inserts a main-workspace row whereworktreePath === localProject.repoPath(the repo root). Runninggit worktree removeon the main working tree is rejected by git withfatal: '<path>' is a main worktree, so every project removal will now log a spurious warning for the main workspace before falling through to thermSync.The simplest guard is to skip the
git worktree removestep when the workspace path equals the repo root:Prompt To Fix With AI
This is a comment left during a code review. Path: packages/host-service/src/trpc/router/project/project.ts Line: 262-273 Comment: **`git worktree remove` will always fail for the main workspace** `project.remove` iterates all entries in the local `workspaces` table and calls `git worktree remove <ws.worktreePath>` for each one. This PR now inserts a main-workspace row where `worktreePath === localProject.repoPath` (the repo root). Running `git worktree remove` on the main working tree is rejected by git with `fatal: '<path>' is a main worktree`, so every project removal will now log a spurious warning for the main workspace before falling through to the `rmSync`. The simplest guard is to skip the `git worktree remove` step when the workspace path equals the repo root: How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/v2-workspace/v2-workspace.ts
Line: 123-132
Comment:
**TOCTOU race in idempotent `kind='main'` creation**
The check-then-insert pattern for `kind='main'` is not atomic. If two callers (e.g., the startup sweep racing with a concurrent `project.setup`) both execute `findFirst` and find no existing row, both will proceed to the `insert`. One insert will be rejected by the unique index with a raw database constraint error — not a clean `TRPCError` — so the client receives an opaque `INTERNAL_SERVER_ERROR` instead of the intended idempotent success.
The unique index is the correct protection layer, but the code above the insert doesn't handle the constraint violation. The fix is to use `onConflictDoNothing().returning()` on the insert and fetch the existing row when the result is empty.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/project/project.ts
Line: 262-273
Comment:
**`git worktree remove` will always fail for the main workspace**
`project.remove` iterates all entries in the local `workspaces` table and calls `git worktree remove <ws.worktreePath>` for each one. This PR now inserts a main-workspace row where `worktreePath === localProject.repoPath` (the repo root). Running `git worktree remove` on the main working tree is rejected by git with `fatal: '<path>' is a main worktree`, so every project removal will now log a spurious warning for the main workspace before falling through to the `rmSync`.
The simplest guard is to skip the `git worktree remove` step when the workspace path equals the repo root:
```suggestion
for (const ws of localWorkspaces) {
if (ws.worktreePath === localProject.repoPath) continue; // main workspace — not a linked worktree
try {
const git = await ctx.git(localProject.repoPath);
await git.raw(["worktree", "remove", ws.worktreePath]);
} catch (err) {
console.warn("[project.remove] failed to remove worktree", {
projectId: input.projectId,
worktreePath: ws.worktreePath,
err,
});
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/host-service/src/runtime/main-workspace-sweep.ts
Line: 30-34
Comment:
**Unsafe `as HostServiceContext` widening cast**
`runMainWorkspaceSweep` intentionally accepts a narrower `Pick<HostServiceContext, "api" | "db" | "git" | "organizationId">`, but passes it to `ensureMainWorkspace` via `ctx as HostServiceContext`. TypeScript's structural check is bypassed, so if `ensureMainWorkspace` is later modified to use an additional `HostServiceContext` property (e.g. `ctx.runtime`), the type system will not catch that this call site doesn't supply it — leaving a runtime error.
Consider narrowing `ensureMainWorkspace`'s own parameter type to the same `Pick`, which is already sufficient for everything it accesses (`api`, `db`, `git`, `organizationId`).
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(desktop): auto-create v2 main works..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/trpc/src/router/v2-workspace/v2-workspace.ts (1)
123-145:⚠️ Potential issue | 🟠 MajorMake
maincreation idempotent under concurrent calls.The pre-check is a TOCTOU race: two callers can both miss
existing, then one insert hits the partial unique index and fails instead of returning the existing main workspace. Catch the unique violation forkind === "main"and re-query, or use an atomic conflict-handling insert.Race-safe retry direction
+function hasPgErrorCode(error: unknown, code: string): boolean { + if (typeof error !== "object" || error === null) return false; + if ("code" in error && (error as { code?: unknown }).code === code) { + return true; + } + if ("cause" in error) { + return hasPgErrorCode((error as { cause?: unknown }).cause, code); + } + return false; +} + @@ - if (input.kind === "main") { - const existing = await dbWs.query.v2Workspaces.findFirst({ + const findExistingMain = () => + dbWs.query.v2Workspaces.findFirst({ where: and( eq(v2Workspaces.projectId, project.id), eq(v2Workspaces.hostId, host.id), eq(v2Workspaces.kind, "main"), ), }); + + if (input.kind === "main") { + const existing = await findExistingMain(); if (existing) return existing; } - const [workspace] = await dbWs - .insert(v2Workspaces) - .values({ - organizationId: project.organizationId, - projectId: project.id, - name: input.name, - branch: input.branch, - hostId: host.id, - kind: input.kind, - createdByUserId: ctx.userId, - }) - .returning(); + let workspace; + try { + [workspace] = await dbWs + .insert(v2Workspaces) + .values({ + organizationId: project.organizationId, + projectId: project.id, + name: input.name, + branch: input.branch, + hostId: host.id, + kind: input.kind, + createdByUserId: ctx.userId, + }) + .returning(); + } catch (err) { + if (input.kind === "main" && hasPgErrorCode(err, "23505")) { + const existing = await findExistingMain(); + if (existing) return existing; + } + throw err; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts` around lines 123 - 145, The pre-check for input.kind === "main" is TOCTOU-prone: keep the existing fast-path but make the insert in create workspace race-safe by handling unique-constraint conflicts on (projectId, hostId, kind === "main"). Wrap the dbWs.insert(...).returning() call in a try/catch that detects the unique-violation for v2Workspaces (or use an upsert/ON CONFLICT DO NOTHING equivalent) and, on conflict for kind "main", re-query via dbWs.query.v2Workspaces.findFirst(...) using the same predicates (eq(v2Workspaces.projectId, project.id), eq(v2Workspaces.hostId, host.id), eq(v2Workspaces.kind, "main")) and return that row instead of propagating the error; keep existing behavior for other kinds/errors.
🧹 Nitpick comments (1)
packages/host-service/src/runtime/main-workspace-sweep.ts (1)
30-34: Avoid widening the sweep context with a type assertion.
ctx as HostServiceContexthides missing-field regressions. Consider changingensureMainWorkspaceto accept the same narrowPick<HostServiceContext, "api" | "db" | "git" | "organizationId">shape instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/runtime/main-workspace-sweep.ts` around lines 30 - 34, The call site widens ctx with a type assertion (ctx as HostServiceContext) which masks missing-field regressions; update ensureMainWorkspace to accept the narrow shape Pick<HostServiceContext, "api" | "db" | "git" | "organizationId"> (or a named interface mirroring those fields) and change the call to pass ctx directly without assertion; update the ensureMainWorkspace function signature and any internal usages to rely only on those four properties so callers no longer need to assert the full HostServiceContext.
🤖 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/plans/20260421-v2-main-workspace-creation.md`:
- Around line 47-48: The implementation in ensure-main-workspace.ts must match
the documented rollback pattern used by finishCheckout: add compensating undo
steps for both the cloud workspace creation and the local DB insert so failures
trigger a rollback sequence instead of leaving partial state; specifically,
modify the ensureMainWorkspace flow to (1) record the cloud create operation and
on error call the same kind of teardown used by finishCheckout to delete the
created cloud workspace, and (2) wrap the local insert (the partial unique index
entry) with a compensating delete when the downstream steps fail —
alternatively, if you choose the doc route, update the design note in
workspace-creation.ts to state it intentionally logs and rolls forward with
idempotency ensured by the partial unique index rather than performing a
finishCheckout-style undo.
In `@packages/db/src/schema/schema.ts`:
- Line 540: Replace the application-only enum usage on the kind column—currently
defined as text({ enum: v2WorkspaceKindValues
}).notNull().default("worktree")—with a database-enforced pgEnum: create a
pgEnum named v2WorkspaceKind containing the values from v2WorkspaceKindValues,
then update the kind column to use
pgEnum(v2WorkspaceKind).notNull().default("worktree") so the DB enforces valid
values; after that regenerate the migration with bunx drizzle-kit generate
--name="v2_workspaces_kind_enum".
In
`@packages/host-service/src/trpc/router/project/utils/ensure-main-workspace.ts`:
- Around line 50-68: The current sequence calls
ctx.api.v2Workspace.create.mutate then performs
ctx.db.insert(workspaces)...onConflictDoNothing().run() without rollback;
implement the finishCheckout-style rollback by wrapping the local insert in a
try/catch: after calling ctx.api.v2Workspace.create.mutate(...) store cloudRow,
then try the ctx.db.insert(workspaces).values(...).run() (consider removing
onConflictDoNothing so insert errors surface); on any insert exception call
ctx.api.v2Workspace.delete.mutate({ id: cloudRow.id }) to remove the cloud row
(log any delete error but rethrow the original insert error), mirroring the
finishCheckout pattern and preventing cloud/local drift.
- Around line 44-70: The ensureMainWorkspace flow should not crash project.setup
on transient cloud/DB errors: wrap the body of ensureMainWorkspace (the calls to
ctx.api.device.ensureV2Host.mutate, ctx.api.v2Workspace.create.mutate and the
ctx.db.insert(...).run()) in a try/catch, on error log the full error using the
project/service logger available on ctx (e.g. ctx.logger.error or ctx.log.error)
with contextual fields (organizationId, projectId, branch) and return null so
callers can proceed; this relies on runMainWorkspaceSweep to backfill later.
---
Outside diff comments:
In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts`:
- Around line 123-145: The pre-check for input.kind === "main" is TOCTOU-prone:
keep the existing fast-path but make the insert in create workspace race-safe by
handling unique-constraint conflicts on (projectId, hostId, kind === "main").
Wrap the dbWs.insert(...).returning() call in a try/catch that detects the
unique-violation for v2Workspaces (or use an upsert/ON CONFLICT DO NOTHING
equivalent) and, on conflict for kind "main", re-query via
dbWs.query.v2Workspaces.findFirst(...) using the same predicates
(eq(v2Workspaces.projectId, project.id), eq(v2Workspaces.hostId, host.id),
eq(v2Workspaces.kind, "main")) and return that row instead of propagating the
error; keep existing behavior for other kinds/errors.
---
Nitpick comments:
In `@packages/host-service/src/runtime/main-workspace-sweep.ts`:
- Around line 30-34: The call site widens ctx with a type assertion (ctx as
HostServiceContext) which masks missing-field regressions; update
ensureMainWorkspace to accept the narrow shape Pick<HostServiceContext, "api" |
"db" | "git" | "organizationId"> (or a named interface mirroring those fields)
and change the call to pass ctx directly without assertion; update the
ensureMainWorkspace function signature and any internal usages to rely only on
those four properties so callers no longer need to assert the full
HostServiceContext.
🪄 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: 53f85ed9-ec96-4b7c-90f0-76bbe93719ce
📒 Files selected for processing (11)
apps/desktop/plans/20260421-v2-main-workspace-creation.mdpackages/db/drizzle/0036_v2_workspaces_kind.sqlpackages/db/drizzle/meta/0036_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/enums.tspackages/db/src/schema/schema.tspackages/host-service/src/app.tspackages/host-service/src/runtime/main-workspace-sweep.tspackages/host-service/src/trpc/router/project/project.tspackages/host-service/src/trpc/router/project/utils/ensure-main-workspace.tspackages/trpc/src/router/v2-workspace/v2-workspace.ts
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
3 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="packages/trpc/src/router/v2-workspace/v2-workspace.ts">
<violation number="1" location="packages/trpc/src/router/v2-workspace/v2-workspace.ts:123">
P2: The `main` workspace idempotency check is race-prone: concurrent requests can still throw a unique-constraint error instead of returning the existing workspace.</violation>
</file>
<file name="packages/host-service/src/trpc/router/project/project.ts">
<violation number="1" location="packages/host-service/src/trpc/router/project/project.ts:207">
P1: Wrap `ensureMainWorkspace` in try/catch here (and at the other call sites in this file). A transient cloud failure (network blip, 5xx) will now crash `project.setup` even though the clone already succeeded on disk — a regression. The startup sweep (`runMainWorkspaceSweep`) exists as the recovery path, so it's safe to log-and-continue here and let the sweep backfill on next boot.</violation>
</file>
<file name="packages/db/src/schema/schema.ts">
<violation number="1" location="packages/db/src/schema/schema.ts:540">
P2: Use `pgEnum` instead of `text({ enum: ... })` for database-level validation of `kind`. The current declaration creates a plain `text` column with no CHECK constraint or PostgreSQL enum type — only TypeScript-level inference. Direct SQL writes can insert arbitrary strings. Other enums in this codebase (`v2ClientType`, `automationSessionKind`, `taskStatus`) use `pgEnum()` for DB-level enforcement; this should follow the same pattern.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/trpc/src/router/v2-workspace/v2-workspace.ts (1)
124-157:⚠️ Potential issue | 🟠 MajorMake main-workspace idempotency atomic.
Line 124 checks for an existing
"main"workspace before Line 145 inserts, but concurrent setup/sweep calls can both observe no row and then race into the partial unique index. One caller will get a constraint error instead of the idempotent “return existing row” behavior this API promises.Please wrap the insert path so unique-conflict on
(projectId, hostId, kind = "main")re-reads and returns the existing row, or use an atomic upsert/transaction pattern for"main"creation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts` around lines 124 - 157, The current non-atomic check-then-insert for a main workspace (code around v2Workspaces, the async block that queries v2Workspaces.findFirst and then dbWs.insert(...).returning()) can race; change the insert path for input.kind === "main" to be atomic: either perform the insert inside a transaction with a SELECT FOR UPDATE on the project/host row and then insert, or catch the unique-constraint error from dbWs.insert and on that error re-query v2Workspaces for the existing main row and return it (instead of surfacing the constraint error); ensure you reference the same v2Workspaces columns (projectId, hostId, kind) when re-reading and return the existing row when found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts`:
- Around line 124-157: The current non-atomic check-then-insert for a main
workspace (code around v2Workspaces, the async block that queries
v2Workspaces.findFirst and then dbWs.insert(...).returning()) can race; change
the insert path for input.kind === "main" to be atomic: either perform the
insert inside a transaction with a SELECT FOR UPDATE on the project/host row and
then insert, or catch the unique-constraint error from dbWs.insert and on that
error re-query v2Workspaces for the existing main row and return it (instead of
surfacing the constraint error); ensure you reference the same v2Workspaces
columns (projectId, hostId, kind) when re-reading and return the existing row
when found.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18246098-7d4e-460c-afb4-9019fb860793
📒 Files selected for processing (7)
packages/db/drizzle/0037_v2_workspaces_pinned_at.sqlpackages/db/drizzle/meta/0037_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/schema.tspackages/host-service/src/trpc/router/project/handlers.tspackages/host-service/src/trpc/router/project/utils/ensure-main-workspace.tspackages/trpc/src/router/v2-workspace/v2-workspace.ts
✅ Files skipped from review due to trivial changes (2)
- packages/db/drizzle/0037_v2_workspaces_pinned_at.sql
- packages/db/drizzle/meta/_journal.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/db/src/schema/schema.ts
- packages/host-service/src/trpc/router/project/utils/ensure-main-workspace.ts
41f8d30 to
3930005
Compare
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 (1)
packages/trpc/src/router/v2-workspace/v2-workspace.ts (1)
124-157:⚠️ Potential issue | 🟠 MajorMake
maincreation idempotent under concurrent calls.The
findFirstpre-check can race with anotherproject.setupor startup sweep: both callers can see no row, then the loser hits the partial unique index on insert and the supposedly idempotent create fails. Handle the unique-conflict path by refetching/backfilling the existingmain, or use an atomic upsert against the partial index.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts` around lines 124 - 157, The current create path for kind === "main" can race: replace the optimistic findFirst + insert with a safe retry-on-unique-conflict or an atomic upsert; specifically, when calling dbWs.insert(...).values(...).returning() for v2Workspaces, catch the unique constraint error for the partial unique index on (projectId, hostId, kind="main"), then re-query v2Workspaces with the same selectors (the same and(eq(v2Workspaces.projectId, project.id), eq(v2Workspaces.hostId, host.id), eq(v2Workspaces.kind, "main"))) to return the existing row; if the newly supplied pinnedAt should be applied and the existing row lacks pinnedAt, issue a targeted update via dbWs.update(v2Workspaces).set({ pinnedAt: input.pinnedAt }).where(eq(v2Workspaces.id, existing.id)).returning() before returning the row—alternatively implement a DB-native upsert that only inserts when no matching main exists.
🤖 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/db/drizzle/0036_v2_workspaces_main_kind_and_pin.sql`:
- Around line 1-3: The migration adds v2_workspaces.kind as plain text which
allows invalid values and can bypass your partial unique index; instead update
the schema source under packages/db/src/schema to constrain kind (either add a
DB enum type or a CHECK on v2_workspaces.kind IN ('main','worktree') in the
schema definition for the v2_workspaces model), then regenerate the migration
with bunx drizzle-kit generate --name="<sample_name_snake_case>" so the proper
enum/check appears in the generated SQL and associated snapshots; do not
manually edit files in packages/db/drizzle/ (including .sql, meta/_journal.json,
or snapshots).
---
Outside diff comments:
In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts`:
- Around line 124-157: The current create path for kind === "main" can race:
replace the optimistic findFirst + insert with a safe retry-on-unique-conflict
or an atomic upsert; specifically, when calling
dbWs.insert(...).values(...).returning() for v2Workspaces, catch the unique
constraint error for the partial unique index on (projectId, hostId,
kind="main"), then re-query v2Workspaces with the same selectors (the same
and(eq(v2Workspaces.projectId, project.id), eq(v2Workspaces.hostId, host.id),
eq(v2Workspaces.kind, "main"))) to return the existing row; if the newly
supplied pinnedAt should be applied and the existing row lacks pinnedAt, issue a
targeted update via dbWs.update(v2Workspaces).set({ pinnedAt: input.pinnedAt
}).where(eq(v2Workspaces.id, existing.id)).returning() before returning the
row—alternatively implement a DB-native upsert that only inserts when no
matching main exists.
🪄 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: 419f9056-d994-4873-af48-2143d5f0ee44
📒 Files selected for processing (12)
apps/desktop/plans/20260421-v2-main-workspace-creation.mdpackages/db/drizzle/0036_v2_workspaces_main_kind_and_pin.sqlpackages/db/drizzle/meta/0036_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/enums.tspackages/db/src/schema/schema.tspackages/host-service/src/app.tspackages/host-service/src/runtime/main-workspace-sweep.tspackages/host-service/src/trpc/router/project/handlers.tspackages/host-service/src/trpc/router/project/project.tspackages/host-service/src/trpc/router/project/utils/ensure-main-workspace.tspackages/trpc/src/router/v2-workspace/v2-workspace.ts
✅ Files skipped from review due to trivial changes (2)
- packages/host-service/src/app.ts
- packages/db/src/schema/schema.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/db/drizzle/meta/_journal.json
- packages/db/src/schema/enums.ts
- packages/host-service/src/runtime/main-workspace-sweep.ts
- packages/host-service/src/trpc/router/project/project.ts
- packages/host-service/src/trpc/router/project/handlers.ts
- apps/desktop/plans/20260421-v2-main-workspace-creation.md
3930005 to
b7af2a1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/project/handlers.ts (1)
61-75: Minor:createFromImportLocaldoesn't roll back onensureMainWorkspacefailure.If
ensureMainWorkspacethrows here, the cloudv2Projectand the local project row are already persisted, and the error propagates to the caller — the user sees a failed import even though the project is actually set up. The startup sweep will backfill the main workspace on next launch, so this is not a correctness bug, but consider either (a) swallowing/loggingensureMainWorkspaceerrors so import still reports success (sweep will retry) or (b) mirroring the try/catch pattern increateFromClonefor consistency.Same pattern applies to
createFromClone(line 46): a failure inensureMainWorkspacecurrently triggers a fullrmSyncof the freshly cloned repo, which is arguably an over-reaction for an idempotent, sweep-recoverable step. Worth deciding whether main-workspace creation should be treated as fatal to setup or as best-effort.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/project/handlers.ts` around lines 61 - 75, The createFromImportLocal and createFromClone flows currently let ensureMainWorkspace failures abort the import (and in createFromClone even rmSync the repo); change both to treat ensureMainWorkspace as best-effort by wrapping the ensureMainWorkspace(ctx, cloudProject.id, resolved.repoPath) call in a try/catch that logs the error (using the existing logger in HostServiceContext) and does not rethrow, or mirror the existing try/catch pattern from createFromClone but remove the aggressive rmSync on ensureMainWorkspace failure; this keeps the cloud project and local row persisted while allowing the startup sweep to backfill the workspace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/host-service/src/trpc/router/project/handlers.ts`:
- Around line 61-75: The createFromImportLocal and createFromClone flows
currently let ensureMainWorkspace failures abort the import (and in
createFromClone even rmSync the repo); change both to treat ensureMainWorkspace
as best-effort by wrapping the ensureMainWorkspace(ctx, cloudProject.id,
resolved.repoPath) call in a try/catch that logs the error (using the existing
logger in HostServiceContext) and does not rethrow, or mirror the existing
try/catch pattern from createFromClone but remove the aggressive rmSync on
ensureMainWorkspace failure; this keeps the cloud project and local row
persisted while allowing the startup sweep to backfill the workspace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10bda4cb-523e-48a9-bb52-1bb50667a192
📒 Files selected for processing (12)
apps/desktop/plans/20260421-v2-main-workspace-creation.mdpackages/db/drizzle/0036_v2_workspaces_main_kind_and_pin.sqlpackages/db/drizzle/meta/0036_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/enums.tspackages/db/src/schema/schema.tspackages/host-service/src/app.tspackages/host-service/src/runtime/main-workspace-sweep.tspackages/host-service/src/trpc/router/project/handlers.tspackages/host-service/src/trpc/router/project/project.tspackages/host-service/src/trpc/router/project/utils/ensure-main-workspace.tspackages/trpc/src/router/v2-workspace/v2-workspace.ts
✅ Files skipped from review due to trivial changes (4)
- packages/db/drizzle/meta/_journal.json
- packages/db/src/schema/enums.ts
- packages/host-service/src/app.ts
- apps/desktop/plans/20260421-v2-main-workspace-creation.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/host-service/src/runtime/main-workspace-sweep.ts
- packages/db/drizzle/0036_v2_workspaces_main_kind_and_pin.sql
- packages/host-service/src/trpc/router/project/utils/ensure-main-workspace.ts
- packages/host-service/src/trpc/router/project/project.ts
Each (projectId, hostId) gets a singleton "main" workspace pointing at the
host's configured repoPath, auto-created when a project is created or set
up on a new host. Mains are auto-pinned at creation; existing mains get
backfilled by the startup sweep.
- Schema: v2_workspaces.type pgEnum ('main' | 'worktree', default 'worktree')
enforced at the DB layer — matches the v2ClientType / v2UsersHostRole
naming precedent. Partial unique index on (project_id, host_id) WHERE
type='main', plus a pinned_at timestamp.
- API: v2Workspace.create uses onConflictDoNothing + re-fetch so concurrent
main creates collapse idempotently instead of racing into a raw
unique-violation error. Backfills pinned_at on existing main rows.
- Helper: ensureMainWorkspace resolves current branch via git symbolic-ref,
creates the cloud + local workspace rows. Log-and-continue on any error
so transient cloud blips don't fail project.setup; the startup sweep
retries on next boot. Skips on detached HEAD.
- Call sites: project.create (createFromClone, createFromImportLocal) and
project.setup (clone, import, including early-return paths).
- Recovery: host-service startup sweep iterates local projects and ensures
each has a main row. Idempotent via the unique index.
b7af2a1 to
86835dc
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/trpc/src/router/v2-workspace/v2-workspace.ts (1)
124-169: Idempotent conflict handling looks correct.Nice race-safety design: the partial unique index +
onConflictDoNothing+ reconciliation fortype === "main"makes concurrentproject.setup/sweep callers converge to the same row. ThepinnedAtbackfill condition (!existing.pinnedAt && input.pinnedAt) also correctly avoids clobbering an existing pin.One small observation: the fallthrough
INTERNAL_SERVER_ERRORon line 166 fires only fortype === "worktree"(no relevant unique constraint) or a PK collision (effectively impossible withdefaultRandom()), so it's unreachable in normal flow. Consider a more specific message to aid debugging if it ever does trigger.♻️ Proposed refactor
throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", - message: "Workspace insert returned no row", + message: `Workspace insert returned no row (type=${input.type}, projectId=${project.id}, hostId=${host.id})`, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts` around lines 124 - 169, The thrown TRPCError at the end of the create-upsert flow uses a generic message ("Workspace insert returned no row"); change it to a more specific diagnostic message that includes context (e.g. include input.type and project.id or mention PK collision vs missing unique index) so future signals are actionable: update the TRPCError instantiation in the block that uses dbWs.insert(...).onConflictDoNothing().returning() (the code that checks inserted and then reconciles for input.type === "main") to construct a clearer message indicating whether this fallthrough likely represents an unexpected PK collision or an insert-that-was-suppressed-for-a-worktree (include input.type and project.id/host.id in the message).packages/db/src/schema/schema.ts (1)
561-563: Use${table.type}instead of the raw identifier in the partial index predicate.Drizzle ORM pg-core supports column interpolation via
${table.column}insidesqltemplates for partial indexes and emits the quoted physical column name in DDL. The raw identifiertypebypasses Drizzle's column-name resolution — a future column rename won't update this predicate, causing silent DDL drift.♻️ Proposed refactor
- uniqueIndex("v2_workspaces_one_main_per_host") - .on(table.projectId, table.hostId) - .where(sql`type = 'main'`), + uniqueIndex("v2_workspaces_one_main_per_host") + .on(table.projectId, table.hostId) + .where(sql`${table.type} = 'main'`),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/src/schema/schema.ts` around lines 561 - 563, The partial index predicate for uniqueIndex("v2_workspaces_one_main_per_host") uses the raw identifier `type` which bypasses Drizzle's column interpolation; update the sql predicate to interpolate the column using `${table.type}` so column renames are tracked (i.e., change the where(sql`type = 'main'`) to use `${table.type}` inside the sql template while keeping the rest of the index on table.projectId and table.hostId).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/db/src/schema/schema.ts`:
- Around line 561-563: The partial index predicate for
uniqueIndex("v2_workspaces_one_main_per_host") uses the raw identifier `type`
which bypasses Drizzle's column interpolation; update the sql predicate to
interpolate the column using `${table.type}` so column renames are tracked
(i.e., change the where(sql`type = 'main'`) to use `${table.type}` inside the
sql template while keeping the rest of the index on table.projectId and
table.hostId).
In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts`:
- Around line 124-169: The thrown TRPCError at the end of the create-upsert flow
uses a generic message ("Workspace insert returned no row"); change it to a more
specific diagnostic message that includes context (e.g. include input.type and
project.id or mention PK collision vs missing unique index) so future signals
are actionable: update the TRPCError instantiation in the block that uses
dbWs.insert(...).onConflictDoNothing().returning() (the code that checks
inserted and then reconciles for input.type === "main") to construct a clearer
message indicating whether this fallthrough likely represents an unexpected PK
collision or an insert-that-was-suppressed-for-a-worktree (include input.type
and project.id/host.id in the message).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14dd35a0-342a-477d-9311-eb6707e79931
📒 Files selected for processing (12)
apps/desktop/plans/20260421-v2-main-workspace-creation.mdpackages/db/drizzle/0036_v2_workspaces_main_type_and_pin.sqlpackages/db/drizzle/meta/0036_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/enums.tspackages/db/src/schema/schema.tspackages/host-service/src/app.tspackages/host-service/src/runtime/main-workspace-sweep.tspackages/host-service/src/trpc/router/project/handlers.tspackages/host-service/src/trpc/router/project/project.tspackages/host-service/src/trpc/router/project/utils/ensure-main-workspace.tspackages/trpc/src/router/v2-workspace/v2-workspace.ts
✅ Files skipped from review due to trivial changes (1)
- packages/db/drizzle/meta/_journal.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/host-service/src/app.ts
- packages/host-service/src/runtime/main-workspace-sweep.ts
- packages/host-service/src/trpc/router/project/handlers.ts
- apps/desktop/plans/20260421-v2-main-workspace-creation.md
…on-in-v2-migration # Conflicts: # apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsx # apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts # apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/types.ts # packages/db/drizzle/meta/0036_snapshot.json # packages/db/drizzle/meta/_journal.json # packages/trpc/src/router/v2-workspace/v2-workspace.ts
…on-in-v2-migration # Conflicts: # packages/host-service/src/trpc/router/project/handlers.ts
3421ea2 to
6aef7a0
Compare
Add v2 project setup section (#3566, #3605, #3606, #3592, #3626, #3632), scheduled agent runs (#3576), Opus 4.7 (#3579), v1 review comments in pane (#3596), configurable v2 link-click (#3600), Copy Branch Name (#3635), safer terminal preset defaults (#3546), and /pricing page (#3639). Expand bug fixes with v2 git correctness, cross-fork PR misattribution, terminal paste/Unicode/Shift+Enter, and security bumps.
…-27) (#3792) * docs: generate weekly changelog 2026-04-27 * docs: reframe weekly changelog around v2 public beta Lead with v2 public beta + Settings → Experimental enable, restructure around the v1→v2 migration story, sidebar overhaul, cross-workspace terminals, and v2 chat. Pull in ~30 v2 PRs the bot missed and demote non-v2 items (Hosts page, marketing menu) to a brief "Also this week". * docs: pull in missed v2 features and bug fixes Add v2 project setup section (#3566, #3605, #3606, #3592, #3626, #3632), scheduled agent runs (#3576), Opus 4.7 (#3579), v1 review comments in pane (#3596), configurable v2 link-click (#3600), Copy Branch Name (#3635), safer terminal preset defaults (#3546), and /pricing page (#3639). Expand bug fixes with v2 git correctness, cross-fork PR misattribution, terminal paste/Unicode/Shift+Enter, and security bumps. * docs(changelog): add v2 public beta hero screenshot * docs(changelog): add Settings → Experimental screenshot, compress hero pngquant compression: v2-public-beta.png 704KB → 166KB (76%), v2-enable-flag.png 160KB → 36KB (78%). No visible quality loss. * docs(changelog): tighten v2 launch prose, condense bullet groups * docs(changelog): reframe cloud-first pillar as remote workspaces * docs(changelog): cut parallel-agents and honest-state pillars, fold into sub-sections * docs(changelog): tweak title and lead phrasing * docs(changelog): rewrite v2 launch lede around Twitter narrative Pull the launch story (physical limits, 3 ex-CTOs, cloud workspaces) into the lede, restructure pillars around Remote workspaces, Reimagined diff view, and Superset CLI, and add v2-remote-workspaces and v2-changes-pane screenshots to back the new sections. * docs(changelog): add CLI install snippet and docs link * docs(changelog): cut narrative lede, match standard changelog tone --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Kiet Ho <hoakiet98@gmail.com>
…-27) (#3792) * docs: generate weekly changelog 2026-04-27 * docs: reframe weekly changelog around v2 public beta Lead with v2 public beta + Settings → Experimental enable, restructure around the v1→v2 migration story, sidebar overhaul, cross-workspace terminals, and v2 chat. Pull in ~30 v2 PRs the bot missed and demote non-v2 items (Hosts page, marketing menu) to a brief "Also this week". * docs: pull in missed v2 features and bug fixes Add v2 project setup section (#3566, #3605, #3606, #3592, #3626, #3632), scheduled agent runs (#3576), Opus 4.7 (#3579), v1 review comments in pane (#3596), configurable v2 link-click (#3600), Copy Branch Name (#3635), safer terminal preset defaults (#3546), and /pricing page (#3639). Expand bug fixes with v2 git correctness, cross-fork PR misattribution, terminal paste/Unicode/Shift+Enter, and security bumps. * docs(changelog): add v2 public beta hero screenshot * docs(changelog): add Settings → Experimental screenshot, compress hero pngquant compression: v2-public-beta.png 704KB → 166KB (76%), v2-enable-flag.png 160KB → 36KB (78%). No visible quality loss. * docs(changelog): tighten v2 launch prose, condense bullet groups * docs(changelog): reframe cloud-first pillar as remote workspaces * docs(changelog): cut parallel-agents and honest-state pillars, fold into sub-sections * docs(changelog): tweak title and lead phrasing * docs(changelog): rewrite v2 launch lede around Twitter narrative Pull the launch story (physical limits, 3 ex-CTOs, cloud workspaces) into the lede, restructure pillars around Remote workspaces, Reimagined diff view, and Superset CLI, and add v2-remote-workspaces and v2-changes-pane screenshots to back the new sections. * docs(changelog): add CLI install snippet and docs link * docs(changelog): cut narrative lede, match standard changelog tone --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Kiet Ho <hoakiet98@gmail.com>
Summary
Ports v1's main-workspace concept to v2. Each
(projectId, hostId)gets asingleton workspace pointing at the host's configured
repoPath— createdautomatically when
project.setupsucceeds, backfilled by a startup sweepfor projects already set up before this shipped. No new UI; the workspace-
creation modal continues to write
kind: 'worktree'only.v2_workspaces.kindenum ('main' | 'worktree', defaults to'worktree') + partial unique index on(project_id, host_id)WHEREkind='main'. Migration0036_v2_workspaces_kind.sql.v2Workspace.createacceptskindand is idempotent formain(returns existing row instead of inserting a duplicate).
ensureMainWorkspace(ctx, projectId, repoPath)resolves thecurrent branch via
git symbolic-ref, ensures a host row, creates thecloud + local workspace rows. Skips on detached HEAD.
project.setup— wired in bothcloneandimportpaths, including theearly-return branches so re-running setup recovers a missing main.
projectsand ensures each hasa main. Runs in background; idempotent via the unique index.
worktree workspaces — no special-casing on host-offline or delete.
Design doc:
apps/desktop/plans/20260421-v2-main-workspace-creation.md.Test plan
project.setup(clone) creates one main row in v2_workspaces + a local workspaces row withworktreePath = repoPath.project.setup(import) does the same.project.setupon an already-configured project is a no-op (unique index protects).project.removecleans up the main row alongside worktree workspaces.Summary by cubic
Auto-creates a per-host v2 “main” workspace during project create/setup and worktree flows, and backfills existing projects on host startup. Main workspaces auto-appear on this device in the sidebar, can be removed from the sidebar, and are protected from delete in all paths; responses now return
mainWorkspaceIdto pin immediately.New Features
v2_workspaces.typeenum ('main' | 'worktree', default'worktree') with a partial unique index allowing one main per(project, host).v2Workspace.createacceptstypeand is race-safe for mains; updates branch/name if an existing main’s branch changed. Addsv2Workspace.getFromHost, blocks delete for mains, and addsv2Workspace.deleteMainForHostfor host project removal.ensureMainWorkspace(projectId, repoPath)resolves the branch, ensures the host, and creates cloud + local rows; used in project create/setup (including early returns) and worktree flows (adopt,checkout,create). A startup sweep backfills mains and skips detached HEAD. Project create/setup/relocate responses includemainWorkspaceId.isHidden. Sidebar ordering, accessible workspaces, and resource usage all respect this visibility. The delete action is disabled for mains in sidebar rows and cleanup flows.Migration
0038_v2_workspaces_main_type.sql(idempotent DDL). The startup sweep creates mains for existing projects. The workspace-creation modal continues to writetype: 'worktree'.Written for commit b9d9d00. Summary will update on new commits.
Summary by CodeRabbit
New Features