Skip to content

Commit 7d85078

Browse files
xesrevinuGit Agent
andcommitted
feat(config): improve config decoding and validation
- Introduce robust YAML config decoding with schemas - Normalize scopes and hooks from local or project maps - Merge local and project values with safe fallbacks - Add detailed ConfigError handling for invalid input refactor/config: add schema-based decoding, normalization of scopes and hooks, and robust error handling for config input. updates ensure local overrides merge correctly with project defaults and backwards compatibility for hook_type. Co-Authored-By: Git Agent <noreply@git-agent.dev>
1 parent 4978e7a commit 7d85078

1 file changed

Lines changed: 148 additions & 69 deletions

File tree

src/config/project.ts

Lines changed: 148 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,39 @@
1-
import { Effect, FileSystem, Path } from "effect";
1+
import { Effect, FileSystem, Path, Schema } from "effect";
22
import { parse, stringify } from "yaml";
33
import type { ProjectConfig, ProjectScope } from "../domain/project";
44
import { emptyProjectConfig } from "../domain/project";
55
import { ConfigError } from "../shared/errors";
66
import { getKeyDef } from "./keys";
77

88
type RawYamlMap = Record<string, unknown>;
9+
type RawScopeInput = string | { readonly name: string; readonly description?: string | undefined };
10+
type RawHookInput = string | ReadonlyArray<string>;
11+
12+
const RawScopeSchema = Schema.Union([
13+
Schema.String,
14+
Schema.Struct({
15+
name: Schema.String,
16+
description: Schema.optional(Schema.String),
17+
}),
18+
]);
19+
const RawHooksSchema = Schema.Union([Schema.String, Schema.Array(Schema.String)]);
20+
21+
const configError = (pathValue: string, message: string) =>
22+
new ConfigError({
23+
message: `invalid config ${pathValue}: ${message}`,
24+
});
25+
26+
const decodeConfigField = <S extends Schema.Top>(
27+
pathValue: string,
28+
key: string,
29+
input: unknown,
30+
schema: S,
31+
) =>
32+
input === undefined
33+
? Effect.succeed(undefined)
34+
: Schema.decodeUnknownEffect(schema)(input).pipe(
35+
Effect.mapError((cause) => configError(pathValue, `${key}: ${cause.message}`)),
36+
);
937

1038
const readYamlMap = Effect.fn(function* (pathValue: string) {
1139
const fs = yield* FileSystem.FileSystem;
@@ -26,69 +54,87 @@ const readYamlMap = Effect.fn(function* (pathValue: string) {
2654
return yield* Effect.try({
2755
try: () => {
2856
const parsed = parse(text);
29-
return typeof parsed === "object" && parsed !== null ? (parsed as RawYamlMap) : {};
57+
if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
58+
throw configError(pathValue, "expected a YAML mapping");
59+
}
60+
return parsed as RawYamlMap;
3061
},
3162
catch: (cause) =>
32-
new ConfigError({
33-
message: `failed to read config ${pathValue}: ${cause instanceof Error ? cause.message : String(cause)}`,
34-
}),
63+
ConfigError.is(cause)
64+
? cause
65+
: new ConfigError({
66+
message: `failed to read config ${pathValue}: ${cause instanceof Error ? cause.message : String(cause)}`,
67+
}),
3568
});
3669
});
3770

38-
const scopeFromUnknown = (input: unknown): ProjectScope | undefined => {
39-
if (typeof input === "string" && input.trim().length > 0) {
40-
return { name: input.trim() };
41-
}
42-
if (
43-
typeof input === "object" &&
44-
input !== null &&
45-
"name" in input &&
46-
typeof input.name === "string"
47-
) {
48-
const description =
49-
"description" in input &&
50-
typeof input.description === "string" &&
51-
input.description.trim().length > 0
52-
? input.description.trim()
53-
: undefined;
54-
return {
55-
name: input.name.trim(),
56-
...(description != null ? { description } : {}),
57-
};
71+
const normalizeScope = (
72+
pathValue: string,
73+
input: RawScopeInput,
74+
index: number,
75+
): Effect.Effect<ProjectScope, ConfigError> => {
76+
if (typeof input === "string") {
77+
const name = input.trim();
78+
return name.length > 0
79+
? Effect.succeed({ name })
80+
: Effect.failSync(() => configError(pathValue, `scopes[${index}] must not be empty`));
5881
}
59-
return undefined;
60-
};
6182

62-
const parseScopes = (input: unknown): Array<ProjectScope> => {
63-
if (!Array.isArray(input)) {
64-
return [];
83+
const name = input.name.trim();
84+
if (name.length === 0) {
85+
return Effect.failSync(() => configError(pathValue, `scopes[${index}].name must not be empty`));
6586
}
66-
return input.map(scopeFromUnknown).filter((scope): scope is ProjectScope => scope != null);
87+
88+
const description = input.description?.trim();
89+
return Effect.succeed({
90+
name,
91+
...(description != null && description.length > 0 ? { description } : {}),
92+
});
6793
};
6894

69-
const parseHooks = (input: unknown, legacy: unknown): Array<string> => {
70-
if (Array.isArray(input)) {
71-
return input.filter(
72-
(item): item is string => typeof item === "string" && item.trim().length > 0,
73-
);
74-
}
75-
if (typeof input === "string" && input.trim().length > 0) {
76-
return input
77-
.split(",")
78-
.map((item) => item.trim())
79-
.filter((item) => item.length > 0);
80-
}
81-
if (typeof legacy === "string" && legacy.trim().length > 0) {
82-
return [legacy.trim()];
95+
const decodeScopes = Effect.fn(function* (pathValue: string, rawMap: RawYamlMap) {
96+
const scopes = yield* decodeConfigField(
97+
pathValue,
98+
"scopes",
99+
rawMap["scopes"],
100+
Schema.Array(RawScopeSchema),
101+
);
102+
if (scopes == null) {
103+
return [] as Array<ProjectScope>;
83104
}
84-
return [];
105+
return yield* Effect.forEach(scopes, (scope, index) => normalizeScope(pathValue, scope, index));
106+
});
107+
108+
const normalizeHookValues = (
109+
pathValue: string,
110+
key: "hook" | "hook_type",
111+
input: RawHookInput,
112+
): Effect.Effect<Array<string>, ConfigError> => {
113+
const normalized = (typeof input === "string" ? input.split(",") : [...input])
114+
.map((item) => item.trim())
115+
.filter((item) => item.length > 0);
116+
return normalized.length > 0
117+
? Effect.succeed(normalized)
118+
: Effect.failSync(() => configError(pathValue, `${key} must not be empty`));
85119
};
86120

87-
const parseBoolean = (input: unknown): boolean | undefined =>
88-
typeof input === "boolean" ? input : undefined;
121+
const decodeHooks = Effect.fn(function* (pathValue: string, rawMap: RawYamlMap) {
122+
const hooks = yield* decodeConfigField(pathValue, "hook", rawMap["hook"], RawHooksSchema);
123+
if (hooks != null) {
124+
return yield* normalizeHookValues(pathValue, "hook", hooks);
125+
}
89126

90-
const parseIntValue = (input: unknown): number | undefined =>
91-
typeof input === "number" && Number.isInteger(input) ? input : undefined;
127+
const legacyHook = yield* decodeConfigField(
128+
pathValue,
129+
"hook_type",
130+
rawMap["hook_type"],
131+
Schema.String,
132+
);
133+
if (legacyHook != null) {
134+
return yield* normalizeHookValues(pathValue, "hook_type", legacyHook);
135+
}
136+
return [] as Array<string>;
137+
});
92138

93139
const gitAgentPath = Effect.fn(function* (repoRoot: string, ...segments: ReadonlyArray<string>) {
94140
const path = yield* Path.Path;
@@ -118,24 +164,57 @@ export const loadProjectConfig = Effect.fn(function* (repoRoot: string) {
118164
const projectRaw = yield* readYamlMap(projectPath);
119165
const localRaw = yield* readYamlMap(localPath);
120166

121-
const mergedScopes =
122-
parseScopes(localRaw["scopes"]).length > 0
123-
? parseScopes(localRaw["scopes"])
124-
: parseScopes(projectRaw["scopes"]);
125-
const mergedHooks =
126-
parseHooks(localRaw["hook"], localRaw["hook_type"]).length > 0
127-
? parseHooks(localRaw["hook"], localRaw["hook_type"])
128-
: parseHooks(projectRaw["hook"], projectRaw["hook_type"]);
129-
const maxDiffLines =
130-
parseIntValue(localRaw["max_diff_lines"]) ?? parseIntValue(projectRaw["max_diff_lines"]) ?? 0;
131-
const noGitAgentCoAuthor =
132-
parseBoolean(localRaw["no_git_agent_co_author"]) ??
133-
parseBoolean(projectRaw["no_git_agent_co_author"]) ??
134-
false;
135-
const noModelCoAuthor =
136-
parseBoolean(localRaw["no_model_co_author"]) ??
137-
parseBoolean(projectRaw["no_model_co_author"]) ??
138-
false;
167+
const [
168+
localScopes,
169+
projectScopes,
170+
localHooks,
171+
projectHooks,
172+
localMaxDiffLines,
173+
projectMaxDiffLines,
174+
] = yield* Effect.all([
175+
decodeScopes(localPath, localRaw),
176+
decodeScopes(projectPath, projectRaw),
177+
decodeHooks(localPath, localRaw),
178+
decodeHooks(projectPath, projectRaw),
179+
decodeConfigField(localPath, "max_diff_lines", localRaw["max_diff_lines"], Schema.Int),
180+
decodeConfigField(projectPath, "max_diff_lines", projectRaw["max_diff_lines"], Schema.Int),
181+
]);
182+
const mergedScopes = localScopes.length > 0 ? localScopes : projectScopes;
183+
const mergedHooks = localHooks.length > 0 ? localHooks : projectHooks;
184+
const maxDiffLines = localMaxDiffLines ?? projectMaxDiffLines ?? 0;
185+
const [
186+
localNoGitAgentCoAuthor,
187+
projectNoGitAgentCoAuthor,
188+
localNoModelCoAuthor,
189+
projectNoModelCoAuthor,
190+
] = yield* Effect.all([
191+
decodeConfigField(
192+
localPath,
193+
"no_git_agent_co_author",
194+
localRaw["no_git_agent_co_author"],
195+
Schema.Boolean,
196+
),
197+
decodeConfigField(
198+
projectPath,
199+
"no_git_agent_co_author",
200+
projectRaw["no_git_agent_co_author"],
201+
Schema.Boolean,
202+
),
203+
decodeConfigField(
204+
localPath,
205+
"no_model_co_author",
206+
localRaw["no_model_co_author"],
207+
Schema.Boolean,
208+
),
209+
decodeConfigField(
210+
projectPath,
211+
"no_model_co_author",
212+
projectRaw["no_model_co_author"],
213+
Schema.Boolean,
214+
),
215+
]);
216+
const noGitAgentCoAuthor = localNoGitAgentCoAuthor ?? projectNoGitAgentCoAuthor ?? false;
217+
const noModelCoAuthor = localNoModelCoAuthor ?? projectNoModelCoAuthor ?? false;
139218

140219
if (
141220
mergedScopes.length === 0 &&
@@ -163,7 +242,7 @@ export const mergeAndSaveScopes = Effect.fn(function* (
163242
const fs = yield* FileSystem.FileSystem;
164243
const path = yield* Path.Path;
165244
const rawMap = yield* readYamlMap(pathValue);
166-
const existingScopes = parseScopes(rawMap["scopes"]);
245+
const existingScopes = yield* decodeScopes(pathValue, rawMap);
167246
const seen = new Set(existingScopes.map((scope) => scope.name.toLowerCase()));
168247
const merged = [...existingScopes];
169248

0 commit comments

Comments
 (0)