Git integration: branch picker + worktrees#3
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:
WalkthroughAdds end-to-end Git integration: new contracts and WS API, server & desktop git utilities, renderer IPC and React Query wiring, per-thread branch/worktree state and persistence, BranchToolbar UI, ChatView worktree creation on first message, tests, and build/tooling/script updates. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant ChatView as ChatView (UI)
participant BranchToolbar as BranchToolbar (UI)
participant Store as Renderer Store
participant NativeAPI as Native API (preload/WS)
participant WebSocket as WebSocket Transport
participant Server as Server (git module)
User->>ChatView: open chat / branch controls
ChatView->>BranchToolbar: render (envMode, envLocked)
BranchToolbar->>NativeAPI: git.listBranches({ cwd })
NativeAPI->>WebSocket: invoke "git:list-branches"
WebSocket->>Server: WS_METHODS.gitListBranches
Server-->>WebSocket: return branches
WebSocket-->>NativeAPI: branches
NativeAPI-->>BranchToolbar: branches list
User->>ChatView: send first message
alt envMode == "worktree" && no worktreePath
ChatView->>NativeAPI: git.createWorktree({ branch, cwd })
NativeAPI->>WebSocket: invoke "git:create-worktree"
WebSocket->>Server: WS_METHODS.gitCreateWorktree
Server->>Server: runTerminalCommand("git worktree add ...")
Server-->>WebSocket: { worktreePath, branch }
WebSocket-->>NativeAPI: result
NativeAPI-->>ChatView: worktreePath
ChatView->>Store: dispatch SET_THREAD_BRANCH(threadId, branch, worktreePath)
else
ChatView->>Store: dispatch SET_THREAD_BRANCH(threadId, branch, null)
end
Store->>Store: update thread.branch & thread.worktreePath
ChatView->>NativeAPI: ensure/start session(cwd = worktreePath ?? project.cwd)
NativeAPI->>WebSocket: start session
WebSocket->>Server: establish session in selected cwd
Server-->>WebSocket: session ready
WebSocket-->>NativeAPI: session active
NativeAPI-->>ChatView: session active
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Add branch picker and worktree flow to ChatView and expose
|
apps/server/src/git.ts
Outdated
| .filter((line) => line.length > 0) | ||
| .map((line) => ({ | ||
| name: line.replace(/^[*+]\s+/, ""), | ||
| current: line.startsWith("* "), |
There was a problem hiding this comment.
🟡 Medium
src/git.ts:45 The child.killed flag becomes true immediately after child.kill("SIGTERM") is called, so the SIGKILL fallback is unreachable. Consider tracking process exit separately (e.g., with a flag set in the close event) instead of relying on child.killed.
🚀 Want me to fix this? Reply ex: "fix it for me".
adf6a26 to
bc70cc4
Compare
| }, input.timeoutMs ?? 30_000); | ||
|
|
||
| child.stdout?.on("data", (chunk: Buffer) => { | ||
| stdout += chunk.toString(); |
There was a problem hiding this comment.
🟢 Low
src/git.ts:55 Multi-byte UTF-8 characters split across chunks will corrupt to �. Consider using StringDecoder from node:string_decoder to properly handle partial characters.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/server/src/git.ts around line 55:
Multi-byte UTF-8 characters split across chunks will corrupt to `�`. Consider using `StringDecoder` from `node:string_decoder` to properly handle partial characters.
Allows users to right-click "+ New thread" to create threads from specific git branches, with optional worktree creation for isolated work. Includes branch sorting (current/default first), graceful handling of non-git repos, and ability to initialize git repos. - New git IPC contract (listBranches, createWorktree, removeWorktree, init) - Branch context menu with worktree sub-option in Sidebar - Thread model extended with branch/worktreePath fields - Session cwd uses worktree path when available - Branch sorting: current > main/master > alphabetical Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Move branch picking from sidebar context menu into ChatView input toolbar. Add env mode toggle (local/worktree) that locks after first message, branch dropdown with create-new-branch inline form, and wire up git:create-branch IPC channel. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wire up git:checkout IPC channel across the full stack so branch selection actually checks out the branch. Clear error banners when a subsequent checkout or branch creation succeeds. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add non-null assertion for activeProject in worktree creation path (guarded by earlier check) and add missing branch/worktreePath to test fixture. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace hardcoded dark-mode colors with theme-aware semantic classes (bg-popover, text-foreground, bg-accent, etc.) so the branch picker works in both light and dark mode. Detect the real default branch via git symbolic-ref and show a "default" label (with "current" taking precedence when both apply). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move git operations (listGitBranches, checkoutGitBranch, createGitWorktree, etc.) from main.ts into git.ts for testability. Add 20 integration tests that run real git commands against temporary repos covering checkout, branch creation, worktrees, conflicts, and thread-switching flows. Also fix: branch checkout now runs regardless of envMode, worktree creation passes cwd to ensureSession to avoid stale closure bug, and current branch auto-fills on new threads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Both openInEditor and the Cmd+O shortcut now use the thread's worktreePath when available instead of always using activeProject.cwd. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use worktree path instead of project cwd for git branch queries so worktree threads correctly report their branch as current. Also update error banner to use semantic color tokens for light mode support. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verifies that querying branches from a worktree directory correctly reports the worktree's branch as current, while the main repo still shows its own branch — the assumption the BranchToolbar fix relies on. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
6465e69 to
9fc5ad2
Compare
…ate build configs
- Move BranchToolbar below the chat input box to match Codex app layout
- Change worktree path to ~/.t3code/worktrees/{repo}/{branch}
- Split desktop tsdown config into separate main/preload builds to prevent
code-splitting from leaking node:os into the sandboxed preload script
- Migrate turbo scripts to turbo watch, add desktop start/smoke-test tasks
- Update server output to .mjs, add CJS format, remove tsx dependency
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Switch from turbo watch to turbo run --parallel for dev scripts - Simplify contracts exports to use ./src/index.ts for types directly - Add --watch --clean to contracts dev script - Root dev task depends on contracts#build, is persistent - Revert typecheck to depend on ^typecheck (contracts types from source) - Remove redundant server dev task override - Desktop dev depends only on t3#build (contracts implied) - Uncomment server dist wait in dev-electron.mjs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tegration # Conflicts: # apps/web/src/store.ts # apps/web/src/wsNativeApi.ts # packages/contracts/src/ipc.ts
|
@greptileai review |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/scripts/smoke-test.mjs (1)
12-56:⚠️ Potential issue | 🟠 MajorHandle spawn errors and non‑zero exits to avoid false positives.
Uncaughterrorevents (e.g., missing Electron binary) and non‑zero exit codes can slip through, leading to flaky or misleading smoke-test results.🛠️ Suggested fix
const child = spawn(electronBin, [mainJs], { stdio: ["pipe", "pipe", "pipe"], env: { ...process.env, VITE_DEV_SERVER_URL: "", ELECTRON_ENABLE_LOGGING: "1", }, }); +child.on("error", (err) => { + clearTimeout(timeout); + console.error("\nDesktop smoke test failed (spawn error):"); + console.error(err); + process.exit(1); +}); + let output = ""; child.stdout.on("data", (chunk) => { output += chunk.toString(); }); child.stderr.on("data", (chunk) => { output += chunk.toString(); }); const timeout = setTimeout(() => { child.kill(); }, 8_000); -child.on("exit", () => { +child.on("exit", (code, signal) => { clearTimeout(timeout); const fatalPatterns = [ "Cannot find module", "MODULE_NOT_FOUND", "Refused to execute", "Uncaught Error", "Uncaught TypeError", "Uncaught ReferenceError", ]; const failures = fatalPatterns.filter((pattern) => output.includes(pattern)); - if (failures.length > 0) { + if (failures.length > 0 || code !== 0 || signal) { console.error("\nDesktop smoke test failed:"); + if (code !== 0) console.error(` - exit code: ${code}`); + if (signal) console.error(` - signal: ${signal}`); for (const failure of failures) { console.error(` - ${failure}`); } console.error("\nFull output:\n" + output); process.exit(1); }
🤖 Fix all issues with AI agents
In @.plans/git-flows-integration-tests.md:
- Around line 93-99: The documentation uses "npx vitest run
apps/desktop/src/git.test.ts" which contradicts the Bun-centric tooling
elsewhere; update the verification snippet to use Bun's runner by replacing the
"npx" invocation with "bunx" (so the second command uses bunx vitest run
apps/desktop/src/git.test.ts) and keep the existing "cd apps/desktop && bun run
test" line unchanged; ensure both examples consistently recommend Bun tooling.
In `@apps/desktop/package.json`:
- Around line 22-24: Update the package.json dev dependency for "vitest" from
"^4.0.0" to "^4.0.18" to pick up recent patch fixes, and optionally add a
comment or package.json note near "tsdown" to clarify it's already at the latest
stable (0.20.3) if you want to signal stability; also review the test script
that passes "--passWithNoTests" (in package.json scripts) and decide whether to
remove or tighten that flag after updating vitest to encourage real test
coverage.
In `@apps/server/src/git.ts`:
- Around line 122-183: The git commands in createGitWorktree, removeGitWorktree,
createGitBranch, and checkoutGitBranch are built as single shell strings with
single quotes (e.g., using escapeSingleQuotes) which breaks on Windows cmd.exe;
change each to spawn git directly with an args array instead of a shell command
string (e.g., call runTerminalCommand or child_process.spawn with ['git',
'worktree', 'add', '-b', input.newBranch, worktreePath, input.branch'] and
shell: false), remove the single-quote escaping (no escapeSingleQuotes
wrapping), keep cwd and timeoutMs, and ensure failures still throw the same
errors based on result.code/result.stderr.
In `@apps/web/package.json`:
- Around line 28-32: The package.json currently pins "vite" to a beta
pre-release ("^8.0.0-beta.12"); change the "vite" dependency entry to a stable
7.x release (for example "^7.21.0" or the latest 7.x), update the lockfile by
running your package manager (npm/yarn/pnpm install), and run the dev/build/test
commands to verify nothing breaks; edit the "vite" key in package.json to
replace the beta version, commit the lockfile changes, and ensure CI passes
(reference: the "vite" dependency entry in package.json).
In `@apps/web/src/components/BranchToolbar.tsx`:
- Around line 61-73: The auto-sync useEffect in BranchToolbar (the effect that
checks branches.find and dispatches SET_THREAD_BRANCH) is overwriting a
user-selected worktree base branch; change the guard so the effect only runs for
true local threads (e.g., when activeThread exists AND activeThread.worktreePath
is falsy AND the thread is in local mode) or only when activeThread.branch is
null/undefined, then proceed to find the current branch and dispatch
SET_THREAD_BRANCH; update the useEffect condition that currently checks
(!activeThread || activeThread.worktreePath) to include the extra guard (or
replace it with a check for activeThread.branch == null) so selecting a
different base branch for a pending worktree is not clobbered.
🧹 Nitpick comments (3)
.plans/git-flows-test-plan.md (2)
28-32: Consider adding guidance on mock cleanup between tests.The factory pattern is solid, but the plan should mention resetting mocks between tests to ensure isolation. Consider adding a note about using
beforeEach(() => vi.clearAllMocks())or similar to reset the mocked Git API functions between test cases.📋 Suggested addition to the Factories section
Add after line 32:
**Setup/Teardown** Use `beforeEach(() => vi.clearAllMocks())` to reset mocked functions between tests, ensuring each test starts with clean mock state.Based on learnings: For integration tests: Prefer deterministic inputs and explicit state checks; avoid relying on logs or timing assumptions. Clean up external resources in
finallyblocks.
94-102: Clarify working directory assumptions for verification commands.The verification commands mix two approaches: line 96 changes directory before running tests, while lines 99-102 use full paths from the repo root. This inconsistency could confuse developers.
🔧 Suggested clarification
Consider adding a comment about the working directory:
## Verification +All commands assume you're at the repository root unless otherwise noted. + ```bash # Run all renderer tests cd apps/renderer && bun run test # Run just the new test file npx vitest run apps/renderer/src/git-flows.test.ts # Run just the store tests npx vitest run apps/renderer/src/store.test.tsAlternatively, make all commands consistent: ```bash # Run all renderer tests npx vitest run apps/renderer # Run just the new test file npx vitest run apps/renderer/src/git-flows.test.ts # Run just the store tests npx vitest run apps/renderer/src/store.test.tsapps/web/src/components/BranchToolbar.tsx (1)
169-309: Consider splitting the toolbar UI into smaller subcomponents.
The render block is large and mixes toggle, menu, and creation form logic; extractingEnvModeToggleandBranchMenusubcomponents (kept in the same file) would reduce complexity and keep logic closer to the JSX it serves.Based on learnings: Applies to **/*.tsx : Extract large React components into multiple subcomponents with granular functionality. Co-locate subcomponents in the same file as the main component. Avoid hoisting callbacks too high up the component tree; prefer colocating logic close to JSX.
| ```bash | ||
| # Run the git integration tests | ||
| cd apps/desktop && bun run test | ||
|
|
||
| # Or just the git test file | ||
| npx vitest run apps/desktop/src/git.test.ts | ||
| ``` |
There was a problem hiding this comment.
Use bunx instead of npx in the verification snippet.
This keeps docs consistent with the Bun-only tooling standard.
✏️ Proposed doc fix
-# Or just the git test file
-npx vitest run apps/desktop/src/git.test.ts
+# Or just the git test file
+bunx vitest run apps/desktop/src/git.test.tsBased on learnings: Use Bun as the package manager. Use bunx <BIN> when executing scripts from npm packages not installed as a project dependency.
🤖 Prompt for AI Agents
In @.plans/git-flows-integration-tests.md around lines 93 - 99, The
documentation uses "npx vitest run apps/desktop/src/git.test.ts" which
contradicts the Bun-centric tooling elsewhere; update the verification snippet
to use Bun's runner by replacing the "npx" invocation with "bunx" (so the second
command uses bunx vitest run apps/desktop/src/git.test.ts) and keep the existing
"cd apps/desktop && bun run test" line unchanged; ensure both examples
consistently recommend Bun tooling.
| "tsdown": "^0.20.3", | ||
| "typescript": "^5.7.3", | ||
| "vitest": "^3.0.0", | ||
| "vitest": "^4.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest versions from npm registry
echo "=== tsdown latest version ==="
npm view tsdown@latest version 2>/dev/null || echo "Failed to fetch"
echo ""
echo "=== vitest latest version ==="
npm view vitest@latest version 2>/dev/null || echo "Failed to fetch"
echo ""
echo "=== tsdown 0.20.3 details ==="
npm view tsdown@0.20.3 2>/dev/null | head -30 || echo "Failed to fetch"
echo ""
echo "=== vitest 4.0.0 details ==="
npm view vitest@4.0.0 2>/dev/null | head -30 || echo "Failed to fetch"Repository: pingdotgg/codething-mvp
Length of output: 1591
🏁 Script executed:
#!/bin/bash
# Check the full package.json context in apps/desktop
echo "=== apps/desktop/package.json content ==="
if [ -f "apps/desktop/package.json" ]; then
cat apps/desktop/package.json
else
echo "File not found"
fiRepository: pingdotgg/codething-mvp
Length of output: 821
🏁 Script executed:
#!/bin/bash
# Search for vitest and tsdown configuration in the repository
echo "=== vitest configuration files ==="
fd -e "vitest.config" -e "vitest.config.ts" -e "vitest.config.js" 2>/dev/null
echo ""
echo "=== tsdown usage in the codebase ==="
rg "tsdown" --type json --type ts --type tsx -l 2>/dev/null | head -20
echo ""
echo "=== vitest usage pattern ==="
rg "vitest" --type json --type ts --type tsx -l 2>/dev/null | head -20Repository: pingdotgg/codething-mvp
Length of output: 169
Consider updating vitest to 4.0.18 and clarify tsdown stability.
tsdown@0.20.3 is already the latest stable release and does not represent a major update. However, vitest@4.0.0 is outdated—the current stable patch is 4.0.18. Both versions are compatible with TypeScript 5.7.3, but updating vitest to the latest patch is recommended for bug fixes and stability improvements. Note that the test script currently uses --passWithNoTests, suggesting minimal active test coverage; consider whether vitest configuration and test suite expansion are needed for this Electron desktop application.
🤖 Prompt for AI Agents
In `@apps/desktop/package.json` around lines 22 - 24, Update the package.json dev
dependency for "vitest" from "^4.0.0" to "^4.0.18" to pick up recent patch
fixes, and optionally add a comment or package.json note near "tsdown" to
clarify it's already at the latest stable (0.20.3) if you want to signal
stability; also review the test script that passes "--passWithNoTests" (in
package.json scripts) and decide whether to remove or tighten that flag after
updating vitest to encourage real test coverage.
| "@vitejs/plugin-react": "^5.1.4", | ||
| "babel-plugin-react-compiler": "^19.0.0-beta-e552027-20250112", | ||
| "tailwindcss": "^4.0.0", | ||
| "typescript": "^5.7.3", | ||
| "vite": "^6.0.5" | ||
| "vite": "^8.0.0-beta.12" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Vite 8 stable release date 2026
💡 Result:
As of February 11, 2026, Vite 8 does not have a stable (non-beta) release date announced or published yet—the project is still shipping 8.0.0 beta pre-releases. The Vite 8 beta was announced on December 3, 2025 and GitHub shows ongoing v8.0.0-beta.x releases (e.g., v8.0.0-beta.13 on February 5, 2026). [1][2]
Sources:
[1] https://vite.dev/blog/announcing-vite8-beta
[2] https://github.com/vitejs/vite/releases
Replace Vite 8 beta with a stable release version.
Vite 8 remains in beta as of February 2026 with no announced stable release date. Using pre-release versions in production code introduces significant risk of breaking changes and instability. Update to Vite 7.x or wait for Vite 8 stable release before merging.
🤖 Prompt for AI Agents
In `@apps/web/package.json` around lines 28 - 32, The package.json currently pins
"vite" to a beta pre-release ("^8.0.0-beta.12"); change the "vite" dependency
entry to a stable 7.x release (for example "^7.21.0" or the latest 7.x), update
the lockfile by running your package manager (npm/yarn/pnpm install), and run
the dev/build/test commands to verify nothing breaks; edit the "vite" key in
package.json to replace the beta version, commit the lockfile changes, and
ensure CI passes (reference: the "vite" dependency entry in package.json).
Greptile OverviewGreptile SummaryThis PR adds git branch/worktree integration across the web UI and websocket server: threads can store Key issues to address before merge:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User
participant Web as Web app (ChatView/BranchToolbar)
participant WS as WS transport
participant Server as Server (wsServer)
participant Git as git CLI
User->>Web: Select envMode + branch
Web->>WS: git.listBranches(cwd)
WS->>Server: WS_METHODS.gitListBranches
Server->>Git: git branch --no-color
Git-->>Server: branches
Server-->>WS: {isRepo, branches}
WS-->>Web: branches
User->>Web: Send first message (worktree mode)
Web->>WS: git.createWorktree({cwd, branch, newBranch})
WS->>Server: WS_METHODS.gitCreateWorktree
Server->>Git: git worktree add -b newBranch path branch
Git-->>Server: worktree created
Server-->>WS: {worktree:{path, branch}}
WS-->>Web: worktree info
Web->>WS: providers.startSession({cwd: worktreePath})
WS->>Server: WS_METHODS.providersStartSession
Server-->>WS: session
WS-->>Web: session
User->>Web: Right-click thread -> Delete
Web->>WS: providers.stopSession(sessionId)
WS->>Server: WS_METHODS.providersStopSession
Server-->>WS: ok
WS-->>Web: ok
Note over Web: Current code deletes thread
Note over Web: Missing: git.removeWorktree({cwd, path})
|
| export default defineConfig({ | ||
| resolve: { | ||
| alias: [ | ||
| { | ||
| find: /^@t3tools\/contracts$/, | ||
| replacement: path.resolve(import.meta.dirname, "./packages/contracts/src/index.ts"), | ||
| }, | ||
| ], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Vitest run broken
This config aliases @t3tools/contracts to the TypeScript source entry (packages/contracts/src/index.ts). With the PR’s build changes, tests now fail at runtime with TypeError: undefined is not an object (evaluating '__vite_ssr_import_0__.z.object'), which indicates the Zod import in those aliased sources isn’t being resolved correctly under Vitest’s SSR transform.
Since bun run test is part of the repo workflow, this needs to be fixed before merge (either by adjusting the alias/optimizeDeps/ssr settings or changing how contracts are imported in tests).
Additional Comments (2)
Consider switching these git helpers to
This only affects threads that have |
- ChatView: change useEffect dependency from [activeThread] (new object ref every dispatch) to [activeThread?.id, activeThread?.worktreePath] so envMode is no longer silently reset to "local" on unrelated store dispatches, making the worktree code path reachable. - git.ts: replace shell command strings (runTerminalCommand + single-quote escaping) with a runGit helper that spawns git directly via argv array. Eliminates Windows quoting issues and the command injection surface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| } | ||
|
|
||
| export async function createGitBranch(input: GitCreateBranchInput): Promise<void> { | ||
| const result = await runGit(["branch", input.branch], input.cwd, 10_000); |
There was a problem hiding this comment.
🟢 Low
src/git.ts:191 Suggestion: user-supplied branch names can be misparsed as options or pathspecs. Add -- before branch arguments across git calls (e.g., branch, checkout), or prefer git switch for switching branches to avoid ambiguity.
| const result = await runGit(["branch", input.branch], input.cwd, 10_000); | |
| const result = await runGit(["branch", "--", input.branch], input.cwd, 10_000); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/server/src/git.ts around line 191:
Suggestion: user-supplied branch names can be misparsed as options or pathspecs. Add `--` before branch arguments across `git` calls (e.g., `branch`, `checkout`), or prefer `git switch` for switching branches to avoid ambiguity.
- Add @tanstack/react-query and wire up QueryClientProvider in App.tsx - Refactor BranchToolbar to use useQuery for branch listing and useMutation for checkout, create branch, and git init, replacing manual useState/useCallback/useEffect async patterns - Fix branch sync effect and ChatView envMode effect to use stable primitive deps instead of full object references, preventing unnecessary re-runs on unrelated store dispatches Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| onError: (error) => | ||
| setThreadError(error instanceof Error ? error.message : "Failed to create branch."), |
There was a problem hiding this comment.
🟢 Low
components/BranchToolbar.tsx:57 The chained createBranch().then(checkout) in createBranchMutation can leave git state out of sync with UI if checkout fails after branch creation succeeds. Consider separating these into distinct operations or refreshing branch state in onError to stay in sync.
- onError: (error) =>
- setThreadError(error instanceof Error ? error.message : "Failed to create branch."),
+ onError: (error) => {
+ queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd] });
+ setThreadError(error instanceof Error ? error.message : "Failed to create branch.");
+ },🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/web/src/components/BranchToolbar.tsx around lines 57-58:
The chained `createBranch().then(checkout)` in `createBranchMutation` can leave git state out of sync with UI if checkout fails after branch creation succeeds. Consider separating these into distinct operations or refreshing branch state in `onError` to stay in sync.
| useEffect(() => { | ||
| if (!activeThreadId || activeWorktreePath) return; |
There was a problem hiding this comment.
🟡 Medium
components/BranchToolbar.tsx:75 The useEffect at line 75 reverts the thread branch to match Git HEAD when activeWorktreePath is null. However, the new worktree mode (lines 118-123) also has null activeWorktreePath but intentionally sets a different branch without checkout. Consider adding envMode !== "worktree" to the effect's early return to prevent it from overwriting the user's base branch selection.
- useEffect(() => {
- if (!activeThreadId || activeWorktreePath) return;
+ useEffect(() => {
+ if (!activeThreadId || activeWorktreePath || envMode === "worktree") return;🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/web/src/components/BranchToolbar.tsx around lines 75-76:
The `useEffect` at line 75 reverts the thread branch to match Git HEAD when `activeWorktreePath` is null. However, the new worktree mode (lines 118-123) also has null `activeWorktreePath` but intentionally sets a different branch without checkout. Consider adding `envMode !== "worktree"` to the effect's early return to prevent it from overwriting the user's base branch selection.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/desktop/turbo.jsonc`:
- Around line 5-23: The JSONC contains trailing commas after object entries
which cause parse errors; remove the trailing commas in the "build", "dev",
"start", and "smoke-test" task objects (specifically the commas following the
"outputs", "persistent", "persistent"/"cache", and "outputs" lines) so each
object ends with a closing brace without a trailing comma and the file parses
correctly.
In `@apps/server/turbo.jsonc`:
- Around line 5-10: The JSONC has trailing commas after the "persistent" and
closing object entries which causes parse errors; edit the "start" task object
in turbo.jsonc (the "start" key and its properties "dependsOn", "cache",
"persistent") and remove the trailing commas after "persistent": true and after
the inner object so the JSONC object closes without extra commas, then re-run
lint/CI.
In `@apps/web/turbo.jsonc`:
- Around line 5-8: The JSONC parse errors are caused by trailing commas in the
"dev" object; remove the trailing comma after "persistent": true and any
trailing comma before the closing brace of the outer object so the "dev" object
and enclosing object are valid JSONC (i.e., edit the "dev" block to eliminate
the comma after the last property and the comma before the closing brace).
🧹 Nitpick comments (1)
apps/web/src/components/BranchToolbar.tsx (1)
44-69: Consider adding error recovery for partial branch creation failure.In
createBranchMutation, ifcreateBranchsucceeds but the chainedcheckoutfails, the branch exists but the UI shows an error. This is acceptable UX since the user can manually select the branch, but consider whether theonErrormessage should indicate the branch was created.💡 Optional: More specific error message
const createBranchMutation = useMutation({ mutationFn: (branch: string) => api!.git .createBranch({ cwd: branchCwd!, branch }) .then(() => api!.git.checkout({ cwd: branchCwd!, branch })), onSuccess: () => queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd] }), onError: (error) => - setThreadError(error instanceof Error ? error.message : "Failed to create branch."), + setThreadError(error instanceof Error ? error.message : "Failed to create or checkout branch."), });
| "build": { | ||
| "dependsOn": ["^build"], | ||
| "outputs": ["dist-electron/**"], | ||
| }, | ||
| "dev": { | ||
| "dependsOn": ["t3#build"], | ||
| "persistent": true, | ||
| }, | ||
| "start": { | ||
| "dependsOn": ["build", "@t3tools/web#build", "t3#build"], | ||
| "cache": false, | ||
| "persistent": true, | ||
| }, | ||
| "smoke-test": { | ||
| "dependsOn": ["build", "@t3tools/web#build", "t3#build"], | ||
| "cache": false, | ||
| "outputs": [], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Remove trailing commas to fix JSONC parse errors.
Biome reports parse errors at the closing braces; the trailing commas before } are the likely cause and will break lint/CI.
✅ Suggested fix
"tasks": {
"build": {
"dependsOn": ["^build"],
- "outputs": ["dist-electron/**"],
+ "outputs": ["dist-electron/**"]
},
"dev": {
"dependsOn": ["t3#build"],
- "persistent": true,
+ "persistent": true
},
"start": {
"dependsOn": ["build", "@t3tools/web#build", "t3#build"],
"cache": false,
- "persistent": true,
+ "persistent": true
},
"smoke-test": {
"dependsOn": ["build", "@t3tools/web#build", "t3#build"],
"cache": false,
- "outputs": [],
- },
- },
+ "outputs": []
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "build": { | |
| "dependsOn": ["^build"], | |
| "outputs": ["dist-electron/**"], | |
| }, | |
| "dev": { | |
| "dependsOn": ["t3#build"], | |
| "persistent": true, | |
| }, | |
| "start": { | |
| "dependsOn": ["build", "@t3tools/web#build", "t3#build"], | |
| "cache": false, | |
| "persistent": true, | |
| }, | |
| "smoke-test": { | |
| "dependsOn": ["build", "@t3tools/web#build", "t3#build"], | |
| "cache": false, | |
| "outputs": [], | |
| }, | |
| }, | |
| "build": { | |
| "dependsOn": ["^build"], | |
| "outputs": ["dist-electron/**"] | |
| }, | |
| "dev": { | |
| "dependsOn": ["t3#build"], | |
| "persistent": true | |
| }, | |
| "start": { | |
| "dependsOn": ["build", "@t3tools/web#build", "t3#build"], | |
| "cache": false, | |
| "persistent": true | |
| }, | |
| "smoke-test": { | |
| "dependsOn": ["build", "@t3tools/web#build", "t3#build"], | |
| "cache": false, | |
| "outputs": [] | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.3.14)
[error] 8-8: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 12-12: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 17-17: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 22-22: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 23-23: Expected a property but instead found '}'.
Expected a property here.
(parse)
🤖 Prompt for AI Agents
In `@apps/desktop/turbo.jsonc` around lines 5 - 23, The JSONC contains trailing
commas after object entries which cause parse errors; remove the trailing commas
in the "build", "dev", "start", and "smoke-test" task objects (specifically the
commas following the "outputs", "persistent", "persistent"/"cache", and
"outputs" lines) so each object ends with a closing brace without a trailing
comma and the file parses correctly.
| "start": { | ||
| "dependsOn": ["build"], | ||
| "cache": false, | ||
| "persistent": true, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Remove trailing commas to fix JSONC parse errors.
Biome reports parse errors at the closing braces; the trailing commas before } are the likely cause and will break lint/CI.
✅ Suggested fix
"tasks": {
"start": {
"dependsOn": ["build"],
"cache": false,
- "persistent": true,
- },
- },
+ "persistent": true
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "start": { | |
| "dependsOn": ["build"], | |
| "cache": false, | |
| "persistent": true, | |
| }, | |
| }, | |
| "start": { | |
| "dependsOn": ["build"], | |
| "cache": false, | |
| "persistent": true | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.3.14)
[error] 9-9: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 10-10: Expected a property but instead found '}'.
Expected a property here.
(parse)
🤖 Prompt for AI Agents
In `@apps/server/turbo.jsonc` around lines 5 - 10, The JSONC has trailing commas
after the "persistent" and closing object entries which causes parse errors;
edit the "start" task object in turbo.jsonc (the "start" key and its properties
"dependsOn", "cache", "persistent") and remove the trailing commas after
"persistent": true and after the inner object so the JSONC object closes without
extra commas, then re-run lint/CI.
| "dev": { | ||
| "persistent": true, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Remove trailing commas to fix JSONC parse errors.
Biome reports parse errors at the closing braces; the trailing commas before } are the likely cause and will break lint/CI.
✅ Suggested fix
"tasks": {
"dev": {
- "persistent": true,
- },
- },
+ "persistent": true
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dev": { | |
| "persistent": true, | |
| }, | |
| }, | |
| "dev": { | |
| "persistent": true | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.3.14)
[error] 7-7: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 8-8: Expected a property but instead found '}'.
Expected a property here.
(parse)
🤖 Prompt for AI Agents
In `@apps/web/turbo.jsonc` around lines 5 - 8, The JSONC parse errors are caused
by trailing commas in the "dev" object; remove the trailing comma after
"persistent": true and any trailing comma before the closing brace of the outer
object so the "dev" object and enclosing object are valid JSONC (i.e., edit the
"dev" block to eliminate the comma after the last property and the comma before
the closing brace).
… HMR config - Add worktreePath to gitBranchSchema and populate from `git worktree list --porcelain` - Redirect to existing worktree when selecting a branch that's already checked out (instead of failing with "already used by worktree") - Distinguish main worktree (project cwd) from secondary worktrees in selection logic - Fix toolbar to show three states: Local, Worktree (existing), New worktree (pending) - Add explicit protocol/host to Vite HMR config for reliable Electron WebSocket - Auto-open DevTools in dev mode for HMR debugging Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/components/BranchToolbar.tsx`:
- Around line 51-59: The chained mutation in createBranchMutation (mutationFn
calling api.git.createBranch then api.git.checkout) can yield partial success
where createBranch succeeds but checkout fails, causing an incorrect "Failed to
create branch" error and no cache refresh; change the flow so createBranch is a
standalone operation that calls queryClient.invalidateQueries({ queryKey:
["git", "branches", branchCwd] }) on its onSuccess, then perform checkout as a
separate step (either a follow-up mutation or a try/catch after createBranch)
and handle checkout errors distinctly by calling setThreadError with a message
like "Branch created but checkout failed: <checkout error>". Make sure to
reference the existing createBranchMutation, api!.git.createBranch,
api!.git.checkout, onSuccess, onError, queryClient.invalidateQueries, and
setThreadError when making these changes.
In `@apps/web/vite.config.ts`:
- Line 5: The dev port is read from process.env.PORT (const port) while the
desktop dev script expects ELECTRON_RENDERER_PORT, causing mismatches; update
the port resolution in the vite config (where const port is defined) to prefer
ELECTRON_RENDERER_PORT first and then fall back to PORT and then 5173 (or switch
both web and desktop scripts to a single agreed env name), and ensure any
related npm scripts or docs reference the chosen env variable so Electron can
always connect to the web dev server.
🧹 Nitpick comments (1)
apps/web/src/components/BranchToolbar.tsx (1)
194-223: Consider adding ARIA attributes for dropdown accessibility.The branch picker dropdown button lacks
aria-expandedandaria-haspopupattributes, which screen readers rely on to announce dropdown state. This is a minor accessibility improvement.♿ Suggested ARIA attributes
<button type="button" className="inline-flex items-center gap-1.5 rounded-lg px-2 py-1 text-[12px] text-muted-foreground/60 transition-colors duration-150 hover:bg-accent/50 hover:text-muted-foreground/80" onClick={() => setIsBranchMenuOpen((open) => !open)} disabled={branchesQuery.isLoading} + aria-haspopup="listbox" + aria-expanded={isBranchMenuOpen} >
| const createBranchMutation = useMutation({ | ||
| mutationFn: (branch: string) => | ||
| api!.git | ||
| .createBranch({ cwd: branchCwd!, branch }) | ||
| .then(() => api!.git.checkout({ cwd: branchCwd!, branch })), | ||
| onSuccess: () => queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd] }), | ||
| onError: (error) => | ||
| setThreadError(error instanceof Error ? error.message : "Failed to create branch."), | ||
| }); |
There was a problem hiding this comment.
Chained mutation may leave inconsistent state on partial failure.
If createBranch succeeds but checkout fails (e.g., due to uncommitted changes), the error message incorrectly says "Failed to create branch" even though the branch was created. Additionally, since onSuccess won't fire, the branch list won't refresh, hiding the newly created branch from the user.
Consider separating into two distinct operations or providing accurate error messaging for partial success scenarios.
🔧 Suggested fix: separate operations or improve error handling
const createBranchMutation = useMutation({
- mutationFn: (branch: string) =>
- api!.git
- .createBranch({ cwd: branchCwd!, branch })
- .then(() => api!.git.checkout({ cwd: branchCwd!, branch })),
- onSuccess: () => queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd] }),
- onError: (error) =>
- setThreadError(error instanceof Error ? error.message : "Failed to create branch."),
+ mutationFn: async (branch: string) => {
+ await api!.git.createBranch({ cwd: branchCwd!, branch });
+ try {
+ await api!.git.checkout({ cwd: branchCwd!, branch });
+ } catch (checkoutError) {
+ // Branch was created but checkout failed - still invalidate to show new branch
+ await queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd] });
+ throw new Error(
+ `Branch created but checkout failed: ${checkoutError instanceof Error ? checkoutError.message : "Unknown error"}`
+ );
+ }
+ },
+ onSuccess: () => queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd] }),
+ onError: (error) =>
+ setThreadError(error instanceof Error ? error.message : "Failed to create branch."),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const createBranchMutation = useMutation({ | |
| mutationFn: (branch: string) => | |
| api!.git | |
| .createBranch({ cwd: branchCwd!, branch }) | |
| .then(() => api!.git.checkout({ cwd: branchCwd!, branch })), | |
| onSuccess: () => queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd] }), | |
| onError: (error) => | |
| setThreadError(error instanceof Error ? error.message : "Failed to create branch."), | |
| }); | |
| const createBranchMutation = useMutation({ | |
| mutationFn: async (branch: string) => { | |
| await api!.git.createBranch({ cwd: branchCwd!, branch }); | |
| try { | |
| await api!.git.checkout({ cwd: branchCwd!, branch }); | |
| } catch (checkoutError) { | |
| // Branch was created but checkout failed - still invalidate to show new branch | |
| await queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd] }); | |
| throw new Error( | |
| `Branch created but checkout failed: ${checkoutError instanceof Error ? checkoutError.message : "Unknown error"}` | |
| ); | |
| } | |
| }, | |
| onSuccess: () => queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd] }), | |
| onError: (error) => | |
| setThreadError(error instanceof Error ? error.message : "Failed to create branch."), | |
| }); |
🤖 Prompt for AI Agents
In `@apps/web/src/components/BranchToolbar.tsx` around lines 51 - 59, The chained
mutation in createBranchMutation (mutationFn calling api.git.createBranch then
api.git.checkout) can yield partial success where createBranch succeeds but
checkout fails, causing an incorrect "Failed to create branch" error and no
cache refresh; change the flow so createBranch is a standalone operation that
calls queryClient.invalidateQueries({ queryKey: ["git", "branches", branchCwd]
}) on its onSuccess, then perform checkout as a separate step (either a
follow-up mutation or a try/catch after createBranch) and handle checkout errors
distinctly by calling setThreadError with a message like "Branch created but
checkout failed: <checkout error>". Make sure to reference the existing
createBranchMutation, api!.git.createBranch, api!.git.checkout, onSuccess,
onError, queryClient.invalidateQueries, and setThreadError when making these
changes.
| import react from "@vitejs/plugin-react"; | ||
| import { defineConfig } from "vite"; | ||
|
|
||
| const port = Number(process.env.PORT ?? 5173); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for PORT and ELECTRON_RENDERER_PORT usage patterns
echo "=== Web app vite.config.ts port usage ==="
grep -n "PORT\|port" apps/web/vite.config.ts | head -20
echo ""
echo "=== Desktop dev script ELECTRON_RENDERER_PORT usage ==="
grep -n "ELECTRON_RENDERER_PORT\|port" apps/desktop/scripts/dev-electron.mjs | head -20
echo ""
echo "=== Search all usages of PORT env var ==="
rg -n "process\.env\.PORT" --type ts --type js -A 2 -B 2
echo ""
echo "=== Search all usages of ELECTRON_RENDERER_PORT ==="
rg -n "ELECTRON_RENDERER_PORT" --type ts --type js --type mjs -A 2 -B 2
echo ""
echo "=== Check for any .env files or examples ==="
fd "\.env" . -type fRepository: pingdotgg/codething-mvp
Length of output: 1296
🏁 Script executed:
#!/bin/bash
# Check for coordinated dev scripts and documentation
echo "=== Root package.json dev scripts ==="
cat package.json | grep -A 20 '"scripts"' | head -30
echo ""
echo "=== Check for development documentation ==="
find . -maxdepth 2 -name "*.md" -type f | xargs grep -l "PORT\|port\|dev" 2>/dev/null | head -5
echo ""
echo "=== List all dev-related scripts in monorepo ==="
fd "package.json" | xargs grep -l "\"dev\"" 2>/dev/null
echo ""
echo "=== Check apps/web package.json ==="
cat apps/web/package.json | grep -A 5 '"scripts"' 2>/dev/null || echo "Not found"
echo ""
echo "=== Check apps/desktop package.json ==="
cat apps/desktop/package.json | grep -A 5 '"scripts"' 2>/dev/null || echo "Not found"Repository: pingdotgg/codething-mvp
Length of output: 2056
Align port environment variable names between web and desktop dev scripts.
The web app uses PORT while the desktop dev script uses ELECTRON_RENDERER_PORT. Both default to 5173, so they match in typical usage, but if someone sets only one to a custom value, the Electron app will fail to connect to the dev server. Either use a single env var name across both or document the relationship clearly.
🤖 Prompt for AI Agents
In `@apps/web/vite.config.ts` at line 5, The dev port is read from
process.env.PORT (const port) while the desktop dev script expects
ELECTRON_RENDERER_PORT, causing mismatches; update the port resolution in the
vite config (where const port is defined) to prefer ELECTRON_RENDERER_PORT first
and then fall back to PORT and then 5173 (or switch both web and desktop scripts
to a single agreed env name), and ensure any related npm scripts or docs
reference the chosen env variable so Electron can always connect to the web dev
server.
When selecting a branch that lives in a different worktree, the store updated the thread's worktreePath but the running session kept its original cwd. The agent continued operating in the old directory. - SET_THREAD_BRANCH now clears the session when worktreePath changes - BranchToolbar stops the server-side session before dispatching the branch change, so the next message creates a fresh session with the correct cwd Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rror When in a secondary worktree and selecting a branch checked out in the main worktree (e.g. main), the code skipped the worktree redirect (because worktreePath === project.cwd) and tried git checkout, which git rejected. Now any branch with a worktreePath is handled as a redirect — main worktree redirects to local (worktreePath=null), secondary worktrees redirect to their path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
branchesQuery.data is undefined while loading, so isRepo defaulted to false — briefly showing the init button even for worktrees. Now defaults to true while loading (only shows init when the query actually returns isRepo: false). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Worktrees in /tmp or other ephemeral locations get cleaned by the OS but remain in git's worktree list as "prunable". These stale entries caused branches to show a worktree label pointing at a non-existent directory, leading to broken state when selected. Now validates each worktree path with existsSync before including it in the branch→worktreePath map. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add message queueing with QueueBar UI
(Web Works need to fix Desktop)
UI Bug Fixes
- Add CODEX_INTEGRATION_ANALYSIS.md with detailed protocol flow analysis * Protocol flow architecture and high-level overview * Request-response JSON-RPC sequence diagrams * Event streaming and transformation pipeline * Session state machine and lifecycle events * Provider dispatch routing analysis * 5 critical improvements ranked by priority * Failure scenarios and recovery analysis * Testing and deployment recommendations - Add CODEX_IMPROVEMENTS_GUIDE.md with step-by-step implementation guide * Improvement pingdotgg#1: Method-specific timeout configuration * Improvement pingdotgg#2: Partial stream recovery buffer * Improvement pingdotgg#3: Circuit breaker pattern implementation * Improvement pingdotgg#4: Graceful shutdown with timeout * Improvement pingdotgg#5: Structured observability logging * Integration checklist and testing procedures * 5-week rollout strategy Architecture grade: B+ (strong foundation, operational concerns remain) Expected impact: 10x reduction in false timeouts, 99.9% → 99.99% availability These guides provide actionable recommendations for improving connection robustness, timeout strategies, error recovery, and observability in the Codex App Server integration.
Closes #7 (Does not implement multiple threads per worktree)
Summary
Users can now right-click on "+ New thread" to create threads from specific git branches, with optional worktree creation for isolated agent work.
Features
Test Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests