Skip to content

Commit 4ed06dd

Browse files
mason: fix historian wrapup coordination
Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
1 parent a89f921 commit 4ed06dd

11 files changed

Lines changed: 405 additions & 65 deletions

packages/pi-plugin/src/commands/ctx-wrapup.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
/// <reference types="bun-types" />
22

33
import { describe, expect, it, mock } from "bun:test";
4+
import {
5+
acquireCompartmentLease,
6+
releaseCompartmentLease,
7+
} from "@magic-context/core/features/magic-context/compartment-lease";
48
import {
59
appendCompartments,
610
getCompartments,
@@ -241,6 +245,37 @@ describe("Pi /ctx-wrapup", () => {
241245
}
242246
});
243247

248+
it("bounds waiting for a foreign compartment lease and clears the wrapup marker", async () => {
249+
const db = createDb();
250+
try {
251+
const sessionId = "pi-wrapup-lease-timeout";
252+
const foreignHolder = "foreign-lease-holder";
253+
expect(
254+
acquireCompartmentLease(db, sessionId, foreignHolder),
255+
).not.toBeNull();
256+
const runPiHistorianForWrapup = mock(async () => {});
257+
258+
const result = await runPiWrapup(
259+
pi().api,
260+
deps(db, {
261+
runPiHistorianForWrapup,
262+
wrapupLeaseWaitTimeoutMs: 0,
263+
}),
264+
ctx(sessionId, 8),
265+
sessionId,
266+
2,
267+
);
268+
269+
expect(result).toContain("## Magic Wrapup — Partial");
270+
expect(result).toContain("Timed out waiting");
271+
expect(runPiHistorianForWrapup).not.toHaveBeenCalled();
272+
expect(getWrapupInProgressState(db, sessionId)).toBeNull();
273+
releaseCompartmentLease(db, sessionId, foreignHolder);
274+
} finally {
275+
closeQuietly(db);
276+
}
277+
});
278+
244279
it("signals deferred history and materialization after a wrapup publish", async () => {
245280
const db = createDb();
246281
try {

packages/pi-plugin/src/commands/ctx-wrapup.ts

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export interface RegisterCtxWrapupDeps {
6060
[modelKey: string]: number | undefined;
6161
};
6262
runPiHistorianForWrapup?: typeof runPiHistorian;
63+
wrapupLeaseWaitTimeoutMs?: number;
6364
resolveRuntimeDeps?: (ctx: { cwd: string }) => CtxWrapupRuntimeDeps;
6465
}
6566

@@ -70,6 +71,18 @@ export type CtxWrapupRuntimeDeps = Omit<
7071

7172
const DEFAULT_MESSAGES_TO_KEEP = 20;
7273
const LEASE_WAIT_MS = 1_000;
74+
const MAX_WRAPUP_LEASE_WAIT_MS = 10 * 60 * 1_000;
75+
76+
type LeaseAcquireResult =
77+
| { ok: true; holderId: string }
78+
| { ok: false; reason: "ownership_lost" | "timeout" };
79+
80+
function resolveWrapupLeaseWaitTimeout(deps: CtxWrapupRuntimeDeps): number {
81+
const configured = deps.wrapupLeaseWaitTimeoutMs ?? MAX_WRAPUP_LEASE_WAIT_MS;
82+
return Number.isFinite(configured) && configured >= 0
83+
? configured
84+
: MAX_WRAPUP_LEASE_WAIT_MS;
85+
}
7386

7487
export function parseWrapupArgs(
7588
raw: string,
@@ -345,17 +358,21 @@ export async function runPiWrapup(
345358
level: "info",
346359
});
347360

348-
const leaseHolder = await acquireCompartmentLeaseEventually(
361+
const leaseResult = await acquireCompartmentLeaseEventually(
349362
deps.db,
350363
sessionId,
351364
renewWrapupMarker,
365+
resolveWrapupLeaseWaitTimeout(deps),
352366
);
353-
if (!leaseHolder) {
367+
if (!leaseResult.ok) {
354368
failure = ownershipLost
355369
? `${ownershipLostReason}; wrapped up through message ${lastEnd}. Run /ctx-wrapup again to continue.`
356-
: "Another Magic Context rebuild started while wrapup was waiting. Run /ctx-wrapup again to continue.";
370+
: leaseResult.reason === "timeout"
371+
? `Timed out waiting for another process to release the compartment-state lease; wrapped up through message ${lastEnd}. Run /ctx-wrapup again to continue.`
372+
: "Another Magic Context rebuild started while wrapup was waiting. Run /ctx-wrapup again to continue.";
357373
break;
358374
}
375+
const leaseHolder = leaseResult.holderId;
359376
const leaseRenewal = setInterval(() => {
360377
try {
361378
renewCompartmentLease(deps.db, sessionId, leaseHolder);
@@ -480,13 +497,20 @@ async function acquireCompartmentLeaseEventually(
480497
renewWrapupMarker: (
481498
updates: Parameters<typeof updateWrapupInProgress>[3],
482499
) => boolean,
483-
): Promise<string | null> {
500+
maxWaitMs: number,
501+
): Promise<LeaseAcquireResult> {
502+
const waitStartedAt = Date.now();
503+
const remainingMs = (): number =>
504+
Math.max(0, waitStartedAt + maxWaitMs - Date.now());
484505
for (;;) {
506+
if (remainingMs() <= 0) return { ok: false, reason: "timeout" };
485507
const holderId = crypto.randomUUID();
486508
const lease = acquireCompartmentLease(db, sessionId, holderId);
487-
if (lease) return holderId;
488-
if (!renewWrapupMarker({})) return null;
489-
await new Promise((resolve) => setTimeout(resolve, LEASE_WAIT_MS));
509+
if (lease) return { ok: true, holderId };
510+
if (!renewWrapupMarker({})) return { ok: false, reason: "ownership_lost" };
511+
await new Promise((resolve) =>
512+
setTimeout(resolve, Math.min(LEASE_WAIT_MS, remainingMs())),
513+
);
490514
}
491515
}
492516

packages/plugin/src/hooks/magic-context/compartment-runner.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { Database } from "../../shared/sqlite";
3434
import { closeQuietly } from "../../shared/sqlite-helpers";
3535
import {
3636
executeContextRecomp,
37+
executeContextRecompWithResult,
3738
getActiveCompartmentRun,
3839
registerActiveCompartmentRun,
3940
runCompartmentAgent,
@@ -175,6 +176,68 @@ describe("executeContextRecomp", () => {
175176
expect(getSessionFacts(db, "ses-recomp")).toHaveLength(0);
176177
});
177178

179+
it("aborts after winning the recomp lease if a wrapup marker appeared meanwhile", async () => {
180+
useTempDataHome("magic-recomp-wrapup-race-");
181+
const sessionId = "ses-recomp-wrapup-race";
182+
createOpenCodeDb(sessionId, [
183+
{ id: "m-1", role: "user", text: "eligible one" },
184+
{ id: "m-2", role: "assistant", text: "eligible two" },
185+
{ id: "m-3", role: "user", text: "protected 1" },
186+
{ id: "m-4", role: "user", text: "protected 2" },
187+
{ id: "m-5", role: "user", text: "protected 3" },
188+
{ id: "m-6", role: "user", text: "protected 4" },
189+
{ id: "m-7", role: "user", text: "protected 5" },
190+
]);
191+
const db = openDatabase();
192+
getOrCreateSessionMeta(db, sessionId);
193+
194+
const client = {
195+
session: {
196+
get: mock(async () => {
197+
throw new Error("recomp runner should not start under a live wrapup");
198+
}),
199+
create: mock(async () => ({ data: { id: "unused" } })),
200+
prompt: mock(async () => ({})),
201+
messages: mock(async () => ({ data: [] })),
202+
delete: mock(async () => ({})),
203+
},
204+
} as unknown as PluginContext["client"];
205+
206+
const result = await executeContextRecompWithResult(
207+
{
208+
client,
209+
db,
210+
sessionId,
211+
historianChunkTokens: 10_000,
212+
directory: "/tmp",
213+
},
214+
{
215+
onLeaseAcquired: () => {
216+
const acquired = acquireWrapupInProgress(db, sessionId, {
217+
holderId: "wrapup-holder",
218+
messagesToKeep: 5,
219+
anchorRawMessageCount: 7,
220+
targetEligibleEndOrdinal: 3,
221+
lastCompartmentEnd: -1,
222+
chunkIndex: 0,
223+
expectedChunks: 1,
224+
});
225+
expect(acquired.ok).toBe(true);
226+
},
227+
},
228+
);
229+
230+
expect(result.published).toBe(false);
231+
expect(result.message).toContain("/ctx-wrapup is already compacting");
232+
expect(client.session.get).not.toHaveBeenCalled();
233+
expect(getCompartments(db, sessionId)).toHaveLength(0);
234+
expect(
235+
db
236+
.prepare("SELECT 1 FROM compartment_state_lease WHERE session_id = ?")
237+
.get(sessionId) ?? null,
238+
).toBeNull();
239+
});
240+
178241
it("keeps published state unchanged when a later recomp pass fails", async () => {
179242
useTempDataHome("magic-recomp-fail-closed-");
180243
createOpenCodeDb("ses-recomp-fail", [

packages/plugin/src/hooks/magic-context/compartment-runner.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,8 @@ export interface ExecuteContextRecompOptions {
206206
* replacing all compartments and facts.
207207
*/
208208
range?: PartialRecompRange;
209+
/** @internal Exercises the lease/marker race without delaying production callers. */
210+
onLeaseAcquired?: () => void;
209211
}
210212

211213
export interface ExecuteContextRecompResult {
@@ -247,6 +249,19 @@ export async function executeContextRecompWithResult(
247249
published: false,
248250
};
249251
}
252+
options.onLeaseAcquired?.();
253+
if (isWrapupInProgress(deps.db, sessionId)) {
254+
// Close the marker/lease race: wrapup publishes its durable ownership
255+
// marker before it waits for the compartment lease, so a recomp that
256+
// checked early can still win the lease after wrapup has started.
257+
sessionLog(sessionId, "recomp skipped: /ctx-wrapup became active");
258+
releaseCompartmentLease(deps.db, sessionId, holderId);
259+
return {
260+
message:
261+
"## Magic Recomp — Skipped\n\n/ctx-wrapup is already compacting this session. Wait for it to finish, then try `/ctx-recomp` again.",
262+
published: false,
263+
};
264+
}
250265
const renewal = startLeaseRenewal(deps, holderId);
251266
const runnerDeps = withPublishedCallback({ ...deps, compartmentLeaseHolderId: holderId });
252267
const promise = options.range

packages/plugin/src/hooks/magic-context/protected-tail-boundary.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,39 @@ it("fingerprints and true-raw tokens change when nested tool output grows with t
510510
expect(longTokens).toBeGreaterThan(shortTokens);
511511
});
512512

513+
it("fingerprints same-length edits to counted content fields", () => {
514+
const before: RawMessage[] = [
515+
{
516+
ordinal: 1,
517+
id: "m1",
518+
role: "assistant",
519+
version: 1,
520+
parts: [
521+
{ type: "text", text: "alpha" },
522+
{ type: "thinking", thinking: "bravo" },
523+
{ type: "tool", callID: "call-1", state: { input: { q: "one" }, output: "delta" } },
524+
],
525+
},
526+
];
527+
const after: RawMessage[] = [
528+
{
529+
ordinal: 1,
530+
id: "m1",
531+
role: "assistant",
532+
version: 1,
533+
parts: [
534+
{ type: "text", text: "omega" },
535+
{ type: "thinking", thinking: "gamma" },
536+
{ type: "tool", callID: "call-1", state: { input: { q: "two" }, output: "sigma" } },
537+
],
538+
},
539+
];
540+
541+
expect(computeRawRangeFingerprint(before, 1, 2)).not.toBe(
542+
computeRawRangeFingerprint(after, 1, 2),
543+
);
544+
});
545+
513546
it("moves a candidate boundary forward to the first later open tool invocation", () => {
514547
expect(
515548
fenceBoundaryForToolArcs(10, [{ callId: "open", invOrdinal: 20, resOrdinal: null }], 9, 10),

packages/plugin/src/hooks/magic-context/read-session-chunk.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,5 +421,63 @@ describe("readSessionChunk", () => {
421421
expect(chunk.messageCount).toBe(1);
422422
expect(chunk.hasMore).toBe(false);
423423
});
424+
425+
it("reports hasMore false when the remaining eligible tail is only filtered noise", () => {
426+
//#given
427+
useTempDataHome("read-session-noise-tail-");
428+
createOpenCodeDbWithMessages("ses-noise-tail", [
429+
{ id: "m-1", role: "user", part: { type: "text", text: "eligible work" } },
430+
{ id: "m-2", role: "assistant", part: { type: "text", text: "done" } },
431+
{
432+
id: "m-3",
433+
role: "user",
434+
part: { type: "text", text: "## Magic Status", ignored: true },
435+
},
436+
{
437+
id: "m-4",
438+
role: "user",
439+
part: {
440+
type: "text",
441+
text: "<system-reminder>background finished</system-reminder>",
442+
},
443+
},
444+
]);
445+
446+
//#when
447+
const chunk = readSessionChunk("ses-noise-tail", 100_000, 1);
448+
449+
//#then: filtered tail noise was scanned, so no semantic work remains.
450+
expect(chunk.endIndex).toBe(2);
451+
expect(chunk.text).toContain("eligible work");
452+
expect(chunk.text).not.toContain("Magic Status");
453+
expect(chunk.hasMore).toBe(false);
454+
});
455+
456+
it("keeps hasMore true when budget-blocked content precedes trailing noise", () => {
457+
//#given
458+
useTempDataHome("read-session-blocked-before-noise-");
459+
createOpenCodeDbWithMessages("ses-blocked-before-noise", [
460+
{ id: "m-1", role: "user", part: { type: "text", text: "first content" } },
461+
{
462+
id: "m-2",
463+
role: "assistant",
464+
part: { type: "text", text: "second content beyond budget" },
465+
},
466+
{
467+
id: "m-3",
468+
role: "user",
469+
part: { type: "text", text: "## Magic Status", ignored: true },
470+
},
471+
]);
472+
473+
//#when
474+
const chunk = readSessionChunk("ses-blocked-before-noise", 1, 1);
475+
476+
//#then
477+
expect(chunk.endIndex).toBe(1);
478+
expect(chunk.text).toContain("first content");
479+
expect(chunk.text).not.toContain("second content");
480+
expect(chunk.hasMore).toBe(true);
481+
});
424482
});
425483
});

0 commit comments

Comments
 (0)