Files
Michael Ramos 9ee2e83287 feat(review): make the CallDiff runtime a strictly opt-in, in-UI install (#1270)
* feat(review): make the CallDiff runtime a strictly opt-in, in-UI install

The merged CallDiff integration eagerly installed a ~784MB runtime for
every user at install time, for a feature that is off by default. The
runtime is now strictly opt-in and the opt-in lives in the review UI:
toggle Call flow, click Install in the panel, watch staged progress, and
use the analysis in the same session.

Installers: the default sequence no longer installs the runtime. Opt in
with --with-call-flow (PowerShell: -WithCallFlow),
PLANNOTATOR_INSTALL_CALLDIFF=1, or { "installCallFlow": true } in
config.json (flag > env > config). PLANNOTATOR_SKIP_CALLDIFF_INSTALL is
deleted; --minimal keeps excluding the runtime; the installer prints an
honest note pointing at the in-app install. The headless CLI path
(plannotator install-runtime call-flow) is unchanged.

Server (both runtimes, contract-identical): POST /api/call-flow/install
starts installCallFlowRuntime() in the background via a single-flighted
coordinator (concurrent POSTs join the in-flight install), runs a
Node 22+ preflight before any download (distinct node-unavailable
error), and rejects cross-origin POSTs with 403. GET
/api/call-flow/install-status reports idle/running/done/error with
stage: downloading, verifying, installing-deps, building. Install
completion invalidates the 30s runtime probe cache so the next
capability advert resolves available without a server restart.

Client: the Call flow Dock's runtime-missing state is now the opt-in
funnel with an honest disclosure (about 800 MB on disk, Node 22+,
one-time), staged reduced-motion-safe progress, and error + retry with
a no-node hint. On done the advert is refreshed through
POST /api/review-analysis and the existing available-change refetch
starts the analysis for the current snapshot with no reload. The intro
dialog and Settings toggle note the separate first-use runtime.

Docs: AGENTS.md env table + Review Server API table, marketing
environment-variables / installation / ui-settings / code-review /
api-endpoints pages, and the CallDiff ADR runtime-boundary and server
contract sections.

* test(review): stop leaking PLANNOTATOR_DATA_DIR from the install endpoint tests

The call-flow install endpoint tests overrode PLANNOTATOR_DATA_DIR at
module-eval time and never restored it. bun runs CI's full suite in one
process and evaluates every test file's module before running tests,
while Pi's generated/storage.ts caches its data dir at import time; the
override therefore made storage's cached dir and later files' live
getPlannotatorDataDir() calls disagree, failing the Pi annotate-history
unwritable-dir test and both durable-submit-record tests.

An afterAll restore alone is not enough: it reproduces the same three
failures with the mismatch inverted (storage caches the leaked dir at
module eval, tests then run against the restored one). The env var is
now never touched at module-eval time at all; it changes only inside
tests and is restored to its original value in afterEach, exactly like
the PORT/PATH pattern. The config writes the advert tests persist
through the process's frozen config module are snapshotted at load and
restored in afterAll so a standalone run never flips a real
config.json setting, and the process-global scope of the mock.module
seams is documented.

Regression proof (previously failing in either mismatch direction, now
green in both orderings):

  bun test packages/server/call-flow-install-endpoint.test.ts \
    apps/pi-extension/server/annotate-history.test.ts \
    apps/pi-extension/server/annotate-submission.test.ts

* feat(review): install CallDiff grammars selectively

* fix(review): harden CallDiff worker environment

* fix(review): close CallDiff verification gaps
2026-08-11 16:28:08 -07:00

194 lines
7.8 KiB
TypeScript

import { describe, expect, test } from "bun:test";
import { CallFlowInstallCoordinator, callFlowInstallOriginAllowed } from "./call-flow-install";
import type { CallFlowInstallStage, CallFlowRuntimeInstallResult } from "./call-flow";
import type { CallFlowLanguageId } from "./call-flow-languages";
function deferred<T>(): { promise: Promise<T>; resolve: (value: T) => void } {
let resolve!: (value: T) => void;
const promise = new Promise<T>((resolvePromise) => {
resolve = resolvePromise;
});
return { promise, resolve };
}
const installed: CallFlowRuntimeInstallResult = {
ok: true,
status: "installed",
runtimeDir: "/tmp/runtime",
languageId: "python",
message: "installed",
};
describe("CallFlowInstallCoordinator", () => {
test("concurrent starts join one in-flight install", async () => {
let installs = 0;
const gate = deferred<CallFlowRuntimeInstallResult>();
const coordinator = new CallFlowInstallCoordinator({
preflight: async () => ({ ok: true }),
install: () => {
installs++;
return gate.promise;
},
});
const [first, second, third] = await Promise.all([
coordinator.start(["python"]),
coordinator.start(["python"]),
coordinator.start(["python"]),
]);
expect(first).toEqual({ state: "running", stage: "downloading", languageIds: ["python"] });
expect(second).toEqual(first);
expect(third).toEqual(first);
expect(installs).toBe(1);
// Still running: a later POST joins rather than restarting.
expect(await coordinator.start(["python"])).toEqual({ state: "running", stage: "downloading", languageIds: ["python"] });
expect(installs).toBe(1);
gate.resolve(installed);
await Bun.sleep(0);
expect(coordinator.getStatus()).toEqual({ state: "done", languageIds: ["python"] });
expect(installs).toBe(1);
});
test("stage callbacks advance the running status in order", async () => {
let emit: ((stage: CallFlowInstallStage) => void) | undefined;
const gate = deferred<CallFlowRuntimeInstallResult>();
const coordinator = new CallFlowInstallCoordinator({
preflight: async () => ({ ok: true }),
install: (_id, onStage) => {
emit = onStage;
return gate.promise;
},
});
await coordinator.start(["python"]);
expect(coordinator.getStatus()).toEqual({ state: "running", stage: "downloading", languageIds: ["python"] });
emit?.("verifying");
expect(coordinator.getStatus()).toEqual({ state: "running", stage: "verifying", languageIds: ["python"], currentLanguageId: "python" });
emit?.("installing-deps");
expect(coordinator.getStatus()).toEqual({ state: "running", stage: "installing-deps", languageIds: ["python"], currentLanguageId: "python" });
emit?.("building");
expect(coordinator.getStatus()).toEqual({ state: "running", stage: "building", languageIds: ["python"], currentLanguageId: "python" });
gate.resolve(installed);
await Bun.sleep(0);
expect(coordinator.getStatus()).toEqual({ state: "done", languageIds: ["python"] });
// A late stage callback can never resurrect a settled status.
emit?.("downloading");
expect(coordinator.getStatus()).toEqual({ state: "done", languageIds: ["python"] });
});
test("a failed Node preflight reports a distinct error before any install work", async () => {
let installs = 0;
const coordinator = new CallFlowInstallCoordinator({
preflight: async () => ({ ok: false, reason: "node-unavailable", message: "Node.js was not found." }),
install: async () => {
installs++;
return installed;
},
});
const status = await coordinator.start(["python"]);
expect(status).toEqual({ state: "error", error: "Node.js was not found.", reason: "node-unavailable", languageIds: ["python"] });
expect(installs).toBe(0);
// The error persists until the next start retries.
expect(coordinator.getStatus()).toEqual(status);
});
test("a failed preflight cannot leak an old review's languages into a later retry", async () => {
let preflights = 0;
const installedIds: CallFlowLanguageId[] = [];
const coordinator = new CallFlowInstallCoordinator({
preflight: async () => ++preflights === 1
? { ok: false, reason: "node-unavailable", message: "Node.js was not found." }
: { ok: true },
install: async (id) => {
installedIds.push(id);
return { ...installed, languageId: id };
},
});
await coordinator.start(["python"]);
await coordinator.start(["go"]);
await Bun.sleep(0);
expect(installedIds).toEqual(["go"]);
expect(coordinator.getStatus()).toEqual({ state: "done", languageIds: ["go"] });
});
test("an install failure persists as error and the next start retries", async () => {
let installs = 0;
const settled: boolean[] = [];
const coordinator = new CallFlowInstallCoordinator({
preflight: async () => ({ ok: true }),
install: async () => {
installs++;
if (installs === 1) {
return { ok: false, status: "failed", runtimeDir: "/tmp/runtime", message: "npm ci failed" };
}
return installed;
},
onSettled: (ok) => settled.push(ok),
});
await coordinator.start(["python"]);
await Bun.sleep(0);
expect(coordinator.getStatus()).toEqual({ state: "error", error: "npm ci failed", languageIds: ["python"], currentLanguageId: "python" });
expect(settled).toEqual([false]);
await coordinator.start(["python"]);
await Bun.sleep(0);
expect(coordinator.getStatus()).toEqual({ state: "done", languageIds: ["python"] });
expect(installs).toBe(2);
expect(settled).toEqual([false, true]);
});
test("a throwing install settles as error instead of leaving running forever", async () => {
const coordinator = new CallFlowInstallCoordinator({
preflight: async () => ({ ok: true }),
install: async () => {
throw new Error("unexpected crash");
},
});
await coordinator.start(["python"]);
await Bun.sleep(0);
expect(coordinator.getStatus()).toEqual({ state: "error", error: "unexpected crash", languageIds: ["python"], currentLanguageId: "python" });
});
test("queues a second language onto the active single flight", async () => {
const calls: string[] = [];
const first = deferred<CallFlowRuntimeInstallResult>();
const coordinator = new CallFlowInstallCoordinator({
preflight: async () => ({ ok: true }),
install: async (id) => {
calls.push(id);
if (id === "python") return first.promise;
return { ...installed, languageId: id };
},
});
await coordinator.start(["python"]);
await coordinator.start(["go"]);
expect(coordinator.getStatus()).toMatchObject({ state: "running", languageIds: ["python", "go"] });
first.resolve(installed);
await Bun.sleep(0);
await Bun.sleep(0);
expect(calls).toEqual(["python", "go"]);
expect(coordinator.getStatus()).toEqual({ state: "done", languageIds: ["python", "go"] });
});
});
describe("callFlowInstallOriginAllowed", () => {
test("permits same-origin and missing Origin, rejects everything else", () => {
expect(callFlowInstallOriginAllowed(null, "127.0.0.1:4321")).toBe(true);
expect(callFlowInstallOriginAllowed(undefined, "127.0.0.1:4321")).toBe(true);
expect(callFlowInstallOriginAllowed("http://127.0.0.1:4321", "127.0.0.1:4321")).toBe(true);
expect(callFlowInstallOriginAllowed("http://localhost:4321", "localhost:4321")).toBe(true);
expect(callFlowInstallOriginAllowed("https://evil.example", "127.0.0.1:4321")).toBe(false);
expect(callFlowInstallOriginAllowed("http://127.0.0.1:9999", "127.0.0.1:4321")).toBe(false);
expect(callFlowInstallOriginAllowed("null", "127.0.0.1:4321")).toBe(false);
expect(callFlowInstallOriginAllowed("not a url", "127.0.0.1:4321")).toBe(false);
});
});