Skip to content

Commit 620c138

Browse files
author
Tehan
committed
fix(scheduler): keep cacheRemainingMs JSON-safe and restore invalid-TTL diagnostics
- rpc-handlers: the cacheNeverExpires branch assigned Infinity to cacheRemainingMs; JSON.stringify converts Infinity to null over RPC, violating the numeric StatusDetail contract. Use 0 and let the cacheNeverExpires flag carry the semantics (the TUI keys on it first). - computeHardCacheExpired: the extraction dropped the invalid-cache-ttl-fallback pass outcome and session log on parse failure. Add an onInvalid callback; the transform call site restores the exact pre-extraction record + log. - test: seed last_response_time in the never-TTL status test — the guarded branch only runs when lastResponseTime > 0, so the assertion was vacuous without it (verified red: Infinity revert now fails it).
1 parent 9a6466a commit 620c138

4 files changed

Lines changed: 61 additions & 6 deletions

File tree

packages/plugin/src/hooks/magic-context/transform-context-state.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,39 @@ describe("computeHardCacheExpired", () => {
163163
const sixMinutesAgo = now - 6 * 60 * 1000;
164164
expect(computeHardCacheExpired("garbage", sixMinutesAgo, now)).toBe(true);
165165
});
166+
167+
it("invokes onInvalid callback for invalid TTL but not for valid ones", () => {
168+
const now = Date.now();
169+
const tenDaysAgo = now - 10 * 24 * 60 * 60 * 1000;
170+
const invalidErrors: unknown[] = [];
171+
172+
// Invalid TTL: callback fires + 5m fallback applied
173+
const result = computeHardCacheExpired("bad-format", tenDaysAgo, now, (error) => {
174+
invalidErrors.push(error);
175+
});
176+
expect(invalidErrors.length).toBe(1);
177+
expect(invalidErrors[0]).toBeInstanceOf(Error);
178+
expect(result).toBe(true); // 5m fallback, 10-day-old → expired
179+
180+
// Valid TTLs: callback NOT invoked
181+
const validErrors: unknown[] = [];
182+
computeHardCacheExpired("never", tenDaysAgo, now, (error) => {
183+
validErrors.push(error);
184+
});
185+
expect(validErrors.length).toBe(0);
186+
187+
computeHardCacheExpired("5m", tenDaysAgo, now, (error) => {
188+
validErrors.push(error);
189+
});
190+
expect(validErrors.length).toBe(0);
191+
});
192+
193+
it("omitting onInvalid is harmless (5m fallback still applies)", () => {
194+
const now = Date.now();
195+
const tenDaysAgo = now - 10 * 24 * 60 * 60 * 1000;
196+
// No callback — should not throw, should still fall back to 5m
197+
expect(computeHardCacheExpired("bad-format", tenDaysAgo, now)).toBe(true);
198+
// "never" with no callback still works
199+
expect(computeHardCacheExpired("never", tenDaysAgo, now)).toBe(false);
200+
});
166201
});

packages/plugin/src/hooks/magic-context/transform.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,16 +242,21 @@ export function __getMessageTokensCacheForTest(
242242
*
243243
* Returns false when cacheTtl is "never" (Infinity) because any finite
244244
* elapsed time is < Infinity.
245+
*
246+
* @param onInvalid Optional callback invoked when cacheTtl fails to parse;
247+
* the 5m fallback is applied AFTER the callback returns.
245248
*/
246249
export function computeHardCacheExpired(
247250
cacheTtl: string,
248251
lastResponseTime: number,
249252
now: number,
253+
onInvalid?: (error: unknown) => void,
250254
): boolean {
251255
let ttlMs: number;
252256
try {
253257
ttlMs = parseCacheTtl(cacheTtl);
254-
} catch {
258+
} catch (error) {
259+
onInvalid?.(error);
255260
ttlMs = 5 * 60 * 1000;
256261
}
257262
return lastResponseTime > 0 && now - lastResponseTime >= ttlMs;
@@ -1896,6 +1901,10 @@ export function createTransform(deps: TransformDeps) {
18961901
sessionMeta.cacheTtl,
18971902
sessionMeta.lastResponseTime,
18981903
Date.now(),
1904+
(error) => {
1905+
passOutcome.record("invalid-cache-ttl-fallback");
1906+
sessionLog(sessionId, "invalid cache_ttl; using the 5m default:", error);
1907+
},
18991908
);
19001909
const m0HardSignals = {
19011910
systemHash: hardSystemHash,

packages/plugin/src/plugin/rpc-handlers.test.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -331,15 +331,23 @@ describe("buildStatusDetail — cacheNeverExpires with 'never' TTL", () => {
331331

332332
// Force-create the session meta row so the UPDATE lands on an existing row.
333333
db.prepare(`INSERT INTO session_meta (session_id) VALUES (?)`).run(sessionId);
334-
db.prepare("UPDATE session_meta SET cache_ttl = ? WHERE session_id = ?").run(
335-
"never",
336-
sessionId,
337-
);
334+
// Seed last_response_time: the cacheNeverExpires branch only runs
335+
// inside `if (lastResponseTime > 0)` — without this the test would
336+
// pass even if Infinity leaked into cacheRemainingMs.
337+
db.prepare(
338+
"UPDATE session_meta SET cache_ttl = ?, last_response_time = ? WHERE session_id = ?",
339+
).run("never", Date.now() - 60_000, sessionId);
338340

339341
const detail = buildStatusDetail(db, sessionId, directory);
340342

341343
expect(detail.cacheNeverExpires).toBe(true);
342344
expect(detail.cacheExpired).toBe(false);
345+
// Infinity must NOT leak into the numeric RPC field — JSON.stringify
346+
// converts Infinity to null, violating the StatusDetail contract.
347+
expect(detail.cacheRemainingMs).toBe(0);
348+
const roundTripped = JSON.parse(JSON.stringify(detail));
349+
expect(roundTripped.cacheRemainingMs).toBe(0);
350+
expect(roundTripped.cacheRemainingMs).not.toBeNull();
343351
} finally {
344352
closeQuietly(db);
345353
}

packages/plugin/src/plugin/rpc-handlers.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,10 @@ export function buildStatusDetail(
703703
if (detail.lastResponseTime > 0) {
704704
const elapsed = Date.now() - detail.lastResponseTime;
705705
if (detail.cacheNeverExpires) {
706-
detail.cacheRemainingMs = Number.POSITIVE_INFINITY;
706+
// Infinity does not survive JSON-RPC; cacheNeverExpires is the
707+
// authoritative flag — the TUI keys on it first.
708+
detail.cacheRemainingMs = 0;
709+
detail.cacheExpired = false;
707710
} else {
708711
detail.cacheRemainingMs = Math.max(0, detail.cacheTtlMs - elapsed);
709712
detail.cacheExpired = detail.cacheRemainingMs === 0;

0 commit comments

Comments
 (0)