mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server reads its configuration through `loadConfig()` in `server/src/config.ts`. Runtime code calls it often: `getStorageService()` calls it on each use, and the native runtime calls it for backup storage, response delivery and attachment reuse > - `loadConfig()` called `detectTailnetBindHost()` on each call. That function runs `tailscale ip -4` with `execFileSync`, which is a synchronous child process. The event loop stops until the child process ends > - A CPU profile of a busy hosted server showed this cost on the main thread, on a server that does not bind to a tailnet > - The tailnet address is an input only to the bind host, and only the listener that the server binds at startup uses the bind host. Most bind configurations never read the address > - This pull request makes `loadConfig()` detect the address only when the bind configuration reads it, and keeps the result for the life of the process > - The benefit is that a `loadConfig()` call no longer starts a child process on runtime paths, so the event loop stays free for requests ## Linked Issues or Issue Description No public issue exists. I searched open and closed pull requests for `tailscale loadConfig` and `detectTailnetBindHost` and found no related work. **What happened?** Each `loadConfig()` call started `tailscale ip -4` as a synchronous child process. A 30 second CPU profile of the main thread of a hosted server (commit556b249897, Node 24, main thread at 96% of one core) showed `detectTailnetBindHost` ← `loadConfig` at 3.1% inclusive and the native child-process spawn at 3.6% self. The callers in the profile were `getStorageService`, `durableNativeHarnessStorage`, `restoreDurableHarnessForExecution` and `createRunnerdBackendWithinSessionClaim`. The server process was about 4.4 GB resident, which makes each spawn more expensive. The cost is the same when the `tailscale` command is not installed, because the spawn still runs. **Expected behavior** `loadConfig()` starts `tailscale` only when the bind configuration reads a tailnet address: `server.bind=tailnet`, or a host without a bind mode that is not loopback and not all interfaces. It starts it at most one time per process. A change to `PAPERCLIP_TAILNET_BIND_HOST`, to another environment variable or to the config file is still visible to the next `loadConfig()` call. **Steps to reproduce** 1. Run `pnpm --filter @paperclipai/server exec tsx scripts/benchmark-config-load.ts --calls=200` on the parent commit. The benchmark puts a counting `tailscale` stand-in first on `PATH`, so it does not need Tailscale. 2. Read the `tailscale execs` column. Each scenario shows 200 execs for 200 calls. **Paperclip version or commit** `master` ate2cf9a670f. The profile is from556b249897. **Deployment mode** All modes. The profile is from an `authenticated` hosted server. The benchmark covers `bind=lan`, the default loopback bind and `bind=tailnet`. **Node.js version** Profile: Node 24. Local benchmark: Node 26.4.0 on darwin arm64. ## What Changed - `server/src/config.ts`: `loadConfig()` calls `detectTailnetBindHost()` only when the bind configuration reads the address. With a configured bind mode, that is `tailnet` only. Without one, it is a host that `inferBindModeFromHost` cannot resolve to loopback or all interfaces without the address. - `server/src/config.ts`: the result of `tailscale ip -4` is kept for the life of the process, also when the command fails. `PAPERCLIP_TAILNET_BIND_HOST` is not kept: each call reads it first, as before. - `server/src/__tests__/config-tailnet-bind-detection.test.ts` (new): regression tests that count `tailscale` starts. - `server/scripts/benchmark-config-load.ts` (new): the reproduction and benchmark. - `doc/DEVELOPING.md`: removed the advice to set `PAPERCLIP_TAILNET_BIND_HOST` for local fixtures to prevent repeated discovery. The server no longer repeats discovery. - `cli/src/config/server-bind.ts` has its own copy of the detection. It is not changed: it detects only for `bind=tailnet` and runs in short CLI commands, not on a server runtime path. ## Verification Regression test, red then green: - `cd server && pnpm exec vitest run src/__tests__/config-tailnet-bind-detection.test.ts` - Parent commit: 11 of 11 tests fail. Examples: `expected 1 to be +0` for `bind=lan` (the command ran when it must not), `expected 3 to be 1` for `bind=tailnet` (the command ran on each call). - This branch: 11 of 11 tests pass. - The tests assert the number of `tailscale` starts and the resolved bind host. They do not assert time. They cover `bind=loopback`, `lan`, `custom`, loopback and all-interfaces hosts without a bind mode, repeated `getStorageService()`, `bind=tailnet`, bind inference from a host, `PAPERCLIP_TAILNET_BIND_HOST` (used without the command, each change visible), a bind mode change in the environment and in the config file, and the unchanged refusal when no tailnet address exists. Benchmark, same machine, 200 calls per scenario, 3 runs each (ranges shown): | Scenario | Before: `tailscale` execs | Before: ms per call | After: `tailscale` execs | After: ms per call | | --- | --- | --- | --- | --- | | `loadConfig()`, `bind=lan` | 200 | 4.5 to 6.0 | 0 | 0.063 to 0.069 | | `getStorageService()`, `bind=lan` | 200 | 4.0 to 4.5 | 0 | 0.043 to 0.060 | | `loadConfig()`, default loopback | 200 | 4.0 to 4.5 | 0 | 0.046 to 0.053 | | `loadConfig()`, `bind=tailnet` | 200 | 3.8 to 5.1 | 1 | 0.74 to 0.92 | - Command: `pnpm --filter @paperclipai/server exec tsx scripts/benchmark-config-load.ts --calls=200` - Each call is synchronous, so the time per call is time that the event loop is blocked. For 200 calls the block went from about 750 to 1200 ms to about 9 to 14 ms where the bind does not read a tailnet address. - The `bind=tailnet` row after the fix contains the one detection (about 150 ms for the first start of the new stand-in file on macOS). The other 199 calls start no process. - `loadConfig()` also reads and validates the config file on each call. After the fix a full call costs about 0.05 ms locally, so the file read is not a material cost and this pull request does not change it. A hosted server reads a stored snapshot from memory, not a file. Other checks: - `cd server && pnpm exec vitest run src/__tests__/managed-runtime-public-url.test.ts src/__tests__/native-harness-backup-expiration-config.test.ts src/__tests__/chat-webhook-public-url.test.ts src/__tests__/managed-cloud-prerequisites.test.ts src/__tests__/managed-loopback-auth.test.ts src/__tests__/cloud-runtime-config.test.ts`: 84 tests pass. - `tsc --noEmit` over the three changed TypeScript files: clean. - `pnpm verify` exits 2 (broader verification required, because of the new file in `server/scripts`). Required CI supplies that coverage. - Not verified: the benchmark on Linux with a multi-gigabyte process, and the hosted server after deploy. The local numbers come from a 200 MB process, where a spawn is cheaper than on the profiled server. ## Risks - Low risk. The resolved `bind`, `host` and `customBindHost` are the same as before for each configuration. A configured bind mode other than `tailnet` never used the detected address, and a loopback or all-interfaces host resolved before the address was read. - Behavior shift: a server process detects the Tailscale address one time. If Tailscale starts, stops or changes its address later, `loadConfig()` in that process still returns the first result until restart. The server binds its listener one time at startup, so the listener already had this property. From the code (not reproduced): before this change, a `bind=tailnet` server whose Tailscale stopped after startup threw from each later `loadConfig()` call on runtime paths. It now continues with the address from startup. - A failed detection is also kept. With `bind=tailnet` the server refuses to start, as before, and the next start detects again. `PAPERCLIP_TAILNET_BIND_HOST` overrides a kept result at once. ## Model Used - Claude Fable 5.2 (Anthropic, model ID `claude-fable-5-2`), in Claude Code with tool use (shell, file edits, GitHub CLI). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
83 lines
3.9 KiB
TypeScript
83 lines
3.9 KiB
TypeScript
import assert from "node:assert/strict";
|
|
import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
|
|
const argument = (name: string) => process.argv.find((value) => value.startsWith(`${name}=`))?.slice(name.length + 1);
|
|
const calls = Number(argument("--calls") ?? 200);
|
|
assert.ok(Number.isInteger(calls) && calls >= 1 && calls <= 100_000, "--calls must be an integer from 1 to 100000");
|
|
assert.notEqual(process.platform, "win32", "the counting tailscale stand-in needs a POSIX shell");
|
|
|
|
const TAILNET_ADDRESS = "100.64.0.1";
|
|
const home = mkdtempSync(path.join(os.tmpdir(), "paperclip-config-load-benchmark-"));
|
|
const configPath = path.join(home, "config.json");
|
|
const execLogPath = path.join(home, "tailscale-execs.log");
|
|
const binDir = path.join(home, "bin");
|
|
|
|
function isolateEnvironment() {
|
|
for (const name of [
|
|
"PAPERCLIP_BIND", "PAPERCLIP_BIND_HOST", "PAPERCLIP_TAILNET_BIND_HOST", "HOST", "PAPERCLIP_DEPLOYMENT_MODE",
|
|
"PAPERCLIP_DEPLOYMENT_EXPOSURE", "PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN", "PAPERCLIP_MANAGED_CONFIG",
|
|
"PAPERCLIP_STORAGE_PROVIDER", "DATABASE_URL",
|
|
]) delete process.env[name];
|
|
process.env.PAPERCLIP_HOME = home;
|
|
process.env.PAPERCLIP_INSTANCE_ID = "default";
|
|
process.env.PAPERCLIP_CONFIG = configPath;
|
|
if (!process.env.PATH?.startsWith(binDir)) process.env.PATH = `${binDir}${path.delimiter}${process.env.PATH ?? ""}`;
|
|
}
|
|
|
|
function writeConfig(server: Record<string, unknown>) {
|
|
writeFileSync(configPath, JSON.stringify({
|
|
$meta: { version: 1, updatedAt: "2026-11-11T00:00:00.000Z", source: "configure" },
|
|
database: { mode: "embedded-postgres" },
|
|
logging: { mode: "file" },
|
|
server,
|
|
}));
|
|
}
|
|
|
|
const tailscaleExecs = () => existsSync(execLogPath) ? readFileSync(execLogPath, "utf8").split("\n").filter(Boolean).length : 0;
|
|
|
|
try {
|
|
isolateEnvironment();
|
|
writeConfig({});
|
|
mkdirSync(binDir);
|
|
const standIn = path.join(binDir, "tailscale");
|
|
writeFileSync(standIn, `#!/bin/sh\necho exec >> '${execLogPath}'\necho ${TAILNET_ADDRESS}\n`);
|
|
chmodSync(standIn, 0o755);
|
|
|
|
const { loadConfig } = await import("../src/config.js");
|
|
const { getStorageService } = await import("../src/storage/index.js");
|
|
isolateEnvironment();
|
|
|
|
const scenarios: Array<{ name: string; server: Record<string, unknown>; expectedHost: string; run: () => void }> = [
|
|
{ name: "loadConfig, bind=lan (hosted shape)", server: { deploymentMode: "authenticated", exposure: "private", bind: "lan" },
|
|
expectedHost: "0.0.0.0", run: () => { loadConfig(); } },
|
|
{ name: "getStorageService, bind=lan", server: { deploymentMode: "authenticated", exposure: "private", bind: "lan" },
|
|
expectedHost: "0.0.0.0", run: () => { getStorageService(); } },
|
|
{ name: "loadConfig, default loopback", server: {}, expectedHost: "127.0.0.1", run: () => { loadConfig(); } },
|
|
{ name: "loadConfig, bind=tailnet", server: { deploymentMode: "authenticated", exposure: "private", bind: "tailnet" },
|
|
expectedHost: TAILNET_ADDRESS, run: () => { loadConfig(); } },
|
|
];
|
|
|
|
const rows = scenarios.map((scenario) => {
|
|
writeConfig(scenario.server);
|
|
const execsBefore = tailscaleExecs();
|
|
const started = process.hrtime.bigint();
|
|
for (let index = 0; index < calls; index += 1) scenario.run();
|
|
const totalMs = Number(process.hrtime.bigint() - started) / 1e6;
|
|
const execs = tailscaleExecs() - execsBefore;
|
|
assert.equal(loadConfig().host, scenario.expectedHost, `${scenario.name}: resolved bind host`);
|
|
return {
|
|
scenario: scenario.name,
|
|
calls,
|
|
"tailscale execs": execs,
|
|
"total ms (event loop blocked)": Number(totalMs.toFixed(1)),
|
|
"ms per call": Number((totalMs / calls).toFixed(3)),
|
|
};
|
|
});
|
|
console.table(rows);
|
|
console.log(`node ${process.version}, ${os.platform()} ${os.arch()}, rss ${Math.round(process.memoryUsage().rss / 1e6)} MB`);
|
|
} finally {
|
|
rmSync(home, { recursive: true, force: true });
|
|
}
|