From c036bbfa98494dcfe2521aab65019a4cd021c769 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 26 Apr 2026 07:52:05 -0500 Subject: [PATCH 1/9] Add first-class security agent role to taxonomy (#4532) ## Thinking Path > - Paperclip is the control plane for AI-agent companies, so agent metadata is part of the platform's governance and audit surface. > - The shared agent taxonomy in `@paperclipai/shared` is the source of truth for allowed agent roles and their UI labels. > - The current taxonomy lacks a `security` role, which causes Security Engineer hires to collapse into `engineer`. > - That breaks separation-of-duties evidence in telemetry and weakens role-level audit fidelity even though it does not directly change permissions. > - This pull request adds `security` as a first-class shared role and covers the prior rejection path with a regression test. > - The benefit is that Security Engineer agents can now be persisted and rendered under the correct role without schema or permission churn. ## What Changed - Added `security` to `AGENT_ROLES` in `packages/shared/src/constants.ts`. - Added the `Security` display label to `AGENT_ROLE_LABELS` so existing UI consumers render the new role automatically. - Added a shared validator regression test proving `createAgentSchema` accepts `role: "security"` and that the label stays stable. ## Verification - `pnpm --filter @paperclipai/shared typecheck` - `pnpm --filter @paperclipai/shared exec vitest run src/adapter-types.test.ts` ## Risks - Low risk. This is a shared enum expansion with no database migration and no permission-model change. - Residual risk: this PR does not backfill existing agents already persisted as `engineer`; it only fixes new validations and labels going forward. > I checked `ROADMAP.md`/`doc` for overlap and did not find an existing planned item covering this taxonomy fix. ## Model Used - OpenAI GPT-5.4 via the Codex local adapter, with tool use and local code execution enabled. The runtime did not surface a separate context-window identifier in agent metadata. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- packages/shared/src/adapter-types.test.ts | 14 +++++++++++++- packages/shared/src/constants.ts | 2 ++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/shared/src/adapter-types.test.ts b/packages/shared/src/adapter-types.test.ts index 29fb6eec9dd..4be83236598 100644 --- a/packages/shared/src/adapter-types.test.ts +++ b/packages/shared/src/adapter-types.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { acceptInviteSchema, createAgentSchema, updateAgentSchema } from "./index.js"; +import { AGENT_ROLE_LABELS, acceptInviteSchema, createAgentSchema, updateAgentSchema } from "./index.js"; describe("dynamic adapter type validation schemas", () => { it("accepts external adapter types in create/update agent schemas", () => { @@ -35,4 +35,16 @@ describe("dynamic adapter type validation schemas", () => { }).adapterType, ).toBe("external_adapter"); }); + + it("accepts the security agent role and exposes its UI label", () => { + expect( + createAgentSchema.parse({ + name: "Security Engineer", + role: "security", + adapterType: "codex_local", + }).role, + ).toBe("security"); + + expect(AGENT_ROLE_LABELS.security).toBe("Security"); + }); }); diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index df99bb3353c..cbcd7878012 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -42,6 +42,7 @@ export const AGENT_ROLES = [ "cto", "cmo", "cfo", + "security", "engineer", "designer", "pm", @@ -57,6 +58,7 @@ export const AGENT_ROLE_LABELS: Record = { cto: "CTO", cmo: "CMO", cfo: "CFO", + security: "Security", engineer: "Engineer", designer: "Designer", pm: "PM", From 91333ec86f0509fa5688408d569ef551c60ae58b Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 26 Apr 2026 11:06:13 -0700 Subject: [PATCH 2/9] feat: add paperclip-dev skill with optional bundled skill support (#3854) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Agents working on the Paperclip codebase itself need guidance on dev workflows: server lifecycle, worktrees, builds, database ops, diagnostics > - There was no bundled skill covering these workflows — agents had to figure it out from scratch each time > - Additionally, not every skill should be force-installed on every agent — a dev-focused skill should be opt-in > - This PR adds a `paperclip-dev` skill with `required: false` frontmatter so it ships with Paperclip but isn't auto-installed > - The skill's PR section references canonical files (`.github/PULL_REQUEST_TEMPLATE.md`, `CONTRIBUTING.md`) instead of duplicating their content, with gated instructions that force agents to read those files before creating any PR > - The benefit is that developers (human or agent) can opt in to structured dev guidance without polluting the default agent skill set or creating drift between duplicated docs ## What Changed - Added `skills/paperclip-dev/SKILL.md` covering server management, worktree lifecycle, builds, database ops, diagnostics, agent operations, and common mistakes - The Pull Requests section uses gated, reference-based instructions — agents MUST read `.github/PULL_REQUEST_TEMPLATE.md` and `CONTRIBUTING.md` before running `gh pr create`, with a brief checklist of required section names (no content duplication) - Updated `packages/adapter-utils/src/server-utils.ts` to respect `required: false` frontmatter — optional skills are bundled but not auto-installed on agents - Added test in `server/src/__tests__/paperclip-skill-utils.test.ts` verifying that optional skills are excluded from the default install set ## Verification ```bash # Run tests pnpm test # Manual verification: create a fresh worktree without seeding npx paperclipai worktree:make test-optional-skill --no-seed cd ~/paperclip-test-optional-skill eval "$(npx paperclipai worktree env)" npx paperclipai run # Verify paperclip-dev appears in company skill library but is NOT auto-assigned # Call listPaperclipSkillEntries() — paperclip-dev should show required: false # Call resolvePaperclipDesiredSkillNames() — paperclip-dev should NOT be in the default set # Cleanup npx paperclipai worktree:cleanup test-optional-skill ``` ## Risks - Low risk. The `required` field defaults to `true` when absent, so all existing skills behave identically. Only the new `paperclip-dev` skill sets `required: false`. ## Model Used Claude Opus 4.6 (`claude-opus-4-6`) via Claude Code, with tool use and extended context. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- packages/adapter-utils/src/server-utils.ts | 33 ++- .../__tests__/paperclip-skill-utils.test.ts | 28 +++ skills/paperclip-dev/SKILL.md | 221 ++++++++++++++++++ 3 files changed, 275 insertions(+), 7 deletions(-) create mode 100644 skills/paperclip-dev/SKILL.md diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 297d57c74c6..661010d3067 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -1037,6 +1037,20 @@ export async function resolvePaperclipSkillsDir( return null; } +async function readSkillRequired(skillDir: string): Promise { + try { + const content = await fs.readFile(path.join(skillDir, "SKILL.md"), "utf8"); + const normalized = content.replace(/\r\n/g, "\n"); + if (!normalized.startsWith("---\n")) return true; + const closing = normalized.indexOf("\n---\n", 4); + if (closing < 0) return true; + const frontmatter = normalized.slice(4, closing); + return !/^\s*required\s*:\s*false\s*$/m.test(frontmatter); + } catch { + return true; + } +} + export async function listPaperclipSkillEntries( moduleDir: string, additionalCandidates: string[] = [], @@ -1046,15 +1060,20 @@ export async function listPaperclipSkillEntries( try { const entries = await fs.readdir(root, { withFileTypes: true }); - return entries - .filter((entry) => entry.isDirectory()) - .map((entry) => ({ + const dirs = entries.filter((entry) => entry.isDirectory()); + return Promise.all(dirs.map(async (entry) => { + const skillDir = path.join(root, entry.name); + const required = await readSkillRequired(skillDir); + return { key: `paperclipai/paperclip/${entry.name}`, runtimeName: entry.name, - source: path.join(root, entry.name), - required: true, - requiredReason: "Bundled Paperclip skills are always available for local adapters.", - })); + source: skillDir, + required, + requiredReason: required + ? "Bundled Paperclip skills are always available for local adapters." + : null, + }; + })); } catch { return []; } diff --git a/server/src/__tests__/paperclip-skill-utils.test.ts b/server/src/__tests__/paperclip-skill-utils.test.ts index 787b3f32f11..b2fa08baa2d 100644 --- a/server/src/__tests__/paperclip-skill-utils.test.ts +++ b/server/src/__tests__/paperclip-skill-utils.test.ts @@ -43,6 +43,34 @@ describe("paperclip skill utils", () => { expect(entries[1]?.source).toBe(path.join(root, "skills", "paperclip-create-agent")); }); + it("marks skills with required: false in SKILL.md frontmatter as optional", async () => { + const root = await makeTempDir("paperclip-skill-optional-"); + cleanupDirs.add(root); + + const moduleDir = path.join(root, "a", "b", "c", "d", "e"); + await fs.mkdir(moduleDir, { recursive: true }); + + // Required skill (no frontmatter flag) + const requiredDir = path.join(root, "skills", "paperclip"); + await fs.mkdir(requiredDir, { recursive: true }); + await fs.writeFile(path.join(requiredDir, "SKILL.md"), "---\nname: paperclip\n---\n\n# Paperclip\n"); + + // Optional skill (required: false) + const optionalDir = path.join(root, "skills", "paperclip-dev"); + await fs.mkdir(optionalDir, { recursive: true }); + await fs.writeFile(path.join(optionalDir, "SKILL.md"), "---\nname: paperclip-dev\nrequired: false\n---\n\n# Dev\n"); + + const entries = await listPaperclipSkillEntries(moduleDir); + entries.sort((a, b) => a.runtimeName.localeCompare(b.runtimeName)); + + expect(entries).toHaveLength(2); + expect(entries[0]?.runtimeName).toBe("paperclip"); + expect(entries[0]?.required).toBe(true); + expect(entries[1]?.runtimeName).toBe("paperclip-dev"); + expect(entries[1]?.required).toBe(false); + expect(entries[1]?.requiredReason).toBeNull(); + }); + it("removes stale maintainer-only symlinks from a shared skills home", async () => { const root = await makeTempDir("paperclip-skill-cleanup-"); cleanupDirs.add(root); diff --git a/skills/paperclip-dev/SKILL.md b/skills/paperclip-dev/SKILL.md new file mode 100644 index 00000000000..cb14c3bf4b3 --- /dev/null +++ b/skills/paperclip-dev/SKILL.md @@ -0,0 +1,221 @@ +--- +name: paperclip-dev +required: false +description: > + Develop and operate a local Paperclip instance — start and stop servers, + pull updates from master, run builds and tests, manage worktrees, back up + databases, and diagnose problems. Use whenever you need to work on the + Paperclip codebase itself or keep a running instance healthy. +--- + +# Paperclip Dev + +This skill covers the day-to-day workflows for developing and operating a local Paperclip instance. It assumes you are working inside the Paperclip repo checkout with `origin` pointing to `git@github.com:paperclipai/paperclip.git`. + +> **MANDATORY:** Before running any CLI command, building, testing, or managing worktrees, you MUST read `doc/DEVELOPING.md` in the Paperclip repo. It is the canonical reference for all `paperclipai` CLI commands, their options, build/test workflows, database operations, worktree management, and diagnostics. Do NOT guess at flags or options — read the doc first. + +## Quick Command Reference + +These are the most common commands. For full option tables and details, see `doc/DEVELOPING.md`. + +| Task | Command | +|------|---------| +| Start server (first time or normal) | `npx paperclipai run` | +| Dev mode with hot reload | `pnpm dev` | +| Stop dev server | `pnpm dev:stop` | +| Build | `pnpm build` | +| Type-check | `pnpm typecheck` | +| Run tests | `pnpm test` | +| Run migrations | `pnpm db:migrate` | +| Regenerate Drizzle client | `pnpm db:generate` | +| Back up database | `npx paperclipai db:backup` | +| Health check | `npx paperclipai doctor --repair` | +| Print env vars | `npx paperclipai env` | +| Trigger agent heartbeat | `npx paperclipai heartbeat run --agent-id ` | +| Install agent skills locally | `npx paperclipai agent local-cli --company-id ` | + +## Pulling from Master + +```bash +git fetch origin && git pull origin master +pnpm install && pnpm build +``` + +If schema changes landed, also run `pnpm db:generate && pnpm db:migrate`. + +## Worktrees + +Paperclip worktrees combine git worktrees with isolated Paperclip instances — each gets its own database, server port, and environment seeded from the primary instance. + +> **MANDATORY:** Before creating or managing worktrees, you MUST read the "Worktree-local Instances" and "Worktree CLI Reference" sections in `doc/DEVELOPING.md`. That is the canonical reference for all worktree commands, their options, seed modes, and environment variables. + +### When to Use Worktrees + +- Starting a feature branch that needs its own Paperclip environment +- Running parallel agent work without cross-contaminating the primary instance +- Testing Paperclip changes in isolation before merging + +### Command Overview + +The CLI has two tiers (see `doc/DEVELOPING.md` for full option tables): + +| Command | Purpose | +|---------|---------| +| `worktree:make ` | Create worktree + isolated instance in one step | +| `worktree:list` | List worktrees and their Paperclip status | +| `worktree:merge-history` | Preview/import issue history between worktrees | +| `worktree:cleanup ` | Remove worktree, branch, and instance data | +| `worktree init` | Bootstrap instance inside existing worktree | +| `worktree env` | Print shell exports for worktree instance | +| `worktree reseed` | Refresh worktree DB from another instance | +| `worktree repair` | Fix broken/missing worktree instance metadata | + +### Typical Workflow + +```bash +# 1. Create a worktree for a feature +npx paperclipai worktree:make my-feature --start-point origin/main + +# 2. Move into the worktree (path printed by worktree:make) and source the environment +cd +eval "$(npx paperclipai worktree env)" + +# 3. Start the isolated Paperclip server +npx paperclipai run + +# 4. Do your work + +# 5. When done, merge history back if needed +npx paperclipai worktree:merge-history --from paperclip-my-feature --to current --apply + +# 6. Clean up +npx paperclipai worktree:cleanup my-feature +``` + +## Pull Requests + +> **MANDATORY PRE-FLIGHT:** Before creating ANY pull request, you MUST read the canonical source files listed below. Do NOT run `gh pr create` until you have read these files and verified your PR body matches every required section. + +### Step 1 — Read the canonical files + +You MUST read all three of these files before creating a PR: + +1. **`.github/PULL_REQUEST_TEMPLATE.md`** — the required PR body structure +2. **`CONTRIBUTING.md`** — contribution conventions, PR requirements, and thinking-path examples +3. **`.github/workflows/pr.yml`** — CI checks that gate merge + +### Step 2 — Validate your PR body against this checklist + +After reading the template, verify your `--body` includes every one of these sections (names must match exactly): + +- [ ] `## Thinking Path` — blockquote style, 5-8 reasoning steps +- [ ] `## What Changed` — bullet list of concrete changes +- [ ] `## Verification` — how a reviewer confirms this works +- [ ] `## Risks` — what could go wrong +- [ ] `## Model Used` — provider, model ID, version, capabilities +- [ ] `## Checklist` — copied from the template, items checked off + +If any section is missing or empty, do NOT submit the PR. Go back and fill it in. + +### Step 3 — Create the PR + +Only after completing Steps 1 and 2, run `gh pr create`. Use the template contents as the structure for `--body` — do not write a freeform summary. + +## Hard Rules — Do NOT Bypass + +These rules exist because agents have caused real damage by improvising around CLI failures. Follow them exactly. + +1. **CLI is the only interface to worktrees and databases.** All worktree and database operations MUST go through `npx paperclipai` / `pnpm paperclipai` commands. You MUST NOT: + - Run `pg_dump`, `pg_restore`, `psql`, `createdb`, `dropdb`, or any raw postgres commands + - Manually set `DATABASE_URL` to point a worktree server at another instance's database + - Run `rm -rf` on any `.paperclip/`, `.paperclip-worktrees/`, or `db/` directory + - Directly manipulate embedded postgres data directories + - Kill postgres processes by PID + +2. **If a CLI command fails, stop and report.** Do NOT attempt workarounds. If `worktree:make`, `worktree reseed`, `worktree init`, `worktree:cleanup`, or any other `paperclipai` command fails: + - Report the exact error message in your task comment + - Set the task to `blocked` + - Suggest running `npx paperclipai doctor --repair` or recreating the worktree from scratch + - Do NOT try to manually replicate what the CLI does + +3. **Never share databases between instances.** Each worktree instance gets its own isolated database. Never override `DATABASE_URL` to point one instance at another's database. This destroys isolation and can corrupt production data. + +4. **Starting a dev server in a worktree requires setup first.** The correct sequence is: + ```bash + # If the worktree already exists but has no running instance: + cd + eval "$(npx paperclipai worktree env)" + pnpm install && pnpm build + npx paperclipai run # or pnpm dev + + # If the worktree needs a fresh database: + npx paperclipai worktree reseed --seed-mode full + + # If the worktree is broken beyond repair: + npx paperclipai worktree:cleanup + npx paperclipai worktree:make --seed-mode full + ``` + If any step fails, follow rule 2 — stop and report. + +5. **Seeding is a CLI operation.** When asked to seed a worktree database from the main instance, use `worktree reseed` or recreate with `worktree:make --seed-mode full`. Read `doc/DEVELOPING.md` for the full option tables. Never attempt manual database copying. + +## Persistent Dev Servers (for Manual Testing) + +When an agent needs to start a dev server that outlives the current heartbeat — for example, so a human or QA agent can manually test against it — the server process **must** be launched in a detached session. A process started directly from a heartbeat shell is killed when the heartbeat exits. + +### Use `tmux` for persistent servers + +```bash +# 1. cd into the worktree (or main repo) and source the environment +cd +eval "$(npx paperclipai worktree env)" # skip if using the primary instance + +# 2. Start the dev server in a named, detached tmux session +tmux new-session -d -s 'pnpm dev' + +# Example with a descriptive name: +tmux new-session -d -s auth-fix-3102 'pnpm dev' +``` + +### Managing the session + +| Task | Command | +|------|---------| +| Check if the session is alive | `tmux has-session -t 2>/dev/null && echo running` | +| View server output | `tmux capture-pane -t -p` | +| Kill the session | `tmux kill-session -t ` | +| List all tmux sessions | `tmux list-sessions` | + +### Verifying the server is reachable + +After launching, confirm the port is listening before reporting success: + +```bash +# Wait briefly for startup, then verify +sleep 3 +curl -sf http://127.0.0.1:/api/health && echo "Server is up" +lsof -nP -iTCP: -sTCP:LISTEN +``` + +### Key rules + +1. **Always use `tmux` (or equivalent)** when a dev server needs to stay running after the heartbeat ends. A server started directly from the agent shell will die when the heartbeat exits, even if it appeared healthy moments before. +2. **Name the session descriptively** — include the worktree name and port (e.g., `auth-fix-3102`). +3. **Verify the server is listening** before reporting the URL to anyone. +4. **Do not use `nohup` or `&` alone** — these are unreliable for agent shells that may have their entire process group killed. +5. **Clean up when done** — kill the tmux session when the testing is complete. + +## Common Mistakes + +| Mistake | Fix | +|---------|-----| +| Server won't start | Run `npx paperclipai doctor --repair` to diagnose and auto-fix | +| Forgetting to source worktree env | Run `eval "$(npx paperclipai worktree env)"` after cd-ing into the worktree | +| Stale dependencies after pull | Run `pnpm install && pnpm build` after pulling | +| Schema out of date after pull | Run `pnpm db:generate && pnpm db:migrate` | +| Reseeding while target DB is running | Stop the target server first, or use `--allow-live-target` | +| Cleaning up with unmerged commits | Merge or push first, or use `--force` if intentionally discarding | +| Running agents against wrong instance | Verify `PAPERCLIP_API_URL` points to the correct port | +| CLI command fails | Do NOT work around it — report the error and block (see Hard Rules above) | +| Agent tries manual postgres operations | NEVER do this — all DB ops go through the CLI (see Hard Rules above) | +| Dev server dies between heartbeats | Launch in a detached `tmux` session — see "Persistent Dev Servers" above | From d1484551eefb98321ce9153488ae644c59d256a7 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 26 Apr 2026 12:14:49 -0700 Subject: [PATCH 3/9] Add open-source hygiene note to paperclip-dev skill (#4541) ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies. > - The `paperclip-dev` skill is part of the contributor and agent workflow layer that tells developers how to work in this repository safely. > - That skill already references the public upstream `origin`, but it did not explicitly say that pushes there must be treated as publishable open-source output. > - Without that reminder, contributors are more likely to leak secrets, PII, private logs, machine-local config, or noisy throwaway git history into the public repo. > - This pull request adds a prominent `OPEN SOURCE HYGIENE` callout near the top of the skill, before the git workflow guidance. > - The benefit is clearer safety guidance for contributors and less accidental disclosure or branch/commit noise on the upstream project. ## What Changed - Added an `OPEN SOURCE HYGIENE` callout near the top of `skills/paperclip-dev/SKILL.md`. - Explicitly warned that anything pushed to `origin` must be publishable. - Called out avoiding secrets, API keys, PII, private logs, machine-local config, and noisy throwaway branches or checkpoint commits. ## Verification - N/a ## Risks - Low risk. This is a docs-only change in a skill file; the main risk is wording tone or placement, not runtime behavior. ## Model Used - OpenAI Codex via the `codex_local` Paperclip adapter, GPT-5-based coding agent runtime. Exact backend serving model ID is not exposed in this heartbeat environment. Tool use, shell execution, and patch application were enabled. ## 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 - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- skills/paperclip-dev/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/skills/paperclip-dev/SKILL.md b/skills/paperclip-dev/SKILL.md index cb14c3bf4b3..43f1ebbcf12 100644 --- a/skills/paperclip-dev/SKILL.md +++ b/skills/paperclip-dev/SKILL.md @@ -12,6 +12,8 @@ description: > This skill covers the day-to-day workflows for developing and operating a local Paperclip instance. It assumes you are working inside the Paperclip repo checkout with `origin` pointing to `git@github.com:paperclipai/paperclip.git`. +> **OPEN SOURCE HYGIENE:** This repository is public-facing. Treat anything you push to `origin` as publishable. Never commit or push secrets, API keys, tokens, private logs, PII, customer data, or machine-local configuration that should stay private. Keep git history tidy as well: avoid pushing throwaway branches, noisy checkpoint commits, or speculative work that does not need to be shared upstream. + > **MANDATORY:** Before running any CLI command, building, testing, or managing worktrees, you MUST read `doc/DEVELOPING.md` in the Paperclip repo. It is the canonical reference for all `paperclipai` CLI commands, their options, build/test workflows, database operations, worktree management, and diagnostics. Do NOT guess at flags or options — read the doc first. ## Quick Command Reference From d47ffa87f07d380414976820a739f3e1daae174a Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 26 Apr 2026 13:57:35 -0700 Subject: [PATCH 4/9] Fix CEO AGENT_HOME paths and centralize workspace env propagation (#4551) ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies. > - The local adapter layer is responsible for turning Paperclip runtime context into the environment seen by the child agent process. > - The CEO onboarding bundle tells the agent where to read and write its persistent memory and fact files. > - That bundle was using `./memory/...` and `./life/...`, which only works when the process cwd happens to equal the agent home directory. > - At the same time, six local adapters each duplicated the same workspace-env propagation logic, including `AGENT_HOME`, which makes this contract easy to drift. > - This pull request fixes the CEO instructions to use `$AGENT_HOME/...` and centralizes workspace-env propagation in one shared helper with shared tests. > - The benefit is a real bug fix for agent memory paths plus a single tested contract that makes future built-in adapter work less likely to forget `AGENT_HOME`. ## What Changed - Updated `server/src/onboarding-assets/ceo/HEARTBEAT.md` to use `$AGENT_HOME/memory/...` and `$AGENT_HOME/life/...` instead of cwd-relative `./memory/...` and `./life/...`. - Added `applyPaperclipWorkspaceEnv(...)` in `packages/adapter-utils/src/server-utils.ts` to centralize `PAPERCLIP_WORKSPACE_*` and `AGENT_HOME` propagation. - Added shared helper coverage in `packages/adapter-utils/src/server-utils.test.ts` for both populated and skip-empty cases. - Switched the built-in local adapters (`claude_local`, `codex_local`, `cursor_local`, `gemini_local`, `opencode_local`, `pi_local`) over to the shared helper instead of inline env assignment blocks. ## Verification - `pnpm install` - `pnpm exec vitest run packages/adapter-utils/src/server-utils.test.ts packages/adapters/claude-local/src/server/execute.remote.test.ts packages/adapters/codex-local/src/server/execute.remote.test.ts packages/adapters/cursor-local/src/server/execute.remote.test.ts packages/adapters/gemini-local/src/server/execute.remote.test.ts packages/adapters/opencode-local/src/server/execute.remote.test.ts packages/adapters/pi-local/src/server/execute.remote.test.ts` - Result: 7 test files passed, 31 tests passed, 0 failures. ## Risks - Low risk. - The only behavioral surface is the shared env propagation refactor across six adapters; if the helper diverged from prior semantics, an adapter could miss a workspace env var. - The shared helper test plus the affected adapter execute tests reduce that risk, and the helper preserves the prior "set only non-empty strings" behavior. ## Model Used - OpenAI Codex via Paperclip `codex_local` agent runtime; tool-assisted coding workflow with shell execution, file patching, git operations, and API interaction. The exact backend model identifier and context window are not surfaced by this local runtime. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- .../adapter-utils/src/server-utils.test.ts | 45 +++++++++++++++++++ packages/adapter-utils/src/server-utils.ts | 35 +++++++++++++++ .../claude-local/src/server/execute.ts | 39 +++++----------- .../codex-local/src/server/execute.ts | 39 +++++----------- .../cursor-local/src/server/execute.ts | 27 ++++------- .../gemini-local/src/server/execute.ts | 15 ++++--- .../opencode-local/src/server/execute.ts | 15 ++++--- .../adapters/pi-local/src/server/execute.ts | 15 ++++--- server/src/onboarding-assets/ceo/HEARTBEAT.md | 6 +-- 9 files changed, 143 insertions(+), 93 deletions(-) diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index 068474456e0..f5e8f716dec 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -1,6 +1,7 @@ import { randomUUID } from "node:crypto"; import { describe, expect, it } from "vitest"; import { + applyPaperclipWorkspaceEnv, appendWithByteCap, DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE, renderPaperclipWakePrompt, @@ -425,6 +426,50 @@ describe("renderPaperclipWakePrompt", () => { }); }); +describe("applyPaperclipWorkspaceEnv", () => { + it("adds shared workspace env vars including AGENT_HOME", () => { + const env = applyPaperclipWorkspaceEnv( + {}, + { + workspaceCwd: "/tmp/workspace", + workspaceSource: "project_primary", + workspaceStrategy: "git_worktree", + workspaceId: "workspace-1", + workspaceRepoUrl: "https://github.com/paperclipai/paperclip.git", + workspaceRepoRef: "main", + workspaceBranch: "feature/test", + workspaceWorktreePath: "/tmp/worktree", + agentHome: "/tmp/agent-home", + }, + ); + + expect(env).toEqual({ + PAPERCLIP_WORKSPACE_CWD: "/tmp/workspace", + PAPERCLIP_WORKSPACE_SOURCE: "project_primary", + PAPERCLIP_WORKSPACE_STRATEGY: "git_worktree", + PAPERCLIP_WORKSPACE_ID: "workspace-1", + PAPERCLIP_WORKSPACE_REPO_URL: "https://github.com/paperclipai/paperclip.git", + PAPERCLIP_WORKSPACE_REPO_REF: "main", + PAPERCLIP_WORKSPACE_BRANCH: "feature/test", + PAPERCLIP_WORKSPACE_WORKTREE_PATH: "/tmp/worktree", + AGENT_HOME: "/tmp/agent-home", + }); + }); + + it("skips empty workspace env values", () => { + const env = applyPaperclipWorkspaceEnv( + {}, + { + workspaceCwd: "", + workspaceSource: null, + agentHome: "", + }, + ); + + expect(env).toEqual({}); + }); +}); + describe("appendWithByteCap", () => { it("keeps valid UTF-8 when trimming through multibyte text", () => { const output = appendWithByteCap("prefix ", "hello — world", 7); diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 661010d3067..425a1f3f0fa 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -835,6 +835,41 @@ export function buildPaperclipEnv(agent: { id: string; companyId: string }): Rec return vars; } +export function applyPaperclipWorkspaceEnv( + env: Record, + input: { + workspaceCwd?: string | null; + workspaceSource?: string | null; + workspaceStrategy?: string | null; + workspaceId?: string | null; + workspaceRepoUrl?: string | null; + workspaceRepoRef?: string | null; + workspaceBranch?: string | null; + workspaceWorktreePath?: string | null; + agentHome?: string | null; + }, +): Record { + const mappings = [ + ["PAPERCLIP_WORKSPACE_CWD", input.workspaceCwd], + ["PAPERCLIP_WORKSPACE_SOURCE", input.workspaceSource], + ["PAPERCLIP_WORKSPACE_STRATEGY", input.workspaceStrategy], + ["PAPERCLIP_WORKSPACE_ID", input.workspaceId], + ["PAPERCLIP_WORKSPACE_REPO_URL", input.workspaceRepoUrl], + ["PAPERCLIP_WORKSPACE_REPO_REF", input.workspaceRepoRef], + ["PAPERCLIP_WORKSPACE_BRANCH", input.workspaceBranch], + ["PAPERCLIP_WORKSPACE_WORKTREE_PATH", input.workspaceWorktreePath], + ["AGENT_HOME", input.agentHome], + ] as const; + + for (const [key, value] of mappings) { + if (typeof value === "string" && value.length > 0) { + env[key] = value; + } + } + + return env; +} + export function sanitizeInheritedPaperclipEnv(baseEnv: NodeJS.ProcessEnv): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = { ...baseEnv }; for (const key of Object.keys(env)) { diff --git a/packages/adapters/claude-local/src/server/execute.ts b/packages/adapters/claude-local/src/server/execute.ts index a79e7224664..365295b6682 100644 --- a/packages/adapters/claude-local/src/server/execute.ts +++ b/packages/adapters/claude-local/src/server/execute.ts @@ -24,6 +24,7 @@ import { asStringArray, parseObject, parseJson, + applyPaperclipWorkspaceEnv, buildPaperclipEnv, readPaperclipRuntimeSkillEntries, joinPromptSections, @@ -193,33 +194,17 @@ async function buildClaudeRuntimeConfig(input: ClaudeExecutionInput): Promise 0) { env.PAPERCLIP_WORKSPACES_JSON = JSON.stringify(workspaceHints); } diff --git a/packages/adapters/codex-local/src/server/execute.ts b/packages/adapters/codex-local/src/server/execute.ts index 2a6b253a164..89f67079127 100644 --- a/packages/adapters/codex-local/src/server/execute.ts +++ b/packages/adapters/codex-local/src/server/execute.ts @@ -19,6 +19,7 @@ import { asString, asNumber, parseObject, + applyPaperclipWorkspaceEnv, buildPaperclipEnv, buildInvocationEnvForLogs, ensureAbsoluteDirectory, @@ -421,33 +422,17 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0) { env.PAPERCLIP_WORKSPACES_JSON = JSON.stringify(workspaceHints); } diff --git a/packages/adapters/cursor-local/src/server/execute.ts b/packages/adapters/cursor-local/src/server/execute.ts index 6d7f79662d4..5056b222fe1 100644 --- a/packages/adapters/cursor-local/src/server/execute.ts +++ b/packages/adapters/cursor-local/src/server/execute.ts @@ -24,6 +24,7 @@ import { asNumber, asStringArray, parseObject, + applyPaperclipWorkspaceEnv, buildPaperclipEnv, buildInvocationEnvForLogs, ensureAbsoluteDirectory, @@ -277,24 +278,14 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0) { env.PAPERCLIP_WORKSPACES_JSON = JSON.stringify(workspaceHints); } diff --git a/packages/adapters/gemini-local/src/server/execute.ts b/packages/adapters/gemini-local/src/server/execute.ts index 4c79b75375a..756e88fde24 100644 --- a/packages/adapters/gemini-local/src/server/execute.ts +++ b/packages/adapters/gemini-local/src/server/execute.ts @@ -25,6 +25,7 @@ import { asNumber, asString, asStringArray, + applyPaperclipWorkspaceEnv, buildPaperclipEnv, buildInvocationEnvForLogs, ensureAbsoluteDirectory, @@ -240,12 +241,14 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0) env.PAPERCLIP_LINKED_ISSUE_IDS = linkedIssueIds.join(","); if (wakePayloadJson) env.PAPERCLIP_WAKE_PAYLOAD_JSON = wakePayloadJson; - if (effectiveWorkspaceCwd) env.PAPERCLIP_WORKSPACE_CWD = effectiveWorkspaceCwd; - if (workspaceSource) env.PAPERCLIP_WORKSPACE_SOURCE = workspaceSource; - if (workspaceId) env.PAPERCLIP_WORKSPACE_ID = workspaceId; - if (workspaceRepoUrl) env.PAPERCLIP_WORKSPACE_REPO_URL = workspaceRepoUrl; - if (workspaceRepoRef) env.PAPERCLIP_WORKSPACE_REPO_REF = workspaceRepoRef; - if (agentHome) env.AGENT_HOME = agentHome; + applyPaperclipWorkspaceEnv(env, { + workspaceCwd: effectiveWorkspaceCwd, + workspaceSource, + workspaceId, + workspaceRepoUrl, + workspaceRepoRef, + agentHome, + }); if (workspaceHints.length > 0) env.PAPERCLIP_WORKSPACES_JSON = JSON.stringify(workspaceHints); const targetPaperclipApiUrl = adapterExecutionTargetPaperclipApiUrl(executionTarget); if (targetPaperclipApiUrl) env.PAPERCLIP_API_URL = targetPaperclipApiUrl; diff --git a/packages/adapters/opencode-local/src/server/execute.ts b/packages/adapters/opencode-local/src/server/execute.ts index 0aa86b171b4..fbb8d433cc1 100644 --- a/packages/adapters/opencode-local/src/server/execute.ts +++ b/packages/adapters/opencode-local/src/server/execute.ts @@ -24,6 +24,7 @@ import { asNumber, asStringArray, parseObject, + applyPaperclipWorkspaceEnv, buildPaperclipEnv, joinPromptSections, buildInvocationEnvForLogs, @@ -199,12 +200,14 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0) env.PAPERCLIP_LINKED_ISSUE_IDS = linkedIssueIds.join(","); if (wakePayloadJson) env.PAPERCLIP_WAKE_PAYLOAD_JSON = wakePayloadJson; - if (effectiveWorkspaceCwd) env.PAPERCLIP_WORKSPACE_CWD = effectiveWorkspaceCwd; - if (workspaceSource) env.PAPERCLIP_WORKSPACE_SOURCE = workspaceSource; - if (workspaceId) env.PAPERCLIP_WORKSPACE_ID = workspaceId; - if (workspaceRepoUrl) env.PAPERCLIP_WORKSPACE_REPO_URL = workspaceRepoUrl; - if (workspaceRepoRef) env.PAPERCLIP_WORKSPACE_REPO_REF = workspaceRepoRef; - if (agentHome) env.AGENT_HOME = agentHome; + applyPaperclipWorkspaceEnv(env, { + workspaceCwd: effectiveWorkspaceCwd, + workspaceSource, + workspaceId, + workspaceRepoUrl, + workspaceRepoRef, + agentHome, + }); if (workspaceHints.length > 0) env.PAPERCLIP_WORKSPACES_JSON = JSON.stringify(workspaceHints); const targetPaperclipApiUrl = adapterExecutionTargetPaperclipApiUrl(executionTarget); if (targetPaperclipApiUrl) env.PAPERCLIP_API_URL = targetPaperclipApiUrl; diff --git a/packages/adapters/pi-local/src/server/execute.ts b/packages/adapters/pi-local/src/server/execute.ts index 819be8e1b31..00b011a6a97 100644 --- a/packages/adapters/pi-local/src/server/execute.ts +++ b/packages/adapters/pi-local/src/server/execute.ts @@ -23,6 +23,7 @@ import { asNumber, asStringArray, parseObject, + applyPaperclipWorkspaceEnv, buildPaperclipEnv, joinPromptSections, buildInvocationEnvForLogs, @@ -228,12 +229,14 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0) env.PAPERCLIP_LINKED_ISSUE_IDS = linkedIssueIds.join(","); if (wakePayloadJson) env.PAPERCLIP_WAKE_PAYLOAD_JSON = wakePayloadJson; - if (workspaceCwd) env.PAPERCLIP_WORKSPACE_CWD = workspaceCwd; - if (workspaceSource) env.PAPERCLIP_WORKSPACE_SOURCE = workspaceSource; - if (workspaceId) env.PAPERCLIP_WORKSPACE_ID = workspaceId; - if (workspaceRepoUrl) env.PAPERCLIP_WORKSPACE_REPO_URL = workspaceRepoUrl; - if (workspaceRepoRef) env.PAPERCLIP_WORKSPACE_REPO_REF = workspaceRepoRef; - if (agentHome) env.AGENT_HOME = agentHome; + applyPaperclipWorkspaceEnv(env, { + workspaceCwd, + workspaceSource, + workspaceId, + workspaceRepoUrl, + workspaceRepoRef, + agentHome, + }); if (workspaceHints.length > 0) env.PAPERCLIP_WORKSPACES_JSON = JSON.stringify(workspaceHints); const targetPaperclipApiUrl = adapterExecutionTargetPaperclipApiUrl(executionTarget); if (targetPaperclipApiUrl) env.PAPERCLIP_API_URL = targetPaperclipApiUrl; diff --git a/server/src/onboarding-assets/ceo/HEARTBEAT.md b/server/src/onboarding-assets/ceo/HEARTBEAT.md index 573dfea57d4..722e4414855 100644 --- a/server/src/onboarding-assets/ceo/HEARTBEAT.md +++ b/server/src/onboarding-assets/ceo/HEARTBEAT.md @@ -9,7 +9,7 @@ Run this checklist on every heartbeat. This covers both your local planning/memo ## 2. Local Planning Check -1. Read today's plan from `./memory/YYYY-MM-DD.md` under "## Today's Plan". +1. Read today's plan from `$AGENT_HOME/memory/YYYY-MM-DD.md` under "## Today's Plan". 2. Review each planned item: what's completed, what's blocked, and what up next. 3. For any blockers, resolve them yourself or escalate to the board. 4. If you're ahead, start on the next highest priority. @@ -57,8 +57,8 @@ Status quick guide: ## 7. Fact Extraction 1. Check for new conversations since last extraction. -2. Extract durable facts to the relevant entity in `./life/` (PARA). -3. Update `./memory/YYYY-MM-DD.md` with timeline entries. +2. Extract durable facts to the relevant entity in `$AGENT_HOME/life/` (PARA). +3. Update `$AGENT_HOME/memory/YYYY-MM-DD.md` with timeline entries. 4. Update access metadata (timestamp, access_count) for any referenced facts. ## 8. Exit From 08af83043076a95bc335a928826fca8117704594 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 26 Apr 2026 14:29:22 -0700 Subject: [PATCH 5/9] Tighten publicBaseUrl port rewriting (#4553) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is a control plane for autonomous agent companies, so its local and authenticated deployment behavior has to stay predictable under port rebinding and worktree isolation. > - This change sits in the server/worktree configuration path that derives runtime URLs and auth origins from `auth.publicBaseUrl`. > - The original hostname-port rewrite change fixed one real gap for private/tailnet host:port worktree setups, but it widened the rewrite rule too far. > - Rewriting every explicit `auth.publicBaseUrl` can corrupt public or reverse-proxy URLs by turning a stable origin like `https://paperclip.example` into a local listen-port URL. > - Paperclip's auth and trusted-origin handling depend on that URL staying semantically correct, so this had to be narrowed before merge. > - This pull request tightens the rewrite rule to explicit-port URLs only and adds regression coverage across the CLI helper, worktree config persistence, and server startup path. > - The benefit is that private host:port worktree flows still work, while public/default-port URLs remain stable and safe. ## What Changed - Tightened `rewriteLocalUrlPort` in `cli/src/commands/worktree-lib.ts`, `server/src/worktree-config.ts`, and `server/src/index.ts` so it only rewrites URLs that already include an explicit port. - Removed the old loopback-only hostname gate from the CLI/worktree helpers and replaced it with the more precise `parsed.port` guard. - Updated CLI helper coverage to assert that explicit-port non-loopback URLs still rewrite while no-port public URLs stay unchanged. - Expanded `server/src/__tests__/worktree-config.test.ts` to cover explicit-port rewrite and no-port stability for both persisted worktree config and in-memory runtime port selection. - Added startup-path coverage in `server/src/__tests__/server-startup-feedback-export.test.ts` for `detect-port` rebinding with both explicit-port and no-port `auth.publicBaseUrl` values. ## Verification - `pnpm --filter @paperclipai/plugin-sdk build` - `npx vitest run server/src/__tests__/server-startup-feedback-export.test.ts` - `npx vitest run cli/src/__tests__/worktree.test.ts server/src/__tests__/worktree-config.test.ts` - All of the above were run locally in this issue worktree and passed. ## Risks - Low risk. The behavior change is deliberately narrower than the reviewed broad-host rewrite and is guarded by regression coverage for both the explicit-port and no-port cases. - The main remaining risk is behavioral only if another code path starts depending on port rewriting for URLs that never declared a port, which would be a separate bug. > 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 local agent using `gpt-5.4` with high reasoning effort, tool use, shell execution, and file editing. - Anthropic Claude local agent using `claude-opus-4-6` for follow-up code review approval on the implementation issue. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- cli/src/__tests__/worktree.test.ts | 3 +- cli/src/commands/worktree-lib.ts | 8 +- .../server-startup-feedback-export.test.ts | 143 +++++++++++------- server/src/__tests__/worktree-config.test.ts | 126 +++++++++++++-- server/src/index.ts | 3 +- server/src/worktree-config.ts | 8 +- 6 files changed, 211 insertions(+), 80 deletions(-) diff --git a/cli/src/__tests__/worktree.test.ts b/cli/src/__tests__/worktree.test.ts index 28e103761ec..afb83f735b6 100644 --- a/cli/src/__tests__/worktree.test.ts +++ b/cli/src/__tests__/worktree.test.ts @@ -190,8 +190,9 @@ describe("worktree helpers", () => { ).toEqual(["worktree", "add", "-b", "my-worktree", "/tmp/my-worktree", "origin/main"]); }); - it("rewrites loopback auth URLs to the new port only", () => { + it("rewrites auth URLs only when they already include a port", () => { expect(rewriteLocalUrlPort("http://127.0.0.1:3100", 3110)).toBe("http://127.0.0.1:3110/"); + expect(rewriteLocalUrlPort("http://my-host.ts.net:3100", 3110)).toBe("http://my-host.ts.net:3110/"); expect(rewriteLocalUrlPort("https://paperclip.example", 3110)).toBe("https://paperclip.example"); }); diff --git a/cli/src/commands/worktree-lib.ts b/cli/src/commands/worktree-lib.ts index 5d4a0721192..2be4528e507 100644 --- a/cli/src/commands/worktree-lib.ts +++ b/cli/src/commands/worktree-lib.ts @@ -75,11 +75,6 @@ function nonEmpty(value: string | null | undefined): string | null { return typeof value === "string" && value.trim().length > 0 ? value.trim() : null; } -function isLoopbackHost(hostname: string): boolean { - const value = hostname.trim().toLowerCase(); - return value === "127.0.0.1" || value === "localhost" || value === "::1"; -} - export function sanitizeWorktreeInstanceId(rawValue: string): string { const trimmed = rawValue.trim().toLowerCase(); const normalized = trimmed @@ -168,7 +163,8 @@ export function rewriteLocalUrlPort(rawUrl: string | undefined, port: number): s if (!rawUrl) return undefined; try { const parsed = new URL(rawUrl); - if (!isLoopbackHost(parsed.hostname)) return rawUrl; + // The URL API normalizes default ports like :80/:443 to "", so treat them as stable URLs. + if (!parsed.port) return rawUrl; parsed.port = String(port); return parsed.toString(); } catch { diff --git a/server/src/__tests__/server-startup-feedback-export.test.ts b/server/src/__tests__/server-startup-feedback-export.test.ts index 082c99a2daa..a08e0f02414 100644 --- a/server/src/__tests__/server-startup-feedback-export.test.ts +++ b/server/src/__tests__/server-startup-feedback-export.test.ts @@ -4,6 +4,7 @@ const { createAppMock, createDbMock, detectPortMock, + loadConfigMock, feedbackExportServiceMock, feedbackServiceFactoryMock, fakeServer, @@ -11,59 +12,7 @@ const { const createAppMock = vi.fn(async () => ((_: unknown, __: unknown) => {}) as never); const createDbMock = vi.fn(() => ({}) as never); const detectPortMock = vi.fn(async (port: number) => port); - const feedbackExportServiceMock = { - flushPendingFeedbackTraces: vi.fn(async () => ({ attempted: 0, sent: 0, failed: 0 })), - }; - const feedbackServiceFactoryMock = vi.fn(() => feedbackExportServiceMock); - const fakeServer = { - once: vi.fn().mockReturnThis(), - off: vi.fn().mockReturnThis(), - listen: vi.fn((_port: number, _host: string, callback?: () => void) => { - callback?.(); - return fakeServer; - }), - close: vi.fn(), - }; - - return { - createAppMock, - createDbMock, - detectPortMock, - feedbackExportServiceMock, - feedbackServiceFactoryMock, - fakeServer, - }; -}); - -vi.mock("node:http", () => ({ - createServer: vi.fn(() => fakeServer), -})); - -vi.mock("detect-port", () => ({ - default: detectPortMock, -})); - -vi.mock("@paperclipai/db", () => ({ - createDb: createDbMock, - ensurePostgresDatabase: vi.fn(), - getPostgresDataDirectory: vi.fn(), - inspectMigrations: vi.fn(async () => ({ status: "upToDate" })), - applyPendingMigrations: vi.fn(), - reconcilePendingMigrationHistory: vi.fn(async () => ({ repairedMigrations: [] })), - formatDatabaseBackupResult: vi.fn(() => "ok"), - runDatabaseBackup: vi.fn(), - authUsers: {}, - companies: {}, - companyMemberships: {}, - instanceUserRoles: {}, -})); - -vi.mock("../app.js", () => ({ - createApp: createAppMock, -})); - -vi.mock("../config.js", () => ({ - loadConfig: vi.fn(() => ({ + const loadConfigMock = vi.fn(() => ({ deploymentMode: "authenticated", deploymentExposure: "private", bind: "loopback", @@ -99,7 +48,61 @@ vi.mock("../config.js", () => ({ heartbeatSchedulerEnabled: false, heartbeatSchedulerIntervalMs: 30000, companyDeletionEnabled: false, - })), + })); + const feedbackExportServiceMock = { + flushPendingFeedbackTraces: vi.fn(async () => ({ attempted: 0, sent: 0, failed: 0 })), + }; + const feedbackServiceFactoryMock = vi.fn(() => feedbackExportServiceMock); + const fakeServer = { + once: vi.fn().mockReturnThis(), + off: vi.fn().mockReturnThis(), + listen: vi.fn((_port: number, _host: string, callback?: () => void) => { + callback?.(); + return fakeServer; + }), + close: vi.fn(), + }; + + return { + createAppMock, + createDbMock, + detectPortMock, + loadConfigMock, + feedbackExportServiceMock, + feedbackServiceFactoryMock, + fakeServer, + }; +}); + +vi.mock("node:http", () => ({ + createServer: vi.fn(() => fakeServer), +})); + +vi.mock("detect-port", () => ({ + default: detectPortMock, +})); + +vi.mock("@paperclipai/db", () => ({ + createDb: createDbMock, + ensurePostgresDatabase: vi.fn(), + getPostgresDataDirectory: vi.fn(), + inspectMigrations: vi.fn(async () => ({ status: "upToDate" })), + applyPendingMigrations: vi.fn(), + reconcilePendingMigrationHistory: vi.fn(async () => ({ repairedMigrations: [] })), + formatDatabaseBackupResult: vi.fn(() => "ok"), + runDatabaseBackup: vi.fn(), + authUsers: {}, + companies: {}, + companyMemberships: {}, + instanceUserRoles: {}, +})); + +vi.mock("../app.js", () => ({ + createApp: createAppMock, +})); + +vi.mock("../config.js", () => ({ + loadConfig: loadConfigMock, })); vi.mock("../middleware/logger.js", () => ({ @@ -216,4 +219,36 @@ describe("startServer PAPERCLIP_API_URL handling", () => { expect(started.apiUrl).toBe("http://127.0.0.1:3210"); expect(process.env.PAPERCLIP_API_URL).toBe("http://127.0.0.1:3210"); }); + + it("rewrites explicit-port auth public URLs when detect-port selects a new port", async () => { + loadConfigMock.mockReturnValueOnce({ + ...loadConfigMock(), + port: 3100, + authBaseUrlMode: "explicit", + authPublicBaseUrl: "http://my-host.ts.net:3100", + }); + detectPortMock.mockResolvedValueOnce(3110); + + const started = await startServer(); + + expect(started.listenPort).toBe(3110); + expect(started.apiUrl).toBe("http://my-host.ts.net:3110"); + expect(process.env.PAPERCLIP_RUNTIME_API_URL).toBe("http://my-host.ts.net:3110"); + }); + + it("keeps no-port auth public URLs stable when detect-port selects a new port", async () => { + loadConfigMock.mockReturnValueOnce({ + ...loadConfigMock(), + port: 3100, + authBaseUrlMode: "explicit", + authPublicBaseUrl: "https://paperclip.example", + }); + detectPortMock.mockResolvedValueOnce(3110); + + const started = await startServer(); + + expect(started.listenPort).toBe(3110); + expect(started.apiUrl).toBe("https://paperclip.example"); + expect(process.env.PAPERCLIP_RUNTIME_API_URL).toBe("https://paperclip.example"); + }); }); diff --git a/server/src/__tests__/worktree-config.test.ts b/server/src/__tests__/worktree-config.test.ts index defc8a05da7..69a9c5eeef5 100644 --- a/server/src/__tests__/worktree-config.test.ts +++ b/server/src/__tests__/worktree-config.test.ts @@ -24,7 +24,7 @@ afterEach(() => { } }); -function buildLegacyConfig(sharedRoot: string) { +function buildLegacyConfig(sharedRoot: string, publicBaseUrl = "http://127.0.0.1:3100") { return { $meta: { version: 1, @@ -56,7 +56,7 @@ function buildLegacyConfig(sharedRoot: string) { }, auth: { baseUrlMode: "explicit" as const, - publicBaseUrl: "http://127.0.0.1:3100", + publicBaseUrl, disableSignUp: false, }, storage: { @@ -439,7 +439,7 @@ describe("worktree config repair", () => { expect(repairedConfig.database.embeddedPostgresPort).toBe(54331); }); - it("persists runtime-selected worktree ports back into config", async () => { + it("persists runtime-selected worktree ports back into explicit-port auth URLs", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-ports-")); const worktreeRoot = path.join(tempRoot, "PAP-878-create-a-mine-tab-in-inbox"); const paperclipDir = path.join(worktreeRoot, ".paperclip"); @@ -452,7 +452,7 @@ describe("worktree config repair", () => { configPath, JSON.stringify( { - ...buildLegacyConfig(instanceRoot), + ...buildLegacyConfig(instanceRoot, "http://my-host.ts.net:3100"), database: { mode: "embedded-postgres", embeddedPostgresDataDir: path.join(instanceRoot, "db"), @@ -518,20 +518,122 @@ describe("worktree config repair", () => { expect(writtenConfig.server.port).toBe(3103); expect(writtenConfig.database.embeddedPostgresPort).toBe(54335); - expect(writtenConfig.auth.publicBaseUrl).toBe("http://127.0.0.1:3103/"); + expect(writtenConfig.auth.publicBaseUrl).toBe("http://my-host.ts.net:3103/"); }); - it("can update the in-memory config without rewriting env-driven ports", () => { - const { config, changed } = applyRuntimePortSelectionToConfig(buildLegacyConfig("/tmp/shared"), { - serverPort: 3104, - databasePort: 54340, - allowServerPortWrite: false, - allowDatabasePortWrite: true, + it("does not rewrite no-port public auth URLs when persisting runtime-selected ports", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-public-ports-")); + const worktreeRoot = path.join(tempRoot, "PAP-125-public-base-url"); + const paperclipDir = path.join(worktreeRoot, ".paperclip"); + const configPath = path.join(paperclipDir, "config.json"); + const isolatedHome = path.join(tempRoot, ".paperclip-worktrees"); + const instanceRoot = path.join(isolatedHome, "instances", "pap-125-public-base-url"); + + await fs.mkdir(paperclipDir, { recursive: true }); + await fs.writeFile( + configPath, + JSON.stringify( + { + ...buildLegacyConfig(instanceRoot, "https://paperclip.example"), + database: { + mode: "embedded-postgres", + embeddedPostgresDataDir: path.join(instanceRoot, "db"), + embeddedPostgresPort: 54331, + backup: { + enabled: true, + intervalMinutes: 60, + retentionDays: 30, + dir: path.join(instanceRoot, "data", "backups"), + }, + }, + logging: { + mode: "file", + logDir: path.join(instanceRoot, "logs"), + }, + server: { + deploymentMode: "local_trusted", + exposure: "private", + host: "127.0.0.1", + port: 3101, + allowedHostnames: [], + serveUi: true, + }, + storage: { + provider: "local_disk", + localDisk: { + baseDir: path.join(instanceRoot, "data", "storage"), + }, + s3: { + bucket: "paperclip", + region: "us-east-1", + prefix: "", + forcePathStyle: false, + }, + }, + secrets: { + provider: "local_encrypted", + strictMode: false, + localEncrypted: { + keyFilePath: path.join(instanceRoot, "secrets", "master.key"), + }, + }, + }, + null, + 2, + ) + "\n", + "utf8", + ); + + process.chdir(worktreeRoot); + process.env.PAPERCLIP_IN_WORKTREE = "true"; + process.env.PAPERCLIP_WORKTREE_NAME = "PAP-125-public-base-url"; + process.env.PAPERCLIP_HOME = isolatedHome; + process.env.PAPERCLIP_INSTANCE_ID = "pap-125-public-base-url"; + process.env.PAPERCLIP_CONFIG = configPath; + + maybePersistWorktreeRuntimePorts({ + serverPort: 3103, + databasePort: 54335, }); + const writtenConfig = JSON.parse(await fs.readFile(configPath, "utf8")); + + expect(writtenConfig.server.port).toBe(3103); + expect(writtenConfig.database.embeddedPostgresPort).toBe(54335); + expect(writtenConfig.auth.publicBaseUrl).toBe("https://paperclip.example"); + }); + + it("can update the in-memory config when auth URL already includes a port", () => { + const { config, changed } = applyRuntimePortSelectionToConfig( + buildLegacyConfig("/tmp/shared", "http://my-host.ts.net:3100"), + { + serverPort: 3104, + databasePort: 54340, + allowServerPortWrite: false, + allowDatabasePortWrite: true, + }, + ); + expect(changed).toBe(true); expect(config.server.port).toBe(3100); expect(config.database.embeddedPostgresPort).toBe(54340); - expect(config.auth.publicBaseUrl).toBe("http://127.0.0.1:3104/"); + expect(config.auth.publicBaseUrl).toBe("http://my-host.ts.net:3104/"); + }); + + it("does not rewrite the in-memory config when auth URL has no explicit port", () => { + const { config, changed } = applyRuntimePortSelectionToConfig( + buildLegacyConfig("/tmp/shared", "https://paperclip.example"), + { + serverPort: 3104, + databasePort: 54340, + allowServerPortWrite: false, + allowDatabasePortWrite: true, + }, + ); + + expect(changed).toBe(true); + expect(config.server.port).toBe(3100); + expect(config.database.embeddedPostgresPort).toBe(54340); + expect(config.auth.publicBaseUrl).toBe("https://paperclip.example"); }); }); diff --git a/server/src/index.ts b/server/src/index.ts index 67eec273539..4dbf346f2d3 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -191,7 +191,8 @@ export async function startServer(): Promise { if (!rawUrl) return undefined; try { const parsed = new URL(rawUrl); - if (!isLoopbackHost(parsed.hostname)) return rawUrl; + // The URL API normalizes default ports like :80/:443 to "", so treat them as stable URLs. + if (!parsed.port) return rawUrl; parsed.port = String(port); return parsed.toString(); } catch { diff --git a/server/src/worktree-config.ts b/server/src/worktree-config.ts index 2c6170bca59..d4f3ccb19d6 100644 --- a/server/src/worktree-config.ts +++ b/server/src/worktree-config.ts @@ -27,16 +27,12 @@ function sanitizeWorktreeInstanceId(rawValue: string): string { return normalized || "worktree"; } -function isLoopbackHost(hostname: string): boolean { - const value = hostname.trim().toLowerCase(); - return value === "127.0.0.1" || value === "localhost" || value === "::1"; -} - function rewriteLocalUrlPort(rawUrl: string | undefined, port: number): string | undefined { if (!rawUrl) return undefined; try { const parsed = new URL(rawUrl); - if (!isLoopbackHost(parsed.hostname)) return rawUrl; + // The URL API normalizes default ports like :80/:443 to "", so treat them as stable URLs. + if (!parsed.port) return rawUrl; parsed.port = String(port); return parsed.toString(); } catch { From b2496c80672f15481f7af48952b0d6d68ec208de Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 26 Apr 2026 15:40:39 -0700 Subject: [PATCH 6/9] fix(auth): trust allowed hostname port variants on detected listen port (#4554) ## Thinking Path > - Paperclip is the control plane for autonomous AI companies, so authenticated board access has to be predictable across local and worktree deployments. > - This change sits in the authenticated-mode server startup and Better Auth origin-trust wiring. > - The original auth branch fixed one real gap by adding port-qualified trusted origins for allowed hostnames on non-default ports. > - Review of that branch found a second-order bug: trusted origins were still derived from the configured port before startup detected the actual listen port. > - In isolated worktrees, that meant a common `3100 -> 3101` port shift could still leave Better Auth trusting the stale origin. > - This pull request keeps the original allowed-hostname port-variant fix, then moves trust derivation onto the resolved listen port and adds regression coverage around startup wiring. > - The benefit is that authenticated sessions keep working on allowed private hostnames even when Paperclip has to auto-shift to a different local port. ## What Changed - Added `:port` trusted-origin variants for authenticated-mode `allowedHostnames` when Paperclip runs on non-default ports. - Changed authenticated startup so `listenPort` is detected before Better Auth initialization, and explicit auth base URLs are rewritten before auth startup. - Updated `deriveAuthTrustedOrigins()` to accept the resolved listen port so Better Auth trusts the actual browser origin instead of the stale configured port. - Added focused regression coverage in `server/src/__tests__/better-auth.test.ts` and `server/src/__tests__/server-startup-feedback-export.test.ts`. ## Verification - `pnpm exec vitest run server/src/__tests__/better-auth.test.ts server/src/__tests__/server-startup-feedback-export.test.ts` - Reviewer re-check: reviewed commits `380f5b9f` and `092bb34c` after the follow-up fix landed and found no remaining issues. ## Risks - Low risk: this only affects authenticated-mode origin derivation and startup ordering around detected listen ports. - Main behavioral shift: startup no longer mutates `config.port` to the selected port; it now carries `requestedListenPort` separately and uses `listenPort` where runtime behavior needs the resolved value. - If another path was implicitly relying on `config.port` being overwritten during startup, that path would need follow-up, though the current startup/test coverage did not reveal one. > I checked `ROADMAP.md` and did not find an overlapping planned core work item for this auth trusted-origin port handling fix. ## Model Used - OpenAI Codex via Paperclip `codex_local` agents for implementation and review. Exact backend model ID/context window were not surfaced in this run context; work was performed through the Codex local adapter with tool use, code execution, and review passes. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- server/src/__tests__/better-auth.test.ts | 35 +++++ .../server-startup-feedback-export.test.ts | 127 +++++++++++++----- server/src/auth/better-auth.ts | 14 +- server/src/index.ts | 23 ++-- 4 files changed, 148 insertions(+), 51 deletions(-) diff --git a/server/src/__tests__/better-auth.test.ts b/server/src/__tests__/better-auth.test.ts index 79bfa3e75bb..dd672d67e44 100644 --- a/server/src/__tests__/better-auth.test.ts +++ b/server/src/__tests__/better-auth.test.ts @@ -4,6 +4,7 @@ import { getCookies } from "better-auth/cookies"; import { buildBetterAuthAdvancedOptions, deriveAuthCookiePrefix, + deriveAuthTrustedOrigins, } from "../auth/better-auth.js"; const ORIGINAL_INSTANCE_ID = process.env.PAPERCLIP_INSTANCE_ID; @@ -40,4 +41,38 @@ describe("Better Auth cookie scoping", () => { useSecureCookies: false, }); }); + + it("adds hostname port variants for authenticated mode on non-default ports", () => { + const trustedOrigins = deriveAuthTrustedOrigins({ + deploymentMode: "authenticated", + authBaseUrlMode: "auto", + authPublicBaseUrl: undefined, + allowedHostnames: ["Board.Example.Test"], + port: 3101, + } as Parameters[0]); + + expect(trustedOrigins).toEqual(expect.arrayContaining([ + "https://board.example.test", + "http://board.example.test", + "https://board.example.test:3101", + "http://board.example.test:3101", + ])); + }); + + it("prefers an explicit resolved listen port over the configured port", () => { + const trustedOrigins = deriveAuthTrustedOrigins({ + deploymentMode: "authenticated", + authBaseUrlMode: "auto", + authPublicBaseUrl: undefined, + allowedHostnames: ["board.example.test"], + port: 3100, + } as Parameters[0], { listenPort: 3101 }); + + expect(trustedOrigins).toEqual(expect.arrayContaining([ + "https://board.example.test:3101", + "http://board.example.test:3101", + ])); + expect(trustedOrigins).not.toContain("https://board.example.test:3100"); + expect(trustedOrigins).not.toContain("http://board.example.test:3100"); + }); }); diff --git a/server/src/__tests__/server-startup-feedback-export.test.ts b/server/src/__tests__/server-startup-feedback-export.test.ts index a08e0f02414..35fb076784d 100644 --- a/server/src/__tests__/server-startup-feedback-export.test.ts +++ b/server/src/__tests__/server-startup-feedback-export.test.ts @@ -2,17 +2,50 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const { createAppMock, + createBetterAuthInstanceMock, createDbMock, detectPortMock, - loadConfigMock, + deriveAuthTrustedOriginsMock, feedbackExportServiceMock, feedbackServiceFactoryMock, fakeServer, + loadConfigMock, } = vi.hoisted(() => { const createAppMock = vi.fn(async () => ((_: unknown, __: unknown) => {}) as never); + const createBetterAuthInstanceMock = vi.fn(() => ({})); const createDbMock = vi.fn(() => ({}) as never); const detectPortMock = vi.fn(async (port: number) => port); - const loadConfigMock = vi.fn(() => ({ + const deriveAuthTrustedOriginsMock = vi.fn(() => []); + const feedbackExportServiceMock = { + flushPendingFeedbackTraces: vi.fn(async () => ({ attempted: 0, sent: 0, failed: 0 })), + }; + const feedbackServiceFactoryMock = vi.fn(() => feedbackExportServiceMock); + const fakeServer = { + once: vi.fn().mockReturnThis(), + off: vi.fn().mockReturnThis(), + listen: vi.fn((_port: number, _host: string, callback?: () => void) => { + callback?.(); + return fakeServer; + }), + close: vi.fn(), + }; + const loadConfigMock = vi.fn(); + + return { + createAppMock, + createBetterAuthInstanceMock, + createDbMock, + detectPortMock, + deriveAuthTrustedOriginsMock, + feedbackExportServiceMock, + feedbackServiceFactoryMock, + fakeServer, + loadConfigMock, + }; +}); + +function buildTestConfig(overrides: Record = {}) { + return { deploymentMode: "authenticated", deploymentExposure: "private", bind: "loopback", @@ -48,31 +81,9 @@ const { heartbeatSchedulerEnabled: false, heartbeatSchedulerIntervalMs: 30000, companyDeletionEnabled: false, - })); - const feedbackExportServiceMock = { - flushPendingFeedbackTraces: vi.fn(async () => ({ attempted: 0, sent: 0, failed: 0 })), + ...overrides, }; - const feedbackServiceFactoryMock = vi.fn(() => feedbackExportServiceMock); - const fakeServer = { - once: vi.fn().mockReturnThis(), - off: vi.fn().mockReturnThis(), - listen: vi.fn((_port: number, _host: string, callback?: () => void) => { - callback?.(); - return fakeServer; - }), - close: vi.fn(), - }; - - return { - createAppMock, - createDbMock, - detectPortMock, - loadConfigMock, - feedbackExportServiceMock, - feedbackServiceFactoryMock, - fakeServer, - }; -}); +} vi.mock("node:http", () => ({ createServer: vi.fn(() => fakeServer), @@ -169,8 +180,8 @@ vi.mock("../board-claim.js", () => ({ vi.mock("../auth/better-auth.js", () => ({ createBetterAuthHandler: vi.fn(() => undefined), - createBetterAuthInstance: vi.fn(() => ({})), - deriveAuthTrustedOrigins: vi.fn(() => []), + createBetterAuthInstance: createBetterAuthInstanceMock, + deriveAuthTrustedOrigins: deriveAuthTrustedOriginsMock, resolveBetterAuthSession: vi.fn(async () => null), resolveBetterAuthSessionFromHeaders: vi.fn(async () => null), })); @@ -180,6 +191,9 @@ import { startServer } from "../index.ts"; describe("startServer feedback export wiring", () => { beforeEach(() => { vi.clearAllMocks(); + loadConfigMock.mockReturnValue(buildTestConfig()); + createBetterAuthInstanceMock.mockReturnValue({}); + deriveAuthTrustedOriginsMock.mockReturnValue([]); process.env.BETTER_AUTH_SECRET = "test-secret"; }); @@ -197,9 +211,56 @@ describe("startServer feedback export wiring", () => { }); }); +describe("startServer authenticated auth origin setup", () => { + beforeEach(() => { + vi.clearAllMocks(); + loadConfigMock.mockReturnValue(buildTestConfig()); + createBetterAuthInstanceMock.mockReturnValue({}); + deriveAuthTrustedOriginsMock.mockReturnValue([]); + process.env.BETTER_AUTH_SECRET = "test-secret"; + }); + + it("derives trusted origins from the detected listen port before auth initializes", async () => { + loadConfigMock.mockReturnValue(buildTestConfig({ + port: 3210, + allowedHostnames: ["board.example.test"], + authBaseUrlMode: "explicit", + authPublicBaseUrl: "http://127.0.0.1:3210", + })); + detectPortMock.mockResolvedValueOnce(3211); + deriveAuthTrustedOriginsMock.mockImplementation( + (_config: { port: number; authPublicBaseUrl?: string }, opts?: { listenPort?: number }) => [ + `http://board.example.test:${opts?.listenPort ?? 0}`, + ], + ); + + await startServer(); + + expect(deriveAuthTrustedOriginsMock).toHaveBeenCalledWith( + expect.objectContaining({ + port: 3210, + authPublicBaseUrl: "http://127.0.0.1:3211/", + }), + { listenPort: 3211 }, + ); + expect(createBetterAuthInstanceMock).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + port: 3210, + authPublicBaseUrl: "http://127.0.0.1:3211/", + }), + ["http://board.example.test:3211"], + ); + expect(createAppMock.mock.calls[0]?.[1]).toMatchObject({ + serverPort: 3211, + }); + }); +}); + describe("startServer PAPERCLIP_API_URL handling", () => { beforeEach(() => { vi.clearAllMocks(); + loadConfigMock.mockReturnValue(buildTestConfig()); process.env.BETTER_AUTH_SECRET = "test-secret"; delete process.env.PAPERCLIP_API_URL; }); @@ -221,12 +282,11 @@ describe("startServer PAPERCLIP_API_URL handling", () => { }); it("rewrites explicit-port auth public URLs when detect-port selects a new port", async () => { - loadConfigMock.mockReturnValueOnce({ - ...loadConfigMock(), + loadConfigMock.mockReturnValueOnce(buildTestConfig({ port: 3100, authBaseUrlMode: "explicit", authPublicBaseUrl: "http://my-host.ts.net:3100", - }); + })); detectPortMock.mockResolvedValueOnce(3110); const started = await startServer(); @@ -237,12 +297,11 @@ describe("startServer PAPERCLIP_API_URL handling", () => { }); it("keeps no-port auth public URLs stable when detect-port selects a new port", async () => { - loadConfigMock.mockReturnValueOnce({ - ...loadConfigMock(), + loadConfigMock.mockReturnValueOnce(buildTestConfig({ port: 3100, authBaseUrlMode: "explicit", authPublicBaseUrl: "https://paperclip.example", - }); + })); detectPortMock.mockResolvedValueOnce(3110); const started = await startServer(); diff --git a/server/src/auth/better-auth.ts b/server/src/auth/better-auth.ts index 402dad52036..3c84b9f555c 100644 --- a/server/src/auth/better-auth.ts +++ b/server/src/auth/better-auth.ts @@ -61,7 +61,7 @@ function headersFromExpressRequest(req: Request): Headers { return headersFromNodeHeaders(req.headers); } -export function deriveAuthTrustedOrigins(config: Config): string[] { +export function deriveAuthTrustedOrigins(config: Config, opts?: { listenPort?: number }): string[] { const baseUrl = config.authBaseUrlMode === "explicit" ? config.authPublicBaseUrl : undefined; const trustedOrigins = new Set(); @@ -73,18 +73,24 @@ export function deriveAuthTrustedOrigins(config: Config): string[] { } } if (config.deploymentMode === "authenticated") { + const port = opts?.listenPort ?? config.port; + const needsPortVariants = port !== 80 && port !== 443; for (const hostname of config.allowedHostnames) { const trimmed = hostname.trim().toLowerCase(); if (!trimmed) continue; trustedOrigins.add(`https://${trimmed}`); trustedOrigins.add(`http://${trimmed}`); + if (needsPortVariants) { + trustedOrigins.add(`https://${trimmed}:${port}`); + trustedOrigins.add(`http://${trimmed}:${port}`); + } } } return Array.from(trustedOrigins); } -export function createBetterAuthInstance(db: Db, config: Config, trustedOrigins?: string[]): BetterAuthInstance { +export function createBetterAuthInstance(db: Db, config: Config, trustedOrigins: string[]): BetterAuthInstance { const baseUrl = config.authBaseUrlMode === "explicit" ? config.authPublicBaseUrl : undefined; const secret = process.env.BETTER_AUTH_SECRET ?? process.env.PAPERCLIP_AGENT_JWT_SECRET; if (!secret) { @@ -93,15 +99,13 @@ export function createBetterAuthInstance(db: Db, config: Config, trustedOrigins? "For local development, set BETTER_AUTH_SECRET=paperclip-dev-secret in your .env file.", ); } - const effectiveTrustedOrigins = trustedOrigins ?? deriveAuthTrustedOrigins(config); - const publicUrl = process.env.PAPERCLIP_PUBLIC_URL ?? baseUrl; const isHttpOnly = publicUrl ? publicUrl.startsWith("http://") : false; const authConfig = { baseURL: baseUrl, secret, - trustedOrigins: effectiveTrustedOrigins, + trustedOrigins, database: drizzleAdapter(db, { provider: "pg", schema: { diff --git a/server/src/index.ts b/server/src/index.ts index 4dbf346f2d3..35e2ca5f943 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -468,6 +468,12 @@ export async function startServer(): Promise { } } } + + const requestedListenPort = config.port; + const listenPort = await detectPort(requestedListenPort); + if (config.authBaseUrlMode === "explicit" && config.authPublicBaseUrl) { + config.authPublicBaseUrl = rewriteLocalUrlPort(config.authPublicBaseUrl, listenPort); + } let authReady = config.deploymentMode === "local_trusted"; let betterAuthHandler: RequestHandler | undefined; @@ -488,7 +494,7 @@ export async function startServer(): Promise { resolveBetterAuthSession, resolveBetterAuthSessionFromHeaders, } = await import("./auth/better-auth.js"); - const derivedTrustedOrigins = deriveAuthTrustedOrigins(config); + const derivedTrustedOrigins = deriveAuthTrustedOrigins(config, { listenPort }); const envTrustedOrigins = (process.env.BETTER_AUTH_TRUSTED_ORIGINS ?? "") .split(",") .map((value) => value.trim()) @@ -513,17 +519,10 @@ export async function startServer(): Promise { await initializeBoardClaimChallenge(db as any, { deploymentMode: config.deploymentMode }); authReady = true; } - - const listenPort = await detectPort(config.port); - if (listenPort !== config.port) { - config.port = listenPort; - } + if (resolvedEmbeddedPostgresPort !== null && resolvedEmbeddedPostgresPort !== config.embeddedPostgresPort) { config.embeddedPostgresPort = resolvedEmbeddedPostgresPort; } - if (config.authBaseUrlMode === "explicit" && config.authPublicBaseUrl) { - config.authPublicBaseUrl = rewriteLocalUrlPort(config.authPublicBaseUrl, listenPort); - } maybePersistWorktreeRuntimePorts({ serverPort: listenPort, databasePort: resolvedEmbeddedPostgresPort, @@ -627,8 +626,8 @@ export async function startServer(): Promise { server.keepAliveTimeout = 185000; server.headersTimeout = 186000; - if (listenPort !== config.port) { - logger.warn(`Requested port is busy; using next free port (requestedPort=${config.port}, selectedPort=${listenPort})`); + if (listenPort !== requestedListenPort) { + logger.warn(`Requested port is busy; using next free port (requestedPort=${requestedListenPort}, selectedPort=${listenPort})`); } const runtimeListenHost = config.host; @@ -821,7 +820,7 @@ export async function startServer(): Promise { deploymentMode: config.deploymentMode, deploymentExposure: config.deploymentExposure, authReady, - requestedPort: config.port, + requestedPort: requestedListenPort, listenPort, uiMode, db: startupDbInfo, From 54ab0d24cdf2ed37d55f93a819a59878f55868fb Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 26 Apr 2026 16:23:53 -0700 Subject: [PATCH 7/9] Fix disappearing issue comments (#4557) ## Thinking Path > - Paperclip is a control plane for AI-agent companies, so issue detail pages are a primary surface for understanding agent work and human feedback. > - The relevant subsystem here is the issue comments/chat experience across the React issue detail page and the server comment pagination API. > - Long issue threads were only surfacing the newest page of comments at first render, which hid earlier human and agent messages behind extra pagination. > - The first UI fix exposed that the descending cursor path on the server could also fail for older-page fetches, leaving the chat tab stuck on an infinite "Loading earlier comments..." state. > - This needed to be addressed in both layers so the chat tab can surface earlier conversation history without manual recovery and without server errors. > - This pull request auto-loads earlier comment pages in the issue detail chat view and fixes the descending cursor predicate used by issue comment pagination. > - The benefit is that long-running issues like `PAPA-103` now show the missing conversation history near the top of the chat surface instead of hiding it or failing to load it. ## What Changed - Auto-load earlier issue comment pages in the issue detail chat tab until the thread reaches a 150-comment cap or there are no older comments left. - Add UI-side guard logic and regression coverage for optimistic issue comment pagination so the autoload behavior stops cleanly. - Replace the raw SQL descending cursor predicate in `issueService.listComments` with typed Drizzle comparisons for the `(createdAt, id)` anchor tuple. - Add a server regression test that paginates earlier comments in descending order from an anchor comment. - Smoke-test the exact previously failing seeded `PAPA-103` cursor path on the isolated dev instance used for review. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/issues-service.test.ts` - `pnpm --filter @paperclipai/server typecheck` - Manual smoke against seeded `PAPA-103` data on the isolated dev server: - `GET /api/issues/PAPA-103/comments?order=desc&limit=50` returns `200` - `GET /api/issues/PAPA-103/comments?after=765d3609-edc6-4d11-a8fe-d466affbe85d&order=desc&limit=50` now returns `200` with 50 comments instead of `500` ## Risks - Moderate UI/perf risk on very large threads because the chat tab now prefetches multiple earlier pages on mount; the cap is intentionally limited to 150 comments to bound that work. - Low API risk because the server fix only changes the cursor predicate construction for anchor-based comment pagination, but any mistake there would affect older-comment paging order. > I checked `ROADMAP.md` before opening this PR and this bug fix does not duplicate planned core work. ## Model Used - OpenAI Codex coding agent in the Paperclip local adapter environment. The exact backend model ID and context window were not exposed in-session. Tool-assisted workflow included shell execution, git/GitHub CLI, local test execution, and targeted code edits. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- server/src/__tests__/issues-service.test.ts | 58 ++++++++++++++++++++ server/src/services/issues.ts | 18 +++--- ui/src/lib/optimistic-issue-comments.test.ts | 40 ++++++++++++++ ui/src/lib/optimistic-issue-comments.ts | 15 +++++ ui/src/pages/IssueDetail.tsx | 18 ++++++ 5 files changed, 140 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index 16afd95a9d7..b6e8e6d01cf 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -895,6 +895,64 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { ); }); + it("paginates earlier comments in descending order from an anchor comment", async () => { + const companyId = randomUUID(); + const issueId = randomUUID(); + const firstCommentId = randomUUID(); + const anchorCommentId = randomUUID(); + const latestCommentId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + + await db.insert(issues).values({ + id: issueId, + companyId, + title: "Paged comments issue", + status: "todo", + priority: "medium", + }); + + await db.insert(issueComments).values([ + { + id: firstCommentId, + companyId, + issueId, + body: "First comment", + createdAt: new Date("2026-03-26T10:00:00.000Z"), + updatedAt: new Date("2026-03-26T10:00:00.000Z"), + }, + { + id: anchorCommentId, + companyId, + issueId, + body: "Anchor comment", + createdAt: new Date("2026-03-26T11:00:00.000Z"), + updatedAt: new Date("2026-03-26T11:00:00.000Z"), + }, + { + id: latestCommentId, + companyId, + issueId, + body: "Latest comment", + createdAt: new Date("2026-03-26T12:00:00.000Z"), + updatedAt: new Date("2026-03-26T12:00:00.000Z"), + }, + ]); + + const comments = await svc.listComments(issueId, { + afterCommentId: anchorCommentId, + order: "desc", + limit: 50, + }); + + expect(comments.map((comment) => comment.id)).toEqual([firstCommentId]); + }); + it("includes blockedBy summaries on list rows in one batched pass", async () => { const companyId = randomUUID(); const blockerId = randomUUID(); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 48893d036c8..d7d466b3cf4 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -1,5 +1,5 @@ import { Buffer } from "node:buffer"; -import { and, asc, desc, eq, inArray, isNull, ne, or, sql } from "drizzle-orm"; +import { and, asc, desc, eq, gt, inArray, isNull, lt, ne, or, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { activityLog, @@ -3138,14 +3138,14 @@ export function issueService(db: Db) { if (!anchor) return []; conditions.push( order === "asc" - ? sql`( - ${issueComments.createdAt} > ${anchor.createdAt} - OR (${issueComments.createdAt} = ${anchor.createdAt} AND ${issueComments.id} > ${anchor.id}) - )` - : sql`( - ${issueComments.createdAt} < ${anchor.createdAt} - OR (${issueComments.createdAt} = ${anchor.createdAt} AND ${issueComments.id} < ${anchor.id}) - )`, + ? or( + gt(issueComments.createdAt, anchor.createdAt), + and(eq(issueComments.createdAt, anchor.createdAt), gt(issueComments.id, anchor.id)), + )! + : or( + lt(issueComments.createdAt, anchor.createdAt), + and(eq(issueComments.createdAt, anchor.createdAt), lt(issueComments.id, anchor.id)), + )!, ); } diff --git a/ui/src/lib/optimistic-issue-comments.test.ts b/ui/src/lib/optimistic-issue-comments.test.ts index 1c53fb7fe93..40d292f3357 100644 --- a/ui/src/lib/optimistic-issue-comments.test.ts +++ b/ui/src/lib/optimistic-issue-comments.test.ts @@ -12,6 +12,7 @@ import { matchesIssueRef, mergeIssueComments, removeIssueCommentFromPages, + shouldAutoloadOlderIssueComments, takeOptimisticIssueComment, upsertIssueComment, upsertIssueCommentInPages, @@ -233,6 +234,45 @@ describe("optimistic issue comments", () => { ).toBe("comment-1"); }); + it("autoloads older chat comments while the initial thread is still under the threshold", () => { + expect( + shouldAutoloadOlderIssueComments({ + activeDetailTab: "chat", + hasOlderComments: true, + loadedCommentCount: 50, + initialPageLoading: false, + olderPageLoading: false, + autoLoadLimit: 150, + }), + ).toBe(true); + }); + + it("does not autoload older comments outside the chat tab", () => { + expect( + shouldAutoloadOlderIssueComments({ + activeDetailTab: "activity", + hasOlderComments: true, + loadedCommentCount: 50, + initialPageLoading: false, + olderPageLoading: false, + autoLoadLimit: 150, + }), + ).toBe(false); + }); + + it("stops autoloading once the initial comment window reaches the cap", () => { + expect( + shouldAutoloadOlderIssueComments({ + activeDetailTab: "chat", + hasOlderComments: true, + loadedCommentCount: 150, + initialPageLoading: false, + olderPageLoading: false, + autoLoadLimit: 150, + }), + ).toBe(false); + }); + it("upserts paged comments without dropping older pages", () => { const nextPages = upsertIssueCommentInPages( [ diff --git a/ui/src/lib/optimistic-issue-comments.ts b/ui/src/lib/optimistic-issue-comments.ts index f35a484c3bf..d216ab06678 100644 --- a/ui/src/lib/optimistic-issue-comments.ts +++ b/ui/src/lib/optimistic-issue-comments.ts @@ -150,6 +150,21 @@ export function getNextIssueCommentPageParam( return lastPage[lastPage.length - 1]?.id; } +export function shouldAutoloadOlderIssueComments(params: { + activeDetailTab: string; + hasOlderComments: boolean; + loadedCommentCount: number; + initialPageLoading: boolean; + olderPageLoading: boolean; + autoLoadLimit: number; +}) { + if (params.activeDetailTab !== "chat") return false; + if (!params.hasOlderComments) return false; + if (params.initialPageLoading || params.olderPageLoading) return false; + if (params.loadedCommentCount === 0) return false; + return params.loadedCommentCount < params.autoLoadLimit; +} + export function upsertIssueComment( comments: IssueComment[] | undefined, nextComment: IssueComment, diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index f5ae861634d..6e335c95c95 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -51,6 +51,7 @@ import { matchesIssueRef, mergeIssueComments, removeIssueCommentFromPages, + shouldAutoloadOlderIssueComments, takeOptimisticIssueComment, upsertIssueCommentInPages, type IssueCommentReassignment, @@ -152,6 +153,7 @@ type IssueDetailComment = (IssueComment | OptimisticIssueComment) & { const FEEDBACK_TERMS_URL = import.meta.env.VITE_FEEDBACK_TERMS_URL?.trim() || "https://paperclip.ing/tos"; const ISSUE_COMMENT_PAGE_SIZE = 50; +const ISSUE_COMMENT_AUTOLOAD_LIMIT = ISSUE_COMMENT_PAGE_SIZE * 3; const TREE_CONTROL_MODE_LABEL: Record = { pause: "Pause subtree", resume: "Resume subtree", @@ -1103,6 +1105,18 @@ export function IssueDetail() { () => flattenIssueCommentPages(commentPages?.pages), [commentPages?.pages], ); + const shouldPrefetchOlderComments = useMemo( + () => + shouldAutoloadOlderIssueComments({ + activeDetailTab: detailTab, + hasOlderComments: hasOlderComments ?? false, + loadedCommentCount: comments.length, + initialPageLoading: commentsLoading, + olderPageLoading: commentsLoadingOlder, + autoLoadLimit: ISSUE_COMMENT_AUTOLOAD_LIMIT, + }), + [comments.length, commentsLoading, commentsLoadingOlder, detailTab, hasOlderComments], + ); const { data: interactions = [] } = useQuery({ queryKey: queryKeys.issues.interactions(issueId!), queryFn: () => issuesApi.listInteractions(issueId!), @@ -2537,6 +2551,10 @@ export function IssueDetail() { const loadOlderComments = useCallback(() => { void fetchOlderComments(); }, [fetchOlderComments]); + useEffect(() => { + if (!shouldPrefetchOlderComments) return; + void fetchOlderComments(); + }, [fetchOlderComments, shouldPrefetchOlderComments]); const handleCommentVote = useCallback(async (commentId: string, vote: "up" | "down", options?: { allowSharing?: boolean; reason?: string }) => { await feedbackVoteMutation.mutateAsync({ targetType: "issue_comment", From 8145141c55f1a16f36be4787fe3bb46340d63553 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 26 Apr 2026 17:19:23 -0700 Subject: [PATCH 8/9] Fix external issue URL rewriting in markdown (#4558) ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies. > - Issue and comment rendering is part of the board UI where humans supervise and inspect agent work. > - External Paperclip issue URLs can appear in comments as references to other runs, review threads, or remote test environments. > - Those links must preserve their full destination, including origin, port, and `#comment-...` fragments, or the operator is taken to the wrong place. > - The bug here was that absolute `http(s)` issue URLs were being normalized into internal `/issues/...` routes in the markdown path. > - This pull request stops rewriting absolute URLs while keeping internal issue-reference behavior for relative paths and identifiers. > - The benefit is that authored external links now navigate exactly where the operator expects, especially for remote test and comment-deep-link workflows. ## What Changed - Stopped `ui/src/lib/issue-reference.ts` from treating absolute `http(s)` URLs as internal issue paths. - Added defense-in-depth in `ui/src/lib/mention-chips.ts` so absolute `http(s)` URLs are never reclassified as issue mention chips. - Updated `ui/src/lib/issue-reference.test.ts` to cover absolute Paperclip URLs with preserved origin, port, and comment hash. - Updated `ui/src/components/MarkdownBody.test.tsx` to assert the reported URL renders as an external link, not an internal `/issues/...` href. ## Verification - `pnpm exec vitest run ui/src/lib/issue-reference.test.ts ui/src/components/MarkdownBody.test.tsx` - Expected result: `2` files passed, `37` tests passed. - Manual spot-check from the issue report path: a URL like `http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-...` should remain an external link with its full destination preserved. ## Risks - Low risk. The change narrows when Paperclip rewrites URLs, so the main risk is if some existing workflow depended on absolute `http(s)` Paperclip URLs being converted into internal issue links. The added regression coverage is aimed at preventing that from regressing silently. ## Model Used - OpenAI Codex local agent via Paperclip `codex_local` - Backing model family: GPT-5-based Codex runtime - Exact backend model ID/version: not exposed by this adapter/runtime surface - Context window: not exposed by this adapter/runtime surface - Capabilities used: tool use, shell command execution, code editing, git operations, and local test execution ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] I will address all Greptile and reviewer comments before requesting merge --- ui/src/components/MarkdownBody.test.tsx | 17 +++++++++-------- ui/src/lib/issue-reference.test.ts | 16 +++++++++------- ui/src/lib/issue-reference.ts | 13 ++----------- ui/src/lib/mention-chips.ts | 4 ++++ 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/ui/src/components/MarkdownBody.test.tsx b/ui/src/components/MarkdownBody.test.tsx index 8f64bf8f6fa..0cae828d543 100644 --- a/ui/src/components/MarkdownBody.test.tsx +++ b/ui/src/components/MarkdownBody.test.tsx @@ -187,16 +187,17 @@ describe("MarkdownBody", () => { expect(html).not.toContain('aria-label="Issue PAP-1271: PAP-1271"'); }); - it("rewrites full issue URLs to internal issue links", () => { - const html = renderMarkdown("See http://localhost:3100/PAP/issues/PAP-1179.", [ - { identifier: "PAP-1179", status: "blocked" }, + it("preserves absolute issue URLs as external links", () => { + const url = "http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-850083f3-24de-43e7-a8cd-bc01f7cc9f0d"; + const html = renderMarkdown(`See ${url}.`, [ + { identifier: "PAPA-115", status: "blocked" }, ]); - expect(html).toContain('href="/issues/PAP-1179"'); - expect(html).toContain("text-red-600"); - expect(html).toContain(">http://localhost:3100/PAP/issues/PAP-1179<"); - expect(html).toContain('data-mention-kind="issue"'); - expect(html).not.toContain("paperclip-mention-chip--issue"); + expect(html).toContain(`href="${url}"`); + expect(html).toContain('target="_blank"'); + expect(html).toContain("lucide-external-link"); + expect(html).not.toContain('href="/issues/PAPA-115"'); + expect(html).not.toContain("paperclip-markdown-issue-ref"); }); it("linkifies plain internal issue paths in markdown text", () => { diff --git a/ui/src/lib/issue-reference.test.ts b/ui/src/lib/issue-reference.test.ts index cd0f038a7ec..54224bfb55c 100644 --- a/ui/src/lib/issue-reference.test.ts +++ b/ui/src/lib/issue-reference.test.ts @@ -9,8 +9,9 @@ describe("issue-reference", () => { expect(parseIssuePathIdFromPath("/issues/:id")).toBeNull(); }); - it("extracts issue ids from full issue URLs", () => { - expect(parseIssuePathIdFromPath("http://localhost:3100/PAP/issues/PAP-1179")).toBe("PAP-1179"); + it("does not treat full issue URLs as internal issue paths", () => { + expect(parseIssuePathIdFromPath("http://localhost:3100/PAP/issues/PAP-1179")).toBeNull(); + expect(parseIssuePathIdFromPath("http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-850083f3-24de-43e7-a8cd-bc01f7cc9f0d")).toBeNull(); }); it("does not treat GitHub issue URLs as internal Paperclip issue links", () => { @@ -24,15 +25,11 @@ describe("issue-reference", () => { expect(parseIssueReferenceFromHref("/issues/:id")).toBeNull(); }); - it("normalizes bare identifiers, issue URLs, and issue scheme links into internal links", () => { + it("normalizes bare identifiers, relative issue paths, and issue scheme links into internal links", () => { expect(parseIssueReferenceFromHref("pap-1271")).toEqual({ issuePathId: "PAP-1271", href: "/issues/PAP-1271", }); - expect(parseIssueReferenceFromHref("http://localhost:3100/PAP/issues/PAP-1179")).toEqual({ - issuePathId: "PAP-1179", - href: "/issues/PAP-1179", - }); expect(parseIssueReferenceFromHref("/PAP/issues/pap-1180")).toEqual({ issuePathId: "PAP-1180", href: "/issues/PAP-1180", @@ -54,6 +51,11 @@ describe("issue-reference", () => { }); }); + it("preserves absolute Paperclip issue URLs so origin, port, and hash are not lost", () => { + expect(parseIssueReferenceFromHref("http://localhost:3100/PAP/issues/PAP-1179")).toBeNull(); + expect(parseIssueReferenceFromHref("http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-850083f3-24de-43e7-a8cd-bc01f7cc9f0d")).toBeNull(); + }); + it("ignores literal route placeholder paths", () => { expect(parseIssueReferenceFromHref("/issues/:id")).toBeNull(); expect(parseIssueReferenceFromHref("http://localhost:3100/api/issues/:id")).toBeNull(); diff --git a/ui/src/lib/issue-reference.ts b/ui/src/lib/issue-reference.ts index cbef86ffd8c..616bb876237 100644 --- a/ui/src/lib/issue-reference.ts +++ b/ui/src/lib/issue-reference.ts @@ -11,18 +11,9 @@ const ISSUE_REFERENCE_TOKEN_RE = /issue:\/\/:?[^\s<>()]+|https?:\/\/[^\s<>()]+|\ export function parseIssuePathIdFromPath(pathOrUrl: string | null | undefined): string | null { if (!pathOrUrl) return null; - let pathname = pathOrUrl.trim(); + const pathname = pathOrUrl.trim(); if (!pathname) return null; - - if (/^https?:\/\//i.test(pathname)) { - try { - const url = new URL(pathname); - if (url.hostname === "github.com" || url.hostname === "www.github.com") return null; - pathname = url.pathname; - } catch { - return null; - } - } + if (/^https?:\/\//i.test(pathname)) return null; const segments = pathname.split("/").filter(Boolean); const issueIndex = segments.findIndex((segment) => segment === "issues"); diff --git a/ui/src/lib/mention-chips.ts b/ui/src/lib/mention-chips.ts index d2dbe1c74b5..b12311b4b16 100644 --- a/ui/src/lib/mention-chips.ts +++ b/ui/src/lib/mention-chips.ts @@ -37,6 +37,10 @@ export type ParsedMentionChip = const iconMaskCache = new Map(); export function parseMentionChipHref(href: string): ParsedMentionChip | null { + if (/^https?:\/\//i.test(href.trim())) { + return null; + } + const issue = parseIssueReferenceHref(href); if (issue) { return { From 1d9f7a5149fe60b66234d696f9ddc468e5afe19e Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 26 Apr 2026 17:30:20 -0700 Subject: [PATCH 9/9] Fix flaky heartbeat recovery teardown CI failure (#4559) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies. > - The linked CI job is in the server test/recovery path, where heartbeat runs and issue cleanup need to leave the control plane in a consistent state even when retries fail. > - In this case the failure was not runtime product behavior but test teardown behavior inside `heartbeat-process-recovery.test.ts`. > - The failing GitHub Actions job showed a foreign-key race on `company_skills_company_id_companies_id_fk` while the test tried to delete the parent company record. > - The surrounding teardown code already uses bounded retry cleanup for other dependent tables (`issues`, `heartbeatRuns`, and `agents`) because this test file intentionally exercises asynchronous recovery flows. > - This pull request applies that same retry pattern to the final `db.delete(companies)` step, re-clearing `companySkills` before each retry. > - The benefit is a targeted fix for the CI flake without changing runtime behavior or expanding the scope beyond the failing teardown path. ## What Changed - Wrapped the final `db.delete(companies)` call in `server/src/__tests__/heartbeat-process-recovery.test.ts` with the same 5-attempt retry pattern already used elsewhere in that teardown. - Re-cleared `companySkills` before each company-delete retry so late-arriving FK-dependent rows do not mask the real test result. - Verified the fix against the originally failing `heartbeat-process-recovery` test file and the broader `pnpm test:run` command under CI-like env conditions. ## Verification - `pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts` - Re-ran `pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts` multiple times locally; the previously failing teardown stayed green. - `env -u PAPERCLIP_API_URL -u PAPERCLIP_RUNTIME_API_URL -u PAPERCLIP_RUN_ID -u PAPERCLIP_TASK_ID -u PAPERCLIP_AGENT_ID -u PAPERCLIP_COMPANY_ID -u PAPERCLIP_API_KEY -u PAPERCLIP_WAKE_REASON -u PAPERCLIP_WAKE_COMMENT_ID -u PAPERCLIP_WAKE_PAYLOAD_JSON -u PAPERCLIP_APPROVAL_ID -u PAPERCLIP_APPROVAL_STATUS pnpm test:run` ## Risks - Low risk. The change is test-only and scoped to teardown retry behavior in a single server test file. - If the underlying async cleanup behavior changes again, this test could still become flaky in a different way, but this PR addresses the specific FK race seen in the linked CI job. > 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 `gpt-5.4` via Paperclip `codex_local`, high reasoning mode, with tool use for shell, git, HTTP API calls, and patch application. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- .../src/__tests__/heartbeat-process-recovery.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 8f870ac602c..d0b581128b3 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -346,7 +346,16 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { await new Promise((resolve) => setTimeout(resolve, 50)); } } - await db.delete(companies); + for (let attempt = 0; attempt < 5; attempt += 1) { + await db.delete(companySkills); + try { + await db.delete(companies); + break; + } catch (error) { + if (attempt === 4) throw error; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } }); afterAll(async () => {