mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
master
1
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Guatuandfuthgar
|
2ae6fa51b1 |
fix: warn when a worktree-mode embedded-postgres data dir is in the OS temp dir (#8283)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work, and it can run as a self-hosted instance backed by an embedded PostgreSQL database. > - To let agents work in isolation, Paperclip supports worktree-local instances, gated by `PAPERCLIP_IN_WORKTREE` / `PAPERCLIP_HOME` (see `server/src/worktree-config.ts`, `cli/src/config/home.ts`). > - Those worktree env vars can leak into a *primary* instance's environment (inherited from an agent/worktree shell, or persisted into the instance env file). When they do, `paperclipai run` resolves the data root from `PAPERCLIP_HOME` and rewrites `config.json` to point the DB/backups/logs/storage at `$PAPERCLIP_HOME/instances/<id>/…`. > - If `$PAPERCLIP_HOME` is a throwaway dir under the OS temp dir, the primary instance boots a brand-new **empty** database. Every login then fails (`better-auth` logs `User not found`; the UI returns a generic `401`), so it looks like a *password* problem while the real data sits untouched in `~/.paperclip`. Nothing warns that the control-plane DB is ephemeral. > - This pull request makes that situation non-silent: the database preflight check (also surfaced by `doctor`) emits a `warn` when a worktree-mode instance's embedded-postgres data dir is inside the OS temp directory, with clear remediation. > - The benefit is that a confusing total lockout becomes an obvious, actionable warning the operator sees at every `run` and `doctor`. ## Linked Issues or Issue Description Refs #8282 Related PRs (not duplicates — complementary work on the same area): - #3030 — *stop leaking server worktree env into unrelated local adapter heartbeats* (tackles one **leak vector** of the same root cause; this PR adds **detection** of the resulting bad state). - #3899 — *fix(db): refuse side-started embedded migration instances* (adjacent embedded-postgres safety hardening). ## What Changed - `cli/src/checks/database-check.ts`: for `embedded-postgres` mode, emit `status: "warn"` when the resolved data dir is inside `os.tmpdir()` **and** `PAPERCLIP_IN_WORKTREE === "true"`. The message explains the ephemerality + likely env leak; the repair hint says to unset `PAPERCLIP_HOME` / `PAPERCLIP_IN_WORKTREE` (or pass `--data-dir`). Added a small `isInsideOsTmpDir()` helper. - Intentionally gated on worktree mode so deliberate ephemeral/CI instances that use a temp data dir without `PAPERCLIP_IN_WORKTREE` are **not** flagged. - `cli/src/__tests__/database-check.test.ts` (new): covers pass (persistent dir), warn (worktree-mode temp dir), and no-warn (temp dir without worktree mode). ## Verification ``` # in cli/ pnpm exec vitest run src/__tests__/database-check.test.ts # 3 passed pnpm exec vitest run src/__tests__/doctor.test.ts # passes (no regression) pnpm exec tsc --noEmit # no new errors in changed file ``` Manual repro of the underlying bug (no warning before this change): ```bash PAPERCLIP_IN_WORKTREE=true PAPERCLIP_HOME="$(mktemp -d)/.paperclip-worktrees" paperclipai run # -> boots an empty DB under /tmp; logins fail with "User not found". # With this change, run/doctor now print a Database WARN pointing at the cause + fix. ``` Note for transparency: one unrelated test (`worktree.test.ts > pauseSeededScheduledRoutines`) fails *locally only* because it shells out to a real `pnpm install` that times out in a sandbox — it does not touch `database-check`. Pre-existing `tsc` errors under `server/src/services/plugin-*` (missing `@paperclipai/plugin-sdk` build artifact) are also unrelated to this change. ## Risks Low risk. Additive, non-fatal `warn` only — no behavior change to startup or existing `pass`/`fail` paths, and gated on `PAPERCLIP_IN_WORKTREE` so it does not fire for intentional ephemeral/CI temp data dirs. A stricter follow-up (refuse to start the primary `run` against a temp-dir data dir unless explicitly opted in) is possible but intentionally out of scope here. ## Model Used - **Provider / model:** Anthropic Claude — Opus 4.8 - **Exact model ID:** `claude-opus-4-8` (1M-context variant) - **Context window:** 1M tokens - **Reasoning mode:** extended thinking enabled - **Capabilities used:** agentic tool use via Claude Code (repository exploration, file edits, local shell, and running the vitest suite locally before pushing) The change was authored by @futhgar with this model as an assistant; the diagnosis, fix, and tests were reviewed and verified locally. ## 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 run tests locally and they pass (new test + affected `doctor` suite; see Verification for one unrelated, environment-only failure) - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots (N/A — no UI change) - [x] I have updated relevant documentation to reflect my changes (the warning message + repair hint are self-documenting; no separate docs change needed) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending CI run) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending review) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: futhgar <futhgar@users.noreply.github.com> |