Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 38 additions & 12 deletions assistant/src/__tests__/config-schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
36 changes: 27 additions & 9 deletions assistant/src/config/schemas/heartbeat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment on lines +59 to +62

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve non-null hour when fixing null mismatch

Adding an issue on activeHoursEnd here causes validateWithSchema to delete both keys for any mixed-null input, so configs like { activeHoursStart: null, activeHoursEnd: 20 } are rewritten to defaults (8, 22) instead of retaining the valid explicit 20. This is a behavioral regression from the previous repair strategy and can silently widen heartbeat windows for users who only made one side null.

Useful? React with πŸ‘Β / πŸ‘Ž.

});
return;
}
Expand All @@ -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,
Comment on lines 82 to +85

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep one explicit hour when resolving equal-hour config

Emitting an additional issue on activeHoursStart makes cleanup drop both sides for every equal-hours input, so { activeHoursStart: 5, activeHoursEnd: 5 } now falls back to (8, 22) instead of preserving one explicit value (previously repaired to 5/22). The cascade you’re fixing only requires dual-emit for the default-collision case (22/22), so this broad change introduces avoidable data loss for other equal-hour typos.

Useful? React with πŸ‘Β / πŸ‘Ž.

});
}
});
Expand Down
Loading