feat(FN-1627): expose global execution concurrency limit in settings
- Add global max concurrent agent limit to settings UI (Scheduling section) - Add null-as-delete semantics for global settings persistence - Add fetchGlobalConcurrency and updateGlobalConcurrency API integrations - Update SettingsModal tests with comprehensive coverage for new features - Fix lint issues in modified files
This commit is contained in:
@@ -172,15 +172,68 @@ describe("GlobalSettingsStore", () => {
|
||||
expect(settings.themeMode).toBe("light");
|
||||
});
|
||||
|
||||
it("can clear a field by setting it to undefined", async () => {
|
||||
it("can clear a field by setting it to null (null-as-delete semantics)", async () => {
|
||||
await store.init();
|
||||
await store.updateSettings({ defaultProvider: "anthropic" });
|
||||
await store.updateSettings({ defaultProvider: undefined });
|
||||
// @ts-expect-error - null is intentionally used to clear field (null-as-delete)
|
||||
await store.updateSettings({ defaultProvider: null });
|
||||
|
||||
const settings = await store.getSettings();
|
||||
expect(settings.defaultProvider).toBeUndefined();
|
||||
});
|
||||
|
||||
it("clearing ntfyTopic with null removes it from disk and returns undefined", async () => {
|
||||
await store.init();
|
||||
await store.updateSettings({ ntfyEnabled: true, ntfyTopic: "my-topic" });
|
||||
|
||||
// Verify it was persisted
|
||||
const raw = JSON.parse(await readFile(join(dir, "settings.json"), "utf-8"));
|
||||
expect(raw.ntfyTopic).toBe("my-topic");
|
||||
|
||||
// Clear the topic with null
|
||||
// @ts-expect-error - null is intentionally used to clear field (null-as-delete)
|
||||
await store.updateSettings({ ntfyTopic: null });
|
||||
|
||||
// Verify it was removed from disk
|
||||
const rawAfter = JSON.parse(await readFile(join(dir, "settings.json"), "utf-8"));
|
||||
expect(rawAfter.ntfyTopic).toBeUndefined();
|
||||
|
||||
// Verify getSettings returns undefined
|
||||
const settings = await store.getSettings();
|
||||
expect(settings.ntfyTopic).toBeUndefined();
|
||||
});
|
||||
|
||||
it("clearing ntfyDashboardHost with null removes it from disk", async () => {
|
||||
await store.init();
|
||||
await store.updateSettings({ ntfyDashboardHost: "https://dashboard.example.com" });
|
||||
|
||||
// @ts-expect-error - null is intentionally used to clear field (null-as-delete)
|
||||
await store.updateSettings({ ntfyDashboardHost: null });
|
||||
|
||||
const raw = JSON.parse(await readFile(join(dir, "settings.json"), "utf-8"));
|
||||
expect(raw.ntfyDashboardHost).toBeUndefined();
|
||||
|
||||
const settings = await store.getSettings();
|
||||
expect(settings.ntfyDashboardHost).toBeUndefined();
|
||||
});
|
||||
|
||||
it("clearing ntfyEvents with null resets to default on read", async () => {
|
||||
await store.init();
|
||||
await store.updateSettings({ ntfyEvents: ["in-review", "failed"] });
|
||||
|
||||
// Verify it was persisted with custom value
|
||||
const raw = JSON.parse(await readFile(join(dir, "settings.json"), "utf-8"));
|
||||
expect(raw.ntfyEvents).toEqual(["in-review", "failed"]);
|
||||
|
||||
// @ts-expect-error - null is intentionally used to clear field (null-as-delete)
|
||||
await store.updateSettings({ ntfyEvents: null });
|
||||
|
||||
// After clear, reading back gives the default value
|
||||
// (either undefined on disk with default applied, or default written directly)
|
||||
const settings = await store.getSettings();
|
||||
expect(settings.ntfyEvents).toEqual(["in-review", "merged", "failed", "awaiting-approval", "awaiting-user-review"]);
|
||||
});
|
||||
|
||||
it("handles concurrent updates safely via locking", async () => {
|
||||
await store.init();
|
||||
|
||||
|
||||
@@ -132,16 +132,42 @@ export class GlobalSettingsStore {
|
||||
* merging, so fields that were removed from the TypeScript schema are not
|
||||
* silently dropped during save cycles.
|
||||
*
|
||||
* **Null-as-delete semantics**: Fields set to `null` in the patch are
|
||||
* explicitly deleted from the settings. This allows the frontend to clear
|
||||
* a setting by sending `null` instead of `undefined` (since JSON.stringify
|
||||
* drops `undefined` values, `null` serves as the explicit clear sentinel).
|
||||
*
|
||||
* @returns The full updated settings after merge.
|
||||
*/
|
||||
async updateSettings(patch: Partial<GlobalSettings>): Promise<GlobalSettings> {
|
||||
async updateSettings(patch: Partial<GlobalSettings> & Record<string, unknown>): Promise<GlobalSettings> {
|
||||
return this.withLock(async () => {
|
||||
const raw = await this.readRaw();
|
||||
const merged = { ...DEFAULT_GLOBAL_SETTINGS, ...raw, ...patch };
|
||||
|
||||
// Apply null-as-delete semantics: null means "remove this field"
|
||||
// Merge order: defaults → raw (disk) → patch
|
||||
// This means: patch values win, then raw, then defaults
|
||||
// But null in patch means "delete" - so we delete from raw first
|
||||
const merged: Record<string, unknown> = { ...raw };
|
||||
|
||||
for (const [key, value] of Object.entries(patch)) {
|
||||
if (value === null) {
|
||||
// null → delete this key from the merged object
|
||||
// This effectively makes it fall through to the default
|
||||
delete merged[key];
|
||||
} else {
|
||||
// normal value → set it
|
||||
merged[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
// After merging, fill in defaults for any missing keys
|
||||
// This ensures fields that were deleted (by null) get their default value
|
||||
const withDefaults = { ...DEFAULT_GLOBAL_SETTINGS, ...merged } as GlobalSettings;
|
||||
|
||||
await mkdir(this.dir, { recursive: true });
|
||||
await this.atomicWrite(merged as GlobalSettings);
|
||||
await this.atomicWrite(withDefaults);
|
||||
// Update the write-through cache
|
||||
this.cachedSettings = merged as GlobalSettings;
|
||||
this.cachedSettings = withDefaults;
|
||||
return this.cachedSettings;
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user