Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const BRANCH_NAME_INSTRUCTIONS =
"Generate a concise git branch name (2-4 words, kebab-case, descriptive, 20 characters or less). Return ONLY the branch name, nothing else.";

const MAX_BRANCH_LENGTH = 100;
const GENERATE_TIMEOUT_MS = 5_000;

/**
* Light sanitizer for AI-generated branch names — lowercase, kebab-case,
Expand Down Expand Up @@ -35,14 +36,22 @@ export async function generateBranchNameFromPrompt(

let generated: string | null;
try {
generated = await generateTitleFromMessage({
message: prompt,
agentModel: model,
agentId: "branch-namer",
agentName: "Branch Namer",
instructions: BRANCH_NAME_INSTRUCTIONS,
tracingContext: { surface: "host-service-branch-name" },
});
generated = await Promise.race([
generateTitleFromMessage({
message: prompt,
agentModel: model,
agentId: "branch-namer",
agentName: "Branch Namer",
instructions: BRANCH_NAME_INSTRUCTIONS,
tracingContext: { surface: "host-service-branch-name" },
}),
new Promise<never>((_, reject) =>
setTimeout(
() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
GENERATE_TIMEOUT_MS,
),
),
]);
Comment on lines +39 to +54
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Same timer-leak as in ai-workspace-names.ts — clear the timeout handle.

The setTimeout fires even when generateTitleFromMessage completes within 5 s. The same finally(() => clearTimeout(timeoutId)) fix applies here.

🛠️ Proposed fix
 	try {
-		generated = await Promise.race([
-			generateTitleFromMessage({
-				message: prompt,
-				agentModel: model,
-				agentId: "branch-namer",
-				agentName: "Branch Namer",
-				instructions: BRANCH_NAME_INSTRUCTIONS,
-				tracingContext: { surface: "host-service-branch-name" },
-			}),
-			new Promise<never>((_, reject) =>
-				setTimeout(
-					() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
-					GENERATE_TIMEOUT_MS,
-				),
-			),
-		]);
+		let timeoutId: ReturnType<typeof setTimeout>;
+		generated = await Promise.race([
+			generateTitleFromMessage({
+				message: prompt,
+				agentModel: model,
+				agentId: "branch-namer",
+				agentName: "Branch Namer",
+				instructions: BRANCH_NAME_INSTRUCTIONS,
+				tracingContext: { surface: "host-service-branch-name" },
+			}),
+			new Promise<never>((_, reject) => {
+				timeoutId = setTimeout(
+					() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
+					GENERATE_TIMEOUT_MS,
+				);
+			}),
+		]).finally(() => clearTimeout(timeoutId));
📝 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.

Suggested change
generated = await Promise.race([
generateTitleFromMessage({
message: prompt,
agentModel: model,
agentId: "branch-namer",
agentName: "Branch Namer",
instructions: BRANCH_NAME_INSTRUCTIONS,
tracingContext: { surface: "host-service-branch-name" },
}),
new Promise<never>((_, reject) =>
setTimeout(
() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
GENERATE_TIMEOUT_MS,
),
),
]);
let timeoutId: ReturnType<typeof setTimeout>;
generated = await Promise.race([
generateTitleFromMessage({
message: prompt,
agentModel: model,
agentId: "branch-namer",
agentName: "Branch Namer",
instructions: BRANCH_NAME_INSTRUCTIONS,
tracingContext: { surface: "host-service-branch-name" },
}),
new Promise<never>((_, reject) => {
timeoutId = setTimeout(
() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
GENERATE_TIMEOUT_MS,
);
}),
]).finally(() => clearTimeout(timeoutId));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/host-service/src/trpc/router/workspace-creation/utils/ai-branch-name.ts`
around lines 39 - 54, The Promise.race call with generateTitleFromMessage and
setTimeout leaks the timer because the timeout still fires if
generateTitleFromMessage resolves first; change the code in ai-branch-name.ts so
you capture the setTimeout handle (e.g. const timeoutId = setTimeout(...)) and
ensure you clearTimeout(timeoutId) once the race settles (use a finally block or
clear after await) around the Promise.race that assigns generated; keep
references to generateTitleFromMessage, BRANCH_NAME_INSTRUCTIONS, and
GENERATE_TIMEOUT_MS so the timeout duration and agent call remain unchanged
while preventing the timer leak.

} catch (error) {
console.warn("[generateBranchNameFromPrompt] generation failed:", error);
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { deduplicateBranchName } from "./sanitize-branch";

const WORKSPACE_TITLE_MAX = 150;
const BRANCH_NAME_MAX = 25;
const GENERATE_TIMEOUT_MS = 5_000;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 GENERATE_TIMEOUT_MS is now declared independently in both ai-workspace-names.ts and ai-branch-name.ts. If the desired timeout is ever adjusted, both files must be updated in sync — a silent divergence risk. Consider moving this constant to a shared constants.ts (or ai-utils.ts) in the same utils/ directory and importing it from both call-sites.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/workspace-creation/utils/ai-workspace-names.ts
Line: 12

Comment:
`GENERATE_TIMEOUT_MS` is now declared independently in both `ai-workspace-names.ts` and `ai-branch-name.ts`. If the desired timeout is ever adjusted, both files must be updated in sync — a silent divergence risk. Consider moving this constant to a shared `constants.ts` (or `ai-utils.ts`) in the same `utils/` directory and importing it from both call-sites.

How can I resolve this? If you propose a fix, please make it concise.


function sanitizeBranchCandidate(raw: string): string {
return raw
Expand Down Expand Up @@ -81,12 +82,20 @@ export async function generateWorkspaceNamesFromPrompt(
});

try {
const { object } = await agent.generate(cleaned, {
structuredOutput: {
schema: workspaceNamesSchema,
jsonPromptInjection: true,
},
});
const { object } = await Promise.race([
agent.generate(cleaned, {
structuredOutput: {
schema: workspaceNamesSchema,
jsonPromptInjection: true,
},
}),
new Promise<never>((_, reject) =>
setTimeout(
() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
GENERATE_TIMEOUT_MS,
),
),
]);
Comment on lines +85 to +98
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clear the timeout handle when agent.generate wins the race.

When the LLM responds before 5 s the setTimeout still fires, allocates an Error, and calls the already-no-op reject. In a busy host-service process this accumulates one dangling 5-second timer per successful workspace creation. Storing the handle and clearing it in a finally (or in each branch) removes the waste entirely.

🛠️ Proposed fix
 	try {
-		const { object } = await Promise.race([
-			agent.generate(cleaned, {
-				structuredOutput: {
-					schema: workspaceNamesSchema,
-					jsonPromptInjection: true,
-				},
-			}),
-			new Promise<never>((_, reject) =>
-				setTimeout(
-					() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
-					GENERATE_TIMEOUT_MS,
-				),
-			),
-		]);
+		let timeoutId: ReturnType<typeof setTimeout>;
+		const { object } = await Promise.race([
+			agent.generate(cleaned, {
+				structuredOutput: {
+					schema: workspaceNamesSchema,
+					jsonPromptInjection: true,
+				},
+			}),
+			new Promise<never>((_, reject) => {
+				timeoutId = setTimeout(
+					() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
+					GENERATE_TIMEOUT_MS,
+				);
+			}),
+		]).finally(() => clearTimeout(timeoutId));
 		return object;
📝 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.

Suggested change
const { object } = await Promise.race([
agent.generate(cleaned, {
structuredOutput: {
schema: workspaceNamesSchema,
jsonPromptInjection: true,
},
}),
new Promise<never>((_, reject) =>
setTimeout(
() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
GENERATE_TIMEOUT_MS,
),
),
]);
let timeoutId: ReturnType<typeof setTimeout>;
const { object } = await Promise.race([
agent.generate(cleaned, {
structuredOutput: {
schema: workspaceNamesSchema,
jsonPromptInjection: true,
},
}),
new Promise<never>((_, reject) => {
timeoutId = setTimeout(
() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
GENERATE_TIMEOUT_MS,
);
}),
]).finally(() => clearTimeout(timeoutId));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/host-service/src/trpc/router/workspace-creation/utils/ai-workspace-names.ts`
around lines 85 - 98, The Promise.race that calls agent.generate(...) alongside
a setTimeout creates a dangling timer when agent.generate resolves first; modify
the implementation around agent.generate, workspaceNamesSchema and
GENERATE_TIMEOUT_MS to store the timer handle returned by setTimeout and ensure
you clear it (clearTimeout) once agent.generate settles (either in a finally
block or immediately after the race resolves) so the timeout callback is
cancelled and no dangling timers accumulate.

return object;
} catch (error) {
Comment on lines 84 to 100
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Timer leak on the happy path: when agent.generate resolves before the 5 s deadline, the setTimeout callback is never cancelled. The timer fires 5 seconds later, calls reject on an already-settled promise (harmless but wasteful), and the dangling handle keeps the garbage collector from reclaiming the closure. Under load, many concurrent workspace-creation calls will accumulate dozens of live timers simultaneously. Clearing the handle when the race resolves avoids this.

Suggested change
try {
const { object } = await agent.generate(cleaned, {
structuredOutput: {
schema: workspaceNamesSchema,
jsonPromptInjection: true,
},
});
const { object } = await Promise.race([
agent.generate(cleaned, {
structuredOutput: {
schema: workspaceNamesSchema,
jsonPromptInjection: true,
},
}),
new Promise<never>((_, reject) =>
setTimeout(
() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
GENERATE_TIMEOUT_MS,
),
),
]);
return object;
} catch (error) {
let timeoutHandle: ReturnType<typeof setTimeout> | undefined;
try {
const { object } = await Promise.race([
agent.generate(cleaned, {
structuredOutput: {
schema: workspaceNamesSchema,
jsonPromptInjection: true,
},
}),
new Promise<never>((_, reject) => {
timeoutHandle = setTimeout(
() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
GENERATE_TIMEOUT_MS,
);
}),
]);
clearTimeout(timeoutHandle);
return object;
} catch (error) {
clearTimeout(timeoutHandle);
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/workspace-creation/utils/ai-workspace-names.ts
Line: 84-100

Comment:
Timer leak on the happy path: when `agent.generate` resolves before the 5 s deadline, the `setTimeout` callback is never cancelled. The timer fires 5 seconds later, calls `reject` on an already-settled promise (harmless but wasteful), and the dangling handle keeps the garbage collector from reclaiming the closure. Under load, many concurrent workspace-creation calls will accumulate dozens of live timers simultaneously. Clearing the handle when the race resolves avoids this.

```suggestion
	let timeoutHandle: ReturnType<typeof setTimeout> | undefined;
	try {
		const { object } = await Promise.race([
			agent.generate(cleaned, {
				structuredOutput: {
					schema: workspaceNamesSchema,
					jsonPromptInjection: true,
				},
			}),
			new Promise<never>((_, reject) => {
				timeoutHandle = setTimeout(
					() => reject(new Error(`timed out after ${GENERATE_TIMEOUT_MS}ms`)),
					GENERATE_TIMEOUT_MS,
				);
			}),
		]);
		clearTimeout(timeoutHandle);
		return object;
	} catch (error) {
		clearTimeout(timeoutHandle);
```

How can I resolve this? If you propose a fix, please make it concise.

console.warn(
Expand Down
Loading