Skip to content

Commit 87ab84d

Browse files
xesrevinuGit Agent
andcommitted
fix: refactor service modules for error handling
- Use HttpClient for gitignore fetch with robust status handling - Normalize and enhance OpenAI model detection logic - Improve VCS ignore handling by introducing checkIgnoreError - Remove direct repoRoot calls and mergeAndSaveScopes for simplification - Improve hooks error mapping to ConfigError across execution refactors across multiple service modules improve error handling and general robustness by introducing http-based fetch, normalization, and clearer error mapping Co-Authored-By: Git Agent <noreply@git-agent.dev>
1 parent 7d85078 commit 87ab84d

5 files changed

Lines changed: 62 additions & 41 deletions

File tree

src/services/commit-service.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { executeHooks } from "./hooks";
99
import { generateCommitMessage, planCommits, type ProviderConfig } from "./openai-client";
1010
import { generateProjectScopes } from "./scope-service";
1111
import type { VcsClient, VcsDiff } from "./vcs";
12-
import { projectConfigPath, mergeAndSaveScopes } from "../config/project";
1312

1413
const maxHookRetries = 3;
1514
const maxReplans = 2;
@@ -204,8 +203,6 @@ const withAutoScopes = Effect.fn(function* (
204203
...(projectConfig ?? emptyProjectConfig()),
205204
scopes,
206205
} satisfies ProjectConfig;
207-
const repoRoot = yield* vcs.repoRoot(cwd);
208-
yield* mergeAndSaveScopes(yield* projectConfigPath(repoRoot), scopes);
209206
return nextConfig;
210207
});
211208

@@ -457,8 +454,6 @@ const commitGit = Effect.fn(function* (request: CommitRequest, config: ProjectCo
457454
},
458455
},
459456
);
460-
const repoRoot = yield* request.vcs.repoRoot(request.cwd);
461-
yield* mergeAndSaveScopes(yield* projectConfigPath(repoRoot), refreshedScopes);
462457
groups = yield* Effect.withSpan(
463458
planGroups(request.provider, promptStaged.files, promptUnstaged.files, request.intent, {
464459
...configWithScopes,

src/services/gitignore-service.ts

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { platform } from "node:os";
22
import { Effect, FileSystem, Path } from "effect";
3+
import { HttpClient } from "effect/unstable/http";
34
import { ApiError } from "../shared/errors";
45
import { buildEnvironment } from "../config/env";
56
import { detectTechnologies, type ProviderConfig } from "./openai-client";
@@ -99,28 +100,30 @@ const mergeGitignore = (existing: string, generated: string): string => {
99100
const fetchGitignore = Effect.fn(function* (technologies: ReadonlyArray<string>) {
100101
const env = yield* buildEnvironment;
101102
const baseUrl = env.gitignoreBaseUrl.replace(/\/$/, "");
102-
return yield* Effect.tryPromise({
103-
try: async () => {
104-
const response = await fetch(
105-
`${baseUrl}/${technologies.map((item) => encodeURIComponent(item)).join(",")}`,
106-
);
107-
const text = await response.text();
108-
if (!response.ok) {
109-
throw new ApiError({
110-
message: `failed to fetch gitignore template (${response.status})`,
111-
status: response.status,
112-
body: text,
113-
});
114-
}
115-
return text;
116-
},
117-
catch: (cause) =>
103+
const url = `${baseUrl}/${technologies.map((item) => encodeURIComponent(item)).join(",")}`;
104+
return yield* HttpClient.get(url).pipe(
105+
Effect.flatMap((response) =>
106+
Effect.flatMap(response.text, (text) =>
107+
response.status >= 200 && response.status < 300
108+
? Effect.succeed(text)
109+
: Effect.failSync(
110+
() =>
111+
new ApiError({
112+
message: `failed to fetch gitignore template (${response.status})`,
113+
status: response.status,
114+
body: text,
115+
}),
116+
),
117+
),
118+
),
119+
Effect.mapError((cause) =>
118120
ApiError.is(cause)
119121
? cause
120122
: new ApiError({
121123
message: cause instanceof Error ? cause.message : String(cause),
122124
}),
123-
});
125+
),
126+
);
124127
});
125128

126129
export const generateGitignore = Effect.fn(function* (

src/services/hooks.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,14 @@ const encodeHookInputToJson = Schema.encodeUnknownSync(Schema.Json);
3131

3232
const executeShellHook = Effect.fn(function* (path: string, input: HookInput) {
3333
const fs = yield* FileSystem.FileSystem;
34-
const info = yield* fs.stat(path).pipe(Effect.catch(() => Effect.succeed(undefined)));
35-
36-
if (info == null) {
37-
return {
38-
exitCode: 0,
39-
stderr: "",
40-
} satisfies HookResult;
41-
}
34+
const info = yield* fs.stat(path).pipe(
35+
Effect.mapError(
36+
(cause) =>
37+
new ConfigError({
38+
message: `failed to read hook "${path}": ${cause.message}`,
39+
}),
40+
),
41+
);
4242

4343
if ((info.mode & 0o111) === 0) {
4444
return yield* new ConfigError({

src/services/openai-client.ts

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,24 +50,39 @@ const llmTransientRetrySchedule = Schedule.either(
5050
),
5151
);
5252

53-
const isReasoningModel = (model: string): boolean =>
54-
["o1", "o3", "o4"].some(
55-
(prefix) =>
56-
model === prefix || model.startsWith(`${prefix}-`) || model.startsWith(`${prefix}/`),
53+
const normalizeModelId = (model: string): string =>
54+
model
55+
.trim()
56+
.toLowerCase()
57+
.split("/")
58+
.filter((segment) => segment.length > 0)
59+
.at(-1) ?? "";
60+
61+
export const isReasoningModel = (model: string): boolean => {
62+
const modelId = normalizeModelId(model);
63+
64+
return (
65+
modelId.startsWith("o1") ||
66+
modelId.startsWith("o3") ||
67+
modelId.startsWith("o4-mini") ||
68+
modelId.startsWith("codex-mini") ||
69+
modelId.startsWith("computer-use-preview") ||
70+
(modelId.startsWith("gpt-5") && !modelId.startsWith("gpt-5-chat"))
5771
);
72+
};
5873

59-
const makeLanguageModelLayer = (config: ProviderConfig, maxTokens: number) =>
74+
const makeLanguageModelLayer = (config: ProviderConfig, maxOutputTokens: number) =>
6075
OpenAi.OpenAiLanguageModel.layer({
6176
model: config.model,
6277
config: isReasoningModel(config.model)
6378
? {
64-
max_output_tokens: maxTokens,
79+
max_output_tokens: maxOutputTokens,
6580
reasoning: {
6681
effort: "low",
6782
},
6883
}
6984
: {
70-
max_output_tokens: maxTokens,
85+
max_output_tokens: maxOutputTokens,
7186
temperature: 0,
7287
},
7388
}).pipe(
@@ -83,7 +98,7 @@ const makeLanguageModelLayer = (config: ProviderConfig, maxTokens: number) =>
8398
),
8499
);
85100

86-
const callLlm = (config: ProviderConfig, system: string, user: string, maxTokens: number) =>
101+
const callLlm = (config: ProviderConfig, system: string, user: string, maxOutputTokens: number) =>
87102
LanguageModel.generateText({
88103
prompt: [
89104
{ role: "system", content: system },
@@ -102,7 +117,7 @@ const callLlm = (config: ProviderConfig, system: string, user: string, maxTokens
102117
}),
103118
),
104119
),
105-
Effect.provide(makeLanguageModelLayer(config, maxTokens)),
120+
Effect.provide(makeLanguageModelLayer(config, maxOutputTokens)),
106121
Effect.catch((cause) =>
107122
ApiError.is(cause)
108123
? Effect.failSync(() => cause)

src/services/vcs.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ const processError = (command: string, cause: unknown) =>
7070
stderr: cause instanceof Error ? cause.message : String(cause),
7171
});
7272

73+
const checkIgnoreError = (result: ProcessResult) =>
74+
new ProcessExecutionError({
75+
command: "git check-ignore --stdin",
76+
exitCode: result.exitCode,
77+
stdout: result.stdout,
78+
stderr: result.stderr,
79+
});
80+
7381
const emptyIgnoreRules = (): IgnoreRules => ({
7482
directoryNames: new Set(),
7583
relativePaths: new Set(),
@@ -265,7 +273,7 @@ export const GitClientLive = Layer.effect(
265273
return new Set([...fallbackIgnored, ...normalizeLines(result.stdout).map(toPortablePath)]);
266274
}
267275

268-
return fallbackIgnored;
276+
return yield* Effect.failSync(() => checkIgnoreError(result));
269277
});
270278

271279
const buildScanState = Effect.fn(function* (root: string) {
@@ -587,7 +595,7 @@ export const JjClientLive = Layer.effect(
587595
return new Set([...fallbackIgnored, ...normalizeLines(result.stdout).map(toPortablePath)]);
588596
}
589597

590-
return fallbackIgnored;
598+
return yield* Effect.failSync(() => checkIgnoreError(result));
591599
});
592600

593601
const buildScanState = Effect.fn(function* (root: string) {

0 commit comments

Comments
 (0)