Skip to content

Commit cbede01

Browse files
jhgaylorclaude
andauthored
fix: bound and share live reads across requests and polling (#367)
A FIFO budget shared across every live read behold spawns, in-flight work shared by project, chant, source stamp, argv and environment, per-subscriber cancellation with a running-read deadline, and a browser refresh queue that retains one pending refresh instead of scheduling four. Re-measured on the rebase: a burst of six reads spawns 9 chant processes instead of 36 on one project and 18 on a three-member estate; the default budget of two costs a composed estate's cold read about two seconds on a many-core machine, and BEHOLD_ESTATE_CONCURRENCY raises it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DoXg2GDWwXJRPQo76t9Nrq
1 parent e345577 commit cbede01

15 files changed

Lines changed: 617 additions & 98 deletions

README.md

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,32 @@ graph updates, no reload. Add `--poll <secs>` (with `--env`) to also re-query li
284284
drift on an interval and push updates when a node's status changes:
285285

286286
```sh
287-
behold serve ./infra --env prod --poll 30 # watch source + poll drift every 30s
287+
behold serve ./infra --env prod --poll 30 # wait 30s between completed drift sweeps
288288
```
289289

290+
**Read budget.** HTTP observations, background polling, and frame captures share
291+
one Chant subprocess budget: two reads at once by default (one on a one-CPU
292+
host). `BEHOLD_ESTATE_CONCURRENCY` overrides that process-wide limit. Up to 64
293+
distinct reads can wait; excess reads fail explicitly rather than growing an
294+
unbounded queue. Identical in-flight reads share work only when the project,
295+
resolved Chant, source stamp, argv, and effective environment match. Completed
296+
live results are not cached. Source watcher invalidation also separates new
297+
requests from work begun before an edit.
298+
299+
A running read has a 180-second deadline, configurable with
300+
`BEHOLD_READ_TIMEOUT_MS`. A disconnected GET releases its subscription; when no
301+
callers remain, the read is canceled. On Unix, cancellation stops the whole
302+
npm/tsx/Node process group, escalating from TERM to KILL after one second. On
303+
Windows only the direct child is terminated. Delegated writes retain their
304+
existing lifecycle and are never deduplicated as reads.
305+
306+
The browser runs one refresh at a time and collapses intervening notifications
307+
into one follow-up. It no longer starts additional reads at 3/8/15-second offsets.
308+
Polling uses the same estate namespace bindings as the HTTP overlay and reuses
309+
the primary member's observation for lanes capture. Slow sweeps extend the poll
310+
period; they do not overlap the next sweep. These bounds control duplicate work;
311+
they do not eliminate Chant's underlying live discovery cost.
312+
290313
behold shells the **project's own** chant (resolved from the project's
291314
`node_modules` first), so the project decides the chant version — pin it to
292315
`@intentius/chant ^0.18.1` or later for the live overlay (`graph --live` observed

src/chant-read-integration.test.ts

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from "node:fs";
3+
import { tmpdir } from "node:os";
4+
import { join } from "node:path";
5+
import { runChantRaw, isScheduledRead, resolveChant } from "./chant.ts";
6+
import { createApp } from "./server.ts";
7+
import { invalidateReadGeneration, withReadSignal } from "./read-scheduler.ts";
8+
9+
// Real subprocesses, no cluster. The fixture writes its instrumentation outside
10+
// the member so observing it cannot accidentally change the source stamp.
11+
let scratch: string, project: string, log: string;
12+
beforeEach(() => {
13+
scratch = mkdtempSync(join(tmpdir(), "behold-read-test-"));
14+
project = join(scratch, "project"); log = join(scratch, "events");
15+
const pkg = join(project, "node_modules/@intentius/chant");
16+
mkdirSync(pkg, { recursive: true });
17+
writeFileSync(join(project, "source.ts"), "// fixture source\n");
18+
writeFileSync(join(pkg, "package.json"), JSON.stringify({ name: "@intentius/chant", version: "0.54.0", main: "chant.cjs", bin: { chant: "chant.cjs" } }));
19+
writeFileSync(join(pkg, "chant.cjs"), `#!/usr/bin/env node
20+
const fs = require('node:fs');
21+
const log = ${JSON.stringify(log)};
22+
const event = (value) => fs.appendFileSync(log, JSON.stringify(value) + '\\n');
23+
event({ event: 'start', pid: process.pid, args: process.argv.slice(2) });
24+
if (process.argv.includes('--tree')) {
25+
const child = require('node:child_process').spawn(process.execPath, ['-e',
26+
"process.on('SIGTERM', () => {}); require('node:fs').appendFileSync(" + JSON.stringify(log) + ", JSON.stringify({event:'child',pid:process.pid})+'\\\\n'); setInterval(() => {}, 1000);"
27+
], { stdio: 'ignore' });
28+
process.on('SIGTERM', () => process.exit(0));
29+
setInterval(() => {}, 1000);
30+
} else {
31+
setTimeout(() => {
32+
event({ event: 'finish', pid: process.pid });
33+
process.stdout.write(JSON.stringify({ pid: process.pid, variant: process.env.READ_TEST_VARIANT }));
34+
}, process.argv.includes('--slow') ? 1000 : 150);
35+
}
36+
`, { mode: 0o755 });
37+
expect(resolveChant(project).source).toBe("project");
38+
vi.stubEnv("BEHOLD_ESTATE_CONCURRENCY", "2");
39+
});
40+
afterEach(() => { vi.unstubAllEnvs(); rmSync(scratch, { recursive: true, force: true }); });
41+
const events = (): { event: string; pid: number; args?: string[] }[] => {
42+
try { return readFileSync(log, "utf8").trim().split("\n").map((s) => JSON.parse(s)); } catch { return []; }
43+
};
44+
45+
describe("real Chant read scheduling", () => {
46+
it("matching HTTP and background reads share a process, then the next read is fresh", async () => {
47+
const args = ["graph", "source.ts", "--live", "--namespace", "app"];
48+
const first = withReadSignal(new AbortController().signal, () => runChantRaw(args, project));
49+
const second = runChantRaw(args, project);
50+
const [a, b] = await Promise.all([first, second]);
51+
expect(a.stdout).toBe(b.stdout);
52+
expect(events().filter((e) => e.event === "start")).toHaveLength(1);
53+
a.stdout = "caller mutation";
54+
expect(b.stdout).not.toBe(a.stdout);
55+
const fresh = await runChantRaw(args, project);
56+
expect(fresh.stdout).not.toBe(b.stdout);
57+
});
58+
59+
it("the HTTP middleware isolates cancellation between matching requests", async () => {
60+
const app = createApp({ projectDir: project, port: 0 });
61+
app.onError((error, c) => c.json({ error: error.message }, 500));
62+
const caller = new AbortController();
63+
const first = app.request(new Request("http://localhost/api/diff?env=home", { signal: caller.signal }));
64+
const second = app.request("/api/diff?env=home");
65+
await vi.waitFor(() => expect(events().filter((e) => e.event === "start")).toHaveLength(1));
66+
caller.abort(new Error("client disconnected"));
67+
expect((await first).status).toBe(500);
68+
expect((await second).status).toBe(200);
69+
expect(events().filter((e) => e.event === "start")).toHaveLength(1);
70+
});
71+
72+
it("namespace, effective environment, source edits, and watcher invalidation isolate reads", async () => {
73+
const args = ["graph", "source.ts", "--live"];
74+
const reads = [runChantRaw(args, project)];
75+
reads.push(runChantRaw([...args, "--namespace", "other"], project));
76+
reads.push(runChantRaw(args, project, { READ_TEST_VARIANT: "other" }));
77+
writeFileSync(join(project, "source.ts"), "// changed while reading\n");
78+
reads.push(runChantRaw(args, project));
79+
invalidateReadGeneration();
80+
reads.push(runChantRaw(args, project));
81+
const results = await Promise.all(reads);
82+
expect(new Set(results.map((r) => JSON.parse(r.stdout).pid)).size).toBe(5);
83+
let active = 0, peak = 0;
84+
for (const e of events()) { active += e.event === "start" ? 1 : -1; peak = Math.max(peak, active); }
85+
expect(peak).toBe(2);
86+
});
87+
88+
it("a timed-out process releases the shared budget", async () => {
89+
vi.stubEnv("BEHOLD_READ_TIMEOUT_MS", "50");
90+
await expect(runChantRaw(["graph", "source.ts", "--slow"], project)).rejects.toThrow("exceeded 50ms");
91+
vi.stubEnv("BEHOLD_READ_TIMEOUT_MS", "3000");
92+
expect((await runChantRaw(["graph", "source.ts"], project)).code).toBe(0);
93+
});
94+
95+
it.skipIf(process.platform === "win32")("disconnect kills the whole worker group, including a descendant ignoring TERM", async () => {
96+
const caller = new AbortController();
97+
const read = withReadSignal(caller.signal, () => runChantRaw(["graph", "source.ts", "--tree"], project));
98+
// Handle rejection immediately; disconnect rejects the subscriber before
99+
// the worker group has necessarily finished closing its pipes.
100+
const rejected = read.catch((error) => error);
101+
let child = 0;
102+
try {
103+
await vi.waitFor(() => { child = events().find((e) => e.event === "child")?.pid ?? 0; expect(child).toBeGreaterThan(0); });
104+
caller.abort(new Error("client left"));
105+
expect((await rejected).message).toBe("client left");
106+
await vi.waitFor(() => expect(() => process.kill(child, 0)).toThrow(), { timeout: 3000 });
107+
} finally {
108+
caller.abort(new Error("client left"));
109+
for (const e of events()) { try { process.kill(e.pid, "SIGKILL"); } catch {} }
110+
}
111+
});
112+
113+
it("delegated mutations are never scheduled or deduplicated as reads", async () => {
114+
for (const args of [["approve", "op", "gate"], ["run", "apply"], ["emulator", "up"], ["carve", "emit"], ["build"]]) {
115+
expect(isScheduledRead(args)).toBe(false);
116+
}
117+
const results = await Promise.all([runChantRaw(["approve", "op", "gate"], project), runChantRaw(["approve", "op", "gate"], project)]);
118+
expect(results[0].stdout).not.toBe(results[1].stdout);
119+
});
120+
});

src/chant.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -562,11 +562,11 @@ describe("applyArgs", () => {
562562
describe("runChantRaw — env override reaches the spawn", () => {
563563
beforeEach(() => vi.mocked(spawnMock).mockReset());
564564

565-
it("spawns with no explicit `env` option when no override is given — inherits process.env as before", async () => {
565+
it("snapshots the inherited environment before a read can queue", async () => {
566566
vi.mocked(spawnMock).mockReturnValue(fakeProc(0, "{}"));
567567
await runChantRaw(["graph", "src", "--format", "ir"], "/proj");
568568
const opts = vi.mocked(spawnMock).mock.calls[0]![2] as { env?: unknown } | undefined;
569-
expect(opts?.env).toBeUndefined();
569+
expect(opts?.env).toEqual(process.env);
570570
});
571571

572572
it("merges the env override over process.env for exactly this spawn (M2 tier/target lenses)", async () => {

src/chant.ts

Lines changed: 75 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ import { stripAnsi } from "./ansi.ts";
2828
// (package.json `exports["./yaml"]` has no compiled-JS condition), which a
2929
// plain `node dist/cli.js` cannot import unbundled (Node refuses to
3030
// type-strip files under node_modules).
31+
import { createHash } from "node:crypto";
32+
import { memberSourceStamp } from "./member-source.ts";
33+
import { ReadScheduler, currentReadSignal, readGeneration } from "./read-scheduler.ts";
3134
import { parseYAML } from "@intentius/chant/yaml";
3235
import { carveStatusArgs, type CarveStatusJson } from "./carve-manifest.ts";
3336
import { dropForeignDeclarations } from "./foreign.ts";
@@ -297,41 +300,94 @@ export interface ChantRun {
297300
}
298301

299302
/** Run the chant bin, capturing stdout/stderr and the exit code. Never rejects on
300-
* a non-zero exit (only on a spawn failure) — a failing exit is data.
303+
* a non-zero exit — a failing exit is data. Scheduled reads can also reject
304+
* on queue saturation, cancellation, or deadline expiry.
301305
* `envOverride` (M2, #54: the tier/target lenses' `envOverridesFor`) merges over
302306
* `process.env` for this one spawn only — never a global mutation, so a picked
303307
* lens on one request can't bleed into a concurrent request on another. */
308+
// Positive read allowlist: delegated mutations (including build/carve emit) must
309+
// never be coalesced or canceled by the observation scheduler.
310+
export function isScheduledRead(args: string[]): boolean {
311+
return args[0] === "graph" ||
312+
(args[0] === "components" && args[1] === "status") ||
313+
(args[0] === "lifecycle" && ["diff", "plan"].includes(args[1])) ||
314+
(args[0] === "helm" && ["renders", "diff"].includes(args[1])) ||
315+
(args[0] === "run" && args[1] === "status") ||
316+
(args[0] === "operator" && ["status", "log"].includes(args[1]));
317+
}
318+
const reads = new ReadScheduler<ChantRun>();
319+
let unstampableRead = 0;
320+
304321
export function runChantRaw(
305322
args: string[],
306323
projectDir?: string,
307324
envOverride?: Record<string, string>,
308325
): Promise<ChantRun> {
326+
const chant = resolveChant(projectDir);
327+
if (!isScheduledRead(args)) return spawnChant(chant.bin, args, projectDir, envOverride);
328+
const dir = resolve(projectDir ?? process.cwd());
329+
const stamp = memberSourceStamp(dir);
330+
const effectiveEnv = { ...process.env, ...envOverride };
331+
const environment = Object.entries(effectiveEnv).sort(([a], [b]) => a.localeCompare(b));
332+
// Include the resolved compiler, exact argv (namespace/lens/env included),
333+
// source identity and effective environment. Unreadable source cannot share.
334+
const key = createHash("sha256").update(JSON.stringify([
335+
dir, chant.bin, chant.version, args, environment, readGeneration(), stamp ?? ++unstampableRead,
336+
])).digest("hex");
337+
return reads.read(key, (signal) => spawnChant(chant.bin, args, projectDir, effectiveEnv, signal), currentReadSignal())
338+
.then((result) => ({ ...result })); // callers own their result; JSON is parsed separately
339+
}
340+
341+
function spawnChant(
342+
bin: string,
343+
args: string[],
344+
projectDir?: string,
345+
envOverride?: NodeJS.ProcessEnv,
346+
signal?: AbortSignal,
347+
): Promise<ChantRun> {
348+
if (signal?.aborted) return Promise.reject(signal.reason);
309349
return new Promise((resolvePromise, reject) => {
310-
// Run in the project dir: `chant graph --live` reads the current working
311-
// directory (not the path arg), so the cwd must be the project for the live
312-
// and overlay paths to observe the right environment.
313-
const proc = spawn(chantBin(projectDir), args, {
350+
// Read workers own a process group: killing only npx leaves tsx/Node alive.
351+
// Writes retain their existing lifetime and process behavior.
352+
const grouped = !!signal && process.platform !== "win32";
353+
const proc = spawn(bin, args, {
314354
...(projectDir ? { cwd: projectDir } : {}),
315-
...(envOverride ? { env: { ...process.env, ...envOverride } } : {}),
355+
...(envOverride ? { env: signal ? envOverride : { ...process.env, ...envOverride } } : {}),
356+
...(grouped ? { detached: true } : {}),
316357
stdio: ["ignore", "pipe", "pipe"],
317358
});
318-
// Accumulate raw Buffer chunks and decode once at the end. Coercing each
319-
// chunk to a string as it arrives (`s += d`) corrupts a multi-byte UTF-8
320-
// character that straddles a chunk boundary — which for loomster's ~200KB
321-
// entity-graph IR reliably mangles the JSON near the 64KB highWaterMark and
322-
// makes `JSON.parse` throw. Concatenating bytes first avoids the split.
323359
const outChunks: Buffer[] = [];
324360
const errChunks: Buffer[] = [];
361+
let escalation: ReturnType<typeof setTimeout> | undefined;
362+
const kill = (name: NodeJS.Signals): void => {
363+
try {
364+
if (grouped && proc.pid) process.kill(-proc.pid, name);
365+
else proc.kill(name);
366+
} catch (error) {
367+
if ((error as NodeJS.ErrnoException).code !== "ESRCH") proc.kill(name);
368+
}
369+
};
370+
const abort = (): void => {
371+
kill("SIGTERM");
372+
escalation = setTimeout(() => kill("SIGKILL"), 1000);
373+
};
374+
signal?.addEventListener("abort", abort, { once: true });
375+
const cleanup = (): void => {
376+
signal?.removeEventListener("abort", abort);
377+
clearTimeout(escalation);
378+
// A wrapper can exit before a descendant that ignores TERM. Finish the
379+
// group before releasing its budget, even if its pipes already closed.
380+
if (signal?.aborted && grouped) kill("SIGKILL");
381+
};
382+
// Decode once: a UTF-8 character can straddle stdout chunks.
325383
proc.stdout.on("data", (d: Buffer) => outChunks.push(d));
326384
proc.stderr.on("data", (d: Buffer) => errChunks.push(d));
327-
proc.on("error", reject);
328-
proc.on("close", (code) =>
329-
resolvePromise({
330-
code: code ?? 1,
331-
stdout: Buffer.concat(outChunks).toString("utf8"),
332-
stderr: Buffer.concat(errChunks).toString("utf8"),
333-
}),
334-
);
385+
proc.on("error", (error) => { cleanup(); reject(error); });
386+
proc.on("close", (code) => {
387+
cleanup();
388+
if (signal?.aborted) { reject(signal.reason); return; }
389+
resolvePromise({ code: code ?? 1, stdout: Buffer.concat(outChunks).toString("utf8"), stderr: Buffer.concat(errChunks).toString("utf8") });
390+
});
335391
});
336392
}
337393

src/estate.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import { joinCarvedSources } from "./carve-manifest.ts";
2222
import { carveStatesFor } from "./carve-discovery.ts";
2323
import { statSync } from "node:fs";
24-
import { availableParallelism } from "node:os";
24+
import { readConcurrency } from "./read-scheduler.ts";
2525
import { join, resolve, sep } from "node:path";
2626
import { composeStacks, shortStackNames, type GraphIR } from "@intentius/pinhole";
2727
import { meetsFloor, resolveChant, type GraphOptions } from "./chant.ts";
@@ -43,15 +43,10 @@ import { CLUSTER_SCOPED } from "./zoom-notes.ts";
4343
// host busy without drowning it.
4444
// ---------------------------------------------------------------------------
4545

46-
/** How many member chant processes one estate read runs at once: the host's
47-
* parallelism capped at 4 (each spawn wants a core-plus for its TS eval),
48-
* never more lanes than members. `BEHOLD_ESTATE_CONCURRENCY` overrides the
49-
* cap for tuning a live estate; anything unparseable or < 1 is ignored
50-
* rather than honoured into a stall. Exported for testing. */
46+
/** Per-composition pipelining; runChantRaw also enforces this budget across
47+
* all simultaneous estate requests, background polls, and frame captures. */
5148
export function estateReadPool(members: number, env: Record<string, string | undefined> = process.env): number {
52-
const override = Number.parseInt(env.BEHOLD_ESTATE_CONCURRENCY ?? "", 10);
53-
const cap = Number.isInteger(override) && override >= 1 ? override : Math.min(4, availableParallelism());
54-
return Math.max(1, Math.min(cap, members));
49+
return Math.max(1, Math.min(readConcurrency(env), members));
5550
}
5651

5752
/** `Promise.all(items.map(fn))` with at most `width` calls in flight.

0 commit comments

Comments
 (0)