From 635442ee0e507e15c2376ea7c55903d3a8de56f8 Mon Sep 17 00:00:00 2001 From: siddseethepalli Date: Wed, 15 Apr 2026 04:46:33 +0000 Subject: [PATCH] fix(config): dual-emit for heartbeat null-mismatch and equal-values --- assistant/src/__tests__/config-schema.test.ts | 50 ++++++++++++++----- assistant/src/config/schemas/heartbeat.ts | 36 +++++++++---- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/assistant/src/__tests__/config-schema.test.ts b/assistant/src/__tests__/config-schema.test.ts index bf428c588fd..b17b9380435 100644 --- a/assistant/src/__tests__/config-schema.test.ts +++ b/assistant/src/__tests__/config-schema.test.ts @@ -2090,32 +2090,58 @@ describe("loadConfig with schema validation", () => { test("recovers from partial heartbeat.activeHours without wiping unrelated fields", () => { // activeHoursStart is explicitly nulled while activeHoursEnd defaults to - // 22 — a mismatch. Single-emit on the null side lets delete-and-retry - // strip activeHoursStart, after which the default (8) restores it. + // 22 — a mismatch. Dual-emit strips both sides; both defaults restore + // (8, 22). maxTokens is unaffected. writeConfig({ maxTokens: 4096, heartbeat: { activeHoursStart: null }, }); const config = loadConfig(); expect(config.maxTokens).toBe(4096); - // Both fall back to the heartbeat defaults (8, 22). expect(config.heartbeat.activeHoursStart).toBe(8); expect(config.heartbeat.activeHoursEnd).toBe(22); }); - test("partial heartbeat.activeHours preserves the explicit non-null value", () => { - // User sets activeHoursEnd: 20 and nulls activeHoursStart. Single-emit - // on the null side strips activeHoursStart only — default 8 restores - // it, user's explicit 20 survives. Dual-emit would strip both and lose - // the 20. + test("recovers from heartbeat.activeHours null-mismatch where explicit value equals opposite default", () => { + // { start: null, end: 8 } — single-emit on the null side would strip + // start, the default 8 would restore it, and the equal-hours check would + // fire, cascading to a full defaults reset that wipes maxTokens. + // Dual-emit strips both sides in one pass. writeConfig({ - maxTokens: 1234, - heartbeat: { activeHoursStart: null, activeHoursEnd: 20 }, + maxTokens: 4096, + heartbeat: { activeHoursStart: null, activeHoursEnd: 8 }, }); const config = loadConfig(); - expect(config.maxTokens).toBe(1234); + expect(config.maxTokens).toBe(4096); expect(config.heartbeat.activeHoursStart).toBe(8); - expect(config.heartbeat.activeHoursEnd).toBe(20); + expect(config.heartbeat.activeHoursEnd).toBe(22); + }); + + test("recovers from heartbeat.activeHours null-mismatch on the end side", () => { + // { start: 22, end: null } — same cascade class as above, mirrored. + writeConfig({ + maxTokens: 4096, + heartbeat: { activeHoursStart: 22, activeHoursEnd: null }, + }); + const config = loadConfig(); + expect(config.maxTokens).toBe(4096); + expect(config.heartbeat.activeHoursStart).toBe(8); + expect(config.heartbeat.activeHoursEnd).toBe(22); + }); + + test("recovers from equal heartbeat.activeHours without wiping unrelated fields", () => { + // { start: 22, end: 22 } — both equal to the default for end. Single-emit + // on one path would strip one side, the default would recreate the + // equal-hours mismatch, and the loader would fall back to full defaults, + // wiping maxTokens. Dual-emit strips both sides at once. + writeConfig({ + maxTokens: 4096, + heartbeat: { activeHoursStart: 22, activeHoursEnd: 22 }, + }); + const config = loadConfig(); + expect(config.maxTokens).toBe(4096); + expect(config.heartbeat.activeHoursStart).toBe(8); + expect(config.heartbeat.activeHoursEnd).toBe(22); }); test("recovers from equal filing.activeHours without wiping unrelated fields", () => { diff --git a/assistant/src/config/schemas/heartbeat.ts b/assistant/src/config/schemas/heartbeat.ts index b246d8fa1e1..91ac0003c2e 100644 --- a/assistant/src/config/schemas/heartbeat.ts +++ b/assistant/src/config/schemas/heartbeat.ts @@ -43,15 +43,23 @@ export const HeartbeatConfigSchema = z const startNull = config.activeHoursStart == null; const endNull = config.activeHoursEnd == null; if (startNull !== endNull) { - // Emit on the null side only. Heartbeat's defaults are 8/22, so - // delete-and-retry strips the null key, letting the non-null default - // restore it while preserving the user's explicit value on the other - // side. + // Emit on both fields so validateWithSchema's delete-and-retry strips + // both sides in one pass. Single-emit on the null side can cascade when + // the explicit value happens to equal the opposite default (e.g. + // { start: null, end: 8 } → strip start → default 8 → equal check fires + // → loader falls back to full defaults, wiping unrelated keys like + // maxTokens). + const message = + "heartbeat.activeHoursStart and heartbeat.activeHoursEnd must both be set or both be null"; ctx.addIssue({ code: z.ZodIssueCode.custom, - path: [startNull ? "activeHoursStart" : "activeHoursEnd"], - message: - "heartbeat.activeHoursStart and heartbeat.activeHoursEnd must both be set or both be null", + path: ["activeHoursStart"], + message, + }); + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["activeHoursEnd"], + message, }); return; } @@ -60,11 +68,21 @@ export const HeartbeatConfigSchema = z config.activeHoursEnd != null && config.activeHoursStart === config.activeHoursEnd ) { + // Emit on both fields. Single-emit would strip one side and the default + // for that side could recreate a new mismatch (e.g. { start: 22, end: 22 } + // → strip end → default 22 → equal again), cascading to a full defaults + // reset that wipes unrelated fields. + const message = + "heartbeat.activeHoursStart and heartbeat.activeHoursEnd must not be equal (would create an empty window)"; + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["activeHoursStart"], + message, + }); ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["activeHoursEnd"], - message: - "heartbeat.activeHoursStart and heartbeat.activeHoursEnd must not be equal (would create an empty window)", + message, }); } });