Skip to content

Commit 4c3fd67

Browse files
committed
refactor: centralize workspace name resolution
Move workspace name length/collision rules into shared helpers so create/rename/fork paths consistently enforce the same constraints: - Add WORKSPACE_NAME_MAX_LENGTH constant and use in validation + task names - Add buildWorkspaceNameWithSuffix() for length-safe suffixing - Add resolveWorkspaceName() with collision strategies (error/random/numeric) - Update create/rename/fork flows to use resolver and avoid name collisions - Ensure auto-fork truncates base names to keep within 64-char limit Tests: - Added workspaceNaming + workspaceNameResolver unit tests - Updated forkNameGenerator tests for length handling - Updated workspaceValidation tests to use shared constant
1 parent 689c8bc commit 4c3fd67

File tree

11 files changed

+253
-74
lines changed

11 files changed

+253
-74
lines changed

src/common/utils/validation/workspaceValidation.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { WORKSPACE_NAME_MAX_LENGTH } from "@/constants/workspaceNaming";
12
import { validateWorkspaceName } from "./workspaceValidation";
23

34
describe("validateWorkspaceName", () => {
@@ -35,7 +36,7 @@ describe("validateWorkspaceName", () => {
3536
});
3637

3738
test("accepts 64 characters", () => {
38-
const name = "a".repeat(64);
39+
const name = "a".repeat(WORKSPACE_NAME_MAX_LENGTH);
3940
expect(validateWorkspaceName(name).valid).toBe(true);
4041
});
4142
});
@@ -48,10 +49,10 @@ describe("validateWorkspaceName", () => {
4849
});
4950

5051
test("rejects names over 64 characters", () => {
51-
const name = "a".repeat(65);
52+
const name = "a".repeat(WORKSPACE_NAME_MAX_LENGTH + 1);
5253
const result = validateWorkspaceName(name);
5354
expect(result.valid).toBe(false);
54-
expect(result.error).toContain("64 characters");
55+
expect(result.error).toContain(`${WORKSPACE_NAME_MAX_LENGTH} characters`);
5556
});
5657

5758
test("rejects uppercase letters", () => {

src/common/utils/validation/workspaceValidation.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@
44
* - Can only contain: lowercase letters, digits, underscore, hyphen
55
* - Pattern: [a-z0-9_-]{1,64}
66
*/
7+
import { WORKSPACE_NAME_MAX_LENGTH } from "@/constants/workspaceNaming";
8+
79
export function validateWorkspaceName(name: string): { valid: boolean; error?: string } {
810
if (!name || name.length === 0) {
911
return { valid: false, error: "Workspace name cannot be empty" };
1012
}
1113

12-
if (name.length > 64) {
13-
return { valid: false, error: "Workspace name cannot exceed 64 characters" };
14+
if (name.length > WORKSPACE_NAME_MAX_LENGTH) {
15+
return {
16+
valid: false,
17+
error: `Workspace name cannot exceed ${WORKSPACE_NAME_MAX_LENGTH} characters`,
18+
};
1419
}
1520

1621
const validPattern = /^[a-z0-9_-]+$/;
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { describe, expect, it } from "bun:test";
2+
import { WORKSPACE_NAME_MAX_LENGTH } from "@/constants/workspaceNaming";
3+
import { buildWorkspaceNameWithSuffix } from "./workspaceNaming";
4+
5+
describe("workspaceNaming", () => {
6+
it("should append suffix without truncation when base fits", () => {
7+
expect(buildWorkspaceNameWithSuffix("short-name", 2)).toBe("short-name-2");
8+
});
9+
10+
it("should truncate base to respect maximum length", () => {
11+
const base = "a".repeat(WORKSPACE_NAME_MAX_LENGTH);
12+
const result = buildWorkspaceNameWithSuffix(base, 2);
13+
expect(result.length).toBe(WORKSPACE_NAME_MAX_LENGTH);
14+
expect(result.endsWith("-2")).toBe(true);
15+
});
16+
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { WORKSPACE_NAME_MAX_LENGTH } from "@/constants/workspaceNaming";
2+
3+
/**
4+
* Build a workspace name with a suffix, trimming the base to fit length limits.
5+
*/
6+
export function buildWorkspaceNameWithSuffix(baseName: string, suffix: string | number): string {
7+
const suffixText = String(suffix);
8+
const reservedLength = suffixText.length + 1; // +1 for '-'
9+
const maxBaseLength = Math.max(1, WORKSPACE_NAME_MAX_LENGTH - reservedLength);
10+
const truncatedBase =
11+
baseName.length > maxBaseLength ? baseName.slice(0, maxBaseLength) : baseName;
12+
return `${truncatedBase}-${suffixText}`;
13+
}

src/constants/workspaceNaming.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
/**
2+
* Workspace naming limits and constraints.
3+
*/
4+
export const WORKSPACE_NAME_MAX_LENGTH = 64;

src/node/services/forkNameGenerator.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, expect, it } from "bun:test";
2+
import { WORKSPACE_NAME_MAX_LENGTH } from "@/constants/workspaceNaming";
23
import {
34
parseWorkspaceName,
45
parseWorkspaceTitle,
@@ -72,6 +73,13 @@ describe("forkNameGenerator", () => {
7273
expect(generateForkName("sidebar")).toBe("sidebar-2");
7374
});
7475

76+
it("should truncate base to fit suffix within limits", () => {
77+
const base = "a".repeat(WORKSPACE_NAME_MAX_LENGTH);
78+
const result = generateForkName(base);
79+
expect(result.length).toBe(WORKSPACE_NAME_MAX_LENGTH);
80+
expect(result.endsWith("-2")).toBe(true);
81+
});
82+
7583
it("should increment suffix for subsequent forks", () => {
7684
expect(generateForkName("bugs-asd23-2")).toBe("bugs-asd23-3");
7785
expect(generateForkName("bugs-asd23-3")).toBe("bugs-asd23-4");

src/node/services/forkNameGenerator.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
* Fork 2: title "Fixing bugs 3", name "bugs-asd23-3"
88
*/
99

10+
import { buildWorkspaceNameWithSuffix } from "@/common/utils/workspaceNaming";
11+
1012
/**
1113
* Extract the base name and current suffix from a workspace name.
1214
* Returns { base, suffix } where suffix is the numeric fork count (0 if not a fork).
@@ -61,7 +63,7 @@ export function parseWorkspaceTitle(title: string): { base: string; suffix: numb
6163
export function generateForkName(sourceName: string): string {
6264
const { base, suffix } = parseWorkspaceName(sourceName);
6365
const nextSuffix = suffix === 0 ? 2 : suffix + 1;
64-
return `${base}-${nextSuffix}`;
66+
return buildWorkspaceNameWithSuffix(base, nextSuffix);
6567
}
6668

6769
/**
@@ -90,7 +92,7 @@ export function generateForkTitleWithSuffix(sourceTitle: string, suffix: number)
9092
*/
9193
export function generateForkNameWithSuffix(sourceName: string, suffix: number): string {
9294
const { base } = parseWorkspaceName(sourceName);
93-
return `${base}-${suffix}`;
95+
return buildWorkspaceNameWithSuffix(base, suffix);
9496
}
9597

9698
/**
@@ -102,7 +104,7 @@ export function findNextForkSuffix(sourceName: string, existingNames: Set<string
102104
let nextSuffix = currentSuffix === 0 ? 2 : currentSuffix + 1;
103105

104106
// Find next available suffix
105-
while (existingNames.has(`${base}-${nextSuffix}`)) {
107+
while (existingNames.has(buildWorkspaceNameWithSuffix(base, nextSuffix))) {
106108
nextSuffix++;
107109
}
108110

src/node/services/taskService.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { createRuntimeForWorkspace } from "@/node/runtime/runtimeHelpers";
2020
import { createRuntime, runBackgroundInit } from "@/node/runtime/runtimeFactory";
2121
import type { InitLogger, WorkspaceCreationResult } from "@/node/runtime/Runtime";
2222
import { validateWorkspaceName } from "@/common/utils/validation/workspaceValidation";
23+
import { WORKSPACE_NAME_MAX_LENGTH } from "@/constants/workspaceNaming";
2324
import { Ok, Err, type Result } from "@/common/types/result";
2425
import type { TaskSettings } from "@/common/types/tasks";
2526
import { DEFAULT_TASK_SETTINGS } from "@/common/types/tasks";
@@ -171,14 +172,16 @@ function sanitizeAgentTypeForName(agentType: string): string {
171172
function buildAgentWorkspaceName(agentType: string, workspaceId: string): string {
172173
const safeType = sanitizeAgentTypeForName(agentType);
173174
const base = `agent_${safeType}_${workspaceId}`;
174-
// Hard cap to validation limit (64). Ensure stable suffix is preserved.
175-
if (base.length <= 64) return base;
175+
// Hard cap to validation limit. Ensure stable suffix is preserved.
176+
if (base.length <= WORKSPACE_NAME_MAX_LENGTH) return base;
176177

177178
const suffix = `_${workspaceId}`;
178-
const maxPrefixLen = 64 - suffix.length;
179+
const maxPrefixLen = WORKSPACE_NAME_MAX_LENGTH - suffix.length;
179180
const prefix = `agent_${safeType}`.slice(0, Math.max(0, maxPrefixLen));
180181
const name = `${prefix}${suffix}`;
181-
return name.length <= 64 ? name : `agent_${workspaceId}`.slice(0, 64);
182+
return name.length <= WORKSPACE_NAME_MAX_LENGTH
183+
? name
184+
: `agent_${workspaceId}`.slice(0, WORKSPACE_NAME_MAX_LENGTH);
182185
}
183186

184187
function getIsoNow(): string {
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { describe, expect, it } from "bun:test";
2+
import { WORKSPACE_NAME_MAX_LENGTH } from "@/constants/workspaceNaming";
3+
import { resolveWorkspaceName } from "./workspaceNameResolver";
4+
5+
describe("workspaceNameResolver", () => {
6+
it("should reject invalid workspace names", () => {
7+
const result = resolveWorkspaceName("Invalid-Name", new Set(), { type: "error" });
8+
expect(result.success).toBe(false);
9+
if (!result.success) {
10+
expect(result.error).toContain("Use only");
11+
}
12+
});
13+
14+
it("should return the requested name when no collision exists", () => {
15+
const result = resolveWorkspaceName("valid-name", new Set(), { type: "error" });
16+
expect(result).toEqual({ success: true, data: { name: "valid-name" } });
17+
});
18+
19+
it("should error on collisions when using error strategy", () => {
20+
const result = resolveWorkspaceName("existing", new Set(["existing"]), { type: "error" });
21+
expect(result.success).toBe(false);
22+
if (!result.success) {
23+
expect(result.error).toContain("already exists");
24+
}
25+
});
26+
27+
it("should generate numeric suffix for forks", () => {
28+
const result = resolveWorkspaceName("feature", new Set(["feature", "feature-2"]), {
29+
type: "numeric-suffix",
30+
});
31+
expect(result).toEqual({ success: true, data: { name: "feature-3", suffix: 3 } });
32+
});
33+
34+
it("should generate random suffix within length limits", () => {
35+
const base = "a".repeat(WORKSPACE_NAME_MAX_LENGTH);
36+
const existingNames = new Set([base]);
37+
const originalRandom = Math.random;
38+
try {
39+
Math.random = () => 0.123456;
40+
const result = resolveWorkspaceName(base, existingNames, {
41+
type: "random-suffix",
42+
maxAttempts: 1,
43+
});
44+
expect(result.success).toBe(true);
45+
if (!result.success) return;
46+
expect(result.data.name.length).toBe(WORKSPACE_NAME_MAX_LENGTH);
47+
expect(result.data.name.endsWith("-4fzy")).toBe(true);
48+
} finally {
49+
Math.random = originalRandom;
50+
}
51+
});
52+
});
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { Err, Ok, type Result } from "@/common/types/result";
2+
import { validateWorkspaceName } from "@/common/utils/validation/workspaceValidation";
3+
import { buildWorkspaceNameWithSuffix } from "@/common/utils/workspaceNaming";
4+
import { findNextForkSuffix, generateForkNameWithSuffix } from "@/node/services/forkNameGenerator";
5+
6+
export type WorkspaceNameCollisionStrategy =
7+
| { type: "error" }
8+
| { type: "random-suffix"; maxAttempts: number }
9+
| { type: "numeric-suffix" };
10+
11+
export interface WorkspaceNameResolution {
12+
name: string;
13+
suffix?: number;
14+
}
15+
16+
function generateRandomSuffix(): string {
17+
return Math.random().toString(36).substring(2, 6);
18+
}
19+
20+
function validateResolvedName(name: string): Result<string> {
21+
const validation = validateWorkspaceName(name);
22+
if (!validation.valid) {
23+
return Err(validation.error ?? "Invalid workspace name");
24+
}
25+
return Ok(name);
26+
}
27+
28+
/**
29+
* Resolve a workspace name using a collision strategy and shared validation rules.
30+
*/
31+
export function resolveWorkspaceName(
32+
requestedName: string,
33+
existingNames: Set<string>,
34+
strategy: WorkspaceNameCollisionStrategy
35+
): Result<WorkspaceNameResolution> {
36+
const validation = validateWorkspaceName(requestedName);
37+
if (!validation.valid) {
38+
return Err(validation.error ?? "Invalid workspace name");
39+
}
40+
41+
if (strategy.type === "numeric-suffix") {
42+
const suffix = findNextForkSuffix(requestedName, existingNames);
43+
const candidate = generateForkNameWithSuffix(requestedName, suffix);
44+
const resolved = validateResolvedName(candidate);
45+
if (!resolved.success) {
46+
return Err(resolved.error);
47+
}
48+
return Ok({ name: resolved.data, suffix });
49+
}
50+
51+
if (!existingNames.has(requestedName)) {
52+
return Ok({ name: requestedName });
53+
}
54+
55+
if (strategy.type === "error") {
56+
return Err(`Workspace with name "${requestedName}" already exists`);
57+
}
58+
59+
const attemptedNames = new Set(existingNames);
60+
for (let attempt = 0; attempt < strategy.maxAttempts; attempt++) {
61+
const suffix = generateRandomSuffix();
62+
const candidate = buildWorkspaceNameWithSuffix(requestedName, suffix);
63+
if (attemptedNames.has(candidate)) {
64+
continue;
65+
}
66+
attemptedNames.add(candidate);
67+
const resolved = validateResolvedName(candidate);
68+
if (!resolved.success) {
69+
return Err(resolved.error);
70+
}
71+
return Ok({ name: resolved.data });
72+
}
73+
74+
return Err(`Unable to resolve unique workspace name for "${requestedName}"`);
75+
}

0 commit comments

Comments
 (0)