From 6524d2b67f4a908f7b54ce82fa370ce7634444f8 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 26 Aug 2026 09:08:31 -0500 Subject: [PATCH] fix(dev): honor --data-dir isolation (#12193) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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=`, 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 --- doc/DEVELOPING.md | 17 ++++ scripts/dev-runner-options.ts | 74 +++++++++++++++++ scripts/dev-runner.ts | 17 +++- scripts/dev-service.ts | 15 ++++ .../src/__tests__/dev-runner-options.test.ts | 83 +++++++++++++++++++ 5 files changed, 203 insertions(+), 3 deletions(-) create mode 100644 scripts/dev-runner-options.ts create mode 100644 server/src/__tests__/dev-runner-options.test.ts diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 20747f2681..33c5666c70 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -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`) diff --git a/scripts/dev-runner-options.ts b/scripts/dev-runner-options.ts new file mode 100644 index 0000000000..c1613b4328 --- /dev/null +++ b/scripts/dev-runner-options.ts @@ -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 }; +} diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 28e152e994..54e7198a74 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -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, }); diff --git a/scripts/dev-service.ts b/scripts/dev-service.ts index 978607ec3f..af03bf930d 100644 --- a/scripts/dev-service.ts +++ b/scripts/dev-service.ts @@ -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>) { @@ -11,6 +12,20 @@ function toDisplayLines(records: Awaited 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 }, diff --git a/server/src/__tests__/dev-runner-options.test.ts b/server/src/__tests__/dev-runner-options.test.ts new file mode 100644 index 0000000000..af0d85091e --- /dev/null +++ b/server/src/__tests__/dev-runner-options.test.ts @@ -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/, + ); + }, + ); +});