Skip to content

Commit 033cb58

Browse files
fix(server): keep settings reload state and write failures together
1 parent dedeacf commit 033cb58

2 files changed

Lines changed: 27 additions & 9 deletions

File tree

‎apps/server/src/serverSettings.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,27 @@ it.layer(NodeServices.layer)("server settings", (it) => {
506506
const saved = yield* decodeServerSettingsJson(yield* fs.readFileString(config.settingsPath));
507507
assert.equal(saved.responseStreamingMode, "turn");
508508
assert.equal(saved.observability.otlpTracesUrl, "http://localhost:4318/v1/traces");
509+
510+
yield* writeFileStringAtomically({
511+
filePath: config.settingsPath,
512+
contents: `{ "responseStreamingMode": "paragraph" }`,
513+
});
514+
const instanceId = ProviderInstanceId.make("acpRegistry_kilo");
515+
yield* serverSettings.updateProviderInstance({
516+
operation: "upsert",
517+
instanceId,
518+
instance: {
519+
driver: ProviderDriverKind.make("acpRegistry"),
520+
displayName: "Kilo",
521+
config: { agentId: "kilo", distribution: "auto" },
522+
},
523+
});
524+
525+
const afterProviderSave = yield* decodeServerSettingsJson(
526+
yield* fs.readFileString(config.settingsPath),
527+
);
528+
assert.equal(afterProviderSave.responseStreamingMode, "paragraph");
529+
assert.equal(afterProviderSave.providerInstances[instanceId]?.displayName, "Kilo");
509530
}).pipe(Effect.provide(layerServerSettings())),
510531
);
511532

@@ -514,6 +535,7 @@ it.layer(NodeServices.layer)("server settings", (it) => {
514535
const config = yield* ServerConfig.ServerConfig;
515536
const fs = yield* FileSystem.FileSystem;
516537
const serverSettings = yield* ServerSettingsModule.ServerSettingsService;
538+
yield* serverSettings.getSettings;
517539
const raw = `{ "responseStreamingMode": "turn", `;
518540
yield* fs.writeFileString(config.settingsPath, raw);
519541

@@ -524,6 +546,8 @@ it.layer(NodeServices.layer)("server settings", (it) => {
524546
);
525547

526548
assert.equal(error.operation, "read-file");
549+
assert.equal(error.settingsPath, config.settingsPath);
550+
assert.instanceOf(error.cause, Error);
527551
assert.equal(yield* fs.readFileString(config.settingsPath), raw);
528552
}).pipe(Effect.provide(layerServerSettings())),
529553
);

‎apps/server/src/serverSettings.ts‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -658,9 +658,6 @@ const make = Effect.gen(function* () {
658658
let settings = DEFAULT_SERVER_SETTINGS;
659659
let legacyProviders: Readonly<Record<string, unknown>> | undefined;
660660
let legacySourceControl: Parameters<typeof migrateLegacySourceControlSettings>[1] = {};
661-
// A file that failed to decode must stay on disk for the user to repair;
662-
// the fold below only writes when it started from the file's real contents.
663-
let settingsFileTrusted = true;
664661
let parseFailure = Option.none<unknown>();
665662

666663
if (yield* readConfigExists) {
@@ -676,7 +673,6 @@ const make = Effect.gen(function* () {
676673
}
677674
if (decoded._tag === "Failure" || legacySettings._tag === "Failure") {
678675
const failure = decoded._tag === "Failure" ? decoded : legacySettings;
679-
settingsFileTrusted = false;
680676
// The file may hold an opt-out we can no longer read; never resume threads on a guess.
681677
settings = { ...DEFAULT_SERVER_SETTINGS, continueThreadsAfterServerUpdate: false };
682678
if (failure._tag === "Failure") {
@@ -692,6 +688,8 @@ const make = Effect.gen(function* () {
692688
}
693689
}
694690

691+
// A file that failed to decode must stay on disk for the user to repair.
692+
const settingsFileTrusted = Option.isNone(parseFailure);
695693
const providerHistory = yield* sql<{
696694
readonly providerName: string;
697695
readonly providerInstanceId: string | null;
@@ -792,11 +790,7 @@ const make = Effect.gen(function* () {
792790
return { settings: migrated, parseFailure };
793791
});
794792

795-
const settingsCache = yield* Cache.makeWith<
796-
typeof cacheKey,
797-
{ readonly settings: ServerSettings; readonly parseFailure: Option.Option<unknown> },
798-
ServerSettingsError
799-
>(() => loadSettingsFromDisk, {
793+
const settingsCache = yield* Cache.makeWith((_key: typeof cacheKey) => loadSettingsFromDisk, {
800794
capacity: 1,
801795
timeToLive: (exit) => (Exit.isSuccess(exit) ? Duration.infinity : Duration.zero),
802796
});

0 commit comments

Comments
 (0)