Skip to content

Commit 84ef549

Browse files
committed
mason: fix channel2 lease and band reset
1 parent fa425db commit 84ef549

18 files changed

Lines changed: 602 additions & 46 deletions

docs/AUDIT-KNOWN-ISSUES.md

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -581,12 +581,22 @@ with the default, so they read `''` (verified `IS NULL → 0`), and the
581581
trigger/delivery CAS matches.
582582

583583
A wedged `'claimed'` lease (crash mid-delivery) is still healed by
584-
`healWedgedChannel2Claims` (`storage-db.ts`), but the heal is now TTL-scoped via
585-
`channel2_nudge_claimed_at`: fresh claims are left alone so a sibling process
586-
boot cannot steal a live in-flight delivery; stale/legacy claims rewind to
587-
`'pending'`. After a send succeeds, confirm failures are not reverted to
588-
`'pending'` because the synthetic user message may already exist. Accepted as
589-
correct.
584+
`healWedgedChannel2Claims` (`storage-db.ts`), and `openDatabase()` now reruns that
585+
TTL-scoped heal on cached-handle reuse too so long-lived processes eventually
586+
unwind stale claims without a restart. The lease uses
587+
`channel2_nudge_claimed_at` as its liveness boundary: fresh claims are left alone
588+
so a sibling process boot cannot steal a live in-flight delivery; stale/legacy
589+
claims rewind to `'pending'`. If a send fails and the `claimed→pending` restore is
590+
locked, the row stays `claimed` with its timestamp intact and later TTL-heals back
591+
to `'pending'`.
592+
593+
One rare duplicate window remains accepted by design: if a process is suspended or
594+
otherwise hangs for longer than the TTL after sending but before confirming, a
595+
sibling can heal that stale claim, redeliver the same reminder, and mark
596+
`'delivered'` first. The original sender now preserves an already-`'delivered'`
597+
row and logs that stolen-lease path distinctly for diagnosis, but it cannot
598+
unsend its already-queued reminder. The cost is one duplicate synthetic message,
599+
not extra cap consumption.
590600

591601
### A37. `NORMAL_HYSTERESIS_TOKENS` (256) eligible-head snap is deliberate (boundary-straddle wobble accepted)
592602

packages/pi-plugin/src/context-handler.test.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import {
1111
addNote,
1212
appendNoteNudgeAnchor,
1313
getHistorianFailureState,
14+
getLastNudgeLevel,
15+
getLastNudgeUndropped,
1416
getNoteNudgeAnchors,
1517
getOrCreateSessionMeta,
1618
getPendingOps,
@@ -19,6 +21,8 @@ import {
1921
incrementHistorianFailure,
2022
insertTag,
2123
queuePendingOp,
24+
setLastNudgeLevel,
25+
setLastNudgeUndropped,
2226
setPendingPiCompactionMarkerState,
2327
updateSessionMeta,
2428
} from "@magic-context/core/features/magic-context/storage";
@@ -164,6 +168,35 @@ describe("registerPiContextHandler", () => {
164168
}
165169
});
166170

171+
it("resets the persisted Channel 1 band when baseline refresh sees a smaller tail", async () => {
172+
const db = createTestDb();
173+
try {
174+
const sessionId = "ses-pi-band-reset";
175+
setLastNudgeUndropped(db, sessionId, 80_000);
176+
setLastNudgeLevel(db, sessionId, "urgent");
177+
178+
const fake = createFakePi();
179+
registerPiContextHandler(fake.pi as never, {
180+
db,
181+
ctxReduceEnabled: true,
182+
});
183+
const handler = fake.handlers.get("context") as (
184+
event: { messages: never[] },
185+
ctx: never,
186+
) => Promise<{ messages: never[] } | undefined>;
187+
188+
await handler(
189+
{ messages: [userMessage("hello", 1)] as never[] },
190+
fakeContext(sessionId) as never,
191+
);
192+
193+
expect(getLastNudgeUndropped(db, sessionId)).toBe(0);
194+
expect(getLastNudgeLevel(db, sessionId)).toBe("");
195+
} finally {
196+
closeQuietly(db);
197+
}
198+
});
199+
167200
it("clears stale compartmentInProgress on first context pass after restart", async () => {
168201
const db = createTestDb();
169202
try {
@@ -958,8 +991,21 @@ describe("registerPiContextHandler", () => {
958991
piInputs.commitClusterTrigger,
959992
);
960993

994+
const stripCreatedAtDeep = (value: unknown): unknown => {
995+
if (Array.isArray(value)) {
996+
return value.map(stripCreatedAtDeep);
997+
}
998+
if (!value || typeof value !== "object") return value;
999+
const entries = Object.entries(value as Record<string, unknown>)
1000+
.filter(([key]) => key !== "createdAt")
1001+
.map(([key, inner]) => [key, stripCreatedAtDeep(inner)]);
1002+
return Object.fromEntries(entries);
1003+
};
1004+
9611005
expect(piInputs.triggerBudget).toBe(triggerBudget);
962-
expect(piDecision).toEqual(opencodeDecision);
1006+
expect(stripCreatedAtDeep(piDecision)).toEqual(
1007+
stripCreatedAtDeep(opencodeDecision),
1008+
);
9631009
expect(piDecision).toMatchObject({
9641010
shouldFire: true,
9651011
reason: "projected_headroom",

packages/pi-plugin/src/context-handler.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ import {
8080
peekDeferredExecutePending,
8181
pruneAutoSearchHintDecisions,
8282
pruneNoteNudgeAnchors,
83+
resetLastNudgeCycleIfTailShrank,
8384
setDeferredExecutePendingIfAbsent,
8485
} from "@magic-context/core/features/magic-context/storage-meta-persisted";
8586
import {
@@ -2220,6 +2221,14 @@ export function registerPiContextHandler(
22202221
0,
22212222
executeThresholdTokensPi - usageInputTokens + liveTailTokens,
22222223
);
2224+
// Same rationale as OpenCode: a historian publish, emergency drop,
2225+
// or pending-op replay can shrink the tail without a ctx_reduce
2226+
// tool call, so a regrowth must not inherit a stale persisted band.
2227+
resetLastNudgeCycleIfTailShrank(
2228+
options.db,
2229+
sessionId,
2230+
tailToolTokens,
2231+
);
22232232
setPiChannel1Baseline(sessionId, {
22242233
tailToolTokens,
22252234
historyBudgetTokens: historyBudgetTokens ?? 0,

packages/pi-plugin/src/ctx-reduce-nudge-pi.test.ts

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
import { describe, expect, it } from "bun:test";
1+
import { afterEach, describe, expect, it, mock, spyOn } from "bun:test";
22
import { readFileSync } from "node:fs";
33
import { join } from "node:path";
44
import {
5+
getChannel2NudgeClaimedAt,
56
getChannel2NudgeState,
67
setChannel2NudgeState,
78
} from "@magic-context/core/features/magic-context/storage";
9+
import * as loggerModule from "@magic-context/core/shared/logger";
810
import {
911
clearPiChannel1State,
1012
computeTailTokenEstimatePi,
@@ -19,6 +21,10 @@ function toolResultMsg(text: string) {
1921
return { role: "toolResult", content: [{ type: "text", text }] };
2022
}
2123

24+
afterEach(() => {
25+
mock.restore();
26+
});
27+
2228
describe("computeTailToolTokensPi", () => {
2329
it("sums non-dropped toolResult text, excludes sentinels", () => {
2430
const big = "x".repeat(40_000); // ~10k tokens
@@ -262,6 +268,90 @@ describe("maybeDeliverChannel2Pi", () => {
262268
expect(getChannel2NudgeState(db, SESSION)).toBe("pending");
263269
});
264270

271+
it("returns false and leaves the claim healable when claimed→pending CAS throws", async () => {
272+
const db = createTestDb();
273+
const session = "ses-ch2-pi-revert-throw";
274+
setChannel2NudgeState(db, session, "pending");
275+
armStrongBaseline(session);
276+
277+
const originalPrepare = db.prepare.bind(db);
278+
(db as unknown as { prepare: typeof db.prepare }).prepare = (
279+
sql: string,
280+
) => {
281+
const statement = originalPrepare(sql);
282+
if (
283+
sql ===
284+
"UPDATE session_meta SET channel2_nudge_state = ?, channel2_nudge_claimed_at = ? WHERE session_id = ? AND channel2_nudge_state = ?"
285+
) {
286+
return {
287+
...statement,
288+
run: (...args: unknown[]) => {
289+
if (
290+
args[0] === "pending" &&
291+
args[1] === 0 &&
292+
args[2] === session &&
293+
args[3] === "claimed"
294+
) {
295+
throw new Error("SQLITE_BUSY: database is locked");
296+
}
297+
return statement.run(
298+
...(args as [unknown, unknown, unknown, unknown]),
299+
);
300+
},
301+
} as typeof statement;
302+
}
303+
return statement;
304+
};
305+
306+
const delivered = maybeDeliverChannel2Pi(
307+
{
308+
sendUserMessage: () => {
309+
throw new Error("transient");
310+
},
311+
},
312+
db,
313+
session,
314+
);
315+
316+
expect(delivered).toBe(false);
317+
expect(getChannel2NudgeState(db, session)).toBe("claimed");
318+
expect(getChannel2NudgeClaimedAt(db, session)).toBeGreaterThan(0);
319+
});
320+
321+
it("preserves a sibling's delivered claim and logs the duplicate window distinctly", async () => {
322+
const db = createTestDb();
323+
const session = "ses-ch2-pi-duplicate";
324+
setChannel2NudgeState(db, session, "pending");
325+
armStrongBaseline(session);
326+
327+
const sessionLog = spyOn(loggerModule, "sessionLog").mockImplementation(
328+
() => {},
329+
);
330+
331+
const delivered = maybeDeliverChannel2Pi(
332+
{
333+
sendUserMessage: () => {
334+
db.prepare(
335+
"UPDATE session_meta SET channel2_nudge_state = 'delivered', channel2_nudge_claimed_at = 0 WHERE session_id = ?",
336+
).run(session);
337+
},
338+
},
339+
db,
340+
session,
341+
);
342+
343+
expect(delivered).toBe(false);
344+
expect(getChannel2NudgeState(db, session)).toBe("delivered");
345+
expect(
346+
sessionLog.mock.calls.some(
347+
(call) =>
348+
call[0] === session &&
349+
typeof call[1] === "string" &&
350+
call[1].includes("duplicate window"),
351+
),
352+
).toBe(true);
353+
});
354+
265355
it("does not re-deliver after success (one nudge per lifetime)", () => {
266356
const db = createTestDb();
267357
setChannel2NudgeState(db, SESSION, "delivered");

packages/pi-plugin/src/ctx-reduce-nudge-pi.ts

Lines changed: 78 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,32 @@ import {
4040
tailToolTokensFromStrings,
4141
toolOutputTokens,
4242
} from "@magic-context/core/hooks/magic-context/ctx-reduce-nudge";
43+
import { sessionLog } from "@magic-context/core/shared/logger";
4344
import type { Database } from "@magic-context/core/shared/sqlite";
4445

4546
export type { Channel1State };
4647

48+
function sealDeliveredAfterUnconfirmedSend(
49+
db: Database,
50+
sessionId: string,
51+
): "already-delivered" | "sealed" | "stuck-claimed" {
52+
try {
53+
if (getChannel2NudgeState(db, sessionId) === "delivered") {
54+
return "already-delivered";
55+
}
56+
} catch {
57+
// Best-effort probe only — if the read fails, still try to seal the cap.
58+
}
59+
60+
try {
61+
setChannel2NudgeState(db, sessionId, "delivered");
62+
return "sealed";
63+
} catch {
64+
// Preserve the stale claim so the shared TTL heal can recover it later.
65+
return "stuck-claimed";
66+
}
67+
}
68+
4769
// Per-session Channel 1 metric baseline. Written at the end of each pipeline
4870
// pass (post-drop), read in the `tool_result` handler. Primary-only: subagents
4971
// never get a baseline, which is how Channel 1 stays off for them (matches
@@ -322,8 +344,22 @@ export function maybeDeliverChannel2Pi(
322344
pi.sendUserMessage(buildChannel2Reminder(undropped), {
323345
deliverAs,
324346
});
325-
} catch {
326-
casChannel2NudgeState(db, sessionId, "claimed", "pending");
347+
} catch (error) {
348+
try {
349+
casChannel2NudgeState(db, sessionId, "claimed", "pending");
350+
} catch (revertError) {
351+
sessionLog(
352+
sessionId,
353+
"channel2 ceiling nudge delivery failed; pending restore was busy so the stale claim will heal later:",
354+
{ deliveryError: error, revertError },
355+
);
356+
return false;
357+
}
358+
sessionLog(
359+
sessionId,
360+
"channel2 ceiling nudge delivery failed (will retry):",
361+
error,
362+
);
327363
return false;
328364
}
329365

@@ -334,22 +370,50 @@ export function maybeDeliverChannel2Pi(
334370
"claimed",
335371
"delivered",
336372
);
337-
if (confirmed) return true;
338-
try {
339-
// The nudge has already been handed to Pi; seal the cap if another
340-
// process rewound the claim before our authoritative confirm.
341-
setChannel2NudgeState(db, sessionId, "delivered");
342-
} catch {
343-
// Best-effort; never re-arm after send.
373+
if (confirmed) {
374+
sessionLog(sessionId, "channel2 ceiling nudge delivered");
375+
return true;
376+
}
377+
378+
const outcome = sealDeliveredAfterUnconfirmedSend(db, sessionId);
379+
if (outcome === "already-delivered") {
380+
sessionLog(
381+
sessionId,
382+
"channel2 ceiling nudge duplicate window: our send returned after a sibling reclaimed the stale lease and already delivered",
383+
);
384+
} else if (outcome === "sealed") {
385+
sessionLog(
386+
sessionId,
387+
"channel2 ceiling nudge sent but claim confirmation was lost; sealed delivered without an authoritative confirm",
388+
);
389+
} else {
390+
sessionLog(
391+
sessionId,
392+
"channel2 ceiling nudge sent but claim confirmation was lost; lease stayed claimed and will heal later",
393+
);
344394
}
345395
return false;
346-
} catch {
396+
} catch (error) {
347397
// The nudge has already been handed to Pi; never re-arm on a post-send
348398
// confirm failure, or a transient DB error can duplicate the one-shot cap.
349-
try {
350-
setChannel2NudgeState(db, sessionId, "delivered");
351-
} catch {
352-
// Best-effort; never re-arm after send.
399+
const outcome = sealDeliveredAfterUnconfirmedSend(db, sessionId);
400+
if (outcome === "already-delivered") {
401+
sessionLog(
402+
sessionId,
403+
"channel2 ceiling nudge duplicate window: our send returned after a sibling reclaimed the stale lease and already delivered",
404+
);
405+
} else if (outcome === "sealed") {
406+
sessionLog(
407+
sessionId,
408+
"channel2 ceiling nudge sent but confirm failed:",
409+
error,
410+
);
411+
} else {
412+
sessionLog(
413+
sessionId,
414+
"channel2 ceiling nudge sent but confirm failed; lease stayed claimed and will heal later:",
415+
error,
416+
);
353417
}
354418
return false;
355419
}

0 commit comments

Comments
 (0)