mason: fix smart-note lifecycle edges

ualtinok · alfonso-magic-context · ualtinok · commit 664d147df450 · 2026-07-07T20:21:10.000+02:00
Co-authored-by: Alfonso [Magic Context] &lt;288211368+alfonso-magic-context@users.noreply.github.com&gt;
diff --git a/packages/plugin/src/features/magic-context/dreamer/evaluate-smart-notes.ts b/packages/plugin/src/features/magic-context/dreamer/evaluate-smart-notes.ts
@@ -187,7 +187,8 @@ async function compileNote(
             sessionDirectory: args.sessionDirectory,
             projectIdentity: args.projectIdentity,
             note,
-            capabilities: createSmartNoteCapabilities({ projectRoot, signal: controller.signal }),
+            capabilityFactory: (signal) => createSmartNoteCapabilities({ projectRoot, signal }),
+            signal: controller.signal,
             deadline: args.deadline,
             model: args.model,
             fallbackModels: args.fallbackModels,
@@ -261,7 +262,7 @@ async function runLivenessCheck(
     try {
         const result = await runCompiledSmartNoteCheck({
             compiledCheck: note.compiledCheck,
-            capabilities: createSmartNoteCapabilities({ projectRoot, signal: controller.signal }),
+            capabilityFactory: (signal) => createSmartNoteCapabilities({ projectRoot, signal }),
             signal: controller.signal,
             timeoutMs: 2_000,
         });
diff --git a/packages/plugin/src/features/magic-context/smart-notes/capabilities.ts b/packages/plugin/src/features/magic-context/smart-notes/capabilities.ts
@@ -24,6 +24,8 @@ export interface SmartNoteCapabilityApi {
     httpGet(url: string): Promise<{ status: number; body: string }>;
 }
 
+export type SmartNoteCapabilityFactory = (signal: AbortSignal) => SmartNoteCapabilityApi;
+
 export interface SmartNoteCapabilitiesOptions {
     projectRoot: string;
     signal: AbortSignal;
diff --git a/packages/plugin/src/features/magic-context/smart-notes/compiler.ts b/packages/plugin/src/features/magic-context/smart-notes/compiler.ts
@@ -8,7 +8,7 @@ import { log } from "../../../shared/logger";
 import { modelBodyField } from "../../../shared/resolve-fallbacks";
 import type { Database } from "../../../shared/sqlite";
 import { recordChildInvocation } from "../subagent-token-capture";
-import type { SmartNoteCapabilityApi } from "./capabilities";
+import type { SmartNoteCapabilityFactory } from "./capabilities";
 import { SMART_NOTE_COMPILER_SYSTEM_PROMPT } from "./compiler-prompt";
 import { runCompiledSmartNoteCheck } from "./sandbox-runner";
 import type {
@@ -24,7 +24,8 @@ interface CompileSmartNoteArgs {
     sessionDirectory: string | undefined;
     projectIdentity: string;
     note: { id: number; content: string; surfaceCondition: string | null };
-    capabilities: SmartNoteCapabilityApi;
+    capabilityFactory: SmartNoteCapabilityFactory;
+    signal: AbortSignal;
     deadline: number;
     model?: string;
     fallbackModels?: readonly string[];
@@ -149,7 +150,8 @@ Remember: output only the JSON object described by the system prompt.`;
         }
         const dryRun = await runCompiledSmartNoteCheck({
             compiledCheck,
-            capabilities: args.capabilities,
+            capabilityFactory: args.capabilityFactory,
+            signal: args.signal,
             timeoutMs: 2_000,
         });
         if (!dryRun.ok) {
diff --git a/packages/plugin/src/features/magic-context/smart-notes/runner.ts b/packages/plugin/src/features/magic-context/smart-notes/runner.ts
@@ -79,10 +79,11 @@ export async function runDueCompiledSmartNoteChecks(
         try {
             const result = await runCompiledSmartNoteCheck({
                 compiledCheck: note.compiledCheck,
-                capabilities: createSmartNoteCapabilities({
-                    projectRoot: args.projectRoot,
-                    signal: controller.signal,
-                }),
+                capabilityFactory: (signal) =>
+                    createSmartNoteCapabilities({
+                        projectRoot: args.projectRoot,
+                        signal,
+                    }),
                 signal: controller.signal,
                 timeoutMs: Math.min(2_000, remaining),
             });
diff --git a/packages/plugin/src/features/magic-context/smart-notes/sandbox-runner.test.ts b/packages/plugin/src/features/magic-context/smart-notes/sandbox-runner.test.ts
@@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test";
 
 import type { SmartNoteCapabilityApi } from "./capabilities";
 import { runCompiledSmartNoteCheck } from "./sandbox-runner";
+import { SmartNoteNetworkError } from "./types";
 
 const fakeCap: SmartNoteCapabilityApi = {
     readFile: async (path) => (path === "ready.txt" ? "ready" : null),
@@ -37,6 +38,67 @@ describe("compiled smart-note QuickJS runner", () => {
         expect(result.ok).toBe(false);
     });
 
+    test("aborts host capabilities on the run timeout and frees the shared lock", async () => {
+        await runCompiledSmartNoteCheck({
+            compiledCheck: `function check() { return { met: true }; }`,
+            capabilities: fakeCap,
+        });
+
+        const startedAt = Date.now();
+        const timedOut = (await Promise.race([
+            runCompiledSmartNoteCheck({
+                compiledCheck: `function check(cap) { cap.httpGet("https://example.test/"); return { met: false }; }`,
+                capabilityFactory: (signal) => ({
+                    ...fakeCap,
+                    httpGet: () =>
+                        new Promise((_resolve, reject) => {
+                            const abort = () =>
+                                reject(new SmartNoteNetworkError("SMART_NOTE_NETWORK: aborted"));
+                            if (signal.aborted) {
+                                abort();
+                                return;
+                            }
+                            signal.addEventListener("abort", abort, { once: true });
+                        }),
+                }),
+                timeoutMs: 100,
+            }),
+            new Promise<never>((_, reject) =>
+                setTimeout(
+                    () => reject(new Error("sandbox timeout did not abort the host capability")),
+                    1_000,
+                ),
+            ),
+        ])) as Awaited<ReturnType<typeof runCompiledSmartNoteCheck>>;
+        const elapsed = Date.now() - startedAt;
+
+        expect(timedOut.ok).toBe(false);
+        if (!timedOut.ok) {
+            expect(timedOut.network).toBe(true);
+        }
+        expect(elapsed).toBeGreaterThanOrEqual(50);
+        expect(elapsed).toBeLessThan(1_000);
+
+        const followup = (await Promise.race([
+            runCompiledSmartNoteCheck({
+                compiledCheck: `function check() { return { met: true }; }`,
+                capabilities: fakeCap,
+            }),
+            new Promise<never>((_, reject) =>
+                setTimeout(
+                    () =>
+                        reject(
+                            new Error(
+                                "follow-up sandbox run stayed blocked behind the timed-out host call",
+                            ),
+                        ),
+                    500,
+                ),
+            ),
+        ])) as Awaited<ReturnType<typeof runCompiledSmartNoteCheck>>;
+        expect(followup).toEqual({ ok: true, result: { met: true } });
+    });
+
     test("serializes concurrent checks whose host calls suspend (shared-module asyncify safety)", async () => {
         // Regression for QuickJSUseAfterFree: the asyncify module has ONE
         // suspension stack; before serialization, two checks suspended in host
diff --git a/packages/plugin/src/features/magic-context/smart-notes/sandbox-runner.ts b/packages/plugin/src/features/magic-context/smart-notes/sandbox-runner.ts
@@ -14,7 +14,7 @@ import {
     type QuickJSHandle,
 } from "quickjs-emscripten";
 
-import type { SmartNoteCapabilityApi } from "./capabilities";
+import type { SmartNoteCapabilityApi, SmartNoteCapabilityFactory } from "./capabilities";
 import { isSmartNoteNetworkError, type SmartNoteCheckResult } from "./types";
 
 /**
@@ -58,7 +58,8 @@ function withSandboxLock<T>(fn: () => Promise<T>): Promise<T> {
 
 export interface RunCompiledSmartNoteCheckOptions {
     compiledCheck: string;
-    capabilities: SmartNoteCapabilityApi;
+    capabilities?: SmartNoteCapabilityApi;
+    capabilityFactory?: SmartNoteCapabilityFactory;
     signal?: AbortSignal;
     timeoutMs?: number;
     heapLimitBytes?: number;
@@ -84,13 +85,36 @@ const DEFAULT_TIMEOUT_MS = 2_000;
 const DEFAULT_HEAP_LIMIT_BYTES = 8 * 1024 * 1024;
 const DEFAULT_STACK_LIMIT_BYTES = 512 * 1024;
 
+// Host calls can outlive the VM interrupt path, so any capability that touches
+// the outside world must listen to this run's controller. Otherwise one tarpit
+// request can keep the shared QuickJS module suspended past the sandbox budget
+// and block the next caller on the process-wide lock.
+function resolveCapabilitiesForRun(
+    options: RunCompiledSmartNoteCheckOptions,
+    signal: AbortSignal,
+): SmartNoteCapabilityApi {
+    if (options.capabilityFactory) {
+        return options.capabilityFactory(signal);
+    }
+    if (options.capabilities) {
+        return options.capabilities;
+    }
+    throw new Error("smart-note check requires capabilities");
+}
+
+function throwIfRunAborted(signal: AbortSignal): void {
+    if (signal.aborted) {
+        throw signal.reason ?? new Error("smart-note check aborted");
+    }
+}
+
 export async function runCompiledSmartNoteCheck(
     options: RunCompiledSmartNoteCheckOptions,
 ): Promise<RunCompiledSmartNoteCheckResult> {
     // Serialize the actual sandbox work (see withSandboxLock): only one
     // asyncify-suspended eval may exist at a time on the shared module. The
-    // per-check timeout starts INSIDE the lock so a check queued behind another
-    // doesn't burn its own budget waiting for the lock.
+    // per-check timeout and host-capability controller start INSIDE the lock so
+    // a check queued behind another doesn't burn its own budget waiting.
     return withSandboxLock(() => runCompiledSmartNoteCheckLocked(options));
 }
 
@@ -100,22 +124,29 @@ async function runCompiledSmartNoteCheckLocked(
     const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS;
     const controller = new AbortController();
     const externalAbort = () => controller.abort(options.signal?.reason);
-    options.signal?.addEventListener("abort", externalAbort, { once: true });
+    if (options.signal?.aborted) {
+        externalAbort();
+    } else {
+        options.signal?.addEventListener("abort", externalAbort, { once: true });
+    }
     const timer = setTimeout(
         () => controller.abort(new Error("smart-note check timed out")),
         timeoutMs,
     );
     try {
+        throwIfRunAborted(controller.signal);
+        const capabilities = resolveCapabilitiesForRun(options, controller.signal);
         const deadline = Date.now() + timeoutMs;
         const quickjs = await getAsyncModule();
+        throwIfRunAborted(controller.signal);
         const context = quickjs.newContext();
         try {
             context.runtime.setMemoryLimit(options.heapLimitBytes ?? DEFAULT_HEAP_LIMIT_BYTES);
             context.runtime.setMaxStackSize(options.stackLimitBytes ?? DEFAULT_STACK_LIMIT_BYTES);
             context.runtime.setInterruptHandler(
                 () => controller.signal.aborted || Date.now() > deadline,
             );
-            installCapabilityObject(context, options.capabilities);
+            installCapabilityObject(context, capabilities);
             disableAmbientDynamicCode(context);
             const result = await evalCheck(context, options.compiledCheck);
             const checkResult = result as { met?: unknown } | null;
diff --git a/packages/plugin/src/features/magic-context/smart-notes/schedule.test.ts b/packages/plugin/src/features/magic-context/smart-notes/schedule.test.ts
@@ -0,0 +1,50 @@
+import { describe, expect, test } from "bun:test";
+
+import { nextSmartNoteCheckDueAt } from "./schedule";
+
+function collectDeltas(
+    cron: string,
+    options: { now: number; floorMs: number; ceilingMs: number; hashPrefix: string },
+): number[] {
+    return Array.from(
+        { length: 128 },
+        (_, index) =>
+            nextSmartNoteCheckDueAt(cron, {
+                now: options.now,
+                noteId: index + 1,
+                hash: `${options.hashPrefix}-${index}`,
+                floorMs: options.floorMs,
+                ceilingMs: options.ceilingMs,
+            }) - options.now,
+    );
+}
+
+describe("nextSmartNoteCheckDueAt", () => {
+    test("keeps a floor-1ms schedule at or above the floor after jitter", () => {
+        const floorMs = 60_000;
+        const ceilingMs = 10 * floorMs;
+        const deltas = collectDeltas("* * * * *", {
+            now: Date.UTC(2026, 0, 1, 0, 0, 59, 999),
+            floorMs,
+            ceilingMs,
+            hashPrefix: "floor",
+        });
+
+        expect(Math.min(...deltas)).toBeGreaterThanOrEqual(floorMs);
+        expect(Math.max(...deltas)).toBeLessThanOrEqual(ceilingMs);
+    });
+
+    test("keeps a ceiling-clamped schedule at or below the ceiling after jitter", () => {
+        const floorMs = 1_000;
+        const ceilingMs = 60_000;
+        const deltas = collectDeltas("0 * * * *", {
+            now: Date.UTC(2026, 0, 1, 0, 0, 0, 0),
+            floorMs,
+            ceilingMs,
+            hashPrefix: "ceiling",
+        });
+
+        expect(Math.min(...deltas)).toBeGreaterThanOrEqual(floorMs);
+        expect(Math.max(...deltas)).toBeLessThanOrEqual(ceilingMs);
+    });
+});
diff --git a/packages/plugin/src/features/magic-context/smart-notes/schedule.ts b/packages/plugin/src/features/magic-context/smart-notes/schedule.ts
@@ -23,7 +23,9 @@ export function nextSmartNoteCheckDueAt(
     const rawNext = cron?.trim() ? nextDueAtMs(cron, now) : null;
     const rawDelta = rawNext ? rawNext - now : SMART_NOTE_CHECK_DEFAULT_INTERVAL_MS;
     const clamped = Math.min(ceilingMs, Math.max(floorMs, rawDelta));
-    return now + clamped + deterministicJitterMs(clamped, options.noteId, options.hash);
+    const jittered = clamped + deterministicJitterMs(clamped, options.noteId, options.hash);
+    const bounded = Math.min(ceilingMs, Math.max(floorMs, jittered));
+    return now + bounded;
 }
 
 function deterministicJitterMs(intervalMs: number, noteId?: number, hash?: string | null): number {
diff --git a/packages/plugin/src/features/magic-context/smart-notes/ssrf-guard.test.ts b/packages/plugin/src/features/magic-context/smart-notes/ssrf-guard.test.ts
@@ -1,6 +1,12 @@
-import { describe, expect, test } from "bun:test";
+import { describe, expect, mock, test } from "bun:test";
 
-import { createPinnedLookup, type SmartNoteResolver, validateSmartNoteHttpUrl } from "./ssrf-guard";
+import {
+    createPinnedLookup,
+    guardedSmartNoteHttpGet,
+    type SmartNoteResolver,
+    validateSmartNoteHttpUrl,
+} from "./ssrf-guard";
+import { SmartNoteNetworkError } from "./types";
 
 const signal = new AbortController().signal;
 
@@ -84,6 +90,80 @@ describe("smart-note SSRF guard", () => {
             "2606:2800:220:1:248:1893:25c8:1946",
         ]);
     });
+
+    test("stops after a terminal per-target failure", async () => {
+        const contacted: string[] = [];
+        const requestAddress = mock(async (_validation, candidate) => {
+            contacted.push(candidate.address);
+            throw new SmartNoteNetworkError("SMART_NOTE_NETWORK: response body too large", {
+                terminal: true,
+            });
+        });
+
+        const error = await guardedSmartNoteHttpGet("https://example.test/", {
+            signal,
+            resolver: resolver([
+                { address: "93.184.216.34", family: 4 },
+                { address: "1.1.1.1", family: 4 },
+            ]),
+            requestAddress,
+        }).catch((error) => error);
+
+        expect(error).toBeInstanceOf(SmartNoteNetworkError);
+        expect((error as SmartNoteNetworkError).terminal).toBe(true);
+        expect(contacted).toEqual(["93.184.216.34"]);
+        expect(requestAddress.mock.calls).toHaveLength(1);
+    });
+
+    test("advances to the next address after a connection-level failure", async () => {
+        const contacted: string[] = [];
+        const requestAddress = mock(async (_validation, candidate) => {
+            contacted.push(candidate.address);
+            if (candidate.address === "93.184.216.34") {
+                throw new SmartNoteNetworkError("SMART_NOTE_NETWORK: connect ECONNREFUSED");
+            }
+            return { status: 200, body: "ok" };
+        });
+
+        const response = await guardedSmartNoteHttpGet("https://example.test/", {
+            signal,
+            resolver: resolver([
+                { address: "93.184.216.34", family: 4 },
+                { address: "1.1.1.1", family: 4 },
+            ]),
+            requestAddress,
+        });
+
+        expect(response).toEqual({ status: 200, body: "ok" });
+        expect(contacted).toEqual(["93.184.216.34", "1.1.1.1"]);
+        expect(requestAddress.mock.calls).toHaveLength(2);
+    });
+
+    test("caps the validated address fanout", async () => {
+        const addresses = [
+            "93.184.216.34",
+            "1.1.1.1",
+            "8.8.8.8",
+            "151.101.1.69",
+            "13.107.42.14",
+            "208.67.222.222",
+        ].map((address) => ({ address, family: 4 as const }));
+        const contacted: string[] = [];
+        const requestAddress = mock(async (_validation, candidate) => {
+            contacted.push(candidate.address);
+            throw new SmartNoteNetworkError("SMART_NOTE_NETWORK: connect ECONNREFUSED");
+        });
+
+        const error = await guardedSmartNoteHttpGet("https://example.test/", {
+            signal,
+            resolver: resolver(addresses),
+            requestAddress,
+        }).catch((error) => error);
+
+        expect(error).toBeInstanceOf(SmartNoteNetworkError);
+        expect(contacted).toEqual(addresses.slice(0, 4).map((candidate) => candidate.address));
+        expect(requestAddress.mock.calls).toHaveLength(4);
+    });
 });
 
 describe("createPinnedLookup", () => {
diff --git a/packages/plugin/src/features/magic-context/smart-notes/ssrf-guard.ts b/packages/plugin/src/features/magic-context/smart-notes/ssrf-guard.ts
diff --git a/packages/plugin/src/features/magic-context/smart-notes/types.ts b/packages/plugin/src/features/magic-context/smart-notes/types.ts

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,8 @@ export interface SmartNoteCapabilityApi {`
`24`	`24`	`httpGet(url: string): Promise<{ status: number; body: string }>;`
`25`	`25`	`}`
`26`	`26`
	`27`	`+export type SmartNoteCapabilityFactory = (signal: AbortSignal) => SmartNoteCapabilityApi;`
	`28`	`+`
`27`	`29`	`export interface SmartNoteCapabilitiesOptions {`
`28`	`30`	`projectRoot: string;`
`29`	`31`	`signal: AbortSignal;`