Repository navigation
Git integration, shared errors, and composer agent/branch UX - #52
Conversation
Threads can be created from git refs with isolated worktrees, and the web UI surfaces branch context, checkout, and diff viewing against the project repo. Co-authored-by: Cursor <cursoragent@cursor.com>
Centralize tagged errors with module-prefixed tags, derive ORPC codes on instances, map Drizzle/Zod failures in database adapters, and replace tryRepo wrappers with repo/repoArgs across repositories. Co-authored-by: Cursor <cursoragent@cursor.com>
Move branch and workspace controls into the composer, load agents at composer level with loading states, and rebind stale sessions on resume failure instead of spamming unhandled rejections. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (28)
📝 WalkthroughWalkthroughThis PR adds Git repository integration across schemas, persistence, CLI services, controller RPCs, React Query hooks, thread creation, branch/worktree controls, and a Git-backed diff panel. It also introduces shared typed error modules and repository error wrappers. ChangesGit integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shared/database/src/repositories/projects.ts (1)
45-57: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDB write before schema validation in
createProjectandwriteProjectName. Both functions persist to the database before runningProjectSchema.parse(), so if validation fails (e.g., emptyname), the invalid row is already committed andrepoArgsonly catches the error after the damage is done.
shared/database/src/repositories/projects.ts#L45-L57: MoveProjectSchema.parse()before theconnection.db.insert()call increateProject.shared/database/src/repositories/projects.ts#L60-L67: MoveProjectSchema.parse()before theconnection.db.update()call inwriteProjectName.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/database/src/repositories/projects.ts` around lines 45 - 57, In shared/database/src/repositories/projects.ts lines 45-57, update createProject to run ProjectSchema.parse() on the new project data before connection.db.insert(), and persist only the validated result. In shared/database/src/repositories/projects.ts lines 60-67, apply the same ordering in writeProjectName by validating the updated project before connection.db.update().Source: Learnings
🧹 Nitpick comments (9)
shared/errors/src/orpc.ts (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider whether
EmptyPayloadis needed in this module.
EmptyPayloadis exported but not referenced in any of the provided files. If it's used elsewhere in the@cyrus/errorspackage or by consumers, this is fine. If not, it adds unused surface area to a new module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/errors/src/orpc.ts` around lines 10 - 11, Verify whether EmptyPayload is referenced elsewhere in the `@cyrus/errors` package or by consumers; if no usages exist, remove the emptyPayloadBrand declaration and exported EmptyPayload type from this module. Retain the type only if it is part of an existing public API contract.apps/cli/src/handlers/controller/git.ts (1)
67-78: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winOrphaned worktree if DB update fails in
createGitWorktree.If
updateThreadWorktreePathfails at line 78, the worktree has already been created on disk (line 67–71) but the thread doesn't reference it. Consider cleaning up the worktree on DB update failure to avoid resource leaks.♻️ Proposed cleanup on failure
const updated = await updateThreadWorktreePath( input.threadId, worktree.value ); - if (updated.isErr()) throwOrpc(updated.error); + if (updated.isErr()) { + await Result.tryPromise(() => + removeGitWorktree(projectCwd.value, worktree.value) + ); + throwOrpc(updated.error); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/handlers/controller/git.ts` around lines 67 - 78, Update the flow around createGitWorktree and updateThreadWorktreePath so that when the database update fails, the newly created worktree is cleaned up before propagating the error. Preserve the existing successful update path and error propagation behavior.apps/cli/src/git/worktree.ts (1)
46-60: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on
Bun.spawnforgit worktree remove.If the subprocess hangs (e.g., locked files on Windows, git deadlock),
proc.exitednever resolves and the promise blocks indefinitely. Consider passing anAbortSignalwith a timeout toBun.spawnso the operation fails fast instead of hanging the caller.♻️ Suggested timeout via AbortController
return ( await Result.tryPromise(async () => { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10_000); const proc = Bun.spawn( ["git", "worktree", "remove", worktreePath, "--force"], { cwd: projectCwd, stdout: "pipe", stderr: "pipe", + signal: controller.signal, } ); const exitCode = await proc.exited; + clearTimeout(timeout); if (exitCode !== 0) { const stderr = await new Response(proc.stderr).text(); throw new Error(stderr.trim() || "Failed to remove worktree"); } }) ).mapError((error) => operationFailedError(error instanceof Error ? error.message : String(error)) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/git/worktree.ts` around lines 46 - 60, Update the git worktree removal flow in the Result.tryPromise callback to create a timed AbortController and pass its signal to Bun.spawn. Ensure the timeout aborts the subprocess so awaiting proc.exited fails promptly, while preserving the existing nonzero-exit stderr handling.apps/cli/src/git/status.ts (1)
49-52: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftAvoid recomputing the diff for every file.
es-git’sDeltaAPI doesn’t expose per-file insertion/deletion counts, so this loop still does O(n) diff work. Derive the per-file counts from the singlediffyou already have instead of callingdiffTreeToWorkdirWithIndexagain for each path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/git/status.ts` around lines 49 - 52, Update the file-status flow around the existing single diff computation to derive each file’s insertion and deletion counts from that diff, rather than calling repo.diffTreeToWorkdirWithIndex inside the per-file loop. Reuse the existing diff entries and preserve the current per-path status output.shared/schemas/src/rtc/git.ts (1)
52-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
.min(1)onthreadIdfor consistency with other identifier fields.
refNameinGitCheckoutInputSchema/GitCreateWorktreeInputSchemarequires non-empty strings, butThreadGitQueryInputSchema.threadId(the base for most Git input schemas) doesn't. Tightening this would reject malformed empty-string thread IDs at the schema boundary rather than downstream.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/schemas/src/rtc/git.ts` around lines 52 - 54, Update ThreadGitQueryInputSchema.threadId to require a non-empty string using the same minimum-length validation applied to refName in GitCheckoutInputSchema and GitCreateWorktreeInputSchema; leave the surrounding schema structure unchanged.apps/web/src/components/chat/main/thread-header.tsx (1)
43-53: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
aria-pressedto the Diffs toggle button.The button visually toggles based on
diffOpenbut doesn't expose this state to assistive technology.♿ Proposed fix
<button + aria-pressed={diffOpen} className={ diffOpen ? "inline-flex h-7 items-center gap-1 rounded-md bg-primary px-2 font-medium text-primary-foreground text-xs" : "inline-flex h-7 items-center gap-1 rounded-md bg-muted/70 px-2 font-medium text-foreground text-xs hover:bg-muted" } onClick={toggleDiffOpen} type="button" >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/chat/main/thread-header.tsx` around lines 43 - 53, Update the Diffs toggle button in the thread header, identified by its onClick handler toggleDiffOpen, to include an aria-pressed attribute bound to the diffOpen state so assistive technology receives the current toggle status.shared/database/src/repositories/conversations.ts (1)
20-45: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winInsert + parent update aren't wrapped in a transaction.
If the
threads.updatedAtupdate fails after the conversation row insert succeeds, the two writes drift out of sync. This behavior predates this diff (only the wrapping utility changed here), but since the block is being touched, consider wrapping both writes inconnection.db.transaction(...)for atomicity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/database/src/repositories/conversations.ts` around lines 20 - 45, Wrap the insert into conversations and the parent threads.updatedAt update within a single connection.db.transaction in appendConversation, preserving the existing values, failure handling, and parsed return behavior while ensuring both writes commit or roll back together.shared/database/src/utils/repo.test.ts (1)
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor:
z.string().uuid()is deprecated in Zod v4.Functionally fine (still supported), but
z.uuid()is the current top-level form; consider updating for consistency with newer Zod idioms.Also applies to: 59-59, 103-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/database/src/utils/repo.test.ts` at line 35, Update the Zod UUID schema usages in the test cases around the `cause` declarations, including the occurrences at the referenced lines, replacing the deprecated `z.string().uuid()` form with the current top-level `z.uuid()` form while preserving the existing validation behavior.shared/hooks/src/connection/use-git.ts (1)
10-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated conditional query pattern across 5 hooks;
invalidateThreadGitQueriesduplicatesinvalidateGitQueries.
useGitStatus,useGitPatch,useProjectGitStatus,useProjectGitRefs, anduseListGitRefsall repeat the same "id present → real query, else skipToken" ternary shape. Also,invalidateThreadGitQueries(154-159) is just a same-signature pass-through ofinvalidateGitQueries. Could be consolidated, though not urgent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/hooks/src/connection/use-git.ts` around lines 10 - 92, Consolidate the repeated enabled-identifier/skipToken query setup used by useGitStatus, useGitPatch, useProjectGitStatus, useProjectGitRefs, and useListGitRefs into a shared helper or consistent abstraction, while preserving each hook’s query keys and inputs. Remove the redundant invalidateThreadGitQueries pass-through and use invalidateGitQueries directly at its call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cli/src/core/threads/coordinator.ts`:
- Around line 135-138: Update the same-agent branch in the coordinator’s
session-close flow to handle the result of withRuntime instead of ignoring it.
Return or propagate a close error consistently with the other branch so
new-session creation does not proceed after a failed close, unless this cleanup
is explicitly intended to be best effort and documented accordingly.
In `@apps/cli/src/git/patch.ts`:
- Line 7: Update the diff retrieval flow around the repository-open check and
diff.print() failure handling so git operation errors are propagated or surfaced
through the established error-reporting mechanism instead of returning an
indistinguishable empty string. Preserve the empty-string result only for a
genuine no-changes diff, using the surrounding function’s existing error
contract.
- Line 9: Wrap the complete git operation in getGitPatch, including
opened.value.head().peelToTree() and diff.print(), inside the existing
Result.try so errors from unborn HEAD repositories are captured and returned
rather than becoming unhandled rejections.
In `@apps/cli/src/git/status.ts`:
- Around line 77-86: Update the `status.match` error branch to preserve and
surface the failure instead of silently returning a clean-repository result: log
the caught error there and add the requested optional error indicator to
`GitStatusOutput`, populating it from the failure while retaining the existing
status fields for compatibility.
In `@apps/cli/src/handlers/controller/git.ts`:
- Around line 83-86: Update the removeGitWorktree handler to explicitly detect a
missing thread after getThread succeeds and throw the established notFound error
via throwOrpc, matching createGitWorktree. Keep the existing worktreePath
absence behavior unchanged for threads that do exist.
- Around line 39-42: Restrict GitCreateWorktreeInputSchema and the
createGitWorktree flow to repository-relative paths: reject absolute paths and
traversal that escapes the thread repository before passing the value to mkdir
or git worktree add. Leave getGitPatch unchanged because its path is only a git
pathspec.
In `@apps/cli/src/handlers/controller/threads.ts`:
- Around line 28-42: Update createThread to inspect the Result returned by
tryCheckoutGitRef and propagate its error instead of returning success when
checkout fails. Update deleteThread to inspect the Result from removeGitWorktree
and log cleanup failures using the existing logging mechanism, while preserving
successful deletion behavior.
In `@apps/web/src/components/chat/composer/agent-model-picker.tsx`:
- Line 75: Update the trigger label expression in the agent model picker to
retain a minimal fallback when both activeModel?.name and activeAgent?.name are
unavailable. Preserve the existing preference order and ensure the button
renders a non-empty label such as the prior “Agent” fallback.
In `@shared/hooks/src/connection/use-agent-catalog.ts`:
- Around line 198-208: Reset the per-thread resume-bind request flag in the bind
mutation completion paths so failures and completed binds can be retried when
appropriate. Update the mutation error/success handling associated with
bindAgentMutation and use the existing resumeBindRequestedByThread state
helpers, while preserving the current guard and markResumeBindRequested flow.
---
Outside diff comments:
In `@shared/database/src/repositories/projects.ts`:
- Around line 45-57: In shared/database/src/repositories/projects.ts lines
45-57, update createProject to run ProjectSchema.parse() on the new project data
before connection.db.insert(), and persist only the validated result. In
shared/database/src/repositories/projects.ts lines 60-67, apply the same
ordering in writeProjectName by validating the updated project before
connection.db.update().
---
Nitpick comments:
In `@apps/cli/src/git/status.ts`:
- Around line 49-52: Update the file-status flow around the existing single diff
computation to derive each file’s insertion and deletion counts from that diff,
rather than calling repo.diffTreeToWorkdirWithIndex inside the per-file loop.
Reuse the existing diff entries and preserve the current per-path status output.
In `@apps/cli/src/git/worktree.ts`:
- Around line 46-60: Update the git worktree removal flow in the
Result.tryPromise callback to create a timed AbortController and pass its signal
to Bun.spawn. Ensure the timeout aborts the subprocess so awaiting proc.exited
fails promptly, while preserving the existing nonzero-exit stderr handling.
In `@apps/cli/src/handlers/controller/git.ts`:
- Around line 67-78: Update the flow around createGitWorktree and
updateThreadWorktreePath so that when the database update fails, the newly
created worktree is cleaned up before propagating the error. Preserve the
existing successful update path and error propagation behavior.
In `@apps/web/src/components/chat/main/thread-header.tsx`:
- Around line 43-53: Update the Diffs toggle button in the thread header,
identified by its onClick handler toggleDiffOpen, to include an aria-pressed
attribute bound to the diffOpen state so assistive technology receives the
current toggle status.
In `@shared/database/src/repositories/conversations.ts`:
- Around line 20-45: Wrap the insert into conversations and the parent
threads.updatedAt update within a single connection.db.transaction in
appendConversation, preserving the existing values, failure handling, and parsed
return behavior while ensuring both writes commit or roll back together.
In `@shared/database/src/utils/repo.test.ts`:
- Line 35: Update the Zod UUID schema usages in the test cases around the
`cause` declarations, including the occurrences at the referenced lines,
replacing the deprecated `z.string().uuid()` form with the current top-level
`z.uuid()` form while preserving the existing validation behavior.
In `@shared/errors/src/orpc.ts`:
- Around line 10-11: Verify whether EmptyPayload is referenced elsewhere in the
`@cyrus/errors` package or by consumers; if no usages exist, remove the
emptyPayloadBrand declaration and exported EmptyPayload type from this module.
Retain the type only if it is part of an existing public API contract.
In `@shared/hooks/src/connection/use-git.ts`:
- Around line 10-92: Consolidate the repeated enabled-identifier/skipToken query
setup used by useGitStatus, useGitPatch, useProjectGitStatus, useProjectGitRefs,
and useListGitRefs into a shared helper or consistent abstraction, while
preserving each hook’s query keys and inputs. Remove the redundant
invalidateThreadGitQueries pass-through and use invalidateGitQueries directly at
its call sites.
In `@shared/schemas/src/rtc/git.ts`:
- Around line 52-54: Update ThreadGitQueryInputSchema.threadId to require a
non-empty string using the same minimum-length validation applied to refName in
GitCheckoutInputSchema and GitCreateWorktreeInputSchema; leave the surrounding
schema structure unchanged.
🪄 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: 1c9332b1-284b-4124-841d-00cd472fdb92
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (78)
apps/cli/__tests__/integration/draft-session-lifecycle.test.tsapps/cli/package.jsonapps/cli/src/core/acp/pool.tsapps/cli/src/core/agents/runtime.tsapps/cli/src/core/threads/coordinator.tsapps/cli/src/errors/coordinator.tsapps/cli/src/git/checkout.tsapps/cli/src/git/diff-options.tsapps/cli/src/git/git.test.tsapps/cli/src/git/init.tsapps/cli/src/git/open.tsapps/cli/src/git/patch.tsapps/cli/src/git/paths.tsapps/cli/src/git/refs.tsapps/cli/src/git/status.tsapps/cli/src/git/worktree.tsapps/cli/src/handlers/controller/agents.tsapps/cli/src/handlers/controller/catalog/effort.tsapps/cli/src/handlers/controller/catalog/mode.tsapps/cli/src/handlers/controller/catalog/model.tsapps/cli/src/handlers/controller/catalog/persona.tsapps/cli/src/handlers/controller/chat.tsapps/cli/src/handlers/controller/git.tsapps/cli/src/handlers/controller/index.tsapps/cli/src/handlers/controller/projects.tsapps/cli/src/handlers/controller/threads.tsapps/cli/src/utils/error.tsapps/web/src/components/chat/composer/agent-model-picker.tsxapps/web/src/components/chat/composer/compact-composer-controls.tsxapps/web/src/components/chat/composer/composer-branch-toolbar.tsxapps/web/src/components/chat/composer/composer-skeleton.tsxapps/web/src/components/chat/composer/composer-unavailable.tsxapps/web/src/components/chat/composer/footer-controls.tsxapps/web/src/components/chat/composer/index.tsxapps/web/src/components/chat/diff/diff-panel.tsxapps/web/src/components/chat/main/thread-header.tsxapps/web/src/components/chat/main/thread-workspace.tsxapps/web/src/components/settings/settings-section-panel.tsxapps/web/src/components/sidebar/projects/project-thread-explorer.tsxapps/web/src/devtools.tsxapps/web/src/hooks/auth/use-error-listener.tsapps/web/src/index.cssopenspec/changes/git-integration/.openspec.yamlopenspec/changes/git-integration/design.mdopenspec/changes/git-integration/proposal.mdopenspec/changes/git-integration/specs/git-diff-panel/spec.mdopenspec/changes/git-integration/specs/git-thread-context/spec.mdopenspec/changes/git-integration/specs/git-worker-service/spec.mdopenspec/changes/git-integration/specs/wire-schemas/spec.mdopenspec/changes/git-integration/tasks.mdshared/connections/src/contracts/controller.tsshared/constants/src/operation-keys.tsshared/database/package.jsonshared/database/src/models/threads.tsshared/database/src/repositories/conversations.tsshared/database/src/repositories/git.tsshared/database/src/repositories/projects.tsshared/database/src/repositories/threads.tsshared/database/src/utils/error.tsshared/database/src/utils/repo.test.tsshared/database/src/utils/repo.tsshared/errors/package.jsonshared/errors/src/common.tsshared/errors/src/coordinator.tsshared/errors/src/git.tsshared/errors/src/orpc.tsshared/errors/src/repository.tsshared/errors/tsconfig.jsonshared/hooks/src/connection/use-agent-catalog.tsshared/hooks/src/connection/use-controller-threads.tsshared/hooks/src/connection/use-git.tsshared/hooks/src/connection/use-list-agents.tsshared/hooks/src/connection/use-threads.tsshared/hooks/src/stores/agent-catalog.tsshared/schemas/src/rtc/common.tsshared/schemas/src/rtc/git.tsshared/schemas/src/rtc/threads.test.tsshared/schemas/src/rtc/threads.ts
💤 Files with no reviewable changes (3)
- apps/cli/src/utils/error.ts
- apps/cli/src/errors/coordinator.ts
- shared/database/src/utils/error.ts
Move the completed change to openspec/changes/archive and add git-diff-panel, git-thread-context, and git-worker-service main specs. Co-authored-by: Cursor <cursoragent@cursor.com>
Harden git/worker flows with typed errors and Result returns, use es-git for worktree removal, and fix session bind, schema validation, and composer edge cases raised in review. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
@cyrus/errorswith repository HOF wrappers and typed error adapters for oRPC, git, and coordinator flows.Test plan
bun checkbun check:typesbun test:unitbun test:integrationMade with Cursor
Summary by CodeRabbit