fix(compact): a synchronous compact throw no longer wedges auto-compaction (audit)

ctx.compact() can throw synchronously (not only via onError); that left the
module-level `compacting` flag stuck true, permanently disabling auto-compaction,
and in the input-preflight path left the promise unresolved so the turn stalled.
New src/dispatch.ts (dispatchCompact) try/catches ctx.compact() and routes a
synchronous throw through onError, which resets the flag and (in compactAndWait)
resolves so the input hook still returns {action:"continue"}.

dispatch unit tests + two extension integration tests. 49 pass, typecheck clean.

Author: hypnguyen1209

extensions/compact.ts

@@ -29,6 +29,7 @@ import { join } from "node:path";
 import { shouldCompact } from "../src/decide.ts";
 import { guardDegeneration } from "../src/degenerate.ts";
 import { anchorInstructions } from "../src/anchors.ts";
+import { dispatchCompact } from "../src/dispatch.ts";
 import { DEFAULT_SETTINGS, resolveSettings, type CompactSettings } from "../src/settings.ts";
 
 type UiContext = ExtensionContext;
@@ -168,7 +169,10 @@ export default function compact(pi: ExtensionAPI) {
 
   function runCompaction(ctx: UiContext, forced: boolean): void {
     compacting = true;
-    ctx.compact({
+    // dispatchCompact routes a SYNCHRONOUS throw from ctx.compact() through
+    // onError (→ onCompactError), so a synchronous failure still clears the
+    // `compacting` flag instead of stranding it true and disabling us for good.
+    dispatchCompact(ctx, {
       customInstructions: compactionInstructions(ctx),
       onComplete: (result) => onCompactComplete(ctx, forced, result),
       onError: (err) => onCompactError(ctx, err),
@@ -179,7 +183,10 @@ export default function compact(pi: ExtensionAPI) {
   function compactAndWait(ctx: UiContext): Promise<void> {
     return new Promise((resolve) => {
       compacting = true;
-      ctx.compact({
+      // A SYNCHRONOUS throw here would otherwise leave both the `compacting`
+      // flag stuck true and this promise unresolved, so the input hook never
+      // returns. Route it through onError, which clears the flag and resolves.
+      dispatchCompact(ctx, {
         customInstructions: compactionInstructions(ctx),
         onComplete: (result) => {
           onCompactComplete(ctx, false, result);

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@pify/compact",
-  "version": "0.4.0",
+  "version": "0.4.1",
   "description": "Proactive auto-compaction: when the context window crosses a threshold, compact between turns so a long session never hits the wall or needs a restart",
   "keywords": [
     "pi-package",

src/dispatch.ts

@@ -0,0 +1,41 @@
+/**
+ * Dispatching a compaction safely — the one bit of the ctx.compact() call that
+ * needs guarding, factored out so it is testable without a live session.
+ *
+ * ctx.compact() reports the usual failures asynchronously via onError, but it
+ * can also throw SYNCHRONOUSLY (e.g. no model configured, or an internal
+ * precondition fails before it dispatches). An unguarded synchronous throw would
+ * escape the caller mid-cleanup: the `compacting` flag stays true — permanently
+ * disabling auto-compaction for the session — and, on the input-preflight path,
+ * the pending promise is never resolved, so the input hook never returns
+ * {action:"continue"} and the turn stalls.
+ *
+ * Routing the synchronous throw through the same onError callback the async path
+ * uses lets each caller run its normal recovery (reset the flag, notify, and —
+ * in the preflight — resolve), so a throwing compact leaves no stuck state.
+ */
+
+export interface CompactDispatchOptions {
+  customInstructions?: string;
+  onComplete?: (result: { tokensBefore: number; estimatedTokensAfter?: number }) => void;
+  onError?: (error: Error) => void;
+}
+
+export interface CompactCapable {
+  compact(options: CompactDispatchOptions): void;
+}
+
+/**
+ * Call ctx.compact(), routing a synchronous throw to options.onError instead of
+ * letting it escape. Returns true when compaction was dispatched, false when it
+ * threw synchronously (onError has already run in that case).
+ */
+export function dispatchCompact(ctx: CompactCapable, options: CompactDispatchOptions): boolean {
+  try {
+    ctx.compact(options);
+    return true;
+  } catch (err) {
+    options.onError?.(err instanceof Error ? err : new Error(String(err)));
+    return false;
+  }
+}

test/compact-extension.test.ts

@@ -0,0 +1,124 @@
+/**
+ * Regression tests for the whole extension: a ctx.compact() that throws
+ * SYNCHRONOUSLY must not strand the `compacting` flag (which permanently
+ * disables auto-compaction) nor, on the input-preflight path, leave the promise
+ * unresolved so the input hook never returns {action:"continue"}.
+ *
+ * The extension only leaves its dormant state when pi's own compaction is off,
+ * so each test points ctx.cwd at a temp project whose .pi/settings.json disables
+ * pi's built-in compaction (roots are injected, never via HOME — os.homedir may
+ * be cached under bun). Everything else is a hand-rolled fake ctx/pi.
+ */
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import compact from "../extensions/compact.ts";
+import type { ExtensionAPI } from "@earendil-works/pi-coding-agent";
+
+type Handler = (event: unknown, ctx: unknown) => unknown;
+
+/** Instantiate the extension against a fake pi, returning its captured handlers. */
+function loadExtension(): Map<string, Handler> {
+  const handlers = new Map<string, Handler>();
+  const pi = {
+    on(evt: string, fn: Handler) {
+      handlers.set(evt, fn);
+    },
+    registerCommand() {},
+  } as unknown as ExtensionAPI;
+  compact(pi);
+  return handlers;
+}
+
+/** A temp project dir with pi's built-in compaction OFF (so the ext is active). */
+function makeActiveProjectDir(): string {
+  const dir = mkdtempSync(join(tmpdir(), "pify-compact-"));
+  mkdirSync(join(dir, ".pi"), { recursive: true });
+  writeFileSync(join(dir, ".pi", "settings.json"), JSON.stringify({ compaction: { enabled: false } }));
+  return dir;
+}
+
+function cleanup(dir: string): void {
+  try {
+    rmSync(dir, { recursive: true, force: true });
+  } catch {
+    // best-effort; a lingering lockfile on Windows must not fail the test
+  }
+}
+
+interface FakeCtx {
+  cwd: string;
+  hasUI: boolean;
+  isProjectTrusted: () => boolean;
+  sessionManager: { getBranch: () => unknown[] };
+  ui: { notify: (msg: string, level: string) => void; setStatus: () => void };
+  getContextUsage: () => { percent: number; tokens: number; contextWindow: number };
+  compact: () => void;
+}
+
+/** ctx whose compact() always throws synchronously, counting each attempt. */
+function makeThrowingCtx(dir: string, usage: { percent: number; tokens: number; contextWindow: number }) {
+  const state = { compactAttempts: 0, warnings: [] as string[] };
+  const ctx: FakeCtx = {
+    cwd: dir,
+    hasUI: true,
+    isProjectTrusted: () => true,
+    sessionManager: { getBranch: () => [] },
+    ui: {
+      notify: (msg, level) => {
+        if (level === "warning") state.warnings.push(msg);
+      },
+      setStatus: () => {},
+    },
+    getContextUsage: () => usage,
+    compact: () => {
+      state.compactAttempts++;
+      throw new Error("synthetic synchronous compact failure");
+    },
+  };
+  return { ctx, state };
+}
+
+test("agent_settled: a synchronous compact throw resets the flag, so the next settle compacts again", async () => {
+  const dir = makeActiveProjectDir();
+  try {
+    const handlers = loadExtension();
+    const { ctx, state } = makeThrowingCtx(dir, { percent: 95, tokens: 500_000, contextWindow: 1_000_000 });
+
+    await handlers.get("session_start")!({}, ctx); // determines active = true
+
+    // First settle: over threshold → compact → ctx.compact() throws synchronously.
+    await handlers.get("agent_settled")!({}, ctx);
+    // Second settle: if the throw had stranded `compacting` at true, shouldCompact
+    // would refuse and this would be a no-op. It must compact again.
+    await handlers.get("agent_settled")!({}, ctx);
+
+    assert.equal(state.compactAttempts, 2, "compaction is attempted on both settles (flag was reset)");
+    assert.equal(state.warnings.length, 2, "each synchronous failure is surfaced as a warning");
+  } finally {
+    cleanup(dir);
+  }
+});
+
+test("input preflight: a synchronous compact throw still resolves and returns continue", async () => {
+  const dir = makeActiveProjectDir();
+  try {
+    const handlers = loadExtension();
+    const { ctx, state } = makeThrowingCtx(dir, { percent: 95, tokens: 850_000, contextWindow: 1_000_000 });
+
+    await handlers.get("session_start")!({}, ctx);
+
+    // A big-enough prompt while idle trips the preflight → compactAndWait → throw.
+    const result = await handlers.get("input")!({ text: "hello", streamingBehavior: undefined }, ctx);
+    assert.deepEqual(result, { action: "continue" }, "input hook returns despite the synchronous throw");
+    assert.equal(state.compactAttempts, 1, "preflight attempted the compaction");
+
+    // Flag must be cleared: a following settle over threshold compacts again.
+    await handlers.get("agent_settled")!({}, ctx);
+    assert.equal(state.compactAttempts, 2, "compacting flag was reset by the preflight failure");
+  } finally {
+    cleanup(dir);
+  }
+});

test/dispatch.test.ts

@@ -0,0 +1,86 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { dispatchCompact, type CompactCapable, type CompactDispatchOptions } from "../src/dispatch.ts";
+
+/** A ctx.compact() that throws synchronously, like pi does when it cannot start. */
+const throwingCtx = (message = "no model configured"): CompactCapable => ({
+  compact() {
+    throw new Error(message);
+  },
+});
+
+/** A ctx.compact() that dispatches fine and reports async (never synchronously). */
+const dispatchingCtx = (): CompactCapable => ({
+  compact() {
+    // returns void; onComplete/onError would fire later in the real runtime
+  },
+});
+
+test("a synchronous throw is routed to onError, not rethrown", () => {
+  let seen: Error | null = null;
+  const ok = dispatchCompact(throwingCtx("boom"), {
+    onError: (err) => {
+      seen = err;
+    },
+  });
+  assert.equal(ok, false, "returns false when compact threw synchronously");
+  assert.ok(seen, "onError was invoked with the thrown error");
+  assert.equal((seen as unknown as Error).message, "boom");
+});
+
+test("a throwing compact runs the caller's flag-reset (compacting → false)", () => {
+  // Mirror the extension: `compacting` is set true just before dispatch and the
+  // onError callback (onCompactError) is what flips it back to false.
+  let compacting = true;
+  const options: CompactDispatchOptions = {
+    onError: () => {
+      compacting = false;
+    },
+  };
+  dispatchCompact(throwingCtx(), options);
+  assert.equal(compacting, false, "flag reset even though compact threw synchronously");
+});
+
+test("the preflight path resolves its promise on a synchronous throw", async () => {
+  // Mirror compactAndWait: onError both clears the flag and resolves the promise.
+  let compacting = true;
+  const done = new Promise<void>((resolve) => {
+    dispatchCompact(throwingCtx(), {
+      onError: () => {
+        compacting = false;
+        resolve();
+      },
+    });
+  });
+  await done; // would hang forever if the throw escaped instead of hitting onError
+  assert.equal(compacting, false);
+});
+
+test("a clean dispatch returns true and does not call onError", () => {
+  let errored = false;
+  const ok = dispatchCompact(dispatchingCtx(), {
+    onError: () => {
+      errored = true;
+    },
+  });
+  assert.equal(ok, true);
+  assert.equal(errored, false, "onError is left for the async path, not fired on a clean dispatch");
+});
+
+test("a non-Error throw is wrapped into an Error for onError", () => {
+  let wasError = false;
+  let message: string | null = null;
+  const ctx: CompactCapable = {
+    compact() {
+      throw "stringly failure";
+    },
+  };
+  dispatchCompact(ctx, {
+    onError: (err) => {
+      wasError = err instanceof Error;
+      message = err.message;
+    },
+  });
+  assert.equal(wasError, true, "a non-Error throw is wrapped into an Error");
+  assert.equal(message, "stringly failure");
+});