authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-02-28 23:27:12-08:00
committergravatar for jmarminge@gmail.comJulius Marminge <jmarminge@gmail.com> 2026-03-01 20:06:21-08:00
log913d0409f6b38b7652435f581f1e38768e4cb27a
tree609d8c74414e07559a8be703fcfad3dd40c9529a
parentac0e720de19e452b75cd1a13cb576b0dec242721

Handle branch-name generation failures as typed errors

- 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

5 files changed, 114 insertions(+), 113 deletions(-)

apps/server/src/git/Layers/CodexTextGeneration.test.ts+17-6
...@@ -374,7 +374,7 @@ it.layer(CodexTextGenerationTestLayer)("CodexTextGenerationLive", (it) => {...@@ -374,7 +374,7 @@ it.layer(CodexTextGenerationTestLayer)("CodexTextGenerationLive", (it) => {
374 ),374 ),
375 );375 );
376376
377 it.effect("returns null branch when codex returns the wrong object shape", () =>377 it.effect("fails with typed TextGenerationError when codex returns wrong branch payload shape", () =>
378 withFakeCodexEnv(378 withFakeCodexEnv(
379 {379 {
380 output: JSON.stringify({380 output: JSON.stringify({
...@@ -384,12 +384,23 @@ it.layer(CodexTextGenerationTestLayer)("CodexTextGenerationLive", (it) => {...@@ -384,12 +384,23 @@ it.layer(CodexTextGenerationTestLayer)("CodexTextGenerationLive", (it) => {
384 Effect.gen(function* () {384 Effect.gen(function* () {
385 const textGeneration = yield* TextGeneration;385 const textGeneration = yield* TextGeneration;
386386
387 const generated = yield* textGeneration.generateBranchName({387 const result = yield* textGeneration
388 cwd: process.cwd(),388 .generateBranchName({
389 message: "Fix websocket reconnect flake",389 cwd: process.cwd(),
390 });390 message: "Fix websocket reconnect flake",
391 })
392 .pipe(
393 Effect.match({
394 onFailure: (error) => ({ _tag: "Left" as const, left: error }),
395 onSuccess: (value) => ({ _tag: "Right" as const, right: value }),
396 }),
397 );
391398
392 expect(generated.branch).toBeNull();399 expect(result._tag).toBe("Left");
400 if (result._tag === "Left") {
401 expect(result.left).toBeInstanceOf(TextGenerationError);
402 expect(result.left.message).toContain("Codex returned invalid structured output");
403 }
393 }),404 }),
394 ),405 ),
395 );406 );
apps/server/src/git/Layers/CodexTextGeneration.ts+2-12
...@@ -552,20 +552,10 @@ const makeCodexTextGeneration = Effect.gen(function* () {...@@ -552,20 +552,10 @@ const makeCodexTextGeneration = Effect.gen(function* () {
552 outputSchemaJson: BRANCH_NAME_OUTPUT_SCHEMA_JSON,552 outputSchemaJson: BRANCH_NAME_OUTPUT_SCHEMA_JSON,
553 imagePaths,553 imagePaths,
554 parse: (raw) => parseBranchNameOutput(raw),554 parse: (raw) => parseBranchNameOutput(raw),
555 }).pipe(555 });
556 Effect.catch((error) =>
557 Effect.gen(function* () {
558 yield* Effect.logWarning("branch-name generation failed, skipping rename", {
559 operation: "generateBranchName",
560 reason: error instanceof Error ? error.message : String(error),
561 });
562 return { branch: null };
563 }),
564 ),
565 );
566556
567 return {557 return {
568 branch: generated.branch ? sanitizeBranchName(generated.branch) : null,558 branch: sanitizeBranchName(generated.branch),
569 } satisfies BranchNameGenerationResult;559 } satisfies BranchNameGenerationResult;
570 });560 });
571 };561 };
apps/server/src/git/Services/TextGeneration.ts+1-1
...@@ -45,7 +45,7 @@ export interface BranchNameGenerationInput {...@@ -45,7 +45,7 @@ export interface BranchNameGenerationInput {
45}45}
4646
47export interface BranchNameGenerationResult {47export interface BranchNameGenerationResult {
48 branch: string | null;48 branch: string;
49}49}
5050
51export interface TextGenerationService {51export interface TextGenerationService {
apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts+10-6
...@@ -18,6 +18,7 @@ import { Effect, Exit, Layer, ManagedRuntime, PubSub, Scope, Stream } from "effe...@@ -18,6 +18,7 @@ import { Effect, Exit, Layer, ManagedRuntime, PubSub, Scope, Stream } from "effe
18import { afterEach, describe, expect, it, vi } from "vitest";18import { afterEach, describe, expect, it, vi } from "vitest";
1919
20import { type ServerConfigShape, ServerConfig } from "../../config.ts";20import { type ServerConfigShape, ServerConfig } from "../../config.ts";
21import { TextGenerationError } from "../../git/Errors.ts";
21import { OrchestrationEventStoreLive } from "../../persistence/Layers/OrchestrationEventStore.ts";22import { OrchestrationEventStoreLive } from "../../persistence/Layers/OrchestrationEventStore.ts";
22import { OrchestrationCommandReceiptRepositoryLive } from "../../persistence/Layers/OrchestrationCommandReceipts.ts";23import { OrchestrationCommandReceiptRepositoryLive } from "../../persistence/Layers/OrchestrationCommandReceipts.ts";
23import { SqlitePersistenceMemory } from "../../persistence/Layers/Sqlite.ts";24import { SqlitePersistenceMemory } from "../../persistence/Layers/Sqlite.ts";
...@@ -123,7 +124,7 @@ describe("ProviderCommandReactor", () => {...@@ -123,7 +124,7 @@ describe("ProviderCommandReactor", () => {
123 );124 );
124 const generateBranchName = vi.fn((_: unknown) =>125 const generateBranchName = vi.fn((_: unknown) =>
125 Effect.succeed({126 Effect.succeed({
126 branch: "generated-name" as string | null,127 branch: "generated-name",
127 }),128 }),
128 );129 );
129130
...@@ -447,13 +448,16 @@ describe("ProviderCommandReactor", () => {...@@ -447,13 +448,16 @@ describe("ProviderCommandReactor", () => {
447 expect(harness.renameBranch.mock.calls.length).toBe(1);448 expect(harness.renameBranch.mock.calls.length).toBe(1);
448 });449 });
449450
450 it("skips worktree rename when generated branch is null", async () => {451 it("skips worktree rename when branch-name generation fails", async () => {
451 const harness = await createHarness();452 const harness = await createHarness();
452 const now = new Date().toISOString();453 const now = new Date().toISOString();
453 harness.generateBranchName.mockImplementationOnce((_: unknown) =>454 harness.generateBranchName.mockImplementationOnce(() =>
454 Effect.succeed({455 Effect.fail(
455 branch: null,456 new TextGenerationError({
456 }),457 operation: "generateBranchName",
458 detail: "model returned invalid payload",
459 }),
460 ),
457 );461 );
458462
459 await Effect.runPromise(463 await Effect.runPromise(
apps/server/src/orchestration/Layers/ProviderCommandReactor.ts+84-88
...@@ -17,7 +17,10 @@ import {...@@ -17,7 +17,10 @@ import {
17import { Cache, Cause, Duration, Effect, Layer, Option, Queue, Stream } from "effect";17import { Cache, Cause, Duration, Effect, Layer, Option, Queue, Stream } from "effect";
1818
19import { resolveThreadWorkspaceCwd } from "../../checkpointing/Utils.ts";19import { resolveThreadWorkspaceCwd } from "../../checkpointing/Utils.ts";
20import { resolveAttachmentRelativePath, resolveAttachmentRoutePath } from "../../attachmentPaths.ts";20import {
21 resolveAttachmentRelativePath,
22 resolveAttachmentRoutePath,
23} from "../../attachmentPaths.ts";
21import { ServerConfig } from "../../config.ts";24import { ServerConfig } from "../../config.ts";
22import { GitCore } from "../../git/Services/GitCore.ts";25import { GitCore } from "../../git/Services/GitCore.ts";
23import { TextGeneration } from "../../git/Services/TextGeneration.ts";26import { TextGeneration } from "../../git/Services/TextGeneration.ts";
...@@ -229,7 +232,8 @@ const make = Effect.gen(function* () {...@@ -229,7 +232,8 @@ const make = Effect.gen(function* () {
229 const existingSessionId = thread.session?.providerSessionId;232 const existingSessionId = thread.session?.providerSessionId;
230 if (existingSessionId) {233 if (existingSessionId) {
231 const approvalPolicyChanged =234 const approvalPolicyChanged =
232 options?.approvalPolicy !== undefined && options.approvalPolicy !== thread.session?.approvalPolicy;235 options?.approvalPolicy !== undefined &&
236 options.approvalPolicy !== thread.session?.approvalPolicy;
233 const sandboxModeChanged =237 const sandboxModeChanged =
234 options?.sandboxMode !== undefined && options.sandboxMode !== thread.session?.sandboxMode;238 options?.sandboxMode !== undefined && options.sandboxMode !== thread.session?.sandboxMode;
235239
...@@ -237,7 +241,9 @@ const make = Effect.gen(function* () {...@@ -237,7 +241,9 @@ const make = Effect.gen(function* () {
237 return existingSessionId;241 return existingSessionId;
238 }242 }
239243
240 const restartedSession = yield* startProviderSession(thread.session?.providerThreadId ?? null);244 const restartedSession = yield* startProviderSession(
245 thread.session?.providerThreadId ?? null,
246 );
241 yield* bindSessionToThread(restartedSession);247 yield* bindSessionToThread(restartedSession);
242 yield* providerService.stopSession({ sessionId: existingSessionId }).pipe(248 yield* providerService.stopSession({ sessionId: existingSessionId }).pipe(
243 Effect.catchCause((cause) =>249 Effect.catchCause((cause) =>
...@@ -286,13 +292,11 @@ const make = Effect.gen(function* () {...@@ -286,13 +292,11 @@ const make = Effect.gen(function* () {
286 });292 });
287 });293 });
288294
289 const resolvePersistedAttachments = Effect.fnUntraced(function* (295 const resolvePersistedAttachments = Effect.fnUntraced(function* (input: {
290 input: {296 readonly threadId: ThreadId;
291 readonly threadId: ThreadId;297 readonly messageId: string;
292 readonly messageId: string;298 readonly attachments: ReadonlyArray<ChatAttachment> | undefined;
293 readonly attachments: ReadonlyArray<ChatAttachment> | undefined;299 }) {
294 },
295 ) {
296 if (!input.attachments || input.attachments.length === 0) {300 if (!input.attachments || input.attachments.length === 0) {
297 return input.attachments ?? [];301 return input.attachments ?? [];
298 }302 }
...@@ -303,54 +307,52 @@ const make = Effect.gen(function* () {...@@ -303,54 +307,52 @@ const make = Effect.gen(function* () {
303 const threadSegment = encodeURIComponent(input.threadId);307 const threadSegment = encodeURIComponent(input.threadId);
304 const messageSegment = encodeURIComponent(input.messageId);308 const messageSegment = encodeURIComponent(input.messageId);
305309
306 return yield* Effect.forEach(310 return yield* Effect.forEach(Array.from(input.attachments.entries()), ([index, attachment]) =>
307 Array.from(input.attachments.entries()),311 Effect.gen(function* () {
308 ([index, attachment]) =>312 if (attachment.type !== "image") {
309 Effect.gen(function* () {313 return attachment;
310 if (attachment.type !== "image") {314 }
311 return attachment;
312 }
313
314 const resolvedRoutePath = resolveAttachmentRoutePath({
315 stateDir: serverConfig.value.stateDir,
316 dataUrl: attachment.dataUrl,
317 });
318 const resolvedMaterializedPath =
319 resolvedRoutePath ??
320 (() => {
321 const parsed = parseBase64DataUrl(attachment.dataUrl);
322 if (!parsed || !parsed.mimeType.startsWith("image/")) {
323 return null;
324 }
325 const extension = inferImageExtension({
326 mimeType: parsed.mimeType,
327 fileName: attachment.name,
328 });
329 return resolveAttachmentRelativePath({
330 stateDir: serverConfig.value.stateDir,
331 relativePath: `${threadSegment}/${messageSegment}-${index}${extension}`,
332 });
333 })();
334
335 if (!resolvedMaterializedPath) {
336 return attachment;
337 }
338315
339 const isFile = yield* Effect.sync(() => {316 const resolvedRoutePath = resolveAttachmentRoutePath({
340 try {317 stateDir: serverConfig.value.stateDir,
341 return fs.statSync(resolvedMaterializedPath).isFile();318 dataUrl: attachment.dataUrl,
342 } catch {319 });
343 return false;320 const resolvedMaterializedPath =
321 resolvedRoutePath ??
322 (() => {
323 const parsed = parseBase64DataUrl(attachment.dataUrl);
324 if (!parsed || !parsed.mimeType.startsWith("image/")) {
325 return null;
344 }326 }
345 });327 const extension = inferImageExtension({
346 if (!isFile) {328 mimeType: parsed.mimeType,
347 return attachment;329 fileName: attachment.name,
330 });
331 return resolveAttachmentRelativePath({
332 stateDir: serverConfig.value.stateDir,
333 relativePath: `${threadSegment}/${messageSegment}-${index}${extension}`,
334 });
335 })();
336
337 if (!resolvedMaterializedPath) {
338 return attachment;
339 }
340
341 const isFile = yield* Effect.sync(() => {
342 try {
343 return fs.statSync(resolvedMaterializedPath).isFile();
344 } catch {
345 return false;
348 }346 }
349 return {347 });
350 ...attachment,348 if (!isFile) {
351 dataUrl: resolvedMaterializedPath,349 return attachment;
352 } satisfies ChatAttachment;350 }
353 }),351 return {
352 ...attachment,
353 dataUrl: resolvedMaterializedPath,
354 } satisfies ChatAttachment;
355 }),
354 );356 );
355 });357 });
356358
...@@ -382,51 +384,45 @@ const make = Effect.gen(function* () {...@@ -382,51 +384,45 @@ const make = Effect.gen(function* () {
382 const oldBranch = input.branch;384 const oldBranch = input.branch;
383 const cwd = input.worktreePath;385 const cwd = input.worktreePath;
384 const attachments = input.attachments ?? [];386 const attachments = input.attachments ?? [];
385 const renameEffect = textGeneration387 yield* textGeneration
386 .generateBranchName({388 .generateBranchName({
387 cwd,389 cwd,
388 message: input.messageText,390 message: input.messageText,
389 ...(attachments.length > 0 ? { attachments } : {}),391 ...(attachments.length > 0 ? { attachments } : {}),
390 })392 })
391 .pipe(393 .pipe(
394 Effect.catch((error) =>
395 Effect.logWarning(
396 "provider command reactor failed to generate worktree branch name; skipping rename",
397 { threadId: input.threadId, cwd, oldBranch, reason: error.message },
398 ),
399 ),
392 Effect.flatMap((generated) => {400 Effect.flatMap((generated) => {
393 if (!generated.branch) {401 if (!generated) return Effect.void;
394 return Effect.void;402
395 }
396 const targetBranch = buildGeneratedWorktreeBranchName(generated.branch);403 const targetBranch = buildGeneratedWorktreeBranchName(generated.branch);
397 if (targetBranch === oldBranch) {404 if (targetBranch === oldBranch) return Effect.void;
398 return Effect.void;405
399 }406 return Effect.flatMap(
400 return git407 git.renameBranch({ cwd, oldBranch, newBranch: targetBranch }),
401 .renameBranch({408 (renamed) =>
402 cwd,409 orchestrationEngine.dispatch({
403 oldBranch,410 type: "thread.meta.update",
404 newBranch: targetBranch,411 commandId: serverCommandId("worktree-branch-rename"),
405 })412 threadId: input.threadId,
406 .pipe(413 branch: renamed.branch,
407 Effect.flatMap((renamed) =>414 worktreePath: cwd,
408 orchestrationEngine.dispatch({415 }),
409 type: "thread.meta.update",416 );
410 commandId: serverCommandId("worktree-branch-rename"),
411 threadId: input.threadId,
412 branch: renamed.branch,
413 worktreePath: cwd,
414 }),
415 ),
416 Effect.asVoid,
417 );
418 }),417 }),
419 Effect.catchCause((cause) =>418 Effect.catchCause((cause) =>
420 Effect.logWarning("provider command reactor failed to generate or rename worktree branch", {419 Effect.logWarning(
421 threadId: input.threadId,420 "provider command reactor failed to generate or rename worktree branch",
422 cwd,421 { threadId: input.threadId, cwd, oldBranch, cause: Cause.pretty(cause) },
423 oldBranch,422 ),
424 cause: Cause.pretty(cause),
425 }),
426 ),423 ),
424 Effect.forkScoped,
427 );425 );
428
429 yield* Effect.forkScoped(renameEffect);
430 });426 });
431427
432 const processTurnStartRequested = Effect.fnUntraced(function* (428 const processTurnStartRequested = Effect.fnUntraced(function* (