Add AI-generated worktree branch naming with safe Git branch rename flow - #129
Conversation
- add `git.generateBranchName`, `git.generateAndRenameBranch`, and `git.renameBranch` websocket flows - pass image attachments into Codex branch-name generation and normalize/sanitize results - add GitCore branch rename with collision-safe numeric suffixing - extract shared image MIME/data-url helpers and expand server/web tests for new behavior
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
- Generate and rename temporary `t3code/*` worktree branches on first user turn in `ProviderCommandReactor` - Remove websocket/client branch-generation RPCs and related contract/API surface - Update Codex text-generation prompt to include attachment metadata only when attachments exist, with expanded tests
- Replace fire-and-forget `Effect.runPromise` call with `Effect.forkScoped` - Keep checkout responsive while tying upstream refresh to scoped effect management
- remove `git.renameBranch` from WS/contracts/native API surface and update server/web tests - use `git branch -m -- ...` in GitCore to safely separate branch rename arguments
- Remove message/time hashing for temporary branch names in `ChatView` - Use an 8-char UUID token while preserving backend-detected suffix format
- add shared attachment path normalization and resolution helpers - reuse persisted image files for turn start and branch name generation - cover persisted attachment flow in text generation and reactor tests
- Make branch-name generation return a required string instead of nullable output - Propagate invalid Codex branch payloads as `TextGenerationError` instead of silently falling back - Update provider command reactor to catch generation failures, log warnings, and skip rename safely
- Replace manual JSON schema constants and parse helpers with `Schema.Struct` definitions - Generate JSON Schema from Effect Schema and decode output via `Schema.fromJsonString` - Map `SchemaError` to `TextGenerationError` for invalid Codex responses - Update `ProviderCommandReactor` test mock typing for `generateBranchName`
- accept `data:image/...;...;base64,...` headers while still rejecting invalid formats - add `imageMime` tests for mime params, non-base64 URLs, and missing mime types - fork branch-generation prompt effect at call site in `ProviderCommandReactor`
- add `ServerConfig.layerTest(cwd, stateDir)` to standardize test wiring - require `ServerConfig` in `CodexTextGeneration` attachment path resolution - update orchestration/projection tests and harness to use managed runtimes with explicit config
Co-authored-by: codex <codex@users.noreply.github.com>
- add attachment store helpers for IDs, route paths, and file resolution - stop projection pipeline from mutating/storing data URLs; keep attachment metadata unchanged - update Codex send/branch generation flows and tests to resolve images via attachment IDs
- Sanitize thread-derived attachment ID segments to safe, bounded tokens - Resolve attachment IDs only when a matching file exists on disk - Return 404 (not 400) for missing attachment IDs in the attachment route - Add attachmentStore tests for ID sanitization and extension-based path lookup
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Inconsistent thread segment derivation breaks attachment cleanup
- Removed the duplicate toSafeThreadAttachmentSegment in ProjectionPipeline.ts (which used encodeURIComponent) and replaced it with an import of the canonical version from attachmentStore.ts (which uses regex sanitization), ensuring consistent thread segment derivation for attachment cleanup.
Or push these changes by commenting:
@cursor push 462881455e
Preview (462881455e)
diff --git a/apps/server/src/attachmentStore.ts b/apps/server/src/attachmentStore.ts
--- a/apps/server/src/attachmentStore.ts
+++ b/apps/server/src/attachmentStore.ts
@@ -13,7 +13,7 @@
const ATTACHMENT_FILENAME_EXTENSIONS = [...SAFE_IMAGE_FILE_EXTENSIONS, ".bin"];
const ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS = 80;
-function toSafeThreadAttachmentSegment(threadId: string): string | null {
+export function toSafeThreadAttachmentSegment(threadId: string): string | null {
const segment = threadId
.trim()
.replace(/[^a-z0-9_-]+/gi, "-")
diff --git a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts
--- a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts
+++ b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts
@@ -37,7 +37,7 @@
OrchestrationProjectionPipeline,
type OrchestrationProjectionPipelineShape,
} from "../Services/ProjectionPipeline.ts";
-import { attachmentRelativePath } from "../../attachmentStore.ts";
+import { attachmentRelativePath, toSafeThreadAttachmentSegment } from "../../attachmentStore.ts";
export const ORCHESTRATION_PROJECTOR_NAMES = {
projects: "projection.projects",
@@ -66,20 +66,6 @@
readonly prunedThreadRelativePaths: Map<string, Set<string>>;
}
-function toSafeThreadAttachmentSegment(threadId: string): string | null {
- const segment = encodeURIComponent(threadId);
- if (
- segment.length === 0 ||
- segment === "." ||
- segment === ".." ||
- segment.includes("/") ||
- segment.includes("\\") ||
- segment.includes("\0")
- ) {
- return null;
- }
- return segment;
-}
const materializeAttachmentsForProjection = Effect.fn(function* (input: {
readonly attachments: ReadonlyArray<ChatAttachment>;- Move stdin capture earlier in fake codex script setup - Ensure stdin-based assertions remain available even when image validation fails
- export and reuse `toSafeThreadAttachmentSegment` from `attachmentStore` - align projection attachment path logic with store-safe thread IDs - update projection pipeline tests to cover thread IDs with spaces
| const ATTACHMENT_FILENAME_EXTENSIONS = [...SAFE_IMAGE_FILE_EXTENSIONS, ".bin"]; | ||
| const ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS = 80; | ||
|
|
||
| export function toSafeThreadAttachmentSegment(threadId: string): string | null { |
There was a problem hiding this comment.
🟡 Medium src/attachmentStore.ts:16
Thread segment sanitization allows hyphens, causing prefix collisions during cleanup. Thread foo produces segment foo, so .startsWith('foo-') incorrectly matches files from thread foo-bar (segment foo-bar-uuid). Consider replacing hyphens with a different character or using a hash-based approach to ensure unique, non-colliding prefixes.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/attachmentStore.ts around line 16:
Thread segment sanitization allows hyphens, causing prefix collisions during cleanup. Thread `foo` produces segment `foo`, so `.startsWith('foo-')` incorrectly matches files from thread `foo-bar` (segment `foo-bar-uuid`). Consider replacing hyphens with a different character or using a hash-based approach to ensure unique, non-colliding prefixes.
Evidence trail:
- apps/server/src/attachmentStore.ts lines 16-26: `toSafeThreadAttachmentSegment` regex `/[^a-z0-9_-]+/gi` preserves hyphens
- apps/server/src/attachmentStore.ts lines 29-34: `createAttachmentId` creates IDs as `${threadSegment}-${randomUUID()}`
- apps/server/src/orchestration/Layers/ProjectionPipeline.ts lines 201, 239, 275: cleanup uses `.startsWith(`${threadSegment}-`)` for prefix matching
- Parse thread segments from attachment IDs using a UUID-aware pattern - Prevent revert/delete cleanup from removing files belonging to similarly prefixed threads - Add tests for prefix-collision cases in attachment store and projection pipeline
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Unused exported functions in new attachment store module
- Removed the unused
attachmentRoutePathfunction;parseAttachmentIdFromRelativePathwas actually used in ProjectionPipeline.ts so it was correctly kept.
- Removed the unused
- ✅ Fixed: Unused exported functions in new attachment paths module
- Removed both unused
resolveAttachmentRoutePathand its only consumerattachmentRouteToRelativePathfrom attachmentPaths.ts.
- Removed both unused
Or push these changes by commenting:
@cursor push 05bb90eef9
Preview (05bb90eef9)
diff --git a/apps/server/src/attachmentPaths.ts b/apps/server/src/attachmentPaths.ts
--- a/apps/server/src/attachmentPaths.ts
+++ b/apps/server/src/attachmentPaths.ts
@@ -10,14 +10,6 @@
return normalized.replace(/\\/g, "/");
}
-export function attachmentRouteToRelativePath(dataUrl: string): string | null {
- const prefix = `${ATTACHMENTS_ROUTE_PREFIX}/`;
- if (!dataUrl.startsWith(prefix)) {
- return null;
- }
- return normalizeAttachmentRelativePath(dataUrl.slice(prefix.length));
-}
-
export function resolveAttachmentRelativePath(input: {
readonly stateDir: string;
readonly relativePath: string;
@@ -34,17 +26,3 @@
}
return filePath;
}
-
-export function resolveAttachmentRoutePath(input: {
- readonly stateDir: string;
- readonly dataUrl: string;
-}): string | null {
- const relativePath = attachmentRouteToRelativePath(input.dataUrl);
- if (!relativePath) {
- return null;
- }
- return resolveAttachmentRelativePath({
- stateDir: input.stateDir,
- relativePath,
- });
-}
diff --git a/apps/server/src/attachmentStore.ts b/apps/server/src/attachmentStore.ts
--- a/apps/server/src/attachmentStore.ts
+++ b/apps/server/src/attachmentStore.ts
@@ -4,7 +4,6 @@
import type { ChatAttachment } from "@t3tools/contracts";
import {
- ATTACHMENTS_ROUTE_PREFIX,
normalizeAttachmentRelativePath,
resolveAttachmentRelativePath,
} from "./attachmentPaths.ts";
@@ -76,10 +75,6 @@
});
}
-export function attachmentRoutePath(attachment: ChatAttachment): string {
- return `${ATTACHMENTS_ROUTE_PREFIX}/${encodeURIComponent(attachment.id)}`;
-}
-
export function resolveAttachmentPathById(input: {
readonly stateDir: string;
readonly attachmentId: string;There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Case mismatch causes attachment cleanup to silently fail
- Added
.toLowerCase()early intoSafeThreadAttachmentSegment's pipeline and removed the now-unnecessaryiregex flag, ensuring it produces the same lowercase segments asparseThreadSegmentFromAttachmentId.
- Added
Or push these changes by commenting:
@cursor push b3d2f18faf
Preview (b3d2f18faf)
diff --git a/apps/server/src/attachmentStore.ts b/apps/server/src/attachmentStore.ts
--- a/apps/server/src/attachmentStore.ts
+++ b/apps/server/src/attachmentStore.ts
@@ -23,7 +23,8 @@
export function toSafeThreadAttachmentSegment(threadId: string): string | null {
const segment = threadId
.trim()
- .replace(/[^a-z0-9_-]+/gi, "-")
+ .toLowerCase()
+ .replace(/[^a-z0-9_-]+/g, "-")
.replace(/-+/g, "-")
.replace(/^[-_]+|[-_]+$/g, "")
.slice(0, ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS)- Lowercase sanitized thread segments when creating attachment IDs - Add coverage for mixed-case thread IDs in attachment store and projection pipeline tests
| } | ||
| const turnStartCommand = input.command; | ||
|
|
||
| const normalizedAttachments = yield* Effect.forEach( |
There was a problem hiding this comment.
🟡 Medium src/wsServer.ts:251
When attachment processing fails partway through a batch, already-persisted files become orphaned. Consider adding cleanup logic (e.g., track written paths, delete on failure) or using a two-phase approach (validate all first, then persist).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/wsServer.ts around line 251:
When attachment processing fails partway through a batch, already-persisted files become orphaned. Consider adding cleanup logic (e.g., track written paths, delete on failure) or using a two-phase approach (validate all first, then persist).
Evidence trail:
apps/server/src/wsServer.ts lines 251-313 (attachment processing loop using Effect.forEach, writes files individually at lines 302-309), lines 608-610 (caller with no cleanup), git_grep for 'cleanup|rollback|delete|unlink|removeFile' in wsServer.ts shows no attachment cleanup logic, git_grep for 'Effect.acquireRelease|Effect.ensuring' shows these patterns ARE used elsewhere in codebase but NOT in the attachment processing code
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Newly exported functions are never imported anywhere
- Removed the three unused exported functions (resolveAttachmentRoutePath, attachmentRouteToRelativePath, attachmentRoutePath) and cleaned up the now-unused ATTACHMENTS_ROUTE_PREFIX import in attachmentStore.ts.
Or push these changes by commenting:
@cursor push 68241ecf86
Preview (68241ecf86)
diff --git a/apps/server/src/attachmentPaths.ts b/apps/server/src/attachmentPaths.ts
--- a/apps/server/src/attachmentPaths.ts
+++ b/apps/server/src/attachmentPaths.ts
@@ -10,14 +10,6 @@
return normalized.replace(/\\/g, "/");
}
-export function attachmentRouteToRelativePath(dataUrl: string): string | null {
- const prefix = `${ATTACHMENTS_ROUTE_PREFIX}/`;
- if (!dataUrl.startsWith(prefix)) {
- return null;
- }
- return normalizeAttachmentRelativePath(dataUrl.slice(prefix.length));
-}
-
export function resolveAttachmentRelativePath(input: {
readonly stateDir: string;
readonly relativePath: string;
@@ -34,17 +26,3 @@
}
return filePath;
}
-
-export function resolveAttachmentRoutePath(input: {
- readonly stateDir: string;
- readonly dataUrl: string;
-}): string | null {
- const relativePath = attachmentRouteToRelativePath(input.dataUrl);
- if (!relativePath) {
- return null;
- }
- return resolveAttachmentRelativePath({
- stateDir: input.stateDir,
- relativePath,
- });
-}
diff --git a/apps/server/src/attachmentStore.ts b/apps/server/src/attachmentStore.ts
--- a/apps/server/src/attachmentStore.ts
+++ b/apps/server/src/attachmentStore.ts
@@ -4,7 +4,6 @@
import type { ChatAttachment } from "@t3tools/contracts";
import {
- ATTACHMENTS_ROUTE_PREFIX,
normalizeAttachmentRelativePath,
resolveAttachmentRelativePath,
} from "./attachmentPaths.ts";
@@ -77,10 +76,6 @@
});
}
-export function attachmentRoutePath(attachment: ChatAttachment): string {
- return `${ATTACHMENTS_ROUTE_PREFIX}/${encodeURIComponent(attachment.id)}`;
-}
-
export function resolveAttachmentPathById(input: {
readonly stateDir: string;
readonly attachmentId: string;- delete route-to-path conversion helpers from `attachmentPaths` - remove unused `attachmentRoutePath` export and route prefix import
Merges `pingdotgg/t3code` `8b2838e0e..a37c664` — 43 commits. `343` files landed against `343` changed in the upstream range; fork delta `723` files. Exact match, so nothing upstream changed was dropped. Details in [`docs/fork/upstream-merge-log.md`](../blob/merge/upstream-2026-09-08/docs/fork/upstream-merge-log.md). ## Two fork deltas this merge had to re-apply **Upstream split the server-update banner into two routes.** pingdotgg#10596 added `useAutoBalanceUpdateBanner` beside the single-machine condition the fork already gates. The conflict was on the first line only, so resolving it correctly still left the auto-balance route ungated — an auto-balanced project would have been offered `npx t3` against a backend that does not implement `server.updateServer`. `FEATURES.serverUpdateBanner` now carries two gates in `ChatView.tsx`. **A new settings page needs a gate even though it degrades politely.** pingdotgg#8103 added `/settings/snap-shot` for desktop window capture. Every control drives `window.desktopBridge`, and upstream renders an "unavailable" notice rather than hiding the page, so a hosted build listed a sidebar section and six searchable rows for a feature it can never run. Gated with `FEATURES.snapShots`. Two smaller fixes: `packages/moatless-api` still ran `tsgo --noEmit` after upstream replaced `@typescript/native-preview` with TypeScript 7.0.2, and `duplicate-adds.mjs` now skips `pnpm-lock.yaml` (it read `iconv-lite: 0.6.3` as taken twice; `d3-dsv` and `encoding` each declare it). ## Usable as-is - Stop-thread keybinding command (pingdotgg#4308). - Project import tolerates servers that predate the git-identity scan (pingdotgg#10547). - Proactive panels open when entering a thread (pingdotgg#10610); pull-request markdown links open in the panel (pingdotgg#10623); markdown images navigate as galleries (pingdotgg#10625); pull-request videos play inline (pingdotgg#10617). - Settings project scopes are searchable and scrollable (pingdotgg#10570); ref picker stays steady when opening (pingdotgg#9472); sidebar timer uses `tabular-nums` (pingdotgg#10592); popup triggers stay steady when pressed (pingdotgg#9468); settled PR colors restore on hover (pingdotgg#10023). - Composer Fast mode persists across new chats (pingdotgg#2981); inserted citations are removed on cancel (pingdotgg#10518). - TypeScript 7.0.2 (pingdotgg#10663) and the knip desktop-export rules (pingdotgg#10269). ## Unsupported in Moatless / needs implementation - **Cross-platform window capture** (pingdotgg#8103) — `apps/desktop/src/snapShot/**`, `apps/web/src/components/settings/SnapShotSettings.tsx`, `apps/web/src/lib/desktopSnapShot.ts`. Needs an Electron `window.desktopBridge`; a browser tab has none. Gated behind `FEATURES.snapShots` in this PR. - **Auto-balance server update** (pingdotgg#10596) — `apps/web/src/components/chat/useAutoBalanceUpdateBanner.tsx`. Needs `server.updateServer`, which Moatless does not dispatch. Gated behind `FEATURES.serverUpdateBanner` in this PR. - **Preview recording transfer** (pingdotgg#10572) — `apps/server/src/mcp/toolkits/preview/handlers.ts`, `apps/web/src/browser/browserRecordingUpload.ts`. Moves a finished preview recording into the agent environment over the desktop bridge. Adds four error types to `packages/contracts/src/previewAutomation.ts` and no new RPC method, so no union changed. Sits behind the `previewAutomation.connect` / `focusHost` / `respond` gap already in the register. - **Local media linked from remote threads** (pingdotgg#10619) and **browser editing shortcuts** (pingdotgg#10621) — Electron shell only. - **iOS Keychain access group** (pingdotgg#3665) and the mobile provider account badge (pingdotgg#9899) — the fork ships no mobile build against Moatless. ## Backend behavior to consider reproducing in Moatless - **Name the usage limit and its reset instead of relaying "out of credits"** (pingdotgg#10473, `apps/server/src/provider/**` Codex adapter). Moatless owns its provider runtime, so the clearer limit message has to be produced there. - **Report usage limits on retried turns** (pingdotgg#10549, Claude adapter). A retry currently loses the limit signal; same ownership. - **Disable executable capabilities in Claude metadata generation** (pingdotgg#4169, `apps/server/src/textGeneration/ClaudeTextGeneration.ts`). Title and metadata generation should not be able to run tools. Worth mirroring wherever Moatless generates thread titles. ## Verification `verify.mjs`: duplicate-adds, tripwires, resolution-check, unsupported-methods (0 ADD, 0 DROP, 2 KEEP), fmt, lint and typecheck all pass. Tests pass except `@t3tools/desktop`, which cannot compile `scripts/browser-secret-native.test.mjs` because the sandbox has no `libsecret-1` — 1283 tests pass, 0 fail, and the file is byte-identical to upstream. New entry in `docs/fork/gaps.md`. `t3` failed `GrokAdapter.test.ts` once under parallel load and passes 42/42 alone. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- Moatless task: https://moatless.soaplabstest.com/tasks/6d8ea486-2fcb-4c25-bd34-dcd15cc4a7ac

Summary
generateBranchName) with strict JSON output parsing and branch-fragment sanitization.--image.-1,-2, etc.) and no-op behavior when old/new names match.t3code/and normalize unsafe input before rename.imageMime.tsand reuse across projection/attachment handling.Testing
apps/server/src/git/Layers/CodexTextGeneration.test.ts: verifies branch generation, normalization, image passthrough to Codex, and invalid payload fallback.apps/server/src/git/Layers/GitCore.test.ts: verifies branch rename success, no-op rename, and collision suffix increment behavior.apps/server/src/wsServer.test.ts: verifiesgit.generateBranchName,git.generateAndRenameBranch,git.renameBranch, and skip-rename when generation returnsnull.apps/server/src/git/Layers/GitManager.test.ts: updates fake text generation layer coverage for branch-name generation.Note
Medium Risk
Touches Git operations (new
renameBranchwith collision handling) and introduces attachment file resolution/cleanup paths used by both Codex and projections, so mistakes could rename the wrong branch or read/delete the wrong files.Overview
AI-generated worktree branch naming: introduces
TextGeneration.generateBranchName(Codex CLI-backed) with schema-validated JSON parsing, branch-fragment sanitization, and optional image context passed via--image.Safe Git branch rename: adds
GitCore.renameBranchwith--argument separation, no-op when names match, and automatic-1/-2/...suffixing to avoid collisions; also switches upstream refresh after checkout toEffect.forkDetach.Attachment storage/usage refactor: adds
attachmentStore/attachmentPathsplusimageMimeutilities, updates projections to stop materializing data URLs intostateDirand instead persist attachment metadata (id,mimeType, etc.) while pruning/deleting attachment files by attachment-id-derived thread segments; Codex adapter now resolves attachment ids fromstateDirand converts them todata:URLs when calling the Codex app server.Wiring/tests: adds
ServerConfig.layerTestand updates integration/unit tests and server layers to provideServerConfig,NodeServices, and the newGitCore/TextGenerationdependencies; updates Codex manager sendTurn input to useattachments[].url.Written by Cursor Bugbot for commit 150e171. This will update automatically on new commits. Configure here.
Note
Generate and rename temporary worktree branches to safe
t3code/<name>on first user turn usingTextGeneration.generateBranchNameandGitCore.renameBranchAdd AI-driven branch naming on the first user turn and rename the current temporary branch with collision-safe suffixing; persist image attachments at upload, pass attachment metadata and images to generation, normalize attachment access via id-based routes, and adjust tests and schemas to use id-based attachments without
dataUrl. ImplementGitCore.renameBranch, branch name sanitization, and id-based attachment resolution across server, adapter, and web layers.📍Where to Start
Start with the first-turn rename flow in
ProviderCommandReactor.maybeGenerateAndRenameWorktreeBranchForFirstTurnin apps/server/src/orchestration/Layers/ProviderCommandReactor.ts.Macroscope summarized 150e171.