fix(pr): harden coordinator hosted gates (#108153)

* fix(pr): route hosted gate reads through cache

* test(control-ui): stabilize process readiness

* fix(ci): stabilize startup memory sampling

* fix(gateway): route OpenClaw approval listing

* fix(state): reuse schema repair transaction

* test(agents): assert host authority scope

* chore(plugin-sdk): refresh API baseline

* test(agents): assert delegate tool fixture

* test(sessions): stabilize concurrency worker startup
This commit is contained in:
Peter Steinberger
2026-07-15 03:45:37 -07:00
committed by GitHub
parent 026b688161
commit c490117a9f
9 changed files with 186 additions and 39 deletions
+4
View File
@@ -9,6 +9,7 @@ export namespace testing {
export { resolveDefaultLimitsMb };
export { runCase };
export { runStartupMemoryCheck };
export { sampleCount };
}
declare const cases: {
id: string;
@@ -32,6 +33,7 @@ declare function readPositiveNumberEnv(
env?: NodeJS.ProcessEnv,
): unknown;
declare const repoRoot: string;
declare const sampleCount: number;
declare function resolveDefaultLimitsMb(platform?: NodeJS.Platform): {
help: number;
pluginsList: number;
@@ -47,6 +49,7 @@ declare function runCase(
command: string;
limitMb: unknown;
maxRssMb: number | null;
rssSamplesMb?: number[];
status: string;
exitCode: unknown;
signal: unknown;
@@ -63,6 +66,7 @@ declare function runStartupMemoryCheck(
command: string;
limitMb: unknown;
maxRssMb: number | null;
rssSamplesMb?: number[];
status: string;
exitCode: unknown;
signal: unknown;
+55 -18
View File
@@ -11,6 +11,7 @@ const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."
const tmpDir = process.env.TMPDIR || process.env.TEMP || process.env.TMP || os.tmpdir();
const MAX_RSS_MARKER = "__OPENCLAW_MAX_RSS_KB__=";
const DEFAULT_COMMAND_TIMEOUT_MS = 60_000;
const STARTUP_MEMORY_SAMPLE_COUNT = 3;
const COMMAND_TIMEOUT_MS = readPositiveIntEnv(
"OPENCLAW_STARTUP_MEMORY_TIMEOUT_MS",
DEFAULT_COMMAND_TIMEOUT_MS,
@@ -191,16 +192,16 @@ function nodeImportSpecifierForPath(filePath) {
return pathToFileURL(filePath).href;
}
function buildBenchEnv() {
if (!tmpHome) {
function buildBenchEnv(homeDir = tmpHome) {
if (!homeDir) {
throw new Error("temporary home is not initialized");
}
const env = {
HOME: tmpHome,
USERPROFILE: tmpHome,
XDG_CONFIG_HOME: path.join(tmpHome, ".config"),
XDG_DATA_HOME: path.join(tmpHome, ".local", "share"),
XDG_CACHE_HOME: path.join(tmpHome, ".cache"),
HOME: homeDir,
USERPROFILE: homeDir,
XDG_CONFIG_HOME: path.join(homeDir, ".config"),
XDG_DATA_HOME: path.join(homeDir, ".local", "share"),
XDG_CACHE_HOME: path.join(homeDir, ".cache"),
PATH: process.env.PATH ?? "",
TMPDIR: tmpDir,
TEMP: tmpDir,
@@ -229,11 +230,16 @@ function buildBenchEnv() {
return env;
}
function runCase(testCase, params = {}) {
function runCaseSample(testCase, sampleIndex, params = {}) {
if (!rssHookPath) {
throw new Error("RSS hook path is not initialized");
}
const env = buildBenchEnv();
if (!tmpHome) {
throw new Error("temporary home is not initialized");
}
const sampleHome = path.join(tmpHome, "homes", `${testCase.id}-${sampleIndex + 1}`);
mkdirSync(sampleHome, { recursive: true });
const env = buildBenchEnv(sampleHome);
const spawn = params.spawnSync ?? defaultSpawnSync;
const timeoutMs = params.timeoutMs ?? COMMAND_TIMEOUT_MS;
const result = spawn(
@@ -295,18 +301,46 @@ function runCase(testCase, params = {}) {
failureMessage: formatFailure(testCase, report.error),
});
}
return report;
}
function median(values) {
const sorted = values.toSorted((left, right) => left - right);
return sorted[Math.floor(sorted.length / 2)];
}
function formatRssSamples(samplesMb) {
return samplesMb.map((value) => value.toFixed(1)).join(", ");
}
function runCase(testCase, params = {}) {
const samples = [];
let report = null;
// Shared CI runners occasionally produce a single allocator/RSS spike. Independent
// homes plus a median keep that outlier from masking regressions; two high samples fail.
for (let sampleIndex = 0; sampleIndex < STARTUP_MEMORY_SAMPLE_COUNT; sampleIndex += 1) {
report = runCaseSample(testCase, sampleIndex, params);
if (report.status !== "pass" || report.maxRssMb == null) {
return report;
}
samples.push(report.maxRssMb);
}
const maxRssMb = median(samples);
report = { ...report, maxRssMb, rssSamplesMb: samples };
if (maxRssMb > testCase.limitMb) {
report.status = "fail";
report.error = `${testCase.label} used ${maxRssMb.toFixed(1)} MB RSS (limit ${
report.error = `${testCase.label} median max RSS ${maxRssMb.toFixed(1)} MB exceeded ${
testCase.limitMb
} MB)`;
} MB (samples: ${formatRssSamples(samples)} MB)`;
return Object.assign(report, {
failureMessage: formatFailure(testCase, report.error),
});
}
console.log(
`[startup-memory] ${testCase.label}: ${maxRssMb.toFixed(1)} MB RSS (limit ${testCase.limitMb} MB)`,
`[startup-memory] ${testCase.label}: ${maxRssMb.toFixed(1)} MB median max RSS ` +
`(limit ${testCase.limitMb} MB; samples ${formatRssSamples(samples)} MB)`,
);
return report;
}
@@ -327,12 +361,14 @@ function writeReport(options, results) {
"",
`Status: ${report.status}`,
"",
...results.map(
(result) =>
`- ${result.label}: ${result.status} RSS ${formatMb(result.maxRssMb)} / ${formatMb(
result.limitMb,
)}`,
),
...results.map((result) => {
const samples = result.rssSamplesMb
? ` (samples: ${result.rssSamplesMb.map(formatMb).join(", ")})`
: "";
return `- ${result.label}: ${result.status} median max RSS ${formatMb(
result.maxRssMb,
)} / ${formatMb(result.limitMb)}${samples}`;
}),
"",
];
if (failed.length > 0) {
@@ -403,6 +439,7 @@ export const testing = {
resolveDefaultLimitsMb,
runCase,
runStartupMemoryCheck,
sampleCount: STARTUP_MEMORY_SAMPLE_COUNT,
};
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
+16 -4
View File
@@ -19,11 +19,23 @@ export function execPlainGh(
export function execPlainGh(
args: readonly string[],
options?: ExecFileSyncOptionsWithBufferEncoding,
): NonSharedBuffer;
): Uint8Array<ArrayBuffer>;
export function execPlainGh(
args: readonly string[],
options?: ExecFileSyncOptions,
): string | NonSharedBuffer;
): string | Uint8Array<ArrayBuffer>;
export function execGhApiRead(
endpoint: string,
options: ExecFileSyncOptionsWithStringEncoding,
): string;
export function execGhApiRead(
endpoint: string,
options?: ExecFileSyncOptionsWithBufferEncoding,
): Uint8Array<ArrayBuffer>;
export function execGhApiRead(
endpoint: string,
options?: ExecFileSyncOptions,
): string | Uint8Array<ArrayBuffer>;
export function spawnPlainGh(
args: readonly string[],
options: SpawnSyncOptionsWithStringEncoding,
@@ -31,9 +43,9 @@ export function spawnPlainGh(
export function spawnPlainGh(
args: readonly string[],
options?: SpawnSyncOptionsWithBufferEncoding,
): SpawnSyncReturns<NonSharedBuffer>;
): SpawnSyncReturns<Uint8Array<ArrayBuffer>>;
export function spawnPlainGh(
args: readonly string[],
options?: SpawnSyncOptions,
): SpawnSyncReturns<string | NonSharedBuffer>;
): SpawnSyncReturns<string | Uint8Array<ArrayBuffer>>;
export const PLAIN_GH_SYSTEM_CANDIDATES: string[];
+11
View File
@@ -88,6 +88,17 @@ export function execPlainGh(args, options = {}) {
});
}
export function execGhApiRead(endpoint, options = {}) {
const env = plainGhEnv(options.env ?? process.env);
// Keep reads on the normal PATH shim; OPENCLAW_GH_BIN pins maintainer writes.
delete env.OPENCLAW_GH_BIN;
return execFileSync("gh", ["api", endpoint, "--method", "GET"], {
...options,
env,
maxBuffer: options.maxBuffer ?? PLAIN_GH_MAX_BUFFER_BYTES,
});
}
export function spawnPlainGh(args, options = {}) {
const env = plainGhEnv(options.env ?? process.env);
const ghBin = resolvePlainGhBin(env);
+5 -8
View File
@@ -2,7 +2,7 @@
import { mkdirSync, writeFileSync } from "node:fs";
import path from "node:path";
import { isDirectRunUrl } from "./lib/direct-run.mjs";
import { execPlainGh } from "./lib/plain-gh.mjs";
import { execGhApiRead } from "./lib/plain-gh.mjs";
export const SCHEDULED_HOSTED_WORKFLOWS = [
"Blacksmith Testbox",
@@ -412,7 +412,7 @@ export function workflowRunQueryPaths(repo, { sha, recentSha, headBranch }, page
function loadWorkflowRunsForQuery(queryForPage) {
const loadPage = (page) =>
parseWorkflowRunPage(
execPlainGh(["api", queryForPage(page)], {
execGhApiRead(queryForPage(page), {
encoding: "utf8",
stdio: ["ignore", "pipe", "pipe"],
}),
@@ -446,11 +446,8 @@ export function compareCommitPageCount(totalCommits) {
function loadPullRequestCommitShas(repo, { baseSha, headSha }) {
const loadPage = (page) =>
JSON.parse(
execPlainGh(
[
"api",
`repos/${repo}/compare/${baseSha}...${headSha}?per_page=${COMPARE_COMMITS_PAGE_SIZE}&page=${page}`,
],
execGhApiRead(
`repos/${repo}/compare/${baseSha}...${headSha}?per_page=${COMPARE_COMMITS_PAGE_SIZE}&page=${page}`,
{
encoding: "utf8",
stdio: ["ignore", "pipe", "pipe"],
@@ -482,7 +479,7 @@ function loadPullRequestCommitShas(repo, { baseSha, headSha }) {
export function main(argv = process.argv.slice(2)) {
const args = parseArgs(argv);
const pullRequest = JSON.parse(
execPlainGh(["api", `repos/${args.repo}/pulls/${args.pr}`], {
execGhApiRead(`repos/${args.repo}/pulls/${args.pr}`, {
encoding: "utf8",
stdio: ["ignore", "pipe", "pipe"],
}),
@@ -4,7 +4,7 @@ import os from "node:os";
import path from "node:path";
import process from "node:process";
import { pathToFileURL } from "node:url";
import { afterAll, afterEach, describe, expect, it, vi } from "vitest";
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import { useAutoCleanupTempDirTracker } from "../../../test/helpers/temp-dir.js";
import {
appendTranscriptMessage,
@@ -79,7 +79,10 @@ type ConcurrencyWorkerMessage =
| { phase: "ready"; requestId: number; value: unknown }
| { phase: "result"; requestId: number; value: unknown };
const WAIT_TIMEOUT_MS = 10_000;
// Cold tsx/module loading competes with other CI shards. Pay that cost once
// with a process-start budget, while keeping each concurrency handshake tight.
const WORKER_BOOT_TIMEOUT_MS = 30_000;
const SCENARIO_TIMEOUT_MS = 10_000;
const SESSION_KEY = "agent:main:main";
const AGENT_ID = "main";
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
@@ -266,7 +269,7 @@ async function waitForWorkerBoot(child: ReturnType<typeof spawn>): Promise<void>
const timeout = setTimeout(() => {
cleanup();
reject(new Error("timeout waiting for concurrency worker startup"));
}, WAIT_TIMEOUT_MS);
}, WORKER_BOOT_TIMEOUT_MS);
const cleanup = () => {
clearTimeout(timeout);
child.off("error", onError);
@@ -336,7 +339,7 @@ async function runConcurrencyScenario<TRequest extends ConcurrencyWorkerRequest>
let readyHandled = false;
const timeout = setTimeout(() => {
fail(new Error(`timeout waiting for concurrency worker ${request.kind}`));
}, WAIT_TIMEOUT_MS);
}, SCENARIO_TIMEOUT_MS);
const cleanup = () => {
clearTimeout(timeout);
child.off("error", onError);
@@ -426,6 +429,10 @@ async function waitForChild(child: ReturnType<typeof spawn>, label: string): Pro
}
describe("session accessor cross-process concurrency", () => {
beforeAll(async () => {
await getConcurrencyWorker();
}, WORKER_BOOT_TIMEOUT_MS + 5_000);
afterAll(async () => {
const child = concurrencyWorker;
concurrencyWorker = undefined;
+64 -2
View File
@@ -20,6 +20,34 @@ function expectNoNodeStack(stderr: string): void {
expect(stderr).not.toContain("\n at ");
}
function runStartupMemoryCheckWithHelpSamples(helpSamplesMb: number[]) {
const tempRoot = makeTempRoot();
let sampleIndex = 0;
return testing.runStartupMemoryCheck(
[
"--json",
path.join(tempRoot, "startup-memory.json"),
"--summary",
path.join(tempRoot, "summary.md"),
],
{
platform: "linux",
spawnSync: () => {
const caseIndex = Math.floor(sampleIndex / testing.sampleCount);
const caseSampleIndex = sampleIndex % testing.sampleCount;
sampleIndex += 1;
const rssMb = caseIndex === 0 ? (helpSamplesMb[caseSampleIndex] ?? 1) : 1;
return {
signal: null,
status: 0,
stderr: `__OPENCLAW_MAX_RSS_KB__=${rssMb * 1024}\n`,
stdout: "",
};
},
},
);
}
afterEach(() => {
for (const root of tempRoots.splice(0)) {
rmSync(root, { recursive: true, force: true });
@@ -67,6 +95,33 @@ describe("check-cli-startup-memory", () => {
expect(testing.resolveDefaultLimitsMb("linux").statusJson).toBe(425);
});
it("uses the median of three cold-start RSS samples", () => {
if (process.platform !== "darwin" && process.platform !== "linux") {
return;
}
const helpSamplesMb = [120, 80, 85];
const result = runStartupMemoryCheckWithHelpSamples(helpSamplesMb);
expect(result.results[0]).toMatchObject({
maxRssMb: 85,
rssSamplesMb: helpSamplesMb,
status: "pass",
});
});
it("still fails when most cold-start RSS samples exceed the budget", () => {
if (process.platform !== "darwin" && process.platform !== "linux") {
return;
}
const helpSamplesMb = [120, 110, 80];
expect(() => runStartupMemoryCheckWithHelpSamples(helpSamplesMb)).toThrow(
"--help median max RSS 110.0 MB exceeded 100 MB",
);
});
it("keeps invalid startup memory env values from bypassing budgets", () => {
expect(() =>
testing.readPositiveNumberEnv("OPENCLAW_STARTUP_MEMORY_HELP_MB", 100, {
@@ -241,6 +296,7 @@ describe("check-cli-startup-memory", () => {
const tempRoot = makeTempRoot();
const seenArgs: string[][] = [];
const seenHomes: string[] = [];
const result = testing.runStartupMemoryCheck(
[
@@ -251,8 +307,13 @@ describe("check-cli-startup-memory", () => {
],
{
platform: "linux",
spawnSync: (_command: string, args: string[]) => {
spawnSync: (_command: string, args: string[], options: { env: Record<string, string> }) => {
seenArgs.push(args);
const home = options.env.HOME;
if (!home) {
throw new Error("benchmark HOME was not set");
}
seenHomes.push(home);
return {
error: null,
signal: null,
@@ -265,7 +326,8 @@ describe("check-cli-startup-memory", () => {
);
expect(result.skipped).toBe(false);
expect(seenArgs).toHaveLength(testing.cases.length);
expect(seenArgs).toHaveLength(testing.cases.length * testing.sampleCount);
expect(new Set(seenHomes).size).toBe(seenArgs.length);
for (const args of seenArgs) {
expect(args[0]).toBe("--import");
expect(args[1]).toMatch(/^file:/u);
+3 -3
View File
@@ -389,9 +389,9 @@ describe("control-ui-i18n process runner", () => {
});
try {
const deadline = Date.now() + 5_000;
const deadline = Date.now() + 30_000;
let fastReady = false;
while (Date.now() < deadline) {
let fastReady = false;
try {
fastReady = readFileSync(fastReadyPath, "utf8") === "ready";
} catch {}
@@ -405,7 +405,7 @@ describe("control-ui-i18n process runner", () => {
setTimeout(resolve, 10);
});
}
expect(readFileSync(fastReadyPath, "utf8")).toBe("ready");
expect(fastReady).toBe(true);
expect(grandchildPid).toBeGreaterThan(0);
expect(processIsAlive(grandchildPid)).toBe(true);
+17
View File
@@ -5,6 +5,7 @@ import { tmpdir } from "node:os";
import path from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import {
execGhApiRead,
execPlainGh,
plainGhEnv,
PLAIN_GH_SYSTEM_CANDIDATES,
@@ -35,6 +36,7 @@ printf 'FORCE_COLOR=%s\\n' "\${FORCE_COLOR-}"
printf 'CLICOLOR=%s\\n' "\${CLICOLOR-}"
printf 'CLICOLOR_FORCE=%s\\n' "\${CLICOLOR_FORCE-}"
printf 'COLORTERM_SET=%s\\n' "\${COLORTERM+x}"
printf 'OPENCLAW_GH_BIN_SET=%s\\n' "\${OPENCLAW_GH_BIN+x}"
`,
);
chmodSync(ghPath, 0o755);
@@ -97,6 +99,21 @@ describe("plain gh helpers", () => {
expect(plainGhEnv({ GH_FORCE_TTY: "120" })).not.toHaveProperty("GH_FORCE_TTY");
});
it("routes explicit GET reads through the PATH shim", () => {
const ghPath = makeFakeGh();
const output = execGhApiRead("repos/openclaw/openclaw/pulls/1", {
encoding: "utf8",
env: {
...process.env,
OPENCLAW_GH_BIN: "/identity-sensitive/plain-gh",
PATH: `${path.dirname(ghPath)}${path.delimiter}${process.env.PATH ?? ""}`,
},
});
expect(output).toContain("argv=api repos/openclaw/openclaw/pulls/1 --method GET");
expect(output).toContain("OPENCLAW_GH_BIN_SET=");
});
it("runs the shell helper with color disabled", () => {
const ghPath = makeFakeGh();
const outputPath = path.join(path.dirname(path.dirname(ghPath)), "output.txt");