Skip to content

fix: config inheritance precedence (project > global > default) #193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bmdavis419 wants to merge 1 commit into
base: codex/pr-a-git-resource-reliability
Choose a base branch
from
Open

fix: config inheritance precedence (project > global > default) #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 49 additions & 0 deletions apps/server/src/config/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,55 @@ describe('Config', () => {
expect(config.resources.length).toBe(1);
expect(config.getResource('global-resource')).toBeDefined();
});

it('inherits global provider/model/settings when project config omits them', async () => {
const globalConfigDir = path.join(testDir, '.config', 'btca');
await fs.mkdir(globalConfigDir, { recursive: true });
const globalConfig = {
$schema: 'https://btca.dev/btca.schema.json',
provider: 'global-provider',
model: 'global-model',
providerTimeoutMs: 123_000,
maxSteps: 55,
resources: [
{
name: 'global-resource',
type: 'git',
url: 'https://github.com/global/repo',
branch: 'main'
}
]
};
await fs.writeFile(
path.join(globalConfigDir, 'btca.config.jsonc'),
JSON.stringify(globalConfig)
);

const projectDir = path.join(testDir, 'my-project');
await fs.mkdir(projectDir, { recursive: true });
const projectConfig = {
$schema: 'https://btca.dev/btca.schema.json',
resources: [
{
name: 'project-resource',
type: 'git',
url: 'https://github.com/project/repo',
branch: 'main'
}
]
};
await fs.writeFile(path.join(projectDir, 'btca.config.jsonc'), JSON.stringify(projectConfig));
process.chdir(projectDir);

const config = await Config.load();

expect(config.provider).toBe('global-provider');
expect(config.model).toBe('global-model');
expect(config.providerTimeoutMs).toBe(123_000);
expect(config.maxSteps).toBe(55);
expect(config.getResource('global-resource')).toBeDefined();
expect(config.getResource('project-resource')).toBeDefined();
});
});

describe('Config mutations (resource leakage prevention)', () => {
Expand Down
46 changes: 29 additions & 17 deletions apps/server/src/config/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ export namespace Config {
newConfigPath: string,
migratedCount: number
): Promise<StoredConfig> => {
const configDir = newConfigPath.slice(0, newConfigPath.lastIndexOf('/'));
const configDir = path.dirname(newConfigPath);
const result = await Result.gen(async function* () {
yield* Result.await(
Result.tryPromise({
Expand Down Expand Up @@ -503,7 +503,7 @@ export namespace Config {
};

const createDefaultConfig = async (configPath: string): Promise<StoredConfig> => {
const configDir = configPath.slice(0, configPath.lastIndexOf('/'));
const configDir = path.dirname(configPath);

const defaultStored: StoredConfig = {
$schema: CONFIG_SCHEMA_URL,
Expand Down Expand Up @@ -610,10 +610,17 @@ export namespace Config {
const getMergedProviderOptions = (): ProviderOptionsMap =>
mergeProviderOptions(currentGlobalConfig, currentProjectConfig);

// Get the config that should be used for model/provider
const getActiveConfig = (): StoredConfig => {
return currentProjectConfig ?? currentGlobalConfig;
};
const getEffectiveModel = (): string =>
currentProjectConfig?.model ?? currentGlobalConfig.model ?? DEFAULT_MODEL;

const getEffectiveProvider = (): string =>
currentProjectConfig?.provider ?? currentGlobalConfig.provider ?? DEFAULT_PROVIDER;

const getEffectiveProviderTimeoutMs = (): number | undefined =>
currentProjectConfig?.providerTimeoutMs ?? currentGlobalConfig.providerTimeoutMs;

const getEffectiveMaxSteps = (): number =>
currentProjectConfig?.maxSteps ?? currentGlobalConfig.maxSteps ?? DEFAULT_MAX_STEPS;

// Get the config that should be mutated
const getMutableConfig = (): StoredConfig => {
Expand All @@ -636,16 +643,16 @@ export namespace Config {
return getMergedResources();
},
get model() {
return getActiveConfig().model ?? DEFAULT_MODEL;
return getEffectiveModel();
},
get provider() {
return getActiveConfig().provider ?? DEFAULT_PROVIDER;
return getEffectiveProvider();
},
get providerTimeoutMs() {
return getActiveConfig().providerTimeoutMs;
return getEffectiveProviderTimeoutMs();
},
get maxSteps() {
return getActiveConfig().maxSteps ?? DEFAULT_MAX_STEPS;
return getEffectiveMaxSteps();
},
getProviderOptions: (providerId: string) => getMergedProviderOptions()[providerId],
getResource: (name: string) => getMergedResources().find((r) => r.name === name),
Expand Down Expand Up @@ -763,7 +770,7 @@ export namespace Config {
// We can't modify global config from project context, so throw an error
throw new ConfigError({
message: `Resource "${name}" is defined in the global config`,
hint: `To remove this resource globally, edit the global config at "${expandHome(GLOBAL_CONFIG_DIR)}/${GLOBAL_CONFIG_FILENAME}" or run the command without a project config present.`
hint: `To remove this resource globally, edit the global config at "${path.join(expandHome(GLOBAL_CONFIG_DIR), GLOBAL_CONFIG_FILENAME)}" or run the command without a project config present.`
});
}
} else {
Expand Down Expand Up @@ -797,7 +804,7 @@ export namespace Config {

for (const item of resourcesDir) {
const removeResult = await Result.tryPromise(() =>
fs.rm(`${resourcesDirectory}/${item}`, { recursive: true, force: true })
fs.rm(path.join(resourcesDirectory, item), { recursive: true, force: true })
);
const removed = removeResult.match({
ok: () => true,
Expand Down Expand Up @@ -845,16 +852,16 @@ export namespace Config {
const cwd = process.cwd();
Metrics.info('config.load.start', { cwd });

const globalConfigPath = `${expandHome(GLOBAL_CONFIG_DIR)}/${GLOBAL_CONFIG_FILENAME}`;
const projectConfigPath = `${cwd}/${PROJECT_CONFIG_FILENAME}`;
const globalConfigPath = path.join(expandHome(GLOBAL_CONFIG_DIR), GLOBAL_CONFIG_FILENAME);
const projectConfigPath = path.join(cwd, PROJECT_CONFIG_FILENAME);

// First, load or create the global config
let globalConfig: StoredConfig;
const globalExists = await Bun.file(globalConfigPath).exists();

if (!globalExists) {
// Check for legacy config to migrate
const legacyConfigPath = `${expandHome(GLOBAL_CONFIG_DIR)}/${LEGACY_CONFIG_FILENAME}`;
const legacyConfigPath = path.join(expandHome(GLOBAL_CONFIG_DIR), LEGACY_CONFIG_FILENAME);
const migrated = await migrateLegacyConfig(legacyConfigPath, globalConfigPath);
if (migrated) {
Metrics.info('config.load.global', { source: 'migrated', path: globalConfigPath });
Expand Down Expand Up @@ -907,7 +914,7 @@ export namespace Config {
return makeService(
globalConfig,
projectConfig,
`${resolvedProjectDataDir}/resources`,
path.join(resolvedProjectDataDir, 'resources'),
projectConfigPath
);
}
Expand All @@ -919,6 +926,11 @@ export namespace Config {
globalDataDir,
expandHome(GLOBAL_CONFIG_DIR)
);
return makeService(globalConfig, null, `${resolvedGlobalDataDir}/resources`, globalConfigPath);
return makeService(
globalConfig,
null,
path.join(resolvedGlobalDataDir, 'resources'),
globalConfigPath
);
};
}