mirror of
https://github.com/feder-cr/Jobs_Applier_AI_Agent.git
synced 2026-11-07 10:36:05 -10:00
cli, api, web, engine: the LOW review findings of the lane fixes
Findings of .local/next/fixes-review.json (severity low), each confirmed and fixed at its origin, with a test that fails before the fix. The cross-lane finding about the Playwright harness and `next start` is not here: it is resolved when the lane-web branch is merged. - kill -9 of `invisible-dots server` orphans the web child on port 3000 -> fixed. A child started with startProcess outlives a parent that is killed outright (confirmed on Linux: the child stays after SIGKILL of the parent; on Windows libuv's job object ends it, so there the finding did not show). `webEnvironment` now gives the child the server's pid (INVISIBLE_DOTS_WEB_PARENT_PID, shared ENV) and the web build's instrumentation.ts (lib/parent.ts, shared processExists) exits the server once that process is gone. Checked on the real standalone build: it answers while the parent lives and is gone 0.4 s after the parent exits. New: web.test (the pid reaches the child; a stand-in server SIGKILLed frees the port), parent.test (3 tests). - The tool calls of a response with no cost still run before the turn fails -> fixed, not documented away. turns.py now calls spend.ensure_priced() before the assistant_tool_calls checkpoint as well as final_response, so the calls are neither written nor run. spend.py and architecture 8.2 say so. New test_turns case: no cost plus a tool call fails the turn, nothing is written, no tool.called. - INVISIBLE_DOTS_WEB_LISTEN: the error cites the API's example, and port 0 passes the parse -> fixed. parseListen takes a ListenSetting (variable, the setting's own example, fixedPort); serve.ts passes the web setting, so the error shows 127.0.0.1:3000 and port 0 fails the command before anything starts. serve.test now uses the real parseListen instead of a copy of its rule. New: config.test (settings), serve.test (2 cases). Removed test: web.test "does not start at all when the build is incomplete, or the port is 0" lost its port 0 assertion, because the refusal moved from startWebServer to parseListen (one owner of the fact, checked at parse time, where it now fails at once); the same assertion is in config.test and serve.test. The incomplete-build assertion stays under its own name. Replaced assertion: config.test's parseListen(value, "VARIABLE") call, the old two-argument form, is now the ListenSetting case. Floors (.github/test-floors.json), raised for 7 added vitest tests, 1 of them in the postgres suite's files, and 1 pytest: vitest linux 750 -> 757, win32 753 -> 760, postgres 219 -> 220, pytest 2289 -> 2290. Nothing lowered.
This commit is contained in:
1 parent
1c89b98694
commit
58cc48e67e
16 files changed
+269
-43
No files matched your search
@@ -1,17 +1,17 @@
|
||||
{
|
||||
"//": "What each suite must reach on each host, read by .github/scripts/test-guard.mjs in CI (tests.yml) and in the pre-push hook. Raise a floor when tests are added; lowering one is a decision to write down in the commit that does it. Linux skips only the tests that drive a real Windows PowerShell; a Windows host compiles dot-agentd's unix-only test files out and skips the two Go tests that need a Linux guest. The pytest floor is the Python engine's (invisible_engine_dots, the engine job of tests.yml, Linux only, nothing may skip).",
|
||||
"vitest": {
|
||||
"linux": { "min_passed": 750, "allow_skipped": "^generated PowerShell on a real Windows PowerShell " },
|
||||
"win32": { "min_passed": 753, "allow_skipped": "" }
|
||||
"linux": { "min_passed": 757, "allow_skipped": "^generated PowerShell on a real Windows PowerShell " },
|
||||
"win32": { "min_passed": 760, "allow_skipped": "" }
|
||||
},
|
||||
"postgres": {
|
||||
"linux": { "min_passed": 219, "allow_skipped": "", "require": { "\\bpg\\b": 93 } }
|
||||
"linux": { "min_passed": 220, "allow_skipped": "", "require": { "\\bpg\\b": 93 } }
|
||||
},
|
||||
"go": {
|
||||
"linux": { "min_passed": 97, "allow_skipped": "" },
|
||||
"win32": { "min_passed": 61, "allow_skipped": "^agentd (TestSystemRoute|TestPowerOffStartsTheCommandAndAnswers202)$" }
|
||||
},
|
||||
"pytest": {
|
||||
"linux": { "min_passed": 2289, "allow_skipped": "" }
|
||||
"linux": { "min_passed": 2290, "allow_skipped": "" }
|
||||
}
|
||||
}
|
||||
+16
-6
@@ -22,11 +22,20 @@ export interface ListenAddress {
|
||||
port: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse `host:port`, `[v6]:port` or a bare port (which binds 127.0.0.1).
|
||||
* `variable` is the setting the value came from, named in the error.
|
||||
*/
|
||||
export function parseListen(value: string = DEFAULT_LISTEN, variable: string = ENV.LISTEN): ListenAddress {
|
||||
/** What differs between the settings parseListen reads. */
|
||||
export interface ListenSetting {
|
||||
/** The environment variable the value came from, named in the errors. */
|
||||
variable: string;
|
||||
/** The address shown as the example in the error: the setting's own default. */
|
||||
example: string;
|
||||
/** Whether port 0 (the system picks one) is refused, for a server whose address the person has to be told. */
|
||||
fixedPort: boolean;
|
||||
}
|
||||
|
||||
export const API_LISTEN: ListenSetting = { variable: ENV.LISTEN, example: DEFAULT_LISTEN, fixedPort: false };
|
||||
|
||||
/** Parse `host:port`, `[v6]:port` or a bare port (which binds 127.0.0.1). */
|
||||
export function parseListen(value: string = DEFAULT_LISTEN, setting: ListenSetting = API_LISTEN): ListenAddress {
|
||||
const text = value.trim();
|
||||
let host = "127.0.0.1";
|
||||
let portText = text;
|
||||
@@ -41,8 +50,9 @@ export function parseListen(value: string = DEFAULT_LISTEN, variable: string = E
|
||||
}
|
||||
const port = Number(portText);
|
||||
if (!host || !/^\d+$/.test(portText) || port < 0 || port > 65535) {
|
||||
throw new Error(`${variable} must look like "127.0.0.1:8787", got "${value}"`);
|
||||
throw new Error(`${setting.variable} must look like "${setting.example}", got "${value}"`);
|
||||
}
|
||||
if (setting.fixedPort && port === 0) throw new Error(`${setting.variable} needs a fixed port, got "${value}"`);
|
||||
return { host, port };
|
||||
}
|
||||
|
||||
|
||||
@@ -30,7 +30,17 @@ describe("server settings", () => {
|
||||
expect(parseListen("[::1]:8787")).toEqual({ host: "::1", port: 8787 });
|
||||
expect(parseListen("8080")).toEqual({ host: "127.0.0.1", port: 8080 });
|
||||
expect(() => parseListen("localhost:http")).toThrow(/INVISIBLE_DOTS_LISTEN/);
|
||||
expect(() => parseListen("localhost:http", "INVISIBLE_DOTS_WEB_LISTEN")).toThrow(/INVISIBLE_DOTS_WEB_LISTEN must look like/);
|
||||
expect(parseListen("127.0.0.1:0")).toEqual({ host: "127.0.0.1", port: 0 });
|
||||
expect(() => parseListen("localhost:http")).toThrow('INVISIBLE_DOTS_LISTEN must look like "127.0.0.1:8787", got "localhost:http"');
|
||||
});
|
||||
|
||||
it("names the setting it was asked for, with its own example, and refuses port 0 only where the setting needs a fixed port", () => {
|
||||
const web = { variable: "INVISIBLE_DOTS_WEB_LISTEN", example: "127.0.0.1:3000", fixedPort: true };
|
||||
expect(parseListen("127.0.0.1:3100", web)).toEqual({ host: "127.0.0.1", port: 3100 });
|
||||
expect(() => parseListen("localhost:http", web)).toThrow('INVISIBLE_DOTS_WEB_LISTEN must look like "127.0.0.1:3000", got "localhost:http"');
|
||||
expect(() => parseListen("70000", web)).toThrow(/must look like/);
|
||||
expect(() => parseListen("0", web)).toThrow('INVISIBLE_DOTS_WEB_LISTEN needs a fixed port, got "0"');
|
||||
expect(() => parseListen("127.0.0.1:0", web)).toThrow(/needs a fixed port/);
|
||||
});
|
||||
|
||||
it("creates the API token once and reads it back; the environment wins; short tokens are refused", async () => {
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* Ctrl+C while the web client is starting must still close the database),
|
||||
* and a stop closes the web client first, then the control plane.
|
||||
*/
|
||||
import type { ListenAddress, StartServerOptions } from "@invisible-dots/api";
|
||||
import type { ListenAddress, ListenSetting, StartServerOptions } from "@invisible-dots/api";
|
||||
import { DEFAULT_WEB_LISTEN, ENV, type HostPaths } from "@invisible-dots/shared";
|
||||
import type { Logger } from "@invisible-dots/vm-manager";
|
||||
import { locateWebBuild, type WebServer, type WebServerOptions } from "./web.js";
|
||||
@@ -14,7 +14,7 @@ import { locateWebBuild, type WebServer, type WebServerOptions } from "./web.js"
|
||||
export interface ServeDeps {
|
||||
startServer(options: StartServerOptions): Promise<{ url: string; paths: HostPaths; close(): Promise<void> }>;
|
||||
untilStopSignal(close: () => Promise<void>, logger: Logger, signals?: NodeJS.EventEmitter): Promise<void>;
|
||||
parseListen(value: string, variable: string): ListenAddress;
|
||||
parseListen(value: string, setting: ListenSetting): ListenAddress;
|
||||
startWebServer(options: WebServerOptions): Promise<WebServer>;
|
||||
}
|
||||
|
||||
@@ -29,10 +29,13 @@ export interface ServeOptions {
|
||||
signals?: NodeJS.EventEmitter;
|
||||
}
|
||||
|
||||
/** The web client's listen address: its own default shown in the error, and a port the person can be told. */
|
||||
const WEB_LISTEN: ListenSetting = { variable: ENV.WEB_LISTEN, example: DEFAULT_WEB_LISTEN, fixedPort: true };
|
||||
|
||||
export async function serve(options: ServeOptions, deps: ServeDeps): Promise<void> {
|
||||
const { env, logger } = options;
|
||||
// Parsed before anything starts, so a bad INVISIBLE_DOTS_WEB_LISTEN fails the command at once.
|
||||
const webListen = options.web ? deps.parseListen(env[ENV.WEB_LISTEN]?.trim() || DEFAULT_WEB_LISTEN, ENV.WEB_LISTEN) : undefined;
|
||||
const webListen = options.web ? deps.parseListen(env[ENV.WEB_LISTEN]?.trim() || DEFAULT_WEB_LISTEN, WEB_LISTEN) : undefined;
|
||||
const server = await deps.startServer({ env, logger });
|
||||
logger.info(`API token in ${server.paths.apiTokenPath}; stop with Ctrl+C (Dots keep running)`);
|
||||
|
||||
|
||||
+9
-6
@@ -47,7 +47,7 @@ export interface WebServer {
|
||||
|
||||
export interface WebServerOptions {
|
||||
build: WebBuild;
|
||||
/** Port 0 is refused: the person has to know where to go. */
|
||||
/** A fixed port: the person has to know where to go (parseListen refuses port 0 for this setting). */
|
||||
listen: { host: string; port: number };
|
||||
/** The control plane's address, where the web server's proxy sends `/api`. */
|
||||
apiUrl: string;
|
||||
@@ -74,13 +74,16 @@ const READY_POLL_MS = 100;
|
||||
* to listen (Next's PORT and HOSTNAME), where the API is, which data
|
||||
* directory holds `api.token` (read on every request, so a rotated token
|
||||
* takes effect), the API token itself only when the server was given it that
|
||||
* way, and the extra host names its Host check allows.
|
||||
* way, and the extra host names its Host check allows. `parentPid` is the
|
||||
* server's own pid: the web server exits when that process is gone, so a
|
||||
* `kill -9` of the server does not leave it holding the port.
|
||||
*/
|
||||
export function webEnvironment(options: Pick<WebServerOptions, "env" | "listen" | "apiUrl">): Record<string, string> {
|
||||
const { env, listen, apiUrl } = options;
|
||||
export function webEnvironment(options: Pick<WebServerOptions, "env" | "listen" | "apiUrl"> & { parentPid: number }): Record<string, string> {
|
||||
const { env, listen, apiUrl, parentPid } = options;
|
||||
return {
|
||||
...allowlistedEnvironment(env, [...BASE_CHILD_ENV_VARS, ENV.HOME, ENV.TOKEN, ENV.WEB_ALLOWED_HOSTS]),
|
||||
[ENV.URL]: apiUrl,
|
||||
[ENV.WEB_PARENT_PID]: String(parentPid),
|
||||
PORT: String(listen.port),
|
||||
HOSTNAME: listen.host,
|
||||
};
|
||||
@@ -122,14 +125,14 @@ export async function startWebServer(options: WebServerOptions): Promise<WebServ
|
||||
throw new WebStartError(`the web client is not built (${build.missing} does not exist); build it with: ${WEB_BUILD_COMMAND}`);
|
||||
}
|
||||
if (options.signal?.aborted) throw new WebStartError("stopped before the web client started");
|
||||
if (listen.port === 0) throw new WebStartError(`${ENV.WEB_LISTEN} needs a fixed port`);
|
||||
const url = urlOf(listen);
|
||||
// Something else answering there would make the wait below succeed for the wrong server.
|
||||
if (await answers(url)) {
|
||||
throw new WebStartError(`${url} is already in use; free the port or set ${ENV.WEB_LISTEN} to another host:port`);
|
||||
}
|
||||
// The server script chdirs into its own directory, so that is where it starts: never the server's directory.
|
||||
const child = (options.start ?? startProcess)(process.execPath, [build.entry], { cwd: dirname(build.entry), env: webEnvironment(options) });
|
||||
const env = webEnvironment({ ...options, parentPid: process.pid });
|
||||
const child = (options.start ?? startProcess)(process.execPath, [build.entry], { cwd: dirname(build.entry), env });
|
||||
|
||||
let exited = false;
|
||||
let started = false;
|
||||
|
||||
@@ -3,6 +3,7 @@ import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { parseListen } from "@invisible-dots/api";
|
||||
import type { HostPaths } from "@invisible-dots/shared";
|
||||
import { serve, type ServeDeps } from "../src/serve.js";
|
||||
import type { WebServer, WebServerOptions } from "../src/web.js";
|
||||
@@ -50,11 +51,7 @@ function harness(over: { webStart?: (options: WebServerOptions) => Promise<WebSe
|
||||
stopped = resolve;
|
||||
emitter?.once("SIGINT", () => void close().then(resolve));
|
||||
}),
|
||||
parseListen: (value, variable) => {
|
||||
const at = value.lastIndexOf(":");
|
||||
if (!/^\d+$/.test(value.slice(at + 1))) throw new Error(`${variable} must look like "127.0.0.1:8787", got "${value}"`);
|
||||
return { host: value.slice(0, at), port: Number(value.slice(at + 1)) };
|
||||
},
|
||||
parseListen,
|
||||
startWebServer:
|
||||
over.webStart ??
|
||||
(async (options) => {
|
||||
@@ -107,6 +104,21 @@ describe("serve", () => {
|
||||
expect(h.log).toEqual([]);
|
||||
});
|
||||
|
||||
it("names the web client's own default, not the API's, when INVISIBLE_DOTS_WEB_LISTEN is malformed", async () => {
|
||||
const h = harness();
|
||||
await expect(h.run({ web: true, env: { INVISIBLE_DOTS_WEB_LISTEN: "three-thousand" } })).rejects.toThrow(
|
||||
'INVISIBLE_DOTS_WEB_LISTEN must look like "127.0.0.1:3000", got "three-thousand"',
|
||||
);
|
||||
});
|
||||
|
||||
it("fails before starting anything when INVISIBLE_DOTS_WEB_LISTEN names port 0", async () => {
|
||||
const h = harness();
|
||||
await expect(h.run({ web: true, env: { INVISIBLE_DOTS_WEB_LISTEN: "127.0.0.1:0" } })).rejects.toThrow(
|
||||
'INVISIBLE_DOTS_WEB_LISTEN needs a fixed port, got "127.0.0.1:0"',
|
||||
);
|
||||
expect(h.log).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not start the web client with --no-web, and does not read INVISIBLE_DOTS_WEB_LISTEN", async () => {
|
||||
await buildWeb();
|
||||
const h = harness();
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import { spawn } from "node:child_process";
|
||||
import { realpathSync } from "node:fs";
|
||||
import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
|
||||
import { createServer } from "node:net";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { locateWebBuild, startWebServer, webEnvironment, WebStartError, type WebServer, type WebServerOptions } from "../src/web.js";
|
||||
|
||||
@@ -88,12 +90,14 @@ describe("webEnvironment", () => {
|
||||
},
|
||||
listen: { host: "127.0.0.1", port: 3000 },
|
||||
apiUrl: "http://127.0.0.1:8787",
|
||||
parentPid: 4242,
|
||||
});
|
||||
expect(env).toEqual({
|
||||
PATH: "/bin",
|
||||
INVISIBLE_DOTS_HOME: "/data",
|
||||
INVISIBLE_DOTS_WEB_ALLOWED_HOSTS: "dots.example",
|
||||
INVISIBLE_DOTS_URL: "http://127.0.0.1:8787",
|
||||
INVISIBLE_DOTS_WEB_PARENT_PID: "4242",
|
||||
PORT: "3000",
|
||||
HOSTNAME: "127.0.0.1",
|
||||
});
|
||||
@@ -104,6 +108,7 @@ describe("webEnvironment", () => {
|
||||
env: { INVISIBLE_DOTS_TOKEN: "token-from-the-environment" },
|
||||
listen: { host: "127.0.0.1", port: 3000 },
|
||||
apiUrl: "http://127.0.0.1:8787",
|
||||
parentPid: 4242,
|
||||
});
|
||||
expect(env.INVISIBLE_DOTS_TOKEN).toBe("token-from-the-environment");
|
||||
});
|
||||
@@ -118,6 +123,7 @@ describe("startWebServer", { timeout: 30_000 }, () => {
|
||||
expect(body.env.PORT).toBe(String(port));
|
||||
expect(body.env.HOSTNAME).toBe("127.0.0.1");
|
||||
expect(body.env.INVISIBLE_DOTS_URL).toBe("http://127.0.0.1:8787");
|
||||
expect(body.env.INVISIBLE_DOTS_WEB_PARENT_PID).toBe(String(process.pid));
|
||||
expect(body.env.DATABASE_URL).toBeUndefined();
|
||||
// It runs in its own directory, never the server's.
|
||||
expect(realpathSync.native(body.cwd)).toBe(realpathSync.native(root));
|
||||
@@ -168,10 +174,61 @@ describe("startWebServer", { timeout: 30_000 }, () => {
|
||||
await expect(start(entry, { signal: controller.signal })).rejects.toThrow("stopped while the web client was starting");
|
||||
});
|
||||
|
||||
it("does not start at all when the build is incomplete, or the port is 0", async () => {
|
||||
it("does not start at all when the build is incomplete", async () => {
|
||||
await expect(start("/nowhere/server.js", { build: { entry: "/nowhere/server.js", missing: "/nowhere/server.js" } })).rejects.toThrow(
|
||||
"the web client is not built (/nowhere/server.js does not exist); build it with: npm run build --workspace @invisible-dots/web",
|
||||
);
|
||||
await expect(start("/nowhere/server.js", { listen: { host: "127.0.0.1", port: 0 } })).rejects.toThrow("INVISIBLE_DOTS_WEB_LISTEN needs a fixed port");
|
||||
});
|
||||
});
|
||||
|
||||
describe("a server that is killed outright", { timeout: 60_000 }, () => {
|
||||
const here = dirname(fileURLToPath(import.meta.url));
|
||||
const watcher = pathToFileURL(join(here, "..", "..", "web", "src", "lib", "parent.ts")).href;
|
||||
const webModule = pathToFileURL(join(here, "..", "src", "web.ts")).href;
|
||||
|
||||
it("does not leave the web server holding its port: it exits once its parent is gone", async () => {
|
||||
// Windows ends a Node child with its parent on its own (libuv puts children in a job object), so the
|
||||
// orphan this guards against is a POSIX one; the web server's own exit is what keeps both the same.
|
||||
// What the web build's instrumentation.ts does with the pid it is given, around a plain HTTP server.
|
||||
const entry = await script(`
|
||||
import { createServer } from "node:http";
|
||||
import { exitWhenParentGone, parentPidFrom } from ${JSON.stringify(watcher)};
|
||||
createServer((req, res) => res.end("ok")).listen(Number(process.env.PORT), process.env.HOSTNAME);
|
||||
const exists = (pid) => { try { process.kill(pid, 0); return true; } catch (error) { return error.code === "EPERM"; } };
|
||||
const parentPid = parentPidFrom(process.env.INVISIBLE_DOTS_WEB_PARENT_PID);
|
||||
if (parentPid !== undefined) exitWhenParentGone(parentPid, { exists, onGone: () => process.exit(0), intervalMs: 100 });
|
||||
`);
|
||||
const port = await freePort();
|
||||
// The server stand-in: a process that starts the web server and then waits to be killed.
|
||||
const parentScript = join(root, "parent.mjs");
|
||||
await writeFile(
|
||||
parentScript,
|
||||
`
|
||||
import { startWebServer } from ${JSON.stringify(webModule)};
|
||||
const web = await startWebServer({
|
||||
build: { entry: ${JSON.stringify(entry)}, missing: undefined },
|
||||
listen: { host: "127.0.0.1", port: ${port} },
|
||||
apiUrl: "http://127.0.0.1:8787",
|
||||
env: { PATH: process.env.PATH },
|
||||
onUnexpectedExit: () => undefined,
|
||||
});
|
||||
console.log(web.url);
|
||||
setInterval(() => {}, 1000);
|
||||
`,
|
||||
);
|
||||
const parent = spawn(process.execPath, ["--import", "tsx", parentScript], { stdio: ["ignore", "pipe", "inherit"], cwd: join(here, "..") });
|
||||
try {
|
||||
const url = await new Promise<string>((resolve, reject) => {
|
||||
parent.once("exit", () => reject(new Error("the stand-in server exited before the web server answered")));
|
||||
parent.stdout.once("data", (chunk: Buffer) => resolve(chunk.toString().trim()));
|
||||
});
|
||||
expect((await fetch(url)).status).toBe(200);
|
||||
parent.kill("SIGKILL");
|
||||
await expect
|
||||
.poll(() => fetch(url, { signal: AbortSignal.timeout(500) }).then(() => "still answering", () => "gone"), { timeout: 15_000, interval: 200 })
|
||||
.toBe("gone");
|
||||
} finally {
|
||||
parent.kill("SIGKILL");
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,12 @@
|
||||
// Next runs register() once when the server starts. Under
|
||||
// `invisible-dots server` it ties the server's life to its parent's (see
|
||||
// lib/parent.ts); started any other way (next dev) there is no parent to follow.
|
||||
export async function register(): Promise<void> {
|
||||
if (process.env.NEXT_RUNTIME !== "nodejs") return;
|
||||
const { ENV } = await import("@invisible-dots/shared/browser");
|
||||
const { exitWhenParentGone, parentPidFrom } = await import("./lib/parent");
|
||||
const parentPid = parentPidFrom(process.env[ENV.WEB_PARENT_PID]);
|
||||
if (parentPid === undefined) return;
|
||||
const { processExists } = await import("@invisible-dots/shared/process");
|
||||
exitWhenParentGone(parentPid, { exists: processExists, onGone: () => process.exit(0) });
|
||||
}
|
||||
@@ -0,0 +1,37 @@
|
||||
/**
|
||||
* The web server as a child of `invisible-dots server` (architecture section
|
||||
* 9.7) must not outlive it. A normal stop kills the child, but a `kill -9` or
|
||||
* a crash of the parent skips that, and the orphan would keep the port (and
|
||||
* an older build) for the next start. So the child watches its parent: the
|
||||
* parent names its pid in INVISIBLE_DOTS_WEB_PARENT_PID, and the child exits
|
||||
* once that process is gone. No dependencies, so a plain `node` can load it.
|
||||
*/
|
||||
|
||||
export interface ParentWatchOptions {
|
||||
/** Whether a process with this pid exists (shared's processExists). */
|
||||
exists: (pid: number) => boolean;
|
||||
/** Called once when the parent is gone. */
|
||||
onGone: () => void;
|
||||
/** How often the parent is looked at. */
|
||||
intervalMs?: number;
|
||||
}
|
||||
|
||||
export const PARENT_WATCH_INTERVAL_MS = 1000;
|
||||
|
||||
/** Watch `parentPid`; returns the function that stops watching. The timer never keeps the process alive by itself. */
|
||||
export function exitWhenParentGone(parentPid: number, options: ParentWatchOptions): () => void {
|
||||
const timer = setInterval(() => {
|
||||
if (options.exists(parentPid)) return;
|
||||
clearInterval(timer);
|
||||
options.onGone();
|
||||
}, options.intervalMs ?? PARENT_WATCH_INTERVAL_MS);
|
||||
timer.unref();
|
||||
return () => clearInterval(timer);
|
||||
}
|
||||
|
||||
/** The parent pid a server was started with, or undefined when it was not started by `invisible-dots server`. */
|
||||
export function parentPidFrom(value: string | undefined): number | undefined {
|
||||
if (value === undefined || !/^[1-9]\d*$/.test(value)) return undefined;
|
||||
const pid = Number(value);
|
||||
return Number.isSafeInteger(pid) ? pid : undefined;
|
||||
}
|
||||
@@ -0,0 +1,51 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { exitWhenParentGone, parentPidFrom } from "../src/lib/parent.js";
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
describe("exitWhenParentGone", () => {
|
||||
it("does nothing while the parent exists, and calls onGone once when it is gone", () => {
|
||||
let alive = true;
|
||||
const asked: number[] = [];
|
||||
const onGone = vi.fn();
|
||||
exitWhenParentGone(4242, {
|
||||
exists: (pid) => {
|
||||
asked.push(pid);
|
||||
return alive;
|
||||
},
|
||||
onGone,
|
||||
intervalMs: 1000,
|
||||
});
|
||||
vi.advanceTimersByTime(3000);
|
||||
expect(onGone).not.toHaveBeenCalled();
|
||||
expect(asked).toEqual([4242, 4242, 4242]);
|
||||
|
||||
alive = false;
|
||||
vi.advanceTimersByTime(5000);
|
||||
expect(onGone).toHaveBeenCalledTimes(1);
|
||||
expect(asked).toHaveLength(4);
|
||||
});
|
||||
|
||||
it("stops looking when told to", () => {
|
||||
const onGone = vi.fn();
|
||||
const stop = exitWhenParentGone(4242, { exists: () => false, onGone, intervalMs: 1000 });
|
||||
stop();
|
||||
vi.advanceTimersByTime(5000);
|
||||
expect(onGone).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("parentPidFrom", () => {
|
||||
it("reads a positive integer and nothing else", () => {
|
||||
expect(parentPidFrom("4242")).toBe(4242);
|
||||
for (const bad of [undefined, "", "0", "-1", "1.5", "12abc", " 7", "1e3", "99999999999999999999"]) {
|
||||
expect(parentPidFrom(bad)).toBeUndefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
+12
-4
@@ -924,9 +924,11 @@ on from what it had spent); the chat's spend starts again with each answer it gi
|
||||
restart does not reset it.
|
||||
A lowered cap applies from the next turn, like the step limit. A response that
|
||||
reports no cost fails the turn, `stopped: OpenRouter reported
|
||||
no cost for a request, so limits.max_cost_per_task_usd cannot be enforced`, at
|
||||
its next check or, when it was the last response, before its answer is written
|
||||
(the answer is not delivered, the task does not complete). The note is a column
|
||||
no cost for a request, so limits.max_cost_per_task_usd cannot be enforced`,
|
||||
before anything is done on that response: its tool calls are not written and do
|
||||
not run, or its answer is not delivered and the task does not complete (a
|
||||
summary request is not acted on this way, so the failure comes at the next
|
||||
check). The note is a column
|
||||
of the same ledger row as the money (`dots_spend.unpriced`), so a restart does
|
||||
not forget it: the cap never runs blind. A request abandoned by a sleep may have cost something
|
||||
that was never reported; that gap is at most one request a sleep.
|
||||
@@ -1540,10 +1542,16 @@ starts it: the build `npm run build --workspace @invisible-dots/web` leaves
|
||||
(Next's standalone server and the browser files beside it, assembled by
|
||||
`apps/web/scripts/standalone.mjs`) is run with `node` as a child process of the
|
||||
server, on `INVISIBLE_DOTS_WEB_LISTEN` (default `127.0.0.1:3000`), with
|
||||
`INVISIBLE_DOTS_URL` set to the control plane's address. The child gets an
|
||||
`INVISIBLE_DOTS_URL` set to the control plane's address; the setting is read
|
||||
before anything starts, so a malformed value or port 0 (the person has to be
|
||||
told where to go) fails the command at once. The child gets an
|
||||
allowlisted environment, not the server's: the data directory (so it reads
|
||||
`api.token` itself on every request), `INVISIBLE_DOTS_TOKEN` only when the
|
||||
server was given the token that way, and `INVISIBLE_DOTS_WEB_ALLOWED_HOSTS`.
|
||||
It also gets the server's pid (`INVISIBLE_DOTS_WEB_PARENT_PID`) and exits when
|
||||
that process is gone (`apps/web/src/instrumentation.ts`), so a `kill -9` of the
|
||||
server does not leave an orphan holding the port: a stop signal ends the child
|
||||
through the server, anything else through the child itself.
|
||||
The web client is a companion, not a dependency: when it is not built, its
|
||||
port is taken or it exits, the server logs why and the control plane and the
|
||||
command line go on; `server --no-web` does not start it. A stop signal closes
|
||||
|
||||
@@ -11,8 +11,10 @@ session:
|
||||
for nothing.
|
||||
- `TurnSpend.check` runs before every iteration of the runner and raises `CostCapReached`
|
||||
when the session has spent the cap: the turn ends as a failure whose text is the exception's.
|
||||
- `TurnSpend.ensure_priced` runs before the turn's final answer is written (which delivers it and
|
||||
completes a task), so the last response of a turn is held to the same rule as the others.
|
||||
- `TurnSpend.ensure_priced` runs before anything is done on the word of a response: before the
|
||||
tool calls of an assistant message are written (so they never run) and before the turn's final
|
||||
answer is written (which delivers it and completes a task). The last response of a turn is held
|
||||
to the same rule as the others, and so is one that asks for tools.
|
||||
|
||||
The spend lives in `dots_spend`, keyed by the session. A task's row is never reset, so the
|
||||
cap holds across a restart, an approval and a resume. The chat's row is emptied by the answer
|
||||
@@ -22,10 +24,11 @@ next: a call parked for approval, a restart or a sleep does not start the count
|
||||
A request cannot be priced before it is answered, so the cap stops the turn from starting
|
||||
another request: it may be exceeded by the last one, and an answer that crosses the cap is
|
||||
delivered. A response that reports no cost leaves the cap unenforceable, so the turn fails
|
||||
rather than carry on unmetered: at its next check, or, when that response was the last one, before
|
||||
its answer is written (the answer is not delivered and the task does not complete). The
|
||||
note lives in the same ledger row as the money, written in the same transaction, so a restart
|
||||
does not forget it; it goes where the row goes (the chat's answer takes it).
|
||||
rather than carry on unmetered: before the response is acted on (its tool calls do not run, its
|
||||
answer is not delivered and the task does not complete) or, for a request the runner does not
|
||||
commit (a summary), at the next check. The note lives in the same ledger row as the money,
|
||||
written in the same transaction, so a restart does not forget it; it goes where the row goes
|
||||
(the chat's answer takes it).
|
||||
|
||||
What the ledger cannot hold is the cost of a request that never completed: OpenRouter reports
|
||||
the cost in the last chunk of a stream, so a stream that stalled or was cut has none to count
|
||||
@@ -101,8 +104,8 @@ class TurnSpend:
|
||||
def ensure_priced(self) -> None:
|
||||
"""Raise `CostCapReached` when a request of the session reported no cost.
|
||||
|
||||
`check` runs before a request, so the last response of a turn is never checked by it; the
|
||||
cap itself does not apply here, because an answer that crosses it is delivered.
|
||||
`check` runs before a request, so a response is never checked by it before it is acted on;
|
||||
the cap itself does not apply here, because an answer that crosses it is delivered.
|
||||
"""
|
||||
if self._store.read(lambda conn: dots_store.has_unpriced(conn, self._session_key)):
|
||||
raise CostCapReached(_UNPRICED_TEXT)
|
||||
|
||||
@@ -72,6 +72,9 @@ class OpeningMessage:
|
||||
metadata: Mapping[str, Any] = field(default_factory=dict)
|
||||
|
||||
|
||||
# The runner's checkpoint phases that act on a model response, so an unpriced one must stop the turn first.
|
||||
_ACTS_ON_A_RESPONSE = frozenset({"assistant_tool_calls", "final_response"})
|
||||
|
||||
# What the engine hands a running chat turn: the inputs accepted since the last look, oldest first.
|
||||
InjectionSource = Callable[[], Awaitable[Sequence[OpeningMessage]]]
|
||||
|
||||
@@ -302,9 +305,10 @@ class TurnRunner:
|
||||
|
||||
async def commit(payload: dict[str, Any]) -> None:
|
||||
final_index = 0 if payload["phase"] == "final_response" else None
|
||||
if final_index is not None:
|
||||
# This write delivers the answer (and completes a task), and no check of the hook
|
||||
# follows the last response: a turn that sent a request it could not price ends here.
|
||||
if payload["phase"] in _ACTS_ON_A_RESPONSE:
|
||||
# These writes act on the response: the answer is delivered (and a task completes), the
|
||||
# tool calls run next, and no check of the hook comes in between. A turn that sent a
|
||||
# request it could not price ends here, before anything is done on its word.
|
||||
spend.ensure_priced()
|
||||
self._store.write(
|
||||
lambda conn: dots_store.append_messages(
|
||||
|
||||
@@ -1199,6 +1199,19 @@ class TestTheCostCap:
|
||||
)
|
||||
assert len(h.provider.requests) == 1
|
||||
|
||||
async def test_the_tool_calls_of_a_response_with_no_cost_do_not_run(self, make_harness: MakeHarness) -> None:
|
||||
h = make_harness([calls(call("c1", "list_dir", path=".")), says("never asked")], limits=cap_limits(1))
|
||||
h.provider.default_cost = None
|
||||
session = h.start_task()
|
||||
|
||||
outcome = await h.run(TurnUnit(session, "t1", (OpeningMessage("do it"),)))
|
||||
|
||||
assert outcome == TurnOutcome.failed(NO_COST_TEXT)
|
||||
assert len(h.provider.requests) == 1
|
||||
# The turn does not act on a response it could not price: no call was recorded or run.
|
||||
assert roles(h.messages(session)) == ["user"]
|
||||
assert h.events_of("tool.called") == []
|
||||
|
||||
async def test_a_final_answer_with_no_cost_fails_the_task_instead_of_completing_it(
|
||||
self, make_harness: MakeHarness
|
||||
) -> None:
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
"./browser": "./src/browser.ts",
|
||||
"./api": "./src/api.ts",
|
||||
"./paths": "./src/paths.ts",
|
||||
"./process": "./src/process.ts",
|
||||
"./api-token": "./src/api-token.ts",
|
||||
"./replace-file": "./src/replace-file.ts"
|
||||
},
|
||||
|
||||
@@ -69,6 +69,8 @@ export const ENV = {
|
||||
WEB_LISTEN: "INVISIBLE_DOTS_WEB_LISTEN",
|
||||
/** Extra host names (comma separated) the web server may be reached by, besides loopback. */
|
||||
WEB_ALLOWED_HOSTS: "INVISIBLE_DOTS_WEB_ALLOWED_HOSTS",
|
||||
/** The pid of the `invisible-dots server` that started the web server, which exits once that process is gone. */
|
||||
WEB_PARENT_PID: "INVISIBLE_DOTS_WEB_PARENT_PID",
|
||||
/** The API token itself, instead of the api.token file: the server's token, or the one a client sends. */
|
||||
TOKEN: "INVISIBLE_DOTS_TOKEN",
|
||||
/** Where clients (CLI, web server) reach the API. Default http://127.0.0.1:8787. */
|
||||
|
||||
Reference in new issue
Block a user