| author | |
| committer | |
| log | b1608f41adff7ac9670c5a762624e13984d820c8 |
| tree | c50db3f5a0ad95881d7978a17dc215ce7cfd3806 |
| parent | 78d89e3d353a9ce7770b34f3546e39f415cddc37 |
Move sanitizeBranchFragment and sanitizeFeatureBranchName from three
duplicate definitions (CodexTextGeneration, GitManager, GitActionsControl)
into packages/contracts/src/git.ts. Remove the redundant double-sanitization
of branch names between the TextGeneration and GitManager layers —
CodexTextGeneration now owns sanitization of model output while GitManager
trusts the result and only sanitizes the custom commit message path.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>4 files changed, 47 insertions(+), 79 deletions(-)
apps/server/src/git/Layers/CodexTextGeneration.ts+3-27| ... | @@ -3,6 +3,8 @@ import { randomUUID } from "node:crypto"; | ... | @@ -3,6 +3,8 @@ import { randomUUID } from "node:crypto"; |
| 3 | import { Effect, FileSystem, Layer, Option, Path, Schema, Stream } from "effect"; | 3 | import { Effect, FileSystem, Layer, Option, Path, Schema, Stream } from "effect"; |
| 4 | import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; | 4 | import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; |
| 5 | 5 | ||
| 6 | import { sanitizeBranchFragment, sanitizeFeatureBranchName } from "@t3tools/contracts"; | ||
| 7 | |||
| 6 | import { resolveAttachmentPath } from "../../attachmentStore.ts"; | 8 | import { resolveAttachmentPath } from "../../attachmentStore.ts"; |
| 7 | import { ServerConfig } from "../../config.ts"; | 9 | import { ServerConfig } from "../../config.ts"; |
| 8 | import { TextGenerationError } from "../Errors.ts"; | 10 | import { TextGenerationError } from "../Errors.ts"; |
| ... | @@ -93,32 +95,6 @@ function sanitizePrTitle(raw: string): string { | ... | @@ -93,32 +95,6 @@ function sanitizePrTitle(raw: string): string { |
| 93 | return "Update project changes"; | 95 | return "Update project changes"; |
| 94 | } | 96 | } |
| 95 | 97 | ||
| 96 | function sanitizeBranchName(raw: string): string { | ||
| 97 | const normalized = raw | ||
| 98 | .trim() | ||
| 99 | .toLowerCase() | ||
| 100 | .replace(/['"`]/g, "") | ||
| 101 | .replace(/^[./\s_-]+|[./\s_-]+$/g, ""); | ||
| 102 | |||
| 103 | const branchFragment = normalized | ||
| 104 | .replace(/[^a-z0-9/_-]+/g, "-") | ||
| 105 | .replace(/\/+/g, "/") | ||
| 106 | .replace(/-+/g, "-") | ||
| 107 | .replace(/^[./_-]+|[./_-]+$/g, "") | ||
| 108 | .slice(0, 64) | ||
| 109 | .replace(/[./_-]+$/g, ""); | ||
| 110 | |||
| 111 | return branchFragment.length > 0 ? branchFragment : "update"; | ||
| 112 | } | ||
| 113 | |||
| 114 | function sanitizeFeatureBranchName(raw: string): string { | ||
| 115 | const sanitized = sanitizeBranchName(raw); | ||
| 116 | if (sanitized.includes("/")) { | ||
| 117 | return sanitized.startsWith("feature/") ? sanitized : `feature/${sanitized}`; | ||
| 118 | } | ||
| 119 | return `feature/${sanitized}`; | ||
| 120 | } | ||
| 121 | |||
| 122 | const makeCodexTextGeneration = Effect.gen(function* () { | 98 | const makeCodexTextGeneration = Effect.gen(function* () { |
| 123 | const fileSystem = yield* FileSystem.FileSystem; | 99 | const fileSystem = yield* FileSystem.FileSystem; |
| 124 | const path = yield* Path.Path; | 100 | const path = yield* Path.Path; |
| ... | @@ -475,7 +451,7 @@ const makeCodexTextGeneration = Effect.gen(function* () { | ... | @@ -475,7 +451,7 @@ const makeCodexTextGeneration = Effect.gen(function* () { |
| 475 | }); | 451 | }); |
| 476 | 452 | ||
| 477 | return { | 453 | return { |
| 478 | branch: sanitizeBranchName(generated.branch), | 454 | branch: sanitizeBranchFragment(generated.branch), |
| 479 | } satisfies BranchNameGenerationResult; | 455 | } satisfies BranchNameGenerationResult; |
| 480 | }); | 456 | }); |
| 481 | }; | 457 | }; |
apps/server/src/git/Layers/GitManager.ts+3-30| ... | @@ -1,6 +1,7 @@ | ... | @@ -1,6 +1,7 @@ |
| 1 | import { randomUUID } from "node:crypto"; | 1 | import { randomUUID } from "node:crypto"; |
| 2 | 2 | ||
| 3 | import { Effect, FileSystem, Layer, Path } from "effect"; | 3 | import { Effect, FileSystem, Layer, Path } from "effect"; |
| 4 | import { sanitizeFeatureBranchName } from "@t3tools/contracts"; | ||
| 4 | 5 | ||
| 5 | import { GitManagerError } from "../Errors.ts"; | 6 | import { GitManagerError } from "../Errors.ts"; |
| 6 | import { GitManager, type GitManagerShape } from "../Services/GitManager.ts"; | 7 | import { GitManager, type GitManagerShape } from "../Services/GitManager.ts"; |
| ... | @@ -104,32 +105,6 @@ function sanitizeCommitMessage(generated: { | ... | @@ -104,32 +105,6 @@ function sanitizeCommitMessage(generated: { |
| 104 | }; | 105 | }; |
| 105 | } | 106 | } |
| 106 | 107 | ||
| 107 | function sanitizeBranchFragment(raw: string): string { | ||
| 108 | const normalized = raw | ||
| 109 | .trim() | ||
| 110 | .toLowerCase() | ||
| 111 | .replace(/['"`]/g, "") | ||
| 112 | .replace(/^[./\s_-]+|[./\s_-]+$/g, ""); | ||
| 113 | |||
| 114 | const branchFragment = normalized | ||
| 115 | .replace(/[^a-z0-9/_-]+/g, "-") | ||
| 116 | .replace(/\/+/g, "/") | ||
| 117 | .replace(/-+/g, "-") | ||
| 118 | .replace(/^[./_-]+|[./_-]+$/g, "") | ||
| 119 | .slice(0, 64) | ||
| 120 | .replace(/[./_-]+$/g, ""); | ||
| 121 | |||
| 122 | return branchFragment.length > 0 ? branchFragment : "update"; | ||
| 123 | } | ||
| 124 | |||
| 125 | function sanitizeFeatureBranchName(raw: string): string { | ||
| 126 | const sanitized = sanitizeBranchFragment(raw); | ||
| 127 | if (sanitized.includes("/")) { | ||
| 128 | return sanitized.startsWith("feature/") ? sanitized : `feature/${sanitized}`; | ||
| 129 | } | ||
| 130 | return `feature/${sanitized}`; | ||
| 131 | } | ||
| 132 | |||
| 133 | function formatCommitMessage(subject: string, body: string): string { | 108 | function formatCommitMessage(subject: string, body: string): string { |
| 134 | const trimmedBody = body.trim(); | 109 | const trimmedBody = body.trim(); |
| 135 | if (trimmedBody.length === 0) { | 110 | if (trimmedBody.length === 0) { |
| ... | @@ -324,16 +299,14 @@ export const makeGitManager = Effect.gen(function* () { | ... | @@ -324,16 +299,14 @@ export const makeGitManager = Effect.gen(function* () { |
| 324 | branch: input.branch, | 299 | branch: input.branch, |
| 325 | stagedSummary: limitContext(context.stagedSummary, 8_000), | 300 | stagedSummary: limitContext(context.stagedSummary, 8_000), |
| 326 | stagedPatch: limitContext(context.stagedPatch, 50_000), | 301 | stagedPatch: limitContext(context.stagedPatch, 50_000), |
| 327 | ...(input.includeBranch !== undefined ? { includeBranch: input.includeBranch } : {}), | 302 | ...(input.includeBranch ? { includeBranch: true } : {}), |
| 328 | }) | 303 | }) |
| 329 | .pipe(Effect.map((result) => sanitizeCommitMessage(result))); | 304 | .pipe(Effect.map((result) => sanitizeCommitMessage(result))); |
| 330 | 305 | ||
| 331 | return { | 306 | return { |
| 332 | subject: generated.subject, | 307 | subject: generated.subject, |
| 333 | body: generated.body, | 308 | body: generated.body, |
| 334 | ...(generated.branch !== undefined | 309 | ...(generated.branch !== undefined ? { branch: generated.branch } : {}), |
| 335 | ? { branch: sanitizeFeatureBranchName(generated.branch) } | ||
| 336 | : {}), | ||
| 337 | commitMessage: formatCommitMessage(generated.subject, generated.body), | 310 | commitMessage: formatCommitMessage(generated.subject, generated.body), |
| 338 | }; | 311 | }; |
| 339 | }); | 312 | }); |
apps/web/src/components/GitActionsControl.logic.ts+5-22| ... | @@ -1,7 +1,8 @@ | ... | @@ -1,7 +1,8 @@ |
| 1 | import type { | 1 | import { |
| 2 | GitRunStackedActionResult, | 2 | type GitRunStackedActionResult, |
| 3 | GitStackedAction, | 3 | type GitStackedAction, |
| 4 | GitStatusResult, | 4 | type GitStatusResult, |
| 5 | sanitizeBranchFragment, | ||
| 5 | } from "@t3tools/contracts"; | 6 | } from "@t3tools/contracts"; |
| 6 | 7 | ||
| 7 | export type GitActionIconName = "commit" | "push" | "pr"; | 8 | export type GitActionIconName = "commit" | "push" | "pr"; |
| ... | @@ -321,24 +322,6 @@ export function resolveDefaultBranchActionDialogCopy(input: { | ... | @@ -321,24 +322,6 @@ export function resolveDefaultBranchActionDialogCopy(input: { |
| 321 | }; | 322 | }; |
| 322 | } | 323 | } |
| 323 | 324 | ||
| 324 | function sanitizeBranchFragment(raw: string): string { | ||
| 325 | const normalized = raw | ||
| 326 | .trim() | ||
| 327 | .toLowerCase() | ||
| 328 | .replace(/['"`]/g, "") | ||
| 329 | .replace(/^[./\s_-]+|[./\s_-]+$/g, ""); | ||
| 330 | |||
| 331 | const branchFragment = normalized | ||
| 332 | .replace(/[^a-z0-9/_-]+/g, "-") | ||
| 333 | .replace(/\/+/g, "/") | ||
| 334 | .replace(/-+/g, "-") | ||
| 335 | .replace(/^[./_-]+|[./_-]+$/g, "") | ||
| 336 | .slice(0, 64) | ||
| 337 | .replace(/[./_-]+$/g, ""); | ||
| 338 | |||
| 339 | return branchFragment.length > 0 ? branchFragment : "update"; | ||
| 340 | } | ||
| 341 | |||
| 342 | export function resolveAutoFeatureBranchName( | 325 | export function resolveAutoFeatureBranchName( |
| 343 | existingBranchNames: readonly string[], | 326 | existingBranchNames: readonly string[], |
| 344 | preferredBranch?: string, | 327 | preferredBranch?: string, |
packages/contracts/src/git.ts+36| ... | @@ -179,3 +179,39 @@ export const GitPullResult = Schema.Struct({ | ... | @@ -179,3 +179,39 @@ export const GitPullResult = Schema.Struct({ |
| 179 | upstreamBranch: TrimmedNonEmptyStringSchema.pipe(Schema.NullOr), | 179 | upstreamBranch: TrimmedNonEmptyStringSchema.pipe(Schema.NullOr), |
| 180 | }); | 180 | }); |
| 181 | export type GitPullResult = typeof GitPullResult.Type; | 181 | export type GitPullResult = typeof GitPullResult.Type; |
| 182 | |||
| 183 | // Branch name utilities | ||
| 184 | |||
| 185 | /** | ||
| 186 | * Sanitize an arbitrary string into a valid, lowercase git branch fragment. | ||
| 187 | * Strips quotes, collapses separators, limits to 64 chars. | ||
| 188 | */ | ||
| 189 | export function sanitizeBranchFragment(raw: string): string { | ||
| 190 | const normalized = raw | ||
| 191 | .trim() | ||
| 192 | .toLowerCase() | ||
| 193 | .replace(/['"`]/g, "") | ||
| 194 | .replace(/^[./\s_-]+|[./\s_-]+$/g, ""); | ||
| 195 | |||
| 196 | const branchFragment = normalized | ||
| 197 | .replace(/[^a-z0-9/_-]+/g, "-") | ||
| 198 | .replace(/\/+/g, "/") | ||
| 199 | .replace(/-+/g, "-") | ||
| 200 | .replace(/^[./_-]+|[./_-]+$/g, "") | ||
| 201 | .slice(0, 64) | ||
| 202 | .replace(/[./_-]+$/g, ""); | ||
| 203 | |||
| 204 | return branchFragment.length > 0 ? branchFragment : "update"; | ||
| 205 | } | ||
| 206 | |||
| 207 | /** | ||
| 208 | * Sanitize a string into a `feature/…` branch name. | ||
| 209 | * Preserves an existing `feature/` prefix or slash-separated namespace. | ||
| 210 | */ | ||
| 211 | export function sanitizeFeatureBranchName(raw: string): string { | ||
| 212 | const sanitized = sanitizeBranchFragment(raw); | ||
| 213 | if (sanitized.includes("/")) { | ||
| 214 | return sanitized.startsWith("feature/") ? sanitized : `feature/${sanitized}`; | ||
| 215 | } | ||
| 216 | return `feature/${sanitized}`; | ||
| 217 | } |