fix(dev): honor --data-dir isolation (#12193)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The development runner starts the local API, UI, and embedded PostgreSQL services. > - Developers need separate data roots when they run more than one local checkout. > - The runner forwarded `--data-dir` to a child process that did not use it. > - The migration check and server therefore continued to use the default Paperclip home. > - This pull request applies the data root before worktree setup and migration checks. > - The benefit is an isolated database and state root for each requested development run. ## Linked Issues or Issue Description Refs #7466 ## What Changed - Parse and consume `--data-dir`, `--data-dir=<path>`, and `-d` in the development runner. - Set isolated default home, config, and context paths before worktree setup and migration checks. - Keep explicit config and context paths unchanged. - Include the normalized data root in the local service identity. - Let `dev:list` and `dev:stop` select the matching isolated service registry. - Keep explicit option environments independent of ambient process instance values. - Add regression tests and development documentation. ## Verification - `PAPERCLIP_INSTANCE_ID=ambient-test-instance pnpm exec vitest run server/src/__tests__/dev-runner-options.test.ts` passes with 8 tests. - `pnpm --filter @paperclipai/server typecheck` passes. - `pnpm --filter @paperclipai/adapter-utils build` passes. - `pnpm -r typecheck` passes. - `pnpm build` passes. - `pnpm dev:list --data-dir ./tmp/dev-service-review-fixture` selects the isolated registry. - A live run of `pnpm dev --data-dir ./tmp/data-dir-pr-smoke` became healthy on port 3101 while another checkout used port 3100. - The live run used `./tmp/data-dir-pr-smoke/instances/default/db` on a separate PostgreSQL port. - The latest-head Linux CI matrix passes, including build, typecheck, canary, all general and serialized test shards, and all e2e shards. - `pnpm test:run` was attempted on macOS. Current `master` has unrelated workspace path failures because `/tmp` resolves to `/private/tmp`. The focused regression suite passes, and the full Linux matrix is green. ## Risks - Risk is low. The change only affects development runs that pass `--data-dir` and matching service-management commands. - Explicit `PAPERCLIP_CONFIG` and `PAPERCLIP_CONTEXT` values still take priority. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex with `gpt-5.6-sol` produced this change. The run used tool-enabled reasoning and code execution. The runtime did not expose its context window size. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
821573ede8
commit
6524d2b67f
|
|
@ -43,6 +43,23 @@ This starts:
|
|||
|
||||
`pnpm dev` and `pnpm dev:once` are now idempotent for the current repo and instance: if the matching Paperclip dev runner is already alive, Paperclip reports the existing process instead of starting a duplicate.
|
||||
|
||||
To run against a separate local state root, pass `--data-dir`. The dev runner
|
||||
translates it to an isolated `PAPERCLIP_HOME` before migration checks or server
|
||||
startup, so embedded PostgreSQL and other default instance state live under that
|
||||
directory:
|
||||
|
||||
```sh
|
||||
pnpm dev --data-dir ./tmp/paperclip-dev
|
||||
```
|
||||
|
||||
Pass the same option to the service-management commands so they use the
|
||||
isolated runtime-service registry:
|
||||
|
||||
```sh
|
||||
pnpm dev:list --data-dir ./tmp/paperclip-dev
|
||||
pnpm dev:stop --data-dir ./tmp/paperclip-dev
|
||||
```
|
||||
|
||||
Issue execution may also use project execution workspace policies and workspace runtime services for per-project worktrees, preview servers, and managed dev commands. Configure those through the project workspace/runtime surfaces rather than starting long-running unmanaged processes when a task needs a reusable service.
|
||||
|
||||
### Mobile-friendly preview (`pnpm dev:mobile`)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,74 @@
|
|||
import path from "node:path";
|
||||
import {
|
||||
DEFAULT_PAPERCLIP_INSTANCE_ID,
|
||||
expandHomePrefix,
|
||||
resolvePaperclipConfigPathForInstance,
|
||||
resolvePaperclipInstanceId,
|
||||
} from "../packages/shared/src/home-paths.ts";
|
||||
|
||||
export interface AppliedDevRunnerOptions {
|
||||
forwardedArgs: string[];
|
||||
dataDir: string | null;
|
||||
}
|
||||
|
||||
function requireOptionValue(
|
||||
args: string[],
|
||||
index: number,
|
||||
option: string,
|
||||
): string {
|
||||
const value = args[index + 1]?.trim();
|
||||
if (!value || value.startsWith("-")) {
|
||||
throw new Error(`${option} requires a value`);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
export function applyDevRunnerOptions(
|
||||
args: string[],
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
cwd: string = process.cwd(),
|
||||
): AppliedDevRunnerOptions {
|
||||
const forwardedArgs: string[] = [];
|
||||
let dataDirRaw: string | null = null;
|
||||
|
||||
for (let index = 0; index < args.length; index += 1) {
|
||||
const arg = args[index];
|
||||
if (arg === "--data-dir" || arg === "-d") {
|
||||
dataDirRaw = requireOptionValue(args, index, arg);
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
if (arg.startsWith("--data-dir=")) {
|
||||
const value = arg.slice("--data-dir=".length).trim();
|
||||
if (!value) throw new Error("--data-dir requires a value");
|
||||
dataDirRaw = value;
|
||||
continue;
|
||||
}
|
||||
forwardedArgs.push(arg);
|
||||
}
|
||||
|
||||
if (!dataDirRaw) {
|
||||
return { forwardedArgs, dataDir: null };
|
||||
}
|
||||
|
||||
const dataDir = path.resolve(cwd, expandHomePrefix(dataDirRaw));
|
||||
const hasExplicitConfig = Boolean(env.PAPERCLIP_CONFIG?.trim());
|
||||
const hasExplicitContext = Boolean(env.PAPERCLIP_CONTEXT?.trim());
|
||||
|
||||
env.PAPERCLIP_HOME = dataDir;
|
||||
if (!hasExplicitConfig) {
|
||||
const instanceId = resolvePaperclipInstanceId(
|
||||
env.PAPERCLIP_INSTANCE_ID ?? DEFAULT_PAPERCLIP_INSTANCE_ID,
|
||||
);
|
||||
env.PAPERCLIP_INSTANCE_ID = instanceId;
|
||||
env.PAPERCLIP_CONFIG = resolvePaperclipConfigPathForInstance({
|
||||
homeDir: dataDir,
|
||||
instanceId,
|
||||
});
|
||||
}
|
||||
if (!hasExplicitContext) {
|
||||
env.PAPERCLIP_CONTEXT = path.resolve(dataDir, "context.json");
|
||||
}
|
||||
|
||||
return { forwardedArgs, dataDir };
|
||||
}
|
||||
|
|
@ -6,6 +6,7 @@ import path from "node:path";
|
|||
import { createInterface } from "node:readline/promises";
|
||||
import { stdin, stdout } from "node:process";
|
||||
import { createCapturedOutputBuffer, parseJsonResponseWithLimit } from "./dev-runner-output.ts";
|
||||
import { applyDevRunnerOptions } from "./dev-runner-options.ts";
|
||||
import { collectWatchedSnapshot as collectDevServerWatchedSnapshot, diffSnapshots } from "./dev-runner-snapshot.mjs";
|
||||
import { createDevServiceIdentity, repoRoot } from "./dev-service-profile.ts";
|
||||
import { bootstrapDevRunnerWorktreeEnv, isWorktreeSeedPending } from "../server/src/dev-runner-worktree.ts";
|
||||
|
|
@ -21,6 +22,18 @@ import {
|
|||
const BIND_MODES = ["loopback", "lan", "tailnet", "custom"] as const;
|
||||
type BindMode = (typeof BIND_MODES)[number];
|
||||
|
||||
const mode = process.argv[2] === "watch" ? "watch" : "dev";
|
||||
let cliArgs: string[];
|
||||
let dataDir: string | null;
|
||||
try {
|
||||
const appliedOptions = applyDevRunnerOptions(process.argv.slice(3), process.env, repoRoot);
|
||||
cliArgs = appliedOptions.forwardedArgs;
|
||||
dataDir = appliedOptions.dataDir;
|
||||
} catch (error) {
|
||||
console.error(`[paperclip] ${error instanceof Error ? error.message : String(error)}`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const worktreeEnvBootstrap = bootstrapDevRunnerWorktreeEnv(repoRoot, process.env);
|
||||
if (worktreeEnvBootstrap.missingEnv) {
|
||||
console.error(
|
||||
|
|
@ -35,8 +48,6 @@ if (isWorktreeSeedPending(repoRoot)) {
|
|||
process.exit(1);
|
||||
}
|
||||
|
||||
const mode = process.argv[2] === "watch" ? "watch" : "dev";
|
||||
const cliArgs = process.argv.slice(3);
|
||||
const scanIntervalMs = 1500;
|
||||
const autoRestartPollIntervalMs = 2500;
|
||||
const gracefulShutdownTimeoutMs = 10_000;
|
||||
|
|
@ -196,7 +207,7 @@ if (tailscaleAuth || bindMode) {
|
|||
const serverPort = Number.parseInt(env.PORT ?? process.env.PORT ?? "3100", 10) || 3100;
|
||||
const devService = createDevServiceIdentity({
|
||||
mode,
|
||||
forwardedArgs,
|
||||
forwardedArgs: dataDir ? [...forwardedArgs, `--data-dir=${dataDir}`] : forwardedArgs,
|
||||
networkProfile: tailscaleAuth ? `legacy:${bindMode ?? "lan"}` : (bindMode ?? "default"),
|
||||
port: serverPort,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
#!/usr/bin/env -S node --import tsx
|
||||
import { listLocalServiceRegistryRecords, removeLocalServiceRegistryRecord, terminateLocalService } from "../server/src/services/local-service-supervisor.ts";
|
||||
import { applyDevRunnerOptions } from "./dev-runner-options.ts";
|
||||
import { repoRoot } from "./dev-service-profile.ts";
|
||||
|
||||
function toDisplayLines(records: Awaited<ReturnType<typeof listLocalServiceRegistryRecords>>) {
|
||||
|
|
@ -11,6 +12,20 @@ function toDisplayLines(records: Awaited<ReturnType<typeof listLocalServiceRegis
|
|||
}
|
||||
|
||||
const command = process.argv[2] ?? "list";
|
||||
try {
|
||||
const { forwardedArgs } = applyDevRunnerOptions(
|
||||
process.argv.slice(3),
|
||||
process.env,
|
||||
repoRoot,
|
||||
);
|
||||
if (forwardedArgs.length > 0) {
|
||||
throw new Error(`Unknown dev-service option: ${forwardedArgs[0]}`);
|
||||
}
|
||||
} catch (error) {
|
||||
console.error(`[paperclip] ${error instanceof Error ? error.message : String(error)}`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const records = await listLocalServiceRegistryRecords({
|
||||
profileKind: "paperclip-dev",
|
||||
metadata: { repoRoot },
|
||||
|
|
|
|||
|
|
@ -0,0 +1,83 @@
|
|||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { applyDevRunnerOptions } from "../../../scripts/dev-runner-options.ts";
|
||||
|
||||
describe("applyDevRunnerOptions", () => {
|
||||
it("turns --data-dir into isolated Paperclip paths and consumes the option", () => {
|
||||
const env: NodeJS.ProcessEnv = {};
|
||||
const cwd = path.join(os.tmpdir(), "paperclip-dev-runner-options");
|
||||
const previousInstanceId = process.env.PAPERCLIP_INSTANCE_ID;
|
||||
process.env.PAPERCLIP_INSTANCE_ID = "ambient-test-instance";
|
||||
|
||||
try {
|
||||
const result = applyDevRunnerOptions(
|
||||
["--bind", "loopback", "--data-dir", "./tmp", "--future-option"],
|
||||
env,
|
||||
cwd,
|
||||
);
|
||||
|
||||
const expectedHome = path.resolve(cwd, "tmp");
|
||||
expect(result).toEqual({
|
||||
forwardedArgs: ["--bind", "loopback", "--future-option"],
|
||||
dataDir: expectedHome,
|
||||
});
|
||||
expect(env.PAPERCLIP_HOME).toBe(expectedHome);
|
||||
expect(env.PAPERCLIP_INSTANCE_ID).toBe("default");
|
||||
expect(env.PAPERCLIP_CONFIG).toBe(
|
||||
path.join(expectedHome, "instances", "default", "config.json"),
|
||||
);
|
||||
expect(env.PAPERCLIP_CONTEXT).toBe(path.join(expectedHome, "context.json"));
|
||||
} finally {
|
||||
if (previousInstanceId === undefined) {
|
||||
delete process.env.PAPERCLIP_INSTANCE_ID;
|
||||
} else {
|
||||
process.env.PAPERCLIP_INSTANCE_ID = previousInstanceId;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
["short option", ["-d", "~/paperclip-dev"]],
|
||||
["equals form", ["--data-dir=~/paperclip-dev"]],
|
||||
])("supports the %s", (_label, args) => {
|
||||
const env: NodeJS.ProcessEnv = {};
|
||||
|
||||
const result = applyDevRunnerOptions(args, env, "/unused");
|
||||
|
||||
expect(result.forwardedArgs).toEqual([]);
|
||||
expect(result.dataDir).toBe(path.join(os.homedir(), "paperclip-dev"));
|
||||
});
|
||||
|
||||
it("uses the selected instance for the default config path", () => {
|
||||
const env: NodeJS.ProcessEnv = { PAPERCLIP_INSTANCE_ID: "experiment" };
|
||||
|
||||
applyDevRunnerOptions(["--data-dir", "/isolated/home"], env, "/unused");
|
||||
|
||||
expect(env.PAPERCLIP_CONFIG).toBe(
|
||||
path.join("/isolated/home", "instances", "experiment", "config.json"),
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves explicit config and context paths", () => {
|
||||
const env: NodeJS.ProcessEnv = {
|
||||
PAPERCLIP_CONFIG: "/explicit/config.json",
|
||||
PAPERCLIP_CONTEXT: "/explicit/context.json",
|
||||
};
|
||||
|
||||
applyDevRunnerOptions(["--data-dir", "/isolated/home"], env, "/unused");
|
||||
|
||||
expect(env.PAPERCLIP_HOME).toBe("/isolated/home");
|
||||
expect(env.PAPERCLIP_CONFIG).toBe("/explicit/config.json");
|
||||
expect(env.PAPERCLIP_CONTEXT).toBe("/explicit/context.json");
|
||||
});
|
||||
|
||||
it.each([["--data-dir"], ["-d"], ["--data-dir="]])(
|
||||
"rejects a missing value for %s",
|
||||
(...args) => {
|
||||
expect(() => applyDevRunnerOptions(args, {}, "/unused")).toThrow(
|
||||
/requires a value/,
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
Loading…
Reference in New Issue