Ship `paperclipai/optional/knowledge/okf-knowledge`, an installable
(not default-on) catalog skill that round-trips company knowledge through
Google's Open Knowledge Format (OKF) v0.1 — a tool-neutral directory of
markdown + YAML-frontmatter concept files with index.md/log.md/okf.yaml.
- scripts/okf-export.mjs: dependency-free builder that turns a Paperclip
knowledge dump (memory, skills, issue decisions, docs, Content Machine
sources) into a conformant OKF bundle.
- scripts/okf-validate.mjs: bundle conformance checker (errors only on
missing/unparseable frontmatter or missing `type`; tolerates broken
cross-links and unknown types per spec).
- references/: condensed OKF spec, Paperclip→OKF export mapping + dump
schema, and an import procedure with a trust-boundary note.
- sample-bundle/: complete conformant example (the exporter's own output).
- src/okf-knowledge.test.ts: 13 unit tests over the builder + validator;
registered the skill in shipped-catalog.test.ts and regenerated the
catalog manifest.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
PAP-11123 / PAP-11099 Phase 3b. The Conference Room now runs a real-agent
conversation over the reusable SelectedAgentChat surface (default target =
company CEO) instead of the board-concierge SSE relay.
Backend: add POST /api/board/chat/conversations to resolve-or-mint the
backing board_chat issue without spawning the concierge subprocess (the web
client can't mint it directly — originKind is stripped by the create schema).
Gated like /board/chat/stream on the experimental flag + instance admin +
company access. Factor the resolve-or-create logic into a shared helper reused
by the stream relay.
UI: BoardChat resolves/mints the board issue and renders the connected
SelectedAgentChat for the room's center column; the history Sheet, new-chat
control, and split-pane ActivityFeed are kept. Durable history is the issue's
comments and live output is the target agent's active run (real reply comment
surfaced by polling) — the streaming-token bubble is gone.
Tests: new server unit test mirroring the selected-agent-chat route test;
BoardChat.test.tsx reworked for the new transport (SSE-stream + staged-intro
assertions dropped; shell/history kept).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements the Phase 3 selected-agent chat experience (PAP-11117) as a
reusable, issue-backed surface that reuses the existing IssueChatThread
renderer.
- SelectedAgentChatView (presentational) + SelectedAgentChat (connected):
real-agent identity header, CEO-default agent switcher (hideable),
durable comment history, target-scoped live/active-run output, inline
delivery error with draft preservation, loading + empty states.
- API: issuesApi.addSelectedAgentChatComment + targetAgentId on the
live-runs/active-run heartbeat endpoints; new query keys.
- Storybook stories for every state + component/unit tests (10 passing).
No board-concierge persona; assistant comments are authored by the real
agent id. Next-step choices render as real suggest_tasks/request_confirmation
interaction cards via IssueChatThread.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Follow-up to board feedback on the inline comment panel:
- Sort threads in document order (by normalizedStart/markdownStart), not
recency, so the comment list stays congruent with the highlights.
- Scroll the focused thread card into view in the pane whenever a thread
becomes focused — both when a new comment is added (optimistic focus)
and when its highlight is clicked in the document.
- Strengthen the focused-thread outline (ring-2 ring-primary) so the
selected comment is obvious in the list.
- Tint a highlight's run on hover in the document so it's clear which
highlight you're pointing at.
- Carry the anchor's document position onto the optimistic thread so a
freshly-posted comment sorts into the right spot immediately.
Adds a regression test asserting document-order rendering.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Streamline the inline document annotation UI per board feedback:
- Drop the "Comments" panel header, the Open/Resolved/Stale/Orphaned
filter chips, and the "No open comments yet" empty state. The revision
indicator now lives top-right next to the close button, and all
anchorable threads render in a single list.
- Remove the per-thread "Open · just now" status/timestamp row; keep the
per-comment timestamp beside each author avatar.
- Show the current user's avatar + name directly above the composer when
writing a new comment.
- Clamp quote snippets to two lines with a roomier line-height so the next
line's letter tops are never clipped.
- Drop the "⌘↵ to submit" hints from the composer/reply placeholders.
- Keep the selected segment brightly highlighted in the document while a
comment is being composed (pendingHighlightText) so the selection no
longer disappears when the panel opens.
- Make thread creation, replies, and resolve/reopen optimistic so
submission feels instant instead of flashing empty then repopulating.
- Clamp the floating desktop panel below the sticky top nav so the
comments thread no longer tucks under the nav bar while scrolling.
Adds a Storybook story (Product/Document Comments) covering the empty,
populated, and composing states. Updates annotation tests for the
filter-less single-list behaviour.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In the issue document annotation panel:
- Render the commenting author's avatar (agent icon or user image/
initials) next to each comment in an expanded thread.
- ⌘/Ctrl+Enter now submits the new-comment composer and thread replies,
with hint text in the placeholders.
Widen the agent map prop to optionally carry the agent icon. Add tests
for both shortcuts and avatar rendering.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The document annotation (Comments) panel sat 12px off the document's
right edge, reading as flush against it. Widen the desktop gap to 24px so
the pane is nudged right with a clear margin when there is room. The
clamped/overlap case (not enough room) is unaffected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The file viewer flashed, lost code-block scroll position, and cleared text
selection on markdown files (PAP-10767). Two compounding causes:
- FileViewerSheet ran a 75ms `elapsedMs` ticker that only drives the loading
skeleton, but it never stopped after the file loaded — re-rendering the whole
sheet ~13x/second. Now the ticker runs only while a preview is still loading.
- MarkdownBody rebuilt its react-markdown `remarkPlugins` array and `components`
map on every render. Because react-markdown uses the `components` values as
component *types*, new function identities each render made React unmount and
remount the entire rendered tree (discarding scroll/selection, causing the
flash). Both are now memoized.
Adds a regression test asserting markdown DOM nodes survive a parent re-render.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WebKit synthesizes an inline-flex box's baseline from its bottom edge
when align-items removes baseline participation (items-center), so the
chip sat entirely above the text baseline and inflated the surrounding
line height on mobile Safari. Chrome uses the inner content baseline, so
desktop looked fine. Switch align-baseline -> align-middle on both the
WorkspaceFileLink and ArtifactFileChip so alignment no longer depends on
baseline synthesis and renders consistently across engines.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
iPadOS Safari never flips the `(hover: hover) and (pointer: fine)` media
query when a trackpad/mouse is attached, so `pointerCanPeek` stayed false
and the collapsed-sidebar hover peek never opened even though a real cursor
was driving the UI (PAP-10725).
Detect a fine pointer at runtime: a genuine mouse/trackpad emits pointer
events with `pointerType: "mouse"` (touch → "touch", Pencil → "pen"), so
listening for the first such event unlocks peek without false-positives on
touch-only input. Peek capability is a one-way latch for the session.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Railway bans VOLUME in Dockerfiles — persistent storage is handled via
Railway volumes instead.
## Thinking Path
<!--
Required. Trace your reasoning from the top of the project down to this
specific change. Start with what Paperclip is, then narrow through the
subsystem, the problem, and why this PR exists. Use blockquote style.
Aim for 5–8 steps. See CONTRIBUTING.md for full examples.
-->
> - Paperclip orchestrates AI agents for zero-human companies
> - [Which subsystem or capability is involved]
> - [What problem or gap exists]
> - [Why it needs to be addressed]
> - This pull request ...
> - The benefit is ...
## What Changed
<!-- Bullet list of concrete changes. One bullet per logical unit. -->
-
## Verification
<!--
How can a reviewer confirm this works? Include test commands, manual
steps, or both. For UI changes, include before/after screenshots.
-->
-
## Risks
<!--
What could go wrong? Mention migration safety, breaking changes,
behavioral shifts, or "Low risk" if genuinely minor.
-->
-
## Model Used
<!--
Required. Specify which AI model was used to produce or assist with
this change. Be as descriptive as possible — include:
• Provider and model name (e.g., Claude, GPT, Gemini, Codex)
• Exact model ID or version (e.g., claude-opus-4-6,
gpt-4-turbo-2024-04-09)
• Context window size if relevant (e.g., 1M context)
• Reasoning/thinking mode if applicable (e.g., extended thinking,
chain-of-thought)
• Any other relevant capability details (e.g., tool use, code execution)
If no AI model was used, write "None — human-authored".
-->
-
## Checklist
- [ ] I have included a thinking path that traces from project context
to this change
- [ ] I have specified the model used (with version and capability
details)
- [ ] I have run tests locally and they pass
- [ ] 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
- [ ] I have considered and documented any risks above
- [ ] I will address all Greptile and reviewer comments before
requesting merge
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The desktop app shell uses a fixed-height `h-dvh` flex column with
`<body>` pinned to `overflow: hidden` so the page itself doesn't scroll
— only `#main-content` does
> - On long threads, submitting a comment fires `scrollIntoView({ block:
"start" })` on the new optimistic message, which by spec walks every
ancestor scroll container and adjusts each
> - With `<body>` (and historically the two shell `<div>`s) marked
`overflow: hidden`, the browser treats them as scroll containers per the
CSS spec; `scrollIntoView` reaches them and scrolls them, visually
shifting the entire shell — sidebar and header included — off the top of
the viewport
> - `overflow: clip` is visually identical to `hidden` but is explicitly
*not* a scroll container, so converting the shell divs and body to
`clip` removes them from `scrollIntoView`'s ancestor walk
> - Chrome additionally drives `document.scrollingElement.scrollTop` via
its internal C++ smooth-scroll algorithm, which bypasses both the JS
`scrollTop` setter and the CSS `overflow` on the root element — so even
with the root viewport clipped, `<html>` still scrolls
> - This PR layers the CSS/JS overflow-clip changes with a capture-phase
`scroll` listener that snaps `documentElement.scrollTop` and
`body.scrollTop` back to 0 whenever they drift, defeating the
root-viewport scroll the browser performs natively
## Linked Issues or Issue Description
Refs #8041 (`fix(ui): don't window-scroll the desktop shell on comment
submit`) — that PR gated the JS-initiated `window.scrollBy(...)` path in
`restoreComposerViewportSnapshot`. The bug this PR fixes is a separate,
browser-internal scroll path triggered by `scrollIntoView`'s ancestor
walk, which the #8041 gate doesn't reach (no JS scroll call to gate).
This PR is complementary, not a revert.
**Bug description (no existing tracked issue):** After submitting a
comment on a long thread in the desktop app, the entire app shell —
sidebar, top nav, everything — visually shifts upward by ~300px, putting
items above "Tasks" in the sidebar off-screen. A soft refresh (Cmd+R)
doesn't fix it; only a hard URL navigation does. Verified the bug
reproduces only on threads long enough that `#main-content.scrollHeight
> clientHeight` by a wide margin.
**Repro:** Open a long-thread issue → submit a one-word comment →
observe sidebar items above the active selection fall off the top of the
viewport.
## What Changed
- `ui/src/components/Layout.tsx`:
- Static desktop-shell `<div>`s: `overflow-hidden` → `overflow-clip` on
both the outer flex column and inner flex row (lines 459, 470 in the
original numbering). Removes them from the scroll-container set.
- JS-applied body overflow: `document.body.style.overflow = isMobile ?
"visible" : "hidden"` → `... : "clip"`. Same rationale, applied to
`<body>`.
- New `useEffect` that registers a capture-phase `scroll` listener on
`window`. On every scroll event, if `documentElement.scrollTop` or
`body.scrollTop` has drifted from 0, it gets snapped back. Gated on
`!isMobile` so the mobile shell (which uses `min-h-dvh` and
intentionally scrolls window) is unaffected.
## Verification
**Reviewer steps:**
1. Open a long-thread issue in the desktop UI (anything where
`#main-content.scrollHeight > clientHeight` substantially — most active
issues qualify).
2. Submit a one-word comment.
3. Confirm the sidebar stays put — items above the active item remain
visible, top nav stays at the top of the viewport.
4. Repeat several times; confirm the shell never drifts upward.
**Tests:** Existing tests for `issue-chat-scroll.ts` and
`IssueChatThread.tsx` still pass (59/59 in
`ui/src/lib/issue-chat-scroll.test.ts` +
`ui/src/components/IssueChatThread.test.tsx`).
**Bisect proof:** During development I confirmed by selectively
reverting the capture-phase scroll listener that the bug returns
immediately when only the listener is removed (with the two
`overflow-clip` changes still in place) — proving the listener is the
decisive fix for the browser-internal root-viewport scroll path. The
probe trace showed `documentElement.scrollTop` animating from 0 →
307.5px over ~400ms during the smooth `scrollIntoView` even with `html {
overflow: clip }` applied in computed style.
## Risks
Low.
- The two `overflow-hidden` → `overflow-clip` swaps are visually
identical and have well-defined browser support (Chrome 90+, Firefox
81+, Safari 16+). Paperclip's desktop targets are well within those
ranges.
- The capture-phase scroll listener fires only when
`documentElement.scrollTop !== 0` or `body.scrollTop !== 0`. On a
correctly-behaving page neither should ever be non-zero on desktop, so
the listener is effectively a no-op for everything except the bug it's
reverting. It's gated behind `!isMobile`.
- Mobile uses `min-h-dvh` (no overflow set) and intentionally scrolls
window for the standard mobile chrome behavior; the listener is gated
off there so mobile UX is unaffected.
## Model Used
Claude Opus 4.7 (`claude-opus-4-7`), 200k context, extended thinking
enabled. Tool use: file edits, bash for git/test execution. Diagnosis
was driven by an instrumented in-browser probe (built and HMR'd into
`Layout.tsx` during development, removed before this PR) that traced
every `scrollBy`/`scrollTo`/`scrollIntoView` call and the
`Element.prototype.scrollTop` setter; the decisive trace identified the
browser-internal scroll path as the unguarded root cause.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have run tests locally and they pass
- [ ] I have added or updated tests where applicable — no new tests; the
bug is a browser-internal scroll behavior that's not exercisable in
jsdom (which doesn't implement the smooth-scroll animation that drives
the bug). Existing snapshot/restore tests in `issue-chat-scroll.test.ts`
cover the JS-initiated paths.
- [ ] If this change affects the UI, I have included before/after
screenshots — see the description text above; the bug manifests as the
entire shell translating up by ~300px on long-thread comment submit
- [x] I have updated relevant documentation to reflect my changes — N/A,
behavior change is non-user-facing CSS/event-handling
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green — will confirm on this PR
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups —
will iterate
- [x] I will address all Greptile and reviewer comments before
requesting merge
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - The UI ships a dark/light theme toggle and persists the user's
explicit choice in `localStorage`
> - For first-time visitors with no stored choice, the pre-React
bootstrap script in `ui/index.html` hardcoded `"dark"` and never
consulted the OS preference
> - As a result, users on a light-themed OS were forced into dark mode
until they clicked the toggle once — a first-impression friction with no
compensating benefit
> - This pull request makes the bootstrap respect `prefers-color-scheme`
for first-time visitors and adds a `matchMedia` listener so the in-app
theme auto-follows OS changes until the user makes an explicit choice
> - The benefit is a friction-free first visit that matches every other
modern web app, with zero impact on users who have already chosen a
theme
## Linked Issues or Issue Description
No existing issue covers this directly — problem described in-PR (bug
shape):
- For first-time visitors with no stored theme choice, the pre-React
bootstrap script in `ui/index.html` hardcoded `"dark"` and never
consulted the OS `prefers-color-scheme` preference.
- Users on a light-themed OS were forced into dark mode until they
clicked the toggle once — first-impression friction with no compensating
benefit.
- This PR supersedes part of PR #3732 (the `prefers-color-scheme`
bootstrap slice); no standalone issue was filed for it.
## What Changed
- **`ui/index.html`** — the pre-React bootstrap script now computes a
`prefersDark` fallback via `matchMedia("(prefers-color-scheme: dark)")`,
guarded by a `typeof window.matchMedia === "function"` feature-detection
check, in place of the hardcoded `"dark"` default. A stored
`localStorage` value still takes precedence.
- **`ui/src/context/ThemeContext.tsx`** — tracks an `hasExplicitChoice`
flag. While `false`, a `MediaQueryList` `change` listener keeps the
in-app theme in sync with OS theme switches. Once `setTheme` /
`toggleTheme` runs, the choice is persisted and the listener is removed.
## Verification
- `pnpm --filter @paperclipai/ui run typecheck` — clean.
- `npx vitest run src/components/SidebarAccountMenu.test.tsx` — 1/1 pass
(the one test that exercises `ThemeContext`).
- Manual: launched the Vite UI dev server with a mocked
`/api/auth/get-session` 401, navigated to `/auth` with no
`localStorage.theme` set, and toggled the browser's emulated
`prefers-color-scheme` between `dark` and `light`. Bootstrap renders the
matching theme without a flash. Screenshots committed.
**Screenshots — first-visit (no stored theme) with system pref:**
| System dark | System light |
| --- | --- |
<img width="1280" height="800" alt="first-visit-prefers-dark"
src="https://github.com/user-attachments/assets/c30274d5-a2e0-4ed8-b7ef-b96b8caac5ca"
/>
<img width="1280" height="800" alt="first-visit-prefers-light"
src="https://github.com/user-attachments/assets/ebd8e9cf-0be1-4b36-a6f8-eb38b00dff5c"
/>
(Without this PR, both screenshots would have rendered dark.)
## Risks
Low. Purely additive:
- A stored `localStorage` value still takes precedence over the OS
preference, so users who have already picked a theme are unaffected.
- SSR-safe: both new code paths are gated on `typeof window !==
"undefined"` (the inline script only runs in the browser, and the React
`matchMedia` listener is attached inside `useEffect`).
- No new dependencies, no API surface change.
## Model Used
Claude Opus 4.7 (1M context), extended thinking mode.
## Checklist
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] Thinking path traces from project context to this change
- [x] Model used specified
- [x] Checked ROADMAP.md — not in conflict with planned core work
- [x] Tests run locally and pass
- [x] No new test cases — the change is observable only via real
`matchMedia` events, which jsdom does not faithfully implement; manual
verification above
- [x] UI change — before/after screenshots in
`docs/pr-screenshots/pr-5873/`
- [x] No documentation updates required
- [x] Documented risks above
- [x] Will address all Greptile and reviewer comments before merge
Supersedes part of #3732 (the `prefers-color-scheme` bootstrap slice).
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Devin Foley <devin@paperclip.ing>
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Routines let users templatize titles and instructions with
`{{name}}` placeholders that get prompted for at run time
> - A user reported that `{{pr_url}}` typed into a routine description
was never detected as a variable, while camelCase placeholders worked
> - Root cause: the routine description is edited via MDXEditor (WYSIWYG
markdown). On save its serializer (`mdast-util-to-markdown`) defensively
escapes intraword underscores, so a user-typed `{{pr_url}}` is stored as
`{{pr\_url}}`. The variable matcher only accepted `[A-Za-z0-9_]` inside
the placeholder, so the backslash broke the match and the variable was
silently dropped
> - This PR widens the matcher to tolerate `\_` and unescapes it back to
`_` on capture, in both extraction and interpolation, so a single name
is detected and resolved regardless of whether the source markdown was
hand-typed or round-tripped through a WYSIWYG editor
> - The benefit is that snake_case placeholders behave the same in the
routine UI and the executor, removing a silent failure mode
## Linked Issues or Issue Description
Refs PAPA-771 — "Fix example for variable_name". Initially scoped as a
docs-copy fix (change the example to camelCase), but investigation
showed the underlying bug was that the parser could not see the variable
when it was authored as `{{pr_url}}` in a WYSIWYG-edited description.
This PR addresses the bug directly so snake_case names work as users
expect. (Supersedes the now-closed PR #8054 — same commit, fresh branch
so the diff is visible.)
## What Changed
- `packages/shared/src/routine-variables.ts`:
- Widened the `ROUTINE_VARIABLE_MATCHER` regex to accept `\_` inside
placeholder names (in addition to `[A-Za-z0-9_]`)
- Added `unescapeRoutineVariableName` and applied it on capture in both
`extractRoutineVariableNames` and `interpolateRoutineTemplate`, so the
looked-up variable name is normalized regardless of the markdown escape
- `packages/shared/src/routine-variables.test.ts`:
- Added tests covering plain snake_case (`{{pr_url}}`), markdown-escaped
(`{{pr\_url}}`), multi-underscore (`{{pr\_url\_v2}}`),
sync-with-template, and interpolation
## Verification
- `pnpm --filter @paperclipai/shared exec vitest run routine-variables`
→ 13 passed (4 new), confirming both plain and escaped underscore
placeholders are detected and interpolated as the same variable name
- Reproduced the root cause out-of-band with `mdast-util-from-markdown`
+ `mdast-util-to-markdown` to confirm the editor's serializer is the
source of the `\_` escape
## Risks
- Low. Change is confined to `packages/shared/src/routine-variables.ts`;
the regex is strictly more permissive in a tightly bounded way (only
`\_` is newly allowed) and the captured name is normalized.
`isValidRoutineVariableName` is unchanged, so stored names are still
strict identifiers.
## Model Used
- Claude (Anthropic), `claude-opus-4-7` (Opus 4.7), via Claude Code
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above — only my own closed#8054 supersedes/duplicates this one.
Open PRs touching `routine-variables.ts` (#7184, #7186, #6993, #7187)
are all for unrelated concerns (server-side PATCH persistence, GH#6525)
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have run tests locally and they pass — `routine-variables` suite
passes (13/13)
- [x] I have added or updated tests where applicable — 4 new tests
covering snake_case + escaped forms
- [ ] If this change affects the UI, I have included before/after
screenshots — no UI change in this PR
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green — pending CI re-run after the
push for the Greptile blank-line nit
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups —
initial review was 5/5; the one P2 (missing blank line) is addressed in
the latest push
- [x] I will address all Greptile and reviewer comments before
requesting merge
Co-authored-by: Paperclip <noreply@paperclip.ing>
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - Operators sign in via the `/auth` page, which renders before any
session exists
> - The signed-in app has a theme toggle inside `SidebarAccountMenu`,
but the signed-out `/auth` page has none — first-time visitors are stuck
in whichever theme was hardcoded at boot
> - Master's existing toggle was inline inside `SidebarAccountMenu.tsx`
as a `MenuAction` row, not exported as a reusable widget; round-1 of
this PR added a standalone `ThemeToggle` but punted on unifying the two
surfaces
> - This pull request makes `ThemeToggle` the canonical theme widget
(one source of truth for label, icon, and toggle behaviour), used both
as a compact icon button on `/auth` and as a full-width menu row in
`SidebarAccountMenu`
> - The benefit is a working pre-auth theme switch and zero risk of the
two call sites drifting out of sync as the theme model evolves
## Linked Issues or Issue Description
No existing issue covers this directly — problem described in-PR
(feature-gap shape):
- The signed-in app has a theme toggle inside `SidebarAccountMenu`, but
the signed-out `/auth` page has none — first-time visitors are stuck in
whichever theme was hardcoded at boot.
- The existing toggle lived inline in `SidebarAccountMenu.tsx` as a
`MenuAction` row, not exported as a reusable widget, so the two surfaces
could drift apart as the theme model evolves.
- This PR supersedes part of PR #3732 (the auth-page toggle slice); no
standalone issue was filed for it.
Duplicate-PR search: related open theme PRs #2769 and #4666 add in-app
three-state system-theme toggles — different surface from this PR
(unauthenticated auth page); sibling PR in this series: #5873.
## What Changed
- **`ui/src/components/ThemeToggle.tsx`** — accepts `variant: "icon" |
"menu-action"` (default `"icon"`) and an `onAfterToggle` callback. Both
variants share `useTheme` and the same label/icon derivation. The
`menu-action` variant matches the existing `MenuAction` row styling.
- **`ui/src/components/SidebarAccountMenu.tsx`** — drops its inline
`useTheme()` + `MenuAction`-for-the-theme-row in favor of `<ThemeToggle
variant="menu-action" onAfterToggle={() => setOpen(false)} />`. Sun/Moon
icon imports and theme state move with it.
- **`ui/src/pages/Auth.tsx`** — unchanged from round 0; renders
`<ThemeToggle />` at top-right of the `/auth` page (already using the
default `icon` variant).
- **`ui/src/components/ThemeToggle.test.tsx`** (new) — covers both
variants, the `onAfterToggle` callback, and the label/icon flip across
themes.
- **`ui/src/components/SidebarAccountMenu.test.tsx`** — unchanged; its
`ThemeContext` mock still works because `ThemeToggle` uses the same
hook.
## Verification
- `pnpm --filter @paperclipai/ui run typecheck` — clean.
- `npx vitest run src/components/ThemeToggle.test.tsx
src/components/SidebarAccountMenu.test.tsx` — 5 passed (4 new + 1
existing).
- Manual: launched `pnpm dev` for the UI, mocked `/api/auth/get-session`
401, navigated to `/auth` — toggle is visible top-right, click flips
light/dark. Screenshots committed.
## Risks
Low. The change is structural — both call sites render the same widget
that already worked on each surface independently.
`SidebarAccountMenu`'s popover behaviour is preserved via
`onAfterToggle`, and `ThemeContext` is untouched.
## Model Used
Claude Opus 4.7 (1M context), extended thinking mode.
## Checklist
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] Thinking path traces from project context to this change
- [x] Model used specified
- [x] Checked ROADMAP.md — not in conflict with planned core work
- [x] Tests run locally and pass
- [x] Added tests for the new ThemeToggle component (both variants)
- [x] UI change — before/after screenshots in
`docs/pr-screenshots/pr-5874/`
- [x] No documentation updates required (purely internal refactor + new
component)
- [x] Documented risks above
- [x] Will address all Greptile and reviewer comments before merge
Supersedes part of #3732.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Devin Foley <devin@paperclip.ing>
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - Production self-hosters increasingly expect telemetry out of the box
— Jaeger, Tempo, Honeycomb, Datadog, Grafana Cloud, Dynatrace all speak
OTLP
> - Today there is no OpenTelemetry bootstrap in the server, so
operators who want traces have to patch their fork or run a sidecar that
captures only HTTP-level info
> - An opt-in bootstrap that costs nothing when disabled is the
minimum-viable surface for this audience
> - The OpenTelemetry packages are heavyweight enough that we don't want
them in the default dependency graph — they should load only when the
operator configures an OTLP endpoint
> - This pull request adds a self-contained
`server/src/instrumentation.ts` that dynamically imports the OTel SDK
and starts it when `OTEL_EXPORTER_OTLP_ENDPOINT` is set, and is a
complete no-op otherwise
## Linked Issues or Issue Description
No existing issue covers this directly — feature-gap description
following the feature-request template:
**Problem or motivation**
Production self-hosters increasingly expect telemetry out of the box —
Jaeger, Tempo, Honeycomb, Datadog, Grafana Cloud, Dynatrace all speak
OTLP — but the server has no OpenTelemetry bootstrap. Operators who want
traces today must patch their fork or run a sidecar that captures only
HTTP-level information.
**Proposed solution**
An opt-in OTel bootstrap gated on `OTEL_EXPORTER_OTLP_ENDPOINT`, loaded
via dynamic `import()` only when configured, so the heavyweight OTel
packages stay out of the default dependency graph.
**Alternatives considered**
Related open PRs found during the duplicate-PR search approach
observability differently: #4894 adds OTLP instrumentation to Paperclip
core unconditionally, and #3752 proposes an observability plugin. Not
duplicates — different layering: this PR keeps the default install
dependency-free via opt-in dynamic import.
## What Changed
- New `server/src/instrumentation.ts` — opt-in OpenTelemetry
auto-instrumentation. Gated on `OTEL_EXPORTER_OTLP_ENDPOINT`. Respects
the standard OTel env vars (`OTEL_SERVICE_NAME`, `OTEL_SERVICE_VERSION`,
`OTEL_EXPORTER_OTLP_ENDPOINT`). Skips the fs/dns/net
auto-instrumentations (too chatty). `sdk.start()` is wrapped in
try/catch so a bad endpoint or missing native bindings doesn't crash the
server. `process.once("SIGTERM" / "SIGINT", …)` for clean shutdown on
the first signal only. OTel packages are loaded via dynamic `import()`
so they are true optional runtime dependencies — no entries in
`package.json`, no lockfile churn.
- `server/src/index.ts` — import `./instrumentation.js` as the very
first statement so auto-instrumentation can patch `http` / `express` /
`pg` before they are evaluated by downstream modules.
## Verification
- `OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4317 pnpm start` after
`pnpm install
@opentelemetry/{sdk-node,auto-instrumentations-node,exporter-trace-otlp-grpc,resources,semantic-conventions}`
in `server/` — traces show up in the configured collector; HTTP,
Express, and Postgres spans are populated.
- `OTEL_EXPORTER_OTLP_ENDPOINT` unset — server starts with no
OTel-shaped output in logs, no behavior change.
- `OTEL_EXPORTER_OTLP_ENDPOINT=…` set but packages not installed —
single `console.warn` at startup telling the operator which packages to
install.
## Risks
Low. No behavior change unless the env var is set. The bootstrap never
throws into the caller; every failure path ends in `console.warn` /
`console.error` and falls through to non-traced operation.
## Model Used
Claude Opus 4.6 (1M context), extended thinking mode.
## Checklist
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] Thinking path traces from project context to this change
- [x] Model used specified
- [x] Tests run locally and pass
- [x] CI green
- [x] Greptile review addressed
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - Express looks at incoming `X-Forwarded-For` headers only when
`app.set("trust proxy", …)` says it should, and uses that resolved
client IP downstream for rate-limiting, audit logging, and any
auth/abuse signal that ties back to source IP
> - The original PR #3729 added `TRUST_PROXY` accepting only `"true"` or
a positive integer, which forces operators to pick between two unsafe
defaults: hop-count (brittle if topology changes) or boolean-true (any
client can spoof `X-Forwarded-For` and bypass rate-limits or pollute
audit logs)
> - `trust proxy: true` is one of the most common Express
misconfigurations and trivially exploitable for IP-spoofing-based
rate-limit bypass; the safest config — trust only the LB's actual CIDR
or only loopback — was unreachable with the previous parser
> - This pull request replaces the parser with full Express 5 support —
unset / `false` / `0` (Express default), positive integer hop count,
comma-separated CIDR list, named subnets (`loopback`, `linklocal`,
`uniquelocal`) — and emits a startup error naming the offending token on
invalid input
> - The benefit is that operators can now trust *only* their actual
ingress and close the spoofing window without leaking client-IP
integrity to downstream layers, while preserving every
previously-working config as a strict superset
## Linked Issues or Issue Description
Refs #1690 — login returns 500 behind a reverse proxy because Express
`trust proxy` is not enabled; this PR ships the configuration surface
(`TRUST_PROXY` with CIDR lists and named subnets) that lets operators
enable it safely. It does not change the default, so #1690 still
requires the operator to set `TRUST_PROXY` — hence Refs, not Fixes.
No other existing issue covers this directly — remaining problem
described in-PR:
- The original `TRUST_PROXY` parser (PR #3729, which this PR supersedes)
accepted only `"true"` or a hop count, forcing operators to choose
between brittle hop-counting and the spoofable `trust proxy: true`.
- `trust proxy: true` lets any client spoof `X-Forwarded-For` and bypass
rate limits or pollute audit logs; the safest config — trusting only the
LB's actual CIDR or only loopback — was unreachable with the previous
parser.
Duplicate-PR search: #1854 / #1714 are earlier minimal trust-proxy
enablement PRs; this PR supersedes #3729 and generalizes beyond a
boolean enable (CIDR lists + named subnets).
## What Changed
- **`server/src/middleware/trust-proxy.ts`** — new helper exposing
`parseTrustProxyEnv` (testable) and `applyTrustProxy(app)` (one-call
boot wiring). Surface:
- Unset / `""` / `false` / `0` → no `app.set("trust proxy", …)` (Express
default: trust nothing).
- `true` → `app.set("trust proxy", true)`. Documented as unsafe in
untrusted-LB deployments.
- Positive integer (e.g. `"2"`) → hop count. Strict parse: rejects
`"01"`, leading/trailing whitespace.
- Comma-separated list of CIDRs and/or named subnets (e.g.
`"loopback,uniquelocal,10.0.0.0/8,fd00::/8"`) → array passed to
`app.set("trust proxy", [...])`.
- Anything else → startup error naming the offending token.
- **`server/src/app.ts`** — one import + one call to
`applyTrustProxy(app)`.
- **`server/src/__tests__/trust-proxy.test.ts`** — 12 cases: unset,
`"true"`, `"0"`, `"2"`, `"01"` rejected, `" 2 "` rejected, `"loopback"`,
`"loopback,uniquelocal"`, `"10.0.0.0/8"`, `"10.0.0.0/8,fd00::/8"`,
`"bogus"` rejected (error names the bad token), mixed-list with one bad
token rejected (error names the offending token specifically).
## Verification
- `pnpm --filter @paperclipai/server run typecheck` — clean.
- `npx vitest run trust-proxy` — 12/12 pass.
## Risks
- **No new required env vars.** Unset means default Express behavior
(trust nothing). Pure superset of #3729's surface — anything that worked
under #3729 still works here.
- **Strict parse.** `"01"` and `" 2 "` are rejected on purpose so
configuration mistakes surface at startup, not as silently-degraded
auth/rate-limit behavior. The error message names the offending token.
- **No runtime cost** — the parse runs once at boot. The downstream
`trust proxy` setting is internal to Express.
- Single-tenant local-first deploys unaffected by default.
## Model Used
Claude Opus 4.7 (1M context), extended thinking mode.
## Checklist
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] Thinking path traces from project context to this change
- [x] Model used specified
- [x] Checked ROADMAP.md — not in conflict with planned core work
- [x] Tests run locally and pass (`trust-proxy` 12/12)
- [x] Added boundary cases (leading-zero, whitespace, unknown token,
mixed-list-with-bad-token)
- [x] No UI changes
- [x] Documented risks above
- [x] Will address all Greptile and reviewer comments before merge
Closes#3729.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - Access to agents, skills, and configurations is governed by a
per-company permission system
> - `agents:create` is a mutation-tier permission that controls who can
create or modify agents
> - `assertCanReadConfigurations` delegates to
`assertCanCreateAgentsForCompany`, effectively requiring `agents:create`
just to *read* agent configs, skills, and config revisions
> - That's a permission regression: any company member without
`agents:create` hits 403 on the Skills tab, agent config pages, and
revision history — but those responses are already secret-redacted
> - This pull request loosens the read gate to company membership only,
while keeping every mutation-adjacent gate at `agents:create`
## Linked Issues or Issue Description
No existing issue covers this directly — problem described in-PR
following the bug-report template:
**What happened**
`assertCanReadConfigurations` delegates to
`assertCanCreateAgentsForCompany`, effectively requiring the
mutation-tier `agents:create` permission just to *read* agent configs,
skills, and config revisions. Any company member without `agents:create`
hits 403 on the Skills tab, agent config pages, and revision history —
even though those responses are already secret-redacted
(`redactAgentConfiguration`, `redactConfigRevision`).
**Expected behavior**
Read-only configuration/skill/revision endpoints are readable by any
company member; only mutation-adjacent endpoints require
`agents:create`.
**Steps to reproduce**
As a company member without `agents:create`, open the Skills tab or an
agent config page (or `GET` the config/skill/revision endpoints) — the
request fails with 403.
## What Changed
- `server/src/routes/agents.ts`:
- `assertCanReadConfigurations` now requires company membership only
(plus the existing agent-key cross-company check). Previously it
required `agents:create`.
- `actorCanReadConfigurationsForCompany` (the boolean twin, used by `GET
/agents/:id` to decide whether to return a restricted detail) now uses
the standard try/catch-around-`assertCompanyAccess` pattern.
- `POST /companies/:companyId/adapters/:type/test-environment` is not a
pure read (it exercises adapter secrets) and now calls
`assertCanCreateAgentsForCompany` directly instead of going through
`assertCanReadConfigurations`. Behavior for this endpoint is unchanged.
## Verification
- Existing tests pass.
- Manual: log in as a company member without an `agents:create` grant.
Visit the Skills tab on an agent and the agent configuration panel —
both load. Try to edit the agent — blocked, as before.
- Manual: POST to
`/companies/:companyId/adapters/:type/test-environment` as the same user
— still 403.
## Risks
Low. The only behavior change is on read endpoints whose responses were
already redacted (\`redactAgentConfiguration\`,
\`redactConfigRevision\`). No secret escapes anywhere.
## Model Used
Claude Opus 4.6 (1M context), extended thinking mode.
## Checklist
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] Thinking path traces from project context to this change
- [x] Model used specified
- [x] Tests run locally and pass
- [x] CI green
- [x] Greptile review addressed
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - The reference container image must be deployable on both Docker
Compose (where it starts as root and `gosu`'s a `USER_UID`/`USER_GID`
switch) and Kubernetes (where the pod is typically constrained by
PodSecurity)
> - The Kubernetes operator (paperclipinc/paperclip-operator#45) sets
`runAsNonRoot: true`, `runAsUser: 1000`, `allowPrivilegeEscalation:
false`, and `drop: ALL` capabilities by default — the unconditional
`usermod` + `gosu` flow in the entrypoint requires root + `CAP_SETUID` /
`CAP_SETGID`, making the image undeployable on any cluster enforcing
baseline or restricted PodSecurity
> - Without root, neither the user remap nor `gosu` can ever succeed —
so the fix is a runtime branch: non-root starts `exec` the command
directly (warning if the runtime UID/GID differs from the requested
one), while root starts keep the existing `usermod`+`gosu` flow
> - This also covers platforms that assign arbitrary UIDs (OpenShift
restricted SCC), which previously crashed with a cryptic `usermod:
Permission denied`
> - The benefit is one image that works for both deployment shapes with
no operator-side workaround — pure superset, no breaking change
## Linked Issues or Issue Description
Refs paperclipinc/paperclip-operator#45 (cross-repo) — the operator's
default pod security context (`runAsNonRoot: true`, `runAsUser: 1000`,
`allowPrivilegeEscalation: false`, `drop: ALL`) is blocked by this
entrypoint behavior. The operator shipped a stopgap
(paperclipinc/paperclip-operator#46 lets the CRD override the security
context); this PR is the image-side fix that makes the secure defaults
work out of the box. Supersedes #2904 (v1 of this branch).
No in-repo issue covers this directly — problem described in-PR
following the bug-report template:
**What happened**
The entrypoint unconditionally runs `usermod`/`groupmod`/`chown` + `exec
gosu node`, which requires root plus `CAP_SETUID` / `CAP_SETGID` — any
non-root start crashes (`gosu` cannot drop privileges; a mismatched UID
dies earlier at `usermod: Permission denied`), making the reference
image undeployable on clusters enforcing baseline or restricted
PodSecurity.
**Expected behavior**
A non-root container `exec`s the command directly (with a clear warning
if its UID/GID differs from the requested `USER_UID`/`USER_GID`, since a
remap is impossible without root). The existing root + `usermod` +
`gosu` flow is preserved for Docker Compose, where the container starts
as root and switches to the requested UID/GID.
**Deployment mode**
Kubernetes with baseline/restricted PodSecurity and OpenShift-style
arbitrary-UID platforms (failing cases); Docker Compose root-start (must
keep working).
## What Changed
- **`scripts/docker-entrypoint.sh`** — branch on the runtime UID:
- **Non-root start** → `exec "$@"` directly. If the runtime UID/GID
differs from `USER_UID`/`USER_GID`, print a one-line warning to stderr
first (the remap is impossible without root; the warning keeps
volume-permission mismatches diagnosable instead of failing cryptically
inside `usermod`).
- **Root start** → unchanged: `usermod`/`groupmod` remap when requested,
`chown` of `/paperclip` when a remap happened, then `exec gosu node
"$@"`.
## Verification
**Automated:** `server/src/__tests__/docker-entrypoint.test.ts` runs the
real entrypoint with `id`/`usermod`/`groupmod`/`chown`/`gosu` stubbed
via PATH and asserts all five privilege branches (root+defaults,
root+remap, non-root match, arbitrary non-root UID, GID mismatch) — runs
in the regular server suite, no Docker needed.
**Manual (Docker):** ran the entrypoint on `node:lts-trixie-slim` (the
image's actual base) across the full matrix, with `gosu` stubbed to a
marker:
- [x] Root start, defaults → no remap, `gosu node` invoked (Docker
Compose flow unchanged)
- [x] Root start, `USER_UID=1001`/`USER_GID=1001` → `Updating node
UID/GID to 1001` + `gosu node` (remap flow unchanged)
- [x] Non-root `--user 1000:1000` (the operator's `runAsUser: 1000`
shape) → silent direct exec, command runs as 1000:1000
- [x] Non-root `--user 1234:1234` (arbitrary UID) → warning `running
unprivileged as 1234:1234; cannot remap to requested 1000:1000`, then
direct exec (previously: crash)
- [x] Non-root `--user 1000:1001` (GID mismatch) → warning, then direct
exec
- [x] Baseline check: master's entrypoint fails for any non-root start
(gosu/usermod require root)
End-to-end cluster verification under restricted PodSecurity exercises
the same branch as the `--user 1000:1000` case above; the operator
repo's deploy is the natural place for that smoke test once this ships
in an image tag.
## Risks
- **Backward-compatible.** Docker Compose / root-entrypoint path is
byte-for-byte the same flow — `usermod`/`gosu` runs whenever the
container starts as root.
- **Behavior change only for previously-broken starts.** Non-root
containers used to crash; they now run. The only observable difference
for a *working* deployment is none.
- **Mismatched non-root UID/GID warns instead of failing.** Deliberate:
the remap is impossible without root, and arbitrary-UID platforms
(OpenShift) rely on group-writable volumes; a hard fail would keep them
broken. The stderr warning preserves diagnosability.
- **No new env vars, no API surface.** Pure entrypoint behavior change
gated on the runtime UID.
- **Restricted PodSecurity ready.** The non-root branch needs no Linux
capabilities — works under `drop: ALL`.
## Model Used
Claude Opus 4.6; rebase, non-root generalization, and verification
matrix by Claude Fable 5 (1M context).
## Checklist
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] Thinking path traces from project context to this change
- [x] Model used specified
- [x] Checked ROADMAP.md — not in conflict with planned core work
(agent-runtime sandbox images use `tini`, no gosu — unaffected)
- [x] Tests run locally and pass (new `docker-entrypoint.test.ts`
covering all five privilege branches, plus a manual Docker matrix on the
real base image; see Verification)
- [x] No UI changes
- [x] Documented risks above
- [x] Will address all Greptile and reviewer comments before merge
Unblocks the default (non-overridden) security context of
paperclipinc/paperclip-operator#45 / #46.
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - The plugin subsystem persists state into four tables
(`plugin_entities`, `plugin_job_runs`, `plugin_logs`,
`plugin_webhook_deliveries`) and those rows currently have no notion of
an owning tenant — so company-deletion doesn't cascade plugin state, and
operators have no way to query "what does this plugin own for company
X?"
> - The fix is one thin slice of tenant-isolation hygiene that doesn't
change any external API: add a nullable `company_id` FK with `ON DELETE
CASCADE` to the four plugin tables, index it, and scope the
`plugin_entities` external-id uniqueness per-tenant
> - The benefit is plugin-row tenant attribution + cascade cleanup, with
zero impact on single-tenant local-first deploys (`NULL` continues to
mean instance-scope)
> **Rebase note (scope narrowed):** This PR originally also hardened the
schedulers (`heartbeat.tickTimers` / `resumeQueuedRuns` /
`enqueueWakeup` and `routines.tickScheduledTriggers`) to skip archived
companies. That half has since landed on `master` via #7478 (`93206f73`,
"Stop archived companies from waking agents") with a stricter
implementation (`status != 'active'` plus a skipped-request audit row).
On rebase those changes were dropped as redundant — `heartbeat.ts` and
`routines.ts` are now identical to `master`, and the scheduler-specific
tests were removed. **This PR is now DB-only.**
## Linked Issues or Issue Description
No existing issue covers this directly — problem described in-PR:
- Four plugin tables (`plugin_entities`, `plugin_job_runs`,
`plugin_logs`, `plugin_webhook_deliveries`) persist rows with no notion
of an owning tenant.
- Company deletion therefore does not cascade plugin state, and
operators have no way to query "what does this plugin own for company
X?"
- One thin slice of tenant-isolation hygiene fixes this without changing
any external API: a nullable `company_id` FK with `ON DELETE CASCADE`,
an index, and per-tenant scoping of the `plugin_entities` external-id
uniqueness.
- Part of the multi-tenant hardening initiative alongside #3967
(cross-tenant 404 oracle) and #5864 (per-company JWT keys).
## What Changed
**Schema (`packages/db/src/schema/plugin_*.ts`):**
- Nullable `companyId` FK with `onDelete: "cascade"` added to
`plugin_entities`, `plugin_job_runs`, `plugin_logs`,
`plugin_webhook_deliveries`.
- A btree index on each new `company_id` column (`<table>_company_idx`).
- `plugin_entities_external_idx` rescoped from `(plugin_id, entity_type,
external_id)` to `(company_id, plugin_id, entity_type, external_id)` and
switched to `UNIQUE … NULLS NOT DISTINCT` so instance-scope rows
(`company_id IS NULL`) keep their dedup guarantee while tenants get
their own namespace.
**Migration:**
- `0095_plugin_company_id_tenant_isolation.sql` — 14 statements: 4
column adds + 4 FK constraints (`ON DELETE CASCADE`) + 4 indexes +
drop/recreate of the external-id unique constraint.
- Journal entry + regenerated `0095_snapshot.json`.
**Tests:**
- `server/src/__tests__/plugin-tenant-isolation.test.ts` — `NULL`
preserves backward compat; `CASCADE` on company delete across all four
tables; per-tenant external-id namespacing; NULL-NULL collision rejected
(`NULLS NOT DISTINCT`).
## Verification
- `pnpm --filter @paperclipai/db run check:migrations` — pass.
- `pnpm --filter @paperclipai/db typecheck` (`tsc`) — pass.
- `vitest run plugin-tenant-isolation` — **4/4 pass** (embedded Postgres
applies `0095` and exercises cascade + NULLS NOT DISTINCT).
## Notes
- **Clean snapshot, no drift.** The earlier revision of this PR shipped
a ~17.6k-line meta snapshot that was almost entirely pre-existing drift.
On rebase the migration was renumbered (the old `0090_brainy_darkhawk`
collided with `master`'s `0090_resource_memberships … 0094`) and
regenerated from the current `master` baseline via `drizzle-kit
generate`. The result is a 14-statement migration containing **only**
the plugin-table changes — no unrelated drift.
- **Backward-compatible.** `NULL company_id` continues to mean
instance-scope (cron jobs, public webhooks). No new env vars, no API
surface change. Single-tenant local-first deploys unaffected.
## Risks
- **Migration is additive and nullable** — `0095` adds nullable
`company_id` columns, FK constraints, and indexes; existing rows stay
valid (`NULL` keeps meaning instance-scope) and no backfill is required.
- **`ON DELETE CASCADE` is a behavioral change**: deleting a company now
removes its plugin rows across all four tables. Intended (it is the
point of the PR), but operators relying on plugin rows surviving company
deletion would be affected. Covered by the cascade tests.
- **Uniqueness semantics change on `plugin_entities`**: the external-id
constraint is rescoped per-tenant and switched to `UNIQUE … NULLS NOT
DISTINCT`, so two instance-scope rows (`company_id IS NULL`) with the
same external id are now rejected instead of coexisting. Covered by the
NULL-NULL collision test.
- **No API surface change, no new env vars.** Single-tenant local-first
deploys unaffected.
(Section added retroactively to match the PR template; distilled from
the What Changed / Notes sections above.)
## Model Used
Same authoring setup as #5864 (same series, same day): Claude Opus 4.7
(1M context), extended thinking mode. (Section added retroactively.)
## Checklist
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] Tests run locally and pass (plugin-tenant-isolation 4/4)
- [x] `check:migrations` + db typecheck pass
- [x] No UI changes
- [x] Migration carries only the intended changes (no snapshot drift)
- [x] Scheduler half dropped as superseded by #7478
Part of the multi-tenant hardening initiative — see also #3967
(cross-tenant 404 oracle) and #5864 (per-company JWT keys).
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Devin Foley <devin@paperclip.ing>
Co-authored-by: Paperclip <noreply@paperclip.ing>
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The routine scheduler lets agents and users run work on cron
schedules; every 30s a tick computes each trigger's next occurrence
> - `computeNextRun`/`nextCronTickInTimeZone` find that occurrence by
stepping forward one minute at a time (capped at 2.6M iterations),
constructing a fresh `Intl.DateTimeFormat` on every step — ~1ms of ICU
work each
> - Sparse schedules (monthly ≈ 43k steps ≈ 40s) and never-matching
crons (the #7529 midnight bug → full 2.6M steps ≈ 45 min) block the Node
event loop for the whole scan, every tick
> - In production this pegs the server at 100% CPU, health checks time
out, and Paperclip Desktop repeatedly shows "the embedded server is no
longer responding" (diagnosed via a CPU sample: 74% of samples inside
`Builtin_DateTimeFormatConstructor`)
> - This pull request caches the formatter per timezone, since
`Intl.DateTimeFormat` instances are immutable and reusable
> - The benefit is each minute-step pays only `formatToParts` (~1µs): a
43k-step scan drops from ~40s of blocked event loop to under a second,
and the server stays responsive while routines are active
## Linked Issues or Issue Description
- Fixes: #8033
- Refs #7529 — a never-matching midnight cron forces the minute-stepper
through its full 2.6M-iteration cap, which is the worst-case trigger for
this perf bug; the two compound
- Refs #7922 — in-flight fix for #7529 touching the same function
(`getZonedMinuteParts`); the changes are compatible (this PR changes
formatter construction, that PR changes hour normalization)
## What Changed
- `server/src/services/routines.ts`: added a per-timezone
`Intl.DateTimeFormat` cache (`getZonedMinuteFormatter`) used by
`getZonedMinuteParts` and `assertTimeZone`, replacing per-call
construction
- Exported `nextCronTickInTimeZone` so the behavior is testable (same
export PR #7922 makes)
- Added `server/src/services/routines-formatter-cache.test.ts`: verifies
a sparse monthly cron resolves to the correct next occurrence across a
DST-bearing timezone, and asserts at most one formatter construction for
a ~43k-minute-step scan (and zero on a warm cache)
## Verification
- `npx vitest run server/src/services/routines-formatter-cache.test.ts`
— 2 tests pass
- `pnpm --filter @paperclipai/server exec tsc --noEmit` — clean
- Live validation: applied the same patch to the server bundled in
Paperclip Desktop 3.2.9, which was hitting 99.3% CPU with 5s
health-check timeouts within 60s of boot on a real workload; after the
patch, CPU idles at 0–3% across scheduler ticks and `/api/health`
answers in ~1ms (observed over multiple 30s ticks)
## Risks
- Low risk. `Intl.DateTimeFormat` instances are immutable and safe to
reuse; the cache key is the timezone string, and entries are small and
bounded by the number of distinct timezones in use
- Invalid timezones still throw in the constructor before anything is
cached, so `assertTimeZone` semantics are unchanged
- Does not change cron matching semantics; minute-stepping remains
(replacing it with cron-field arithmetic is noted in #8033 as a
follow-up)
## Model Used
- Claude (Anthropic), model ID `claude-fable-5` (Fable 5), via Claude
Code CLI with extended thinking and tool use (profiling, patching, and
live verification performed by the model under user supervision)
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] If this change affects the UI, I have included before/after
screenshots (N/A — server-only change)
- [ ] I have updated relevant documentation to reflect my changes (N/A —
internal perf fix, no doc surface)
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(pending review)
- [x] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
